TST-49: Frontend view and component coverage gaps#817
Conversation
…Alert, TdSkeleton, TdSpinner, TdTag, TdToast)
… ShellKeyboardHelp, KeyboardShortcutsHelp, ToastContainer, WorkspaceHelpCallout)
Self-review findingsWhat was coveredAll 16 previously untested Vue components now have test files: UI primitives (7): TdBadge, TdEmptyState, TdInlineAlert, TdSkeleton, TdSpinner, TdTag, TdToast Quality checks passed
What tests did NOT cover (by design)
Verification
|
There was a problem hiding this comment.
Pull request overview
This PR closes frontend component coverage gaps by adding new Vitest/@vue/test-utils specs for previously untested UI primitives and shell/common/activity components, aligning with the test-coverage goals in #716.
Changes:
- Adds 16 new unit test files covering UI primitives, shell/common components, activity components, and the PWA service-worker update prompt.
- Introduces focused mocks for stores/composables (e.g., toast store, workspace/session stores, virtual list composable) to validate component behavior deterministically.
- Verifies key accessibility attributes/roles across multiple components (e.g.,
role,aria-live, dialog semantics).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/taskdeck-web/src/tests/components/WorkspaceHelpCallout.spec.ts | Adds coverage for help callout visible/dismissed states, slots, and actions. |
| frontend/taskdeck-web/src/tests/components/ui/TdToast.spec.ts | Tests toast variants, dismiss behavior, and a11y attributes. |
| frontend/taskdeck-web/src/tests/components/ui/TdTag.spec.ts | Tests tag slot rendering, removable behavior, and custom color styling. |
| frontend/taskdeck-web/src/tests/components/ui/TdSpinner.spec.ts | Tests spinner label/size defaults, SVG structure, and role="status". |
| frontend/taskdeck-web/src/tests/components/ui/TdSkeleton.spec.ts | Tests skeleton sizing defaults, rounded/circle behavior, and aria-hidden. |
| frontend/taskdeck-web/src/tests/components/ui/TdInlineAlert.spec.ts | Tests alert variants, dismissibility, emitted events, and a11y role. |
| frontend/taskdeck-web/src/tests/components/ui/TdEmptyState.spec.ts | Tests title/description rendering and icon/action slots. |
| frontend/taskdeck-web/src/tests/components/ui/TdBadge.spec.ts | Tests badge variants/sizes and slot rendering. |
| frontend/taskdeck-web/src/tests/components/ToastContainer.spec.ts | Tests toast store integration, roles for error toasts, and close behavior. |
| frontend/taskdeck-web/src/tests/components/SwUpdatePrompt.spec.ts | Tests SW update prompt lifecycle, update/dismiss actions, and a11y attributes. |
| frontend/taskdeck-web/src/tests/components/ShellTopbar.spec.ts | Tests workspace mode selector, mode descriptions, and emitted palette event. |
| frontend/taskdeck-web/src/tests/components/ShellSidebar.spec.ts | Tests nav rendering across modes, feature-flag behavior, badges, and emitted events. |
| frontend/taskdeck-web/src/tests/components/ShellKeyboardHelp.spec.ts | Tests teleported keyboard help dialog rendering and close behavior. |
| frontend/taskdeck-web/src/tests/components/KeyboardShortcutsHelp.spec.ts | Tests teleported shortcuts dialog content and close actions. |
| frontend/taskdeck-web/src/tests/components/ActivitySelector.spec.ts | Tests selector controls, emitted updates, fetch/copy actions, and conditional UI. |
| frontend/taskdeck-web/src/tests/components/ActivityResults.spec.ts | Tests loading/empty/timeline states with mocked virtual list and formatting helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('uses custom dismiss and replay labels', () => { | ||
| visibleRef.value = true | ||
| const wrapper = mount(WorkspaceHelpCallout, { | ||
| props: { | ||
| topic: 'board', | ||
| title: 'Board', | ||
| description: 'Desc', | ||
| dismissLabel: 'Got it', | ||
| }, | ||
| }) | ||
| const dismissBtn = wrapper.findAll('button').find((b) => b.text().includes('Got it')) | ||
| expect(dismissBtn).toBeDefined() | ||
| }) |
There was a problem hiding this comment.
Test case name says it "uses custom dismiss and replay labels", but the test only sets/asserts dismissLabel (no replayLabel is provided or verified). Either rename the test to match what it covers, or also pass replayLabel and assert it appears in the dismissed state to keep intent aligned with coverage.
| vi.mock('../../composables/useEscapeStack', () => ({ | ||
| registerEscapeHandler: vi.fn(() => vi.fn()), | ||
| })) |
There was a problem hiding this comment.
This spec mocks ../../composables/useEscapeStack, but ShellKeyboardHelp.vue does not import or use that composable. The unused mock adds noise and can be removed to keep the test focused on the component’s actual dependencies.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of unit tests for various frontend components using Vitest and Vue Test Utils. The tests cover activity-related components, shell layout elements like the sidebar and topbar, UI primitives, and utility components. Feedback focuses on improving test safety in ActivityResults by replacing non-null assertions with explicit existence checks and reducing boilerplate in the ShellSidebar tests by implementing a mounting helper function.
| const reviewBtn = wrapper.findAll('button').find((b) => b.text() === 'Open Review') | ||
| await reviewBtn!.trigger('click') |
There was a problem hiding this comment.
Using a non-null assertion (!) on the result of find can lead to cryptic errors if the button is not found. It's safer to explicitly assert that the button exists before triggering an event.
| const reviewBtn = wrapper.findAll('button').find((b) => b.text() === 'Open Review') | |
| await reviewBtn!.trigger('click') | |
| const reviewBtn = wrapper.findAll('button').find((b) => b.text() === 'Open Review') | |
| expect(reviewBtn).toBeDefined() | |
| await reviewBtn!.trigger('click') |
| it('renders the Taskdeck brand title', () => { | ||
| const wrapper = mount(ShellSidebar, { | ||
| props: { isAuthenticated: true }, | ||
| global: { stubs: { 'router-link': { template: '<a><slot /></a>', props: ['to'] } } }, | ||
| }) |
There was a problem hiding this comment.
The router-link stub and mounting configuration are repeated across many tests in this file. Creating a helper function for mounting the component would significantly reduce boilerplate and improve maintainability.
| it('renders the Taskdeck brand title', () => { | |
| const wrapper = mount(ShellSidebar, { | |
| props: { isAuthenticated: true }, | |
| global: { stubs: { 'router-link': { template: '<a><slot /></a>', props: ['to'] } } }, | |
| }) | |
| const mountSidebar = (props = { isAuthenticated: true }) => mount(ShellSidebar, { | |
| props, | |
| global: { stubs: { 'router-link': { template: '<a><slot /></a>', props: ['to'] } } }, | |
| }) | |
| it('renders the Taskdeck brand title', () => { | |
| const wrapper = mountSidebar() |
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Review: Quality improvements for new test files
Good coverage addition overall -- 16 test files with meaningful assertions across all component categories. The tests go well beyond "renders without crashing" and check interactions, slots, props, emissions, and accessibility attributes. Here are the issues I'm fixing:
Bot-flagged issues (all confirmed)
-
WorkspaceHelpCallout.spec.ts line 110: Test named "uses custom dismiss and replay labels" only tests
dismissLabel-- never passes or assertsreplayLabel. Will addreplayLabelcoverage in the dismissed state. -
ShellKeyboardHelp.spec.ts line 5-7: Mocks
useEscapeStackbut the component (ShellKeyboardHelp.vue) does not import or use it. Removing the unused mock. -
ActivityResults.spec.ts line 82: Non-null assertion (
!) on.find()result can produce cryptic runtime errors when the element is not found. Fixing across all new files.
Additional findings from adversarial review
-
Non-null assertions without guard across multiple files: The
reviewBtn!.trigger('click')pattern appears in ActivityResults (lines 82, 86), ActivitySelector (115, 124, 148), WorkspaceHelpCallout (60, 88), and KeyboardShortcutsHelp (97). In each case the precedingexpect(...).toBeDefined()is not sufficient because TypeScript still sees| undefined. Will replace with explicitexpect(btn).toBeTruthy()followed by a guarded call, or useif (!btn) throw ...pattern. -
ShellSidebar.spec.ts line 216:
inboxItem?.classes()uses optional chaining, but if the element is missing,expect(undefined).toContain(...)gives an unclear failure message. Will add existence assertion. -
ShellSidebar.spec.ts: Repeated router-link stub definition in every test (12 times). Extracting to a shared
mountSidebar()helper to reduce boilerplate and make tests easier to maintain.
All fixes incoming as focused commits.
…ve non-null assertions Split "uses custom dismiss and replay labels" into two focused tests that each cover one prop. Add missing replayLabel assertion in dismissed state. Replace non-null assertions with toBeTruthy() + optional chaining.
ShellKeyboardHelp.vue does not import useEscapeStack, so the mock served no purpose and was misleading about the component's dependencies.
…s test Use toBeTruthy() to assert element existence before interacting, then optional chaining instead of ! to avoid cryptic runtime errors if the element is unexpectedly missing.
…or test Add explicit existence checks before interacting with dynamically found buttons. Use optional chaining instead of non-null assertions.
…sHelp test Use toBeTruthy() + optional chaining for the Got it button interaction to produce a clear failure message if the element is missing.
Replace 14 repeated inline mount+stubs configurations with a shared mountSidebar() helper. Add explicit existence assertion for the active route test to prevent unclear failures from optional chaining.
Summary
Closes #716
Test plan