test: add 50 tests for 6 previously untested hooks#1625
Conversation
Adds comprehensive test coverage for hooks that had zero tests: - useToast: 16 tests (show/dismiss, auto-dismiss, pause/resume, helpers) - useMetaKeyDown: 5 tests (keydown/keyup, blur reset) - useAnimatedPresence: 7 tests (mount/unmount, exit delay, reduced motion) - usePromptAutocomplete: 13 tests (token extraction, suggestions, accept) - useWaveform: 4 tests (null key, async loading, peak computation) - useReducedMotion: 4 tests (media query sync, manual override) All 7070 tests pass. Zero TypeScript errors. https://claude.ai/code/session_01AAbmUU34sN8t5XRPsvuyzh
There was a problem hiding this comment.
Pull request overview
Adds Vitest coverage for six previously untested React hooks, addressing #1622 by introducing a new test suite for toast notifications, keyboard/meta tracking, animated presence, prompt autocomplete, waveform loading, and reduced motion syncing.
Changes:
- Added 50 new Vitest tests across 6 new hook test files.
- Added mocks/stubs for audio engine, waveform peak computation, and media-query behavior to support hook testing.
- Validated hook behaviors including timers, keyboard events, and store/media-query synchronization.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/tests/useToast.test.ts | Adds store/timer behavior tests for toast creation, dismissal, pause/resume, and helpers. |
| src/hooks/tests/useMetaKeyDown.test.ts | Adds keyboard event tests for Meta key down/up tracking and blur reset. |
| src/hooks/tests/useAnimatedPresence.test.ts | Adds tests for mount/unmount timing, exit delay, cancellation, and reduced-motion behavior. |
| src/hooks/tests/usePromptAutocomplete.test.ts | Adds tests for token extraction and basic open/close/accept/dismiss behavior. |
| src/hooks/tests/useWaveform.test.ts | Adds tests for null key behavior, async waveform loading, and default numPeaks. |
| src/hooks/tests/useReducedMotion.test.ts | Adds tests for matchMedia listener wiring and store updates respecting override state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('loads peaks for valid audioKey', async () => { | ||
| const { result } = renderHook(() => useWaveform('audio-key-1', 100)); | ||
| // Wait for async loading | ||
| await act(async () => { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)); | ||
| }); | ||
| expect(result.current).toHaveLength(100); |
There was a problem hiding this comment.
These tests wait for the async effect by sleeping setTimeout(..., 10), which is timing-dependent and can be flaky on slow/loaded CI. Prefer waitFor (e.g., wait until result.current becomes non-null/has expected length) or await the mocked promises directly so the test only proceeds when the state update has happened.
| it('resets to null when audioKey changes to null', async () => { | ||
| const { result, rerender } = renderHook( | ||
| ({ key }) => useWaveform(key), | ||
| { initialProps: { key: 'audio-1' as string | null } }, | ||
| ); | ||
| await act(async () => { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)); | ||
| }); |
There was a problem hiding this comment.
Same as above: relying on an arbitrary 10ms timeout to let effects run makes this test timing-dependent. Use waitFor (or another deterministic mechanism) to wait for the hook state to update before asserting, and to ensure the rerender-to-null path is actually processed after the effect runs.
| it('uses default numPeaks of 100', async () => { | ||
| const { computeWaveformPeaks } = await import('../../utils/waveformPeaks'); | ||
| const { result } = renderHook(() => useWaveform('audio-1')); | ||
| await act(async () => { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)); | ||
| }); | ||
| expect(vi.mocked(computeWaveformPeaks)).toHaveBeenCalledWith( |
There was a problem hiding this comment.
This test also uses a fixed setTimeout(..., 10) to wait for async work, which can be flaky. Prefer waitFor(() => expect(computeWaveformPeaks).toHaveBeenCalled()) (or waiting for result.current to be non-null) so the assertion is synchronized to the actual completion of the effect.
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| // Ensure prefers-reduced-motion is not set | ||
| Object.defineProperty(window, 'matchMedia', { | ||
| writable: true, | ||
| value: vi.fn().mockImplementation((query: string) => ({ | ||
| matches: false, | ||
| media: query, | ||
| addEventListener: vi.fn(), | ||
| removeEventListener: vi.fn(), | ||
| })), | ||
| }); |
There was a problem hiding this comment.
window.matchMedia is overwritten in beforeEach, but never restored. This can leak into other test files (and into store initializers that read matchMedia at import time), making the suite order-dependent. Save the original matchMedia and restore it in afterEach (or use vi.stubGlobal + vi.unstubAllGlobals() / vi.restoreAllMocks()).
| if (result.current.suggestions.length > 0) { | ||
| expect(result.current.isOpen).toBe(true); | ||
| } |
There was a problem hiding this comment.
This test conditionally asserts isOpen only when suggestions.length > 0, which means it can silently pass even if the hook incorrectly returns no suggestions for a known token (e.g. "pop"). Since getPromptSuggestions('pop') is expected to return at least one result, assert suggestions.length > 0 and isOpen === true unconditionally to make the test meaningful.
| if (result.current.suggestions.length > 0) { | |
| expect(result.current.isOpen).toBe(true); | |
| } | |
| expect(result.current.suggestions.length).toBeGreaterThan(0); | |
| expect(result.current.isOpen).toBe(true); |
| beforeEach(() => { | ||
| addListenerSpy = vi.fn(); | ||
| removeListenerSpy = vi.fn(); | ||
| useUIStore.setState({ reducedMotionOverride: false }); | ||
| Object.defineProperty(window, 'matchMedia', { | ||
| writable: true, | ||
| value: vi.fn().mockImplementation((query: string) => ({ | ||
| matches: false, | ||
| media: query, | ||
| addEventListener: addListenerSpy, | ||
| removeEventListener: removeListenerSpy, | ||
| })), | ||
| }); |
There was a problem hiding this comment.
useUIStore initializes reducedMotion from window.matchMedia(...) at module import time, but this file only stubs window.matchMedia inside beforeEach (after the store has already been imported). That makes the store’s initial reducedMotion value depend on whichever matchMedia was present earlier in the test run. To keep tests deterministic, explicitly reset useUIStore state (at least reducedMotion + reducedMotionOverride) in beforeEach, and restore the original window.matchMedia in afterEach so it doesn’t leak to other suites.
- usePromptAutocomplete: make assertion unconditional — assert suggestions.length > 0 and isOpen === true for 'pop' token - useWaveform: replace setTimeout(10) with waitFor() for deterministic async assertions (3 occurrences) - useAnimatedPresence: save/restore original window.matchMedia in afterEach to prevent leaking into other test suites - useReducedMotion: explicitly reset reducedMotion + reducedMotionOverride in beforeEach, restore matchMedia in afterEach https://claude.ai/code/session_01AAbmUU34sN8t5XRPsvuyzh
Summary
Test plan
npm test— all 7070 tests passnpx tsc --noEmit— 0 errorsCloses #1622
https://claude.ai/code/session_01AAbmUU34sN8t5XRPsvuyzh