test(e2e): cover every area, and fix the overflow this found - #267
Merged
Conversation
The end-to-end suite reached 29 of the app's 38 routes. The match report flow had no coverage at all, which is how its review screen shipped with two independently side-scrolling stat tables and 40px inputs. Coverage: - match-report.spec.ts covers the area that had none: authorization, the selection step, division grouping, search filtering, honest failure when the report cannot be created, no console errors, and no horizontal overflow at 390/768/1440/1920. One assertion walks every element under <main> and fails if any scrolls sideways -- the direct regression guard for the review layout. - uncovered-routes.spec.ts covers the rest: the token-bound host review and private ticket pages rendering their shell for an unknown id, the auth error page, the god draft session page, 404s for unknown announcement and scouter ids, and every admin area that had no rendering or overflow coverage. - site.spec.ts now asserts the logged-out redirect for all fourteen admin routes rather than seven, and opens every nav item. Two real bugs the new tests caught, both horizontal overflow at 390px: - Admin seasons: `shrink-0` on the wrapping per-season button group stopped it shrinking below its max-content width, so a 658px group ran off a 390px viewport. It wraps internally, so it does not need shrink-0. - Admin registrations: the filter row never wrapped, so its tabs plus the trailing count pushed 78px past the viewport. 391 e2e tests pass, up from 343. Typecheck clean, no new lint warnings, 781 unit tests pass.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fbd72a53a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The overflow scan only loaded the selection step. The stat editors render in the review step, so a regression restoring the independently scrolling tables would have passed the very test named after it — the assertion promised more than it checked. Creating a report is the one Supabase write on the path, so it is stubbed and the rest is real client code: clicking a match, then Skip to Manual Entry, builds the blank roster rows from the mock league data the server already serves and lands in the review step. That makes four assertions real rather than nominal: - both team editors render, ten roster rows with their stat fields, two winner controls and the submit action; - stat inputs are ≥32px tall and ≥36px wide, guarding the 0.75rem micro-inputs directly; - nothing under <main> scrolls sideways, at 390/768/1440/1920; - adding a game keeps the layout intact. Proved the guard works by reintroducing the regression (min-w-[900px] rows inside an overflow-x-auto container): three of the four viewports fail, and 1920 passes because 900px genuinely fits there. Restored, 397 e2e tests pass. An input bound to a datalist reports role combobox rather than textbox, so the row locators match on the accessible label instead.
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
1 participant
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.
Problem
The end-to-end suite reached 29 of the app's 38 routes. The match report flow had no coverage at all — which is how its review screen shipped with two independently side-scrolling stat tables and 40px inputs (#266).
Uncovered before this change:
/admin/match-report,/admin/audit,/admin/teams,/admin/seasons/[id]/roster,/announcements/[id],/auth/error,/draft/god/[sessionId],/match-reports/[id]/review,/report-a-bug/tickets/[publicTicketId],/scouters/[matchId].Coverage added
tests/e2e/match-report.spec.ts— the area that had none. Authorization, the selection step, division grouping, search filtering, honest failure when the report cannot be created, and no console errors.It also drives the client into the review step, where the stat editors actually render. Creating a report is the only Supabase write on that path, so it is stubbed; everything after — clicking a match, then Skip → Manual Entry, building the blank roster rows from the mock league data the server already serves — is real client code. That makes four assertions real rather than nominal:
<main>scrolls sideways, at 390 / 768 / 1440 / 1920;tests/e2e/uncovered-routes.spec.ts— the rest. The token-bound host review and private ticket pages rendering their shell for an unknown id (rather than 500-ing), the auth error page, the god draft session page, 404s for unknown announcement and scouter ids, and every admin area that had no rendering or overflow coverage.tests/e2e/site.spec.ts— the logged-out redirect now covers all fourteen admin routes rather than seven, and every nav item is opened.Two real bugs the new tests caught
Both horizontal overflow at 390px, the same class of bug as #266:
/admin/seasonsshrink-0on the wrapping per-season button group stopped it shrinking below its max-content width — a 658px group on a 390px viewportshrink-0; the group already wraps internally/admin/registrationsflex-wrap items-center/admin/auditalso failed initially, but that was my own wrong assertion — its heading is "Pending Deletions"; "Audit Log" is only the nav label.Verification
min-w-[900px]rows inside anoverflow-x-autocontainer): 3 of the 4 viewports fail, and 1920 passes because 900px genuinely fits there. Reverted before pushing.npx tsc --noEmitclean;npm run lintreports no new warnings;npm run test— 781 passed, 26 skipped, 0 failed.An incidental find while writing the locators: an
<input>bound to a<datalist>reports rolecombobox, nottextbox.