test: incremental unit test coverage (67.7% -> 83.7%) - #21
Merged
Conversation
Adds vitest coverage for previously-untested ui/ logic (actions/utils, use-mousetrap, KeyboardShortcuts, the movies list page, and route param parsing), and fixes the vitest coverage config to report all source files instead of only the ones a test happened to import.
Test coverage
|
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This is the first automated coverage run on this repo (no prior
.coverage-ledger.json).Config fix first:
ui/vite.config.ts'stest.coverageblock had noinclude, so the v8 provider only reported files a test happened toimport— 15 of 41 source files — and showed a misleading 99.5%. Addedcoverage.include: ['src/**/*.{ts,tsx}'](with tests/setup/main.tsx excluded) so the report reflects the wholesrc/tree. That alone dropped the honest starting baseline toui/: 220/370 stmts (59.5%). Combined with the backend's unchanged 250/324 (77.2%), the true starting total was 470/694 = 67.7%.New tests, following the repo's existing testify/vitest conventions:
ui/src/actions/utils.test.ts— the fetch wrapper'sget/post/put/delete, including the non-JSON error-body fallback pathui/src/hooks/use-mousetrap.test.ts— bind/unbind/rebind lifecycle, mocking themousetrappackageui/src/shortcuts/KeyboardShortcuts.test.tsx—g h/g mnavigate wiringui/src/pages/movies/list.test.tsx— loading/error/empty/populated states and the create-dialog toolbar button, rendered against the realDataTableui/src/routes.test.tsx— route param parsing for/movies/:idand/movies/:id/edit, exercised through the real router andSideMenulayoutAlso added a
ResizeObserverstub toui/src/test/setup.ts— jsdom doesn't implement it, andSideMenu'sreact-resizable-panelsneeds it just to mount, so nothing rendering that layout could be tested before this.Rendering
list.tsxandroutes.tsxagainst the real component tree (rather than mocking child components) transitively covered several previously-0% shadcn primitives they actually use:data-table.tsx,table.tsx,resizable.tsx,toast.tsx,toaster.tsx.No production code was changed — only the coverage config and new/modified test files.
Coverage delta
+16.0pp against the +15pp target — met.
Measured with:
What was skipped, and why
ui/src/components/ui/alert-dialog.tsx(27 stmts, 0%) is dead code — not imported anywhere insrc/. Testing it would be padding; it should probably be deleted instead (left as-is here since that's a separate call).cmd/movie-catalog/main.go'srun()(44 stmts) blocks on a realnet.Listener/OS signals; testing it meaningfully needs an injectable-listener refactor, which felt like a deliberate decision rather than a nightly one.ui.go's "UI is built" branch — the existingui_test.goalready documents this as intentionally untested, sincedist/only ever holds a real build artifact in this repo, not a test fixture. Respected that convention rather than working around it.Suggested next target
toast.tsx's dismiss/update/pause-on-hover branches andtoaster.tsx's empty-state render need a test drivinguseToast()directly (5+1 stmts).App.tsx(2 stmts) just needs a smoke-render test of the real component. On the backend,main.go'srun()is the largest remaining gap, pending the listener-injection refactor mentioned above.Test plan
cd backend && go test ./...— all pre-existing tests pass, unmodifiedcd ui && yarn test— 73/73 tests pass (8 pre-existing files + 5 new)cd ui && npx tsc -b --noEmit— type-checks cleanGenerated by Claude Code