CodeRabbit Generated Unit Tests: Add renderer unit tests for components and composables #1027
Conversation
…ts and composables
WalkthroughA comprehensive test suite is added across 14 new unit test files for Vue renderer components and composables. Tests cover HTMLArtifact and MessageActionButtons components, and composables spanning artifact handling, UI interactions, message operations, model configuration, and viewport sizing using Vitest and mocking infrastructure. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The diff introduces 14 new test files following consistent testing patterns (setup mocks, arrange-act-assert). While the file count is substantial, homogeneous test structure across composables and components reduces review complexity. Each test file is independent with similar scope, requiring verification of test logic correctness and appropriate mock usage rather than diverse code patterns. Possibly related PRs
Poem
Pre-merge checks✅ Passed checks (3 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
test/renderer/components/HTMLArtifact.test.ts (2)
13-14: Add explicit cleanup for DOM-attached component.When using
attachTo: document.body, the mounted component is attached to the actual DOM. While Vue Test Utils should handle cleanup automatically, it's best practice to explicitly callwrapper.unmount()after assertions to ensure proper cleanup between tests.Apply this diff to add explicit cleanup:
- }) + }), + wrapper.unmount()Or wrap in a try-finally block:
- const wrapper = mount(HTMLArtifact, { - props: { - block: { content: '<html><body>Hello</body></html>', artifact: { type: 'text/html', title: 'doc' } }, - isPreview: true, - viewportSize: 'mobile' - }, - attachTo: document.body - }) - - const iframe = wrapper.find('iframe') - expect(iframe.exists()).toBe(true) - const cls = iframe.attributes('class') || '' - expect(cls).toContain('html-iframe-wrapper') - expect(cls).toContain('border') - - const style = iframe.attributes('style') || '' - expect(style).toContain('width: 375px') - expect(style).toContain('height: 667px') + try { + const wrapper = mount(HTMLArtifact, { + props: { + block: { content: '<html><body>Hello</body></html>', artifact: { type: 'text/html', title: 'doc' } }, + isPreview: true, + viewportSize: 'mobile' + }, + attachTo: document.body + }) + + const iframe = wrapper.find('iframe') + expect(iframe.exists()).toBe(true) + const cls = iframe.attributes('class') || '' + expect(cls).toContain('html-iframe-wrapper') + expect(cls).toContain('border') + + const style = iframe.attributes('style') || '' + expect(style).toContain('width: 375px') + expect(style).toContain('height: 667px') + } finally { + wrapper?.unmount() + }
22-24: Consider making dimension checks more flexible.The test asserts exact pixel values (
width: 375px,height: 667px), which are brittle if mobile viewport constants are updated. Consider checking against the composable's exported constants or using a less strict assertion.Example alternative approach:
const style = iframe.attributes('style') || '' expect(style).toMatch(/width:\s*\d+px/) expect(style).toMatch(/height:\s*\d+px/)Or if the composable exports constants:
import { MOBILE_WIDTH, MOBILE_HEIGHT } from '@/composables/useViewportSize' // ... expect(style).toContain(`width: ${MOBILE_WIDTH}px`) expect(style).toContain(`height: ${MOBILE_HEIGHT}px`)test/renderer/composables/useDragAndDrop.test.ts (1)
5-15: Test bypasses actual timer behavior.The test directly calls
resetDragState()instead of waiting for the timer to execute naturally. This doesn't validate the actual timing behavior that users experience. The comment on line 12 acknowledges this limitation.Consider using Vitest's fake timers to properly test the debounce/timer logic.
Apply this diff to test the actual timer behavior:
+import { describe, it, expect, vi } from 'vitest' -import { describe, it, expect } from 'vitest' import { useDragAndDrop } from '@/components/prompt-input/composables/useDragAndDrop' describe('useDragAndDrop', () => { it('tracks drag state with proper counters and timers', () => { + vi.useFakeTimers() const api = useDragAndDrop() const evt = { dataTransfer: { types: ['Files'] } } as any as DragEvent api.handleDragEnter(evt) expect(api.isDragging.value).toBe(true) api.handleDragOver() api.handleDragLeave() - // we cannot await timer here; call reset directly - api.resetDragState() + + // Advance timers to trigger the debounced reset + vi.runAllTimers() expect(api.isDragging.value).toBe(false) + vi.useRealTimers() }) })test/renderer/composables/useArtifactContext.test.ts (1)
15-18: Fragile assertion assumes componentKey increments.Line 18 uses
toBeGreaterThan(prevKey), which assumescomponentKeyis a monotonically increasing counter. If the implementation changes to use a hash, timestamp, or other non-sequential identifier, this assertion will break despite the component working correctly.Apply this diff to make the assertion more resilient:
// same artifact id but different message produces new key messageId.value = 'm-2' expect(activeArtifactContext.value).toBe('t-1:m-2:art-1') - expect(componentKey.value).toBeGreaterThan(prevKey) + expect(componentKey.value).not.toBe(prevKey)This validates that the key changes (which is the actual requirement) without assuming a specific implementation detail.
test/renderer/composables/useModelCapabilities.test.ts (1)
24-28: Incomplete verification of reset behavior.The reset test (lines 24-28) only verifies
supportsReasoningandbudgetRangebecome null whenproviderIdis undefined, but doesn't check whethersupportsSearchandsearchDefaultsalso reset. This creates incomplete test coverage for the reset logic.Apply this diff to verify all capabilities reset:
// reset path providerId.value = undefined await Promise.resolve() expect(api.supportsReasoning.value).toBeNull() expect(api.budgetRange.value).toBeNull() + expect(api.supportsSearch.value).toBeNull() + expect(api.searchDefaults.value).toBeNull()test/renderer/composables/useArtifactExport.test.ts (1)
42-42: Consider moving navigator.clipboard mock to beforeEach for consistency.The inline
Object.assign(navigator, ...)mock could persist across tests. While Vitest typically isolates tests, it's more maintainable to set up persistent mocks inbeforeEachalongside the other setup.beforeEach(() => { // @ts-ignore global.URL.createObjectURL = vi.fn(() => 'blob://x') // @ts-ignore global.URL.revokeObjectURL = vi.fn() vi.spyOn(document.body, 'appendChild') vi.spyOn(document.body, 'removeChild') + Object.assign(navigator, { clipboard: { writeText: vi.fn().mockResolvedValue(void 0) } }) })Then remove the inline assignment from line 42.
test/renderer/composables/useInputHistory.test.ts (1)
4-14: Add beforeEach to reset shared mock state between tests.The module-level
entriesandidxvariables are shared across all tests. While the current tests don't mutate this state, future tests that calladdSearchwould cause state to leak between tests, leading to non-deterministic behavior.Add a
beforeEachhook to reset the state:+import { describe, it, expect, vi, beforeEach } from 'vitest' -import { describe, it, expect, vi } from 'vitest'describe('useInputHistory', () => { + beforeEach(() => { + entries = ['alpha', 'beta', 'gamma'] + idx = entries.length + }) + it('manages placeholder and confirms fill', () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
test/renderer/components/HTMLArtifact.test.ts(1 hunks)test/renderer/components/MessageActionButtons.test.ts(1 hunks)test/renderer/composables/useArtifactContext.test.ts(1 hunks)test/renderer/composables/useArtifactExport.test.ts(1 hunks)test/renderer/composables/useArtifactViewMode.test.ts(1 hunks)test/renderer/composables/useDragAndDrop.test.ts(1 hunks)test/renderer/composables/useInputHistory.test.ts(1 hunks)test/renderer/composables/useMessageCapture.test.ts(1 hunks)test/renderer/composables/useMessageScroll.test.ts(1 hunks)test/renderer/composables/useModelCapabilities.test.ts(1 hunks)test/renderer/composables/useModelTypeDetection.test.ts(1 hunks)test/renderer/composables/useSearchConfig.test.ts(1 hunks)test/renderer/composables/useThinkingBudget.test.ts(1 hunks)test/renderer/composables/useViewportSize.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: AGENTS.md:0-0
Timestamp: 2025-10-14T08:02:59.495Z
Learning: Use Vitest (+ jsdom) and Vue Test Utils as the testing framework
📚 Learning: 2025-10-14T08:02:59.495Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: AGENTS.md:0-0
Timestamp: 2025-10-14T08:02:59.495Z
Learning: Use Vitest (+ jsdom) and Vue Test Utils as the testing framework
Applied to files:
test/renderer/composables/useArtifactViewMode.test.tstest/renderer/composables/useModelTypeDetection.test.tstest/renderer/composables/useArtifactExport.test.tstest/renderer/components/HTMLArtifact.test.tstest/renderer/composables/useSearchConfig.test.ts
🔇 Additional comments (10)
test/renderer/composables/useSearchConfig.test.ts (1)
5-24: LGTM! Test comprehensively covers reactive behavior.The test validates all three key scenarios:
- Search supported with complete defaults
- Search not supported (showSearchConfig becomes false)
- Search supported but missing defaults (options become false)
The assertions correctly verify the computed reactive properties respond to input changes.
test/renderer/composables/useMessageCapture.test.ts (2)
30-50: LGTM! Test correctly validates message capture with parent/child relationship.The test properly sets up DOM elements with data attributes, invokes the capture API, and cleans up manually appended elements.
52-66: LGTM! Test correctly validates "from top" capture behavior.The test properly creates message elements in order and verifies the fromTop flag functionality with appropriate cleanup.
test/renderer/composables/useArtifactViewMode.test.ts (1)
9-34: LGTM! Tests comprehensively verify view mode behavior.The tests effectively validate:
- Auto-preview logic based on artifact type
- User preference persistence and reset on artifact change
- Loading status gating of preview mode
The reactive state changes are properly tested with clear assertions.
test/renderer/composables/useArtifactExport.test.ts (2)
10-17: LGTM! Mock setup is appropriate for blob download testing.The beforeEach correctly mocks URL APIs and spies on DOM methods needed for download simulation. Vitest will automatically restore these between tests.
22-57: LGTM! Tests comprehensively cover export functionality.The test suite validates:
- SVG export via mermaid rendering and download
- Error handling for invalid SVG content
- Code export and clipboard operations
- Image capture delegation with proper configuration
All assertions are appropriate and test logic is sound.
test/renderer/composables/useInputHistory.test.ts (1)
25-52: LGTM! Tests effectively validate input history behavior.The tests properly verify:
- Dynamic placeholder updates and history confirmation
- Arrow key navigation conditional on empty input state
- Editor command invocation with correct arguments
Test logic and assertions are sound.
test/renderer/composables/useThinkingBudget.test.ts (1)
10-54: LGTM! Tests thoroughly validate thinking budget logic.Once the import issue is fixed, these tests effectively verify:
- Conditional display based on reasoning support and budget range presence
- Min/max validation with appropriate error messages
- Gemini provider's special -1 value handling
The reactive state management and validation logic are properly exercised.
test/renderer/composables/useMessageScroll.test.ts (2)
4-14: LGTM! IntersectionObserver mock is appropriate for testing.The minimal IO class provides the necessary interface for the scroll observer, triggering the callback immediately with
isIntersecting: falseto test threshold behavior.
16-46: LGTM! Test comprehensively validates scroll behavior.The test effectively verifies:
- Scroll observer initialization and threshold detection
- Programmatic scroll to bottom with proper async handling
- Scroll to specific message by ID with DOM cleanup
The use of Promise.resolve() to yield to the microtask queue is appropriate, and DOM cleanup is properly handled.
| await wrapper.find('[key="new-chat"]').trigger('click') | ||
| await wrapper.find('[key="scroll-bottom"]').trigger('click') |
There was a problem hiding this comment.
🧩 Analysis chain
The key attribute selector will likely fail.
Vue's key attribute is used for internal list reconciliation and is not rendered to the DOM. The selectors [key="new-chat"] and [key="scroll-bottom"] won't match any elements unless the component explicitly renders a key HTML attribute (which would be non-standard).
Consider using data-testid attributes instead for test-friendly element selection.
Apply this diff to use proper test selectors:
- await wrapper.find('[key="new-chat"]').trigger('click')
- await wrapper.find('[key="scroll-bottom"]').trigger('click')
+ await wrapper.find('[data-testid="new-chat"]').trigger('click')
+ await wrapper.find('[data-testid="scroll-bottom"]').trigger('click')Ensure the MessageActionButtons component includes corresponding data-testid attributes on the clickable elements.
Run this script to verify how the component renders these elements:
🏁 Script executed:
#!/bin/bash
# Check if MessageActionButtons.vue uses key as a DOM attribute or data-testid
rg -n -C3 'new-chat|scroll-bottom' --type=vue --type=tsLength of output: 27
I need to search for the MessageActionButtons component file and examine its actual implementation.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the MessageActionButtons component file
fd -name 'MessageActionButtons*' --type f
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Search for references to new-chat and scroll-bottom in test file
rg -n 'new-chat|scroll-bottom' test/renderer/components/MessageActionButtons.test.ts -A2 -B2Length of output: 260
🏁 Script executed:
#!/bin/bash
# Search for MessageActionButtons component implementation
rg -n 'new-chat|scroll-bottom' --type=ts --type=js -A2 -B2 | head -100Length of output: 1444
🏁 Script executed:
#!/bin/bash
# Look for data-testid or key attributes in component files
find . -name 'MessageActionButtons*' -type f | head -20Length of output: 118
I need to verify how Vue Test Utils handles key attribute selectors, as this is critical to confirming whether the original review comment is correct.
---
🌐 Web query:
```web
Vue Test Utils find key attribute selector
💡 Result:
Short answer: you can't select by Vue "key" because key is a virtual-DOM-only attribute (not exposed on the rendered DOM or component API). Instead use one of these approaches:
- Add a stable DOM attribute (data-test, id, class) and query that: wrapper.find('[data-test="item-42"]').
- If selecting a child component in a v-for, use findAllComponents() and filter by a prop or other exposed value:
const items = wrapper.findAllComponents(MyItem);
const item = items.wrappers.find(w => w.props('id') === '42'); - Or give the item a ref (or data-test) when you need direct access.
References: Vue Test Utils docs (selectors / find / findAll) and community answer explaining key is not queryable. [1][2]
Sources
[1] Stack Overflow — "Can a component be found using its key?" (explains key not accessible).
[2] Vue Test Utils API / selectors (find, findAll, attributes).
The review comment is correct about the problem, but the solution is incomplete—both the component and test files need updates.
The web search confirms Vue's key attribute is virtual-DOM-only and cannot be queried via CSS selectors like [key="..."]. The test selectors will fail.
However, the fix requires changes to both files:
- Update test selectors:
[key="..."]→[data-testid="..."](lines 10-11 in test file) - Add
data-testidattributes to the Button elements in the component template (lines 18 and 29 insrc/renderer/src/components/message/MessageActionButtons.vue)
The component file currently only has key directives without corresponding data-testid attributes, so applying only the test changes would not resolve the issue.
🤖 Prompt for AI Agents
In test/renderer/components/MessageActionButtons.test.ts around lines 10-11 and
src/renderer/src/components/message/MessageActionButtons.vue around lines 18 and
29: the test uses CSS selectors querying Vue-only key attributes ([key="..."])
which cannot be selected at runtime; update the test to use data-testid
selectors (e.g., [data-testid="new-chat"] and [data-testid="scroll-bottom"]) and
add matching data-testid attributes to the corresponding Button elements in the
component template at the indicated lines (keep the existing key attributes
intact). Ensure attribute names match exactly between test and component.
| beforeEach(() => { | ||
| // container | ||
| const container = document.createElement('div') | ||
| container.className = 'message-list-container' | ||
| document.body.appendChild(container) | ||
| }) |
There was a problem hiding this comment.
Add cleanup in afterEach to prevent DOM pollution across tests.
The beforeEach hook creates and appends a container element to document.body, but there's no corresponding cleanup. This will cause the container to accumulate across test runs, potentially causing memory leaks and test interference.
Add an afterEach hook to clean up:
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
-import { describe, it, expect, vi, beforeEach } from 'vitest' describe('useMessageCapture', () => {
+ let container: HTMLElement
+
beforeEach(() => {
- const container = document.createElement('div')
+ container = document.createElement('div')
container.className = 'message-list-container'
document.body.appendChild(container)
})
+
+ afterEach(() => {
+ if (container && document.body.contains(container)) {
+ document.body.removeChild(container)
+ }
+ })🤖 Prompt for AI Agents
In test/renderer/composables/useMessageCapture.test.ts around lines 23 to 28,
the beforeEach creates and appends a .message-list-container but no cleanup is
run; add an afterEach that removes the appended container to avoid DOM pollution
across tests (e.g., query document.querySelector('.message-list-container') and
remove it if present, or reset document.body.innerHTML = ''), ensuring the DOM
is restored after each test.
| const modelId = ref<string|undefined>('gpt-5-pro') | ||
| const providerId = ref<string|undefined>('gemini') | ||
| const modelType = ref<'chat'|'imageGeneration'|'embedding'|'rerank'|undefined>('imageGeneration') |
There was a problem hiding this comment.
Test uses contradictory model and provider combination.
The test sets modelId to 'gpt-5-pro' (an OpenAI model) while providerId is set to 'gemini' (Google's provider). This combination is unrealistic and creates a misleading test scenario.
While the test may pass due to the composable performing independent string checks, it doesn't validate realistic usage patterns and could mask integration issues.
Apply this diff to use consistent test data:
- const modelId = ref<string|undefined>('gpt-5-pro')
- const providerId = ref<string|undefined>('gemini')
+ const modelId = ref<string|undefined>('gemini-2.0-flash')
+ const providerId = ref<string|undefined>('gemini')
const modelType = ref<'chat'|'imageGeneration'|'embedding'|'rerank'|undefined>('imageGeneration')
const api = useModelTypeDetection({ modelId, providerId, modelType })
expect(api.isImageGenerationModel.value).toBe(true)
- expect(api.isGPT5Model.value).toBe(true)
+ expect(api.isGPT5Model.value).toBe(false)
expect(api.isGeminiProvider.value).toBe(true)Or test each provider/model combination separately:
it('detects OpenAI GPT-5 model', async () => {
const modelId = ref<string|undefined>('gpt-5-pro')
const providerId = ref<string|undefined>('openai')
// ... test OpenAI-specific behavior
})
it('detects Gemini provider', async () => {
const modelId = ref<string|undefined>('gemini-2.0-flash')
const providerId = ref<string|undefined>('gemini')
// ... test Gemini-specific behavior
})🤖 Prompt for AI Agents
In test/renderer/composables/useModelTypeDetection.test.ts around lines 13-15,
the test sets modelId='gpt-5-pro' while providerId='gemini', an inconsistent
OpenAI/Gemini pairing; update the test to use a consistent provider/model pair
(e.g. set providerId='openai' to match 'gpt-5-pro') or split into two separate
tests that each use realistic combinations (one for OpenAI models with
providerId='openai' and one for Gemini models with providerId='gemini') so the
test scenarios reflect real usage.
| @@ -0,0 +1,55 @@ | |||
| import { ref, computed } from 'vue' | |||
| import { describe, it, expect } from 'vitest' | |||
There was a problem hiding this comment.
Missing vi import causes ReferenceError.
Line 6 uses vi.mock() but vi is not imported from 'vitest' on line 2. This will cause a ReferenceError when the test file is executed.
Apply this fix:
-import { describe, it, expect } from 'vitest'
+import { describe, it, expect, vi } from 'vitest'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { describe, it, expect } from 'vitest' | |
| import { describe, it, expect, vi } from 'vitest' |
🤖 Prompt for AI Agents
In test/renderer/composables/useThinkingBudget.test.ts around line 2, the test
uses vi.mock() but vi was not imported, causing a ReferenceError; update the
import from 'vitest' to include vi (e.g., add vi to the named imports alongside
describe, it, expect) so vi.mock() is available to the test runner.
| api.setViewportSize('mobile') | ||
| expect(api.getDimensions()).toEqual({ width: api.MOBILE_WIDTH, height: api.MOBILE_HEIGHT }) |
There was a problem hiding this comment.
Missing assertion for viewportSize after setting to mobile.
After calling setViewportSize('mobile') on line 14, the test only checks getDimensions() but doesn't verify that viewportSize.value actually updated to 'mobile'. This is inconsistent with the tablet test path (lines 10-11) and could miss a state update bug.
Apply this diff to add the missing assertion:
api.setViewportSize('mobile')
+ expect(api.viewportSize.value).toBe('mobile')
expect(api.getDimensions()).toEqual({ width: api.MOBILE_WIDTH, height: api.MOBILE_HEIGHT })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| api.setViewportSize('mobile') | |
| expect(api.getDimensions()).toEqual({ width: api.MOBILE_WIDTH, height: api.MOBILE_HEIGHT }) | |
| api.setViewportSize('mobile') | |
| expect(api.viewportSize.value).toBe('mobile') | |
| expect(api.getDimensions()).toEqual({ width: api.MOBILE_WIDTH, height: api.MOBILE_HEIGHT }) |
🤖 Prompt for AI Agents
In test/renderer/composables/useViewportSize.test.ts around lines 14 to 15, the
test calls api.setViewportSize('mobile') and asserts dimensions but does not
assert that viewportSize.value was updated; add a line after the setViewportSize
call to assert that api.viewportSize.value === 'mobile' (mirroring the tablet
path assertions on lines 10-11) so the reactive state change is verified.
Unit test generation was requested by @deepinfect.
The following files were modified:
test/renderer/components/HTMLArtifact.test.tstest/renderer/components/MessageActionButtons.test.tstest/renderer/composables/useArtifactContext.test.tstest/renderer/composables/useArtifactExport.test.tstest/renderer/composables/useArtifactViewMode.test.tstest/renderer/composables/useDragAndDrop.test.tstest/renderer/composables/useInputHistory.test.tstest/renderer/composables/useMessageCapture.test.tstest/renderer/composables/useMessageScroll.test.tstest/renderer/composables/useModelCapabilities.test.tstest/renderer/composables/useModelTypeDetection.test.tstest/renderer/composables/useSearchConfig.test.tstest/renderer/composables/useThinkingBudget.test.tstest/renderer/composables/useViewportSize.test.tsSummary by CodeRabbit