[3/7] Add frontend test infrastructure (Vitest + Testing Library) - #274
Open
alex-clickhouse wants to merge 1 commit into
Open
[3/7] Add frontend test infrastructure (Vitest + Testing Library)#274alex-clickhouse wants to merge 1 commit into
alex-clickhouse wants to merge 1 commit into
Conversation
This was referenced Aug 5, 2026
There was a problem hiding this comment.
Pull request overview
Adds a frontend testing harness for the web/ app using Vitest + jsdom + Testing Library, and introduces an initial worked example test suite for the shared Modal component. This closes the current gap where frontend changes are only validated via typechecking and bundling.
Changes:
- Configure Vitest in
vite.config.ts(jsdom env, setup file, scoped test glob, CSS disabled). - Add global test setup shims and cleanup in
src/test/setup.ts. - Add a comprehensive
Modalinteraction test suite and wirenpm testinto CI.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
web/vite.config.ts |
Adds Vitest configuration alongside existing Vite config to share resolve/plugins. |
web/src/test/setup.ts |
Establishes global test setup (jest-dom, cleanup, jsdom shims). |
web/src/components/ui/Modal.test.tsx |
Adds behavioral specs for the Modal component (dismissal, focus, stacking, scroll lock). |
web/package.json |
Adds test scripts and Vitest/Testing Library/jsdom devDependencies. |
web/package-lock.json |
Locks new testing dependencies and transitive packages. |
.github/workflows/ci.yml |
Runs npm test in the existing frontend CI job. |
Files not reviewed (1)
- web/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 tasks
web/ had no test tooling and no test files. Everything in the frontend was verified by `tsc -b` and `vite build`, which prove code compiles and bundles — and say nothing about whether it behaves. Sets up Vitest with jsdom and Testing Library, wires `npm test` into the existing frontend CI job, and takes the Modal as its first subject. The Modal is close to an ideal starting point: it is almost entirely interaction (Escape precedence, focus trapping, stacking, the mousedown-not-click backdrop), so essentially none of it was reachable by the checks already in place. That was not hypothetical. Writing these specs immediately surfaced a bug the type checker could not: the body scroll lock read modalStack.length to decide whether it was the last dialog out, but React runs effect cleanups in declaration order, so the closing dialog was still in the stack and overflow:hidden was never lifted. Closing any dialog left the page unscrollable. Fixed in the preceding commit. Two jsdom gaps are patched in the setup file rather than worked around per test. requestAnimationFrame is missing, and the Modal defers its initial focus through it. More subtly, jsdom reports offsetParent as null for every element because nothing has layout — and the focus trap filters candidates on exactly that, so without the shim the trap would find nothing focusable and every trap assertion would pass vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
alex-clickhouse
force-pushed
the
alex-clickhouse/web-test-harness
branch
from
August 5, 2026 12:07
bb10782 to
adc2904
Compare
alex-clickhouse
marked this pull request as ready for review
August 5, 2026 12:26
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.
Why
web/has no test tooling and zero test files. Every frontend change is verified bytsc -bandvite build— which prove code compiles and bundles, and say nothing about whether it behaves.That gap is widest for exactly the kind of code #273 just added. A focus trap, Escape precedence against document-level handlers, dialog stacking,
mousedown-vs-click— none of it is reachable by a type checker.It caught a bug immediately
Writing the Modal specs surfaced a real defect that
tscandvite buildwere both happy with:Fixed in
be57e5fon the #273 branch, so that PR doesn't ship the bug. The page looks completely normal until you try to scroll it, which is the sort of thing a manual pass misses and a spec doesn't.What's here
vitest+jsdom+@testing-library/{react,user-event,jest-dom}as dev dependenciestestblock invite.config.ts(shares the existing resolve/plugin config — no second config file to drift)src/test/setup.ts—jest-dommatchers,cleanupbetween tests, and two jsdom shimsnpm test/npm run test:watchnpm testadded to the existing Frontend build (Vite) CI jobModalcovering portal target, ARIA semantics, Escape (including that it does not reach app-level handlers), backdrop dismissal, the drag-release-outside case, focus entry/wrap/restore, stacked-dialog Escape isolation, and scroll-lock refcountingThe two jsdom shims are load-bearing
Both live in
setup.tswith comments, but they're the sort of thing worth flagging in review:requestAnimationFrame— jsdom doesn't implement it, and the Modal defers its initial focus through it.offsetParent— jsdom gives every element no layout, sooffsetParentisnullfor everything. The focus trap filters candidates on exactly that. Without the shim the trap would find nothing focusable and every trap assertion would pass vacuously — worse than having no tests, because it looks like coverage.Notes
includetosrc/**/*.test.{ts,tsx}; the default glob walksnode_modules.css: false— specs assert behaviour and ARIA, never Tailwind classes.npm ciin CI is cached onweb/package-lock.json, and the test step adds ~2s to the job.Testing
npm test— 19 passednpx tsc -b— cleannpm run build— cleannpx eslinton both new files — clean🤖 Generated with Claude Code