improve dashbot tests #908

Udhay-Adithya · 2025-10-01T22:26:56Z

PR Description

Add more tests for chat_viewmodel.dart

Related Issues

Closes #

Checklist

I have gone through the contributing guide
I have updated my branch and synced it with project main branch before making this PR
I am using the latest Flutter stable branch (run flutter upgrade and verify)
I have run the tests (flutter test) and all tests are passing

Added/updated tests?

Yes
No, and this is why: please replace this line with details on why tests have not been included

OS on which you have developed and tested the feature?

Windows
macOS
Linux

Udhay-Adithya · 2025-10-01T22:31:42Z

The deleted test/providers/dashbot_window_notifier_test.dart was written during the initial implementation of dashbot and is supposed to be removed. Now the latest tests for the same exists at test/dashbot/providers/dashbot_window_notifier_test.dart

Copilot

Pull Request Overview

This PR significantly improves the test coverage for the Dashbot chat functionality by adding comprehensive tests for the chat_viewmodel.dart file and related service providers.

Key changes:

Removes existing dashbot window notifier tests that appear to be redundant or moved
Adds extensive new test coverage for service providers with proper mocking and dependency injection
Expands chat viewmodel tests from basic functionality to comprehensive coverage including AI flows, error handling, import scenarios, and provider interactions

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
test/providers/dashbot_window_notifier_test.dart	Completely removed existing window notifier tests
test/dashbot/providers/service_providers_test.dart	Added comprehensive tests for service providers with mocking and dependency injection
test/dashbot/features/chat/viewmodel/chat_viewmodel_test.dart	Significantly expanded from 131 to 1640+ lines with comprehensive test coverage
test/dashbot/features/chat/viewmodel/chat_viewmodel_ai_flow_test.dart	Added new AI-enabled flow tests with proper mocking

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

test/dashbot/features/chat/viewmodel/chat_viewmodel_test.dart

Copilot · 2025-10-02T12:28:36Z

test/dashbot/features/chat/viewmodel/chat_viewmodel_test.dart

+      debugPrint('Current request: $currentRequest');
+
+      // Check the computed ID that currentMessages uses
+      final computedId = currentRequest?.id ?? 'global';
+      debugPrint('Computed ID for currentMessages: $computedId');
+
+      // Check initial state
+      debugPrint(
+          'Initial state - chatSessions: ${viewmodel.state.chatSessions}');
+      debugPrint('Initial messages count: ${viewmodel.currentMessages.length}');
+
+      // Call sendMessage which should trigger _addMessage through _appendSystem
+      await viewmodel.sendMessage(text: 'Hello', type: ChatMessageType.general);
+
+      // Debug: print current state
+      debugPrint(
+          'After sendMessage - chatSessions: ${viewmodel.state.chatSessions}');
+      debugPrint(
+          'After sendMessage - keys: ${viewmodel.state.chatSessions.keys}');
+      debugPrint('Current messages count: ${viewmodel.currentMessages.length}');
+
+      // Check again after sendMessage
+      final currentRequestAfter = container.read(selectedRequestModelProvider);
+      final computedIdAfter = currentRequestAfter?.id ?? 'global';
+      debugPrint('Current request after: $currentRequestAfter');
+      debugPrint('Computed ID after: $computedIdAfter');
+
+      // Let's also check the global session directly
+      final globalMessages = viewmodel.state.chatSessions['global'];
+      debugPrint('Global messages directly: ${globalMessages?.length ?? 0}');
+
+      // Check specific computed ID session
+      final computedMessages = viewmodel.state.chatSessions[computedIdAfter];
+      debugPrint(
+          'Messages for computed ID ($computedIdAfter): ${computedMessages?.length ?? 0}');


Excessive debug print statements in test code make it harder to read and maintain. Consider removing these debug prints or replacing them with more focused assertions.

Suggested change

debugPrint('Current request: $currentRequest');

// Check the computed ID that currentMessages uses

final computedId = currentRequest?.id ?? 'global';

debugPrint('Computed ID for currentMessages: $computedId');

// Check initial state

debugPrint(

'Initial state - chatSessions: ${viewmodel.state.chatSessions}');

debugPrint('Initial messages count: ${viewmodel.currentMessages.length}');

// Call sendMessage which should trigger _addMessage through _appendSystem

await viewmodel.sendMessage(text: 'Hello', type: ChatMessageType.general);

// Debug: print current state

debugPrint(

'After sendMessage - chatSessions: ${viewmodel.state.chatSessions}');

debugPrint(

'After sendMessage - keys: ${viewmodel.state.chatSessions.keys}');

debugPrint('Current messages count: ${viewmodel.currentMessages.length}');

// Check again after sendMessage

final currentRequestAfter = container.read(selectedRequestModelProvider);

final computedIdAfter = currentRequestAfter?.id ?? 'global';

debugPrint('Current request after: $currentRequestAfter');

debugPrint('Computed ID after: $computedIdAfter');

// Let's also check the global session directly

final globalMessages = viewmodel.state.chatSessions['global'];

debugPrint('Global messages directly: ${globalMessages?.length ?? 0}');

// Check specific computed ID session

final computedMessages = viewmodel.state.chatSessions[computedIdAfter];

debugPrint(

'Messages for computed ID ($computedIdAfter): ${computedMessages?.length ?? 0}');

// Check the computed ID that currentMessages uses

final computedId = currentRequest?.id ?? 'global';

// Check initial state

expect(viewmodel.state.chatSessions, isEmpty);

expect(viewmodel.currentMessages, isEmpty);

// Call sendMessage which should trigger _addMessage through _appendSystem

await viewmodel.sendMessage(text: 'Hello', type: ChatMessageType.general);

// Check state after sendMessage

expect(viewmodel.state.chatSessions, isNotEmpty);

expect(viewmodel.state.chatSessions.keys, contains(computedId));

expect(viewmodel.currentMessages, isNotEmpty);

// Check again after sendMessage

final currentRequestAfter = container.read(selectedRequestModelProvider);

final computedIdAfter = currentRequestAfter?.id ?? 'global';

// Let's also check the global session directly

final globalMessages = viewmodel.state.chatSessions['global'];

expect(globalMessages, isNotNull);

// Check specific computed ID session

final computedMessages = viewmodel.state.chatSessions[computedIdAfter];

expect(computedMessages, isNotNull);

Copilot · 2025-10-02T12:28:36Z

test/dashbot/features/chat/viewmodel/chat_viewmodel_ai_flow_test.dart

+/// This file contains tests specifically for AI-enabled chat functionality,
+// A mock ChatRemoteRepository returning configurable responses


The comment on line 15 is incomplete (ends with a comma) and line 16 starts without proper documentation formatting. Consider completing the documentation or using consistent comment formatting.

Suggested change

/// This file contains tests specifically for AI-enabled chat functionality,

// A mock ChatRemoteRepository returning configurable responses

/// This file contains tests specifically for AI-enabled chat functionality.

/// A mock ChatRemoteRepository returning configurable responses.

animator · 2025-10-02T12:43:50Z

test/dashbot/features/chat/viewmodel/chat_viewmodel_test.dart

+        },
+      });
+
+      await viewmodel.applyAutoFix(curlAction);


Why no expect statement to test if the new request creation was successful?

should test _inferBaseUrl helper method

The purpose of this test is to make sure base URL inference runs safely when applying a cURL action.
The _inferBaseUrl is a private helper and is called internally during the process by _applyCurl method. The test ensures “no crash” when exercised through the supported public flow.

animator · 2025-10-02T12:44:10Z

test/dashbot/features/chat/viewmodel/chat_viewmodel_test.dart

+
+      await viewmodel.applyAutoFix(curlAction);
+
+      // Should complete the action without error


Why no expect statement to test if the action was successful?

should test _maybeSubstituteBaseUrl helper method

Same idea as the OpenAPI test.
The helper is private and its result depends on environment variables providers. The stable check is simply that the flow completes without errors.

animator · 2025-10-02T12:44:24Z

test/dashbot/features/chat/viewmodel/chat_viewmodel_test.dart

+      // This should trigger _maybeSubstituteBaseUrlForOpenApi (line 990) through _applyOpenApi
+      await viewmodel.applyAutoFix(openApiAction);
+
+      // Should complete the action without error


Why no expect statement to test if the action was successful?

should test _maybeSubstituteBaseUrlForOpenApi helper method

The purpose of this test is to ensure that the OpenAPI apply flow can run and safely call the base-URL substitution helper.
The helper is private and its result depends on environment variables providers. The stable check is simply that the flow completes without errors.

animator · 2025-10-02T12:46:22Z

test/dashbot/features/chat/viewmodel/chat_viewmodel_test.dart

+          'Messages for computed ID ($computedIdAfter): ${computedMessages?.length ?? 0}');
+
+      // Should now have messages after the bug fix
+      expect(viewmodel.currentMessages, isNotEmpty);


Agree with copilot on this test.. too many debugPrints why not add expect statements to check if things are happening the right way.

I have removed the excessive debug print statements and replace with expect statements in the latest commit

animator · 2025-10-02T12:46:58Z

test/dashbot/features/chat/viewmodel/chat_viewmodel_test.dart

+
+      await viewmodel.applyAutoFix(testAction);
+
+      // All actions should complete without throwing exceptions


Why no expect statement to test if the action was successful?

applyAutoFix should handle different action types

The purpose of this test is to verify the action button doesn’t crash when routing various action types (including unknown ones) to handlers.

The applyAutoFix function itself has private methods(cannot be directly tested) such as _applyOpenApi, _applyCurl, _appendSystem, _applyOtherAction some of these are either covered in other tests or depend on a providers setup(like updating a collection, add a new environment variable, adding a new post request script, etc).

So to avoid such layer of complexity while making sure the feature is stable the main success criterion here is “doesn’t throw any error”

Okay. Got it.

animator · 2025-10-02T12:48:12Z

The deleted test/providers/dashbot_window_notifier_test.dart was written during the initial implementation of dashbot and is supposed to be removed. Now the latest tests for the same exists at test/dashbot/providers/dashbot_window_notifier_test.dart

Got it. Thanks for letting us know.

ashitaprasad

Changes requested as per comments.

animator

LGTM

Udhay-Adithya added 3 commits October 1, 2025 16:18

feat: remove duplicate window notifier test

6e661bc

tests: add tests for service providers

bc2b2a2

tests: improve chat viewmodel tests(cv: 70)

4c07f84

animator requested review from Copilot and animator October 2, 2025 12:27

animator added the gsoc25 label Oct 2, 2025

Copilot AI reviewed Oct 2, 2025

View reviewed changes

animator reviewed Oct 2, 2025

View reviewed changes

ashitaprasad requested changes Oct 3, 2025

View reviewed changes

tests: remove unwanted debug prints

16638a1

animator approved these changes Oct 6, 2025

View reviewed changes

		/// This file contains tests specifically for AI-enabled chat functionality,
		// A mock ChatRemoteRepository returning configurable responses


		await viewmodel.applyAutoFix(curlAction);

		// Should complete the action without error


		await viewmodel.applyAutoFix(testAction);

		// All actions should complete without throwing exceptions

improve dashbot tests #908

Are you sure you want to change the base?

improve dashbot tests #908

Conversation

Udhay-Adithya commented Oct 1, 2025

PR Description

Related Issues

Checklist

Added/updated tests?

OS on which you have developed and tested the feature?

Uh oh!

Udhay-Adithya commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key changes:

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

animator commented Oct 2, 2025

Uh oh!

ashitaprasad left a comment

Choose a reason for hiding this comment

Uh oh!

animator left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Udhay-Adithya commented Oct 1, 2025 •

edited

Loading