Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Jun 30, 2025

Description

This PR fixes failing tests in the ChatView component where the VersionIndicator was appearing when a task was active, despite tests expecting it to only show on the welcome screen.

Changes Made

  • Moved VersionIndicator inside the welcome screen conditional block (!task) in ChatView.tsx
  • Fixed the mock implementation in ChatView.spec.tsx to use the correct import path (../../common/VersionIndicator)
  • Updated the mock to properly handle Vitest hoisting issues using vi.mocked
  • Ensured tests that need to interact with VersionIndicator can override the mock

Testing

  • All existing tests pass
  • Ran the full ChatView test suite: 21 tests passing
  • Verified VersionIndicator only appears when no task exists (welcome screen)
  • Verified VersionIndicator does not appear when there is an active task

Technical Details

The issue was caused by:

  1. The VersionIndicator component being placed outside the conditional rendering logic
  2. The mock path not matching the actual import path in the source file
  3. Vitest hoisting issues when accessing mock variables

The fix ensures proper conditional rendering and correct mocking behavior.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Tests updated and passing
  • No breaking changes

Important

Fix VersionIndicator in ChatView to only display on the welcome screen and update tests accordingly.

  • Behavior:
    • Move VersionIndicator inside the welcome screen conditional block in ChatView.tsx to ensure it only shows when no task is active.
    • Update ChatView.spec.tsx to mock VersionIndicator using the correct import path and handle Vitest hoisting issues with vi.mocked.
    • Ensure tests can override the VersionIndicator mock when needed.
  • Testing:
    • Verified VersionIndicator only appears on the welcome screen and not when a task is active.
    • All existing tests pass, including 21 tests in the full ChatView test suite.

This description was created by Ellipsis for 6daaf22. You can customize this summary. It will automatically update as commits are pushed.

- Move VersionIndicator inside the welcome screen conditional block
- Fix mock implementation to use correct import path
- Update tests to properly mock VersionIndicator visibility
- Resolves test failures where VersionIndicator appeared with active tasks
Copilot AI review requested due to automatic review settings June 30, 2025 17:39
@hannesrudolph hannesrudolph requested review from cte, jr and mrubens as code owners June 30, 2025 17:39
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 30, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR resolves failing tests by ensuring the VersionIndicator is only rendered on the welcome screen when there is no active task. Key changes include:

  • Moving the VersionIndicator component inside the welcome screen conditional in ChatView.tsx.
  • Updating the VersionIndicator mock in ChatView.spec.tsx to use the correct import path and handling for Vitest hoisting.
  • Adjusting the container styling in ChatView.tsx to support proper absolute positioning of the VersionIndicator.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
webview-ui/src/components/chat/tests/ChatView.spec.tsx Updated the mock implementation, import path, and temporary overrides for VersionIndicator tests
webview-ui/src/components/chat/ChatView.tsx Moved VersionIndicator into the conditional rendering block and updated container styling
Comments suppressed due to low confidence (1)

webview-ui/src/components/chat/tests/ChatView.spec.tsx:1158

  • [nitpick] Ensure that the aria-label values for the VersionIndicator mock are consistent across tests to avoid confusion; consider unifying the label ('Version 3.22.5' vs 'chat:versionIndicator.ariaLabel') for clarity.
					aria-label="Version 3.22.5">

@delve-auditor
Copy link

delve-auditor bot commented Jun 30, 2025

No security or compliance issues detected. Reviewed everything up to 6daaf22.

Security Overview
  • 🔎 Scanned files: 2 changed file(s)
Detected Code Changes
Change Type Relevant files
Refactor ► ChatView.tsx
    Relocated version indicator component
    Updated component positioning
► ChatView.spec.tsx
    Extracted version indicator mock into helper function
    Updated version indicator tests

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 30, 2025
- Created createMockVersionIndicator helper to reduce code duplication
- Replaced 5 repeated mock implementations with calls to the helper
- Maintains same functionality with improved maintainability
- Addresses PR review feedback from Copilot
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 30, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 30, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jun 30, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 30, 2025
@mrubens mrubens merged commit 298908f into main Jun 30, 2025
17 checks passed
@mrubens mrubens deleted the fix/version-indicator-test-failures branch June 30, 2025 18:07
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 30, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 30, 2025
hannesrudolph added a commit that referenced this pull request Jul 3, 2025
utarn pushed a commit to modelharbor/ModelHarbor-Agent that referenced this pull request Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Preliminary Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants