Test the attendee page, attendees list, listing loaders and store-and-refund - #1993
Conversation
NOT GREEN as a finished job — both suites pass, but the attendee-page mutation run still has survivors to close, and two more modules (listing-page-data, store-refund) have no tests yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
NOT GREEN as a finished job — mutation re-run for attendee-page is still in flight, and listing-page-data's loaders plus store-refund have no tests yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
NOT GREEN as a finished job — attendee-page mutation re-run in flight, listing-page-data loaders and store-refund's DB paths still untested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
…loaders NOT GREEN as a finished job — mutation re-runs still to do for attendees-list, listing-page-data and store-refund. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
NOT GREEN as a finished job — store-refund mutation re-run in flight and listing-page-data has not been mutation-tested yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
NOT GREEN as a finished job — mutation re-run in flight, and listing-page-data still needs its first mutation run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
The gate refuses to start on a source that has no test at its mirrored path, so any branch touching these four was blocked however small the change. Each now has one. Scores after the pass: attendees-list 100%, attendee-page 98.1% (one recorded equivalent), store-refund 96.6%, listing-page-data 77.8%. The last two are not finished, and TODO.md records exactly what each remaining survivor needs. Along the way the filler-attendee seeder moved into test-utils, so the new suite and the integration suite share one copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
📝 WalkthroughWalkthroughAdds mirrored admin feature coverage, listing-data and payment-refund tests, mutation-equivalence entries, and a reusable filler-attendee database helper for integration tests. ChangesAdmin attendee and listing coverage
Payment refund coverage
Shared attendee fixture
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c7db9f79d
ℹ️ 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".
| export const seedFillerAttendees = async ( | ||
| listingId: number, | ||
| count: number, | ||
| ): Promise<void> => { |
There was a problem hiding this comment.
Split the attendee helper back below 400 lines
Adding this helper leaves test/test-utils/db-helpers/attendees.ts at 417 lines. This repository explicitly applies its roughly 400-line limit to test code, so move the page-filling helper into a smaller focused helper module rather than growing this already overloaded file.
AGENTS.md reference: AGENTS.md:L82-L82
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in de1aa11. The seeder moved to test/test-utils/db-helpers/attendee-seeding.ts (40 lines), leaving attendees.ts at 382.
Your follow-up on the stale import in the integration suite was right too — that is fixed in 861d524, and it was the CI failure. My extraction rewrote one import but not the other, because the formatter had already reflowed it to multi-line and my edit matched the single-line form. Precommit passes now.
Generated by Claude Code
| test("is still stored, because capacity must never lose the record of a payment", async () => { | ||
| const listing = await createTestListing({ maxAttendees: 1 }); | ||
| // Fill the listing, so a capacity-gated insert would refuse. | ||
| await createTestAttendee( |
There was a problem hiding this comment.
Make the overbook test exercise an over-capacity date
This fixture only fills a capacity-1 listing with one attendee, then inserts a quantity-0 placeholder. The ordinary capacity condition still accepts 1 + 0 <= 1, so changing allowOverbook to false does not fail this test and the assertion does not protect the behavior named in the test. Put the existing booking and placeholder on the same dated capacity bucket and make that bucket genuinely over capacity so the test fails without the flag.
AGENTS.md reference: AGENTS.md:L76-L76
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in de1aa11 — and you were right on both the fact and the reasoning, which I had got wrong twice.
I had believed I already pushed this fixture over capacity; in fact that edit silently failed to apply and the file still held the capacity-1 version you describe. So your reading of the code was accurate and my earlier claim was not. The arithmetic you name is exactly the trap: at exactly full, 1 + 0 <= 1 still passes, so only going genuinely over makes the gate refuse a quantity-0 row.
It now books two attendees on a capacity-2 listing and then shrinks the listing to 1, so the day is over its limit. Verified both directions: with allowOverbook: false the test fails, with true it passes. store-refund.ts is at 100% (29/29), up from 96.6%, and the TODO entry I had written about this being unclosed is deleted.
For the record, one detail differs from your suggestion: a dated bucket was not needed, and the daily-listing route did not work because the booking fixture will not book a daily listing without a date. The plain over-capacity listing is enough.
Generated by Claude Code
| ## Four feature modules had no test at their mirrored path — now they do | ||
|
|
||
| *Origin: `deno task precommit:mutation` on the notes-migration branch, which | ||
| could not start.* | ||
|
|
||
| The mutation gate refuses to run a source that has mutants but no test at its | ||
| mirrored path under `test/`. Four modules are in that state: | ||
|
|
||
| - `src/features/admin/attendee-page.ts` (55 mutants) | ||
| - `src/features/admin/attendees-list.ts` (37) | ||
| - `src/features/admin/listing-page-data.ts` (45) | ||
| - `src/features/api/payment-processing/store-refund.ts` (29) | ||
|
|
||
| Nothing needs moving: no test imports any of them. They are reached only | ||
| through the app, by integration and Cucumber journeys, so each needs a direct | ||
| test written at `test/features/…` to match its path. | ||
|
|
||
| This blocks the gate for *any* branch that touches one of them, however small | ||
| the change — the notes migration only swapped an import in each. Until they | ||
| have direct tests, a branch touching them can prove its own work with a | ||
| targeted `deno task mutation <source> <tests>` run instead. | ||
|
|
||
| `src/features/admin/attendee-notes.ts` was in the same state and is now fixed: | ||
| its route suite drives real pages through the session helpers, so it moved from | ||
| `test/integration/admin/` to `test/features/admin/`, which is where that kind | ||
| of suite belongs (see "Let the misplaced-test list see past request helpers" | ||
| above). The other four have no such suite to move. | ||
|
|
||
| --- | ||
|
|
||
| ## An answer filed under a listing nobody booked | ||
|
|
||
| *Origin: review of PR #1990 (the booking-check slice), 2026-07-29.* | ||
| could not start. Closed by the direct-test pass that followed.* |
There was a problem hiding this comment.
Restore the still-open lost-answer TODO
This replacement also deletes the unrelated “An answer filed under a listing nobody booked” item that came from the review of PR #1990, although the bug is still present: BookingIntentSchema accepts any numeric listing key, and saveSessionAnswers only visits createdEntries, so answers keyed to an unbooked listing are silently discarded after payment. Restore that TODO until the booking code validates the answer keys, rather than losing an acknowledged out-of-scope review issue.
AGENTS.md reference: AGENTS.md:L99-L99
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in de1aa11, and thank you for catching it — this was real data loss, not a judgement call. My edit replaced from the "Four feature modules" heading to end of file, so it took the still-open lost-answer item with it. The section is restored verbatim from origin/main, and git diff origin/main -- TODO.md now shows only the one heading I meant to rewrite.
Generated by Claude Code
| describe("the listing filter", () => { | ||
| test("shows attendees from every listing when nothing is chosen", async () => { |
There was a problem hiding this comment.
Consolidate the duplicate attendees-list suites
This new suite repeats the authentication, listing-filter, sort, and paging scenarios already exercised in test/integration/server/attendees-list.test.ts, while its CSV section also repeats test/integration/server/attendees-csv.test.ts. Keeping parallel suites for the same behavior increases runtime and lets their fixtures and expectations drift; move or consolidate the existing route coverage into the mirrored feature suite and add the direct handler import there instead.
AGENTS.md reference: AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed on the substance, and recorded rather than dropped — see "Two suites now cover the attendees list" in TODO.md (162ff25), which names the three files and what consolidation should look like.
Not done here for two reasons. The mirrored suite had to exist before anything could be moved into it, since the gate keys off that path; and consolidating means opening and rewriting two integration suites this PR otherwise has no reason to touch, which would bury the change it is actually making. The duplication is real and costs runtime, so it is worth doing — just as its own change, next time either file is opened.
Generated by Claude Code
| const listingOfType = (listing_type: string): ListingWithCount => | ||
| ({ id: 1, listing_type }) as ListingWithCount; | ||
|
|
||
| const DAILY = listingOfType("daily"); | ||
| const ONE_OFF = listingOfType("one_off"); |
There was a problem hiding this comment.
Use supported listing types in the new fixtures
ListingTypeSchema permits only "standard" and "daily", but this helper accepts any string and constructs a "one_off" listing by assertion; store-refund.test.ts repeats the same invalid literal behind any. These tests therefore exercise an impossible fallback state instead of the real standard-listing path, and future exhaustive listing-type logic could make their assertions irrelevant. Build typed fixtures with "standard" rather than bypassing the model.
AGENTS.md reference: AGENTS.md:L90-L90
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in de1aa11. ListingTypeSchema is v.picklist(["standard", "daily"]), so "one_off" was a state the model cannot hold — the roster-filter fixture now takes a typed ListingType and uses "standard", and store-refund.test.ts uses "standard" too. The tests read the same and now exercise a listing that can actually exist.
Generated by Claude Code
| answers. The buyer answered a question and the answer quietly goes nowhere. | ||
| - `src/features/admin/attendee-page.ts` → `test/features/admin/attendee-page.test.ts` (98.1%, one recorded equivalent) | ||
| - `src/features/admin/attendees-list.ts` → `test/features/admin/attendees-list.test.ts` (100%) | ||
| - `src/features/admin/listing-page-data.ts` → `test/features/admin/listing-page-data/` (77.8%) |
There was a problem hiding this comment.
Finish the listing-page-data mutation suite
The mirrored listing-page-data suite is recorded here at only 77.8%, with ten surviving branches enumerated earlier in this same change. Since the mutation gate requires 100% whenever that source file is touched, the next change to this common admin loader still cannot pass without also completing this commit's unfinished fixtures; add the hidden-package, daily-date, question, note, child-listing, and permission cases before marking the direct-test work complete.
AGENTS.md reference: AGENTS.md:L83-L83
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done — listing-page-data.ts is now at 100% (44/44, one recorded equivalent), up from the 77.8% this TODO reported.
The fixtures you listed are all built: the hidden package member, a parent with a named child, a daily listing whose booked day appears in the roster's date picker, an attendee note that names who it is about, and a question the listing asks. Two assertions had to be sharpened along the way — the daily one was passing on the attendee row's own date rather than the picker, and the ledger one needed the exact per-listing href.
The one mutation left is ?? → || on the child-name lookup, recorded as equivalent with its proof: Map.get returns ListingWithCount[] | undefined, and every array — empty included — is truthy, so only undefined reaches the fallback and both operators yield the same [].
All four modules in this PR are now at 100%, so the gate no longer blocks a change to any of them. The stale ledger entry is deleted rather than left describing work that is done.
Generated by Claude Code
…tures - Restore the lost-answer TODO my edit truncated away. The replacement ran to end-of-file and took an unrelated, still-open item with it. - Move the page-filling seeder into its own module; attendees.ts was over the 400-line limit at 417. - Use real listing types in the new fixtures; "one_off" is not one the schema permits. - Make the overbook test push the listing OVER capacity. At exactly full a quantity-0 row still fits, so the earlier fixture proved nothing — the mutant is now killed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de1aa113b2
ℹ️ 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".
| import { | ||
| createMultiBookingAttendee, | ||
| createTestAttendeeDirect, | ||
| seedFillerAttendees, |
There was a problem hiding this comment.
Import the extracted helper from its new module
seedFillerAttendees is now exported only by test/test-utils/db-helpers/attendee-seeding.ts, but this added import still requests it from attendees.ts, which has no such export. As a result, the test-file typecheck in deno task precommit fails before this integration suite can run; import the helper from the new module, as the new feature test already does.
AGENTS.md reference: AGENTS.md:L101-L101
Useful? React with 👍 / 👎.
The extraction left this import pointing at attendees.ts, which no longer exports it, so the test typecheck failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 162ff25ea7
ℹ️ 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".
| testRequiresAuth, | ||
| } from "#test-utils/assertions.ts"; | ||
| import { describeWithEnv } from "#test-utils/db.ts"; | ||
| import { seedFillerAttendees } from "#test-utils/db-helpers/attendee-seeding.ts"; |
There was a problem hiding this comment.
Move the seeder documentation with the seeder
When the helper is extracted through this import, its old JSDoc at lines 130–137 is left behind and now attaches to the following paging test, describing cloning and autoincrement behavior that the test itself does not implement. Remove that orphan block or move any needed details to attendee-seeding.ts so this suite does not misdescribe its current code.
AGENTS.md reference: AGENTS.md:L71-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0317da4. The orphaned block is gone from the integration suite, and its useful part — why the cloned rows get higher ids, and so behave like later bookings under newest-first paging — now sits on the helper itself in attendee-seeding.ts.
Generated by Claude Code
Also moves the seeder's doc comment onto the seeder, where the review noted it had been orphaned above an unrelated test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0317da45a0
ℹ️ 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".
| `test/features/admin/listing-page-data/` now exists and takes the file from | ||
| 0 to 77.8% (35 of 45). The gate wants 100% for a changed file, so a branch | ||
| touching this module still has work to do. Each survivor needs a fixture the | ||
| current suite does not build: |
There was a problem hiding this comment.
Refresh the mutation-gap ledger after adding fixtures
Fresh evidence in the reviewed tree is that loaders.test.ts now builds and asserts the hidden-package, child-listing, daily-date, and attendee-note cases at lines 202–295, but this TODO still reports the old 35/45 score and says those fixtures do not exist. Leaving resolved work listed as open misdirects the next mutation pass and makes the summary later in this file inaccurate; rerun the mutation suite and retain only the survivors that remain.
AGENTS.md reference: AGENTS.md:L71-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. The ledger is now accurate: listing-page-data.ts finished at 100% (44/44, one recorded equivalent), so the "remaining gaps" section is deleted rather than left describing work that is done, and the summary lists all four modules at 100%.
The attendee-page.ts figure in that summary was stale for the same reason — it read 98.1%, which was the score before its equivalent was recorded. Re-ran it to check rather than assume: 100% (53/53, two suppressed).
Generated by Claude Code
Fixes the CI typecheck too: the bookable day is optional, so it is checked rather than asserted away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3ed7ff0da
ℹ️ 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".
| * (id-ordered) paging sees them exactly like real bookings made after an | ||
| * earlier fixture attendee. | ||
| */ | ||
| export const seedFillerAttendees = async ( |
There was a problem hiding this comment.
Update the seeder path in the test guide
Extracting seedFillerAttendees here leaves the repository's test-performance guide pointing contributors to its former implementation in test/integration/server/attendees-list.test.ts; that file now contains only an import and calls, so readers looking for the documented batching example cannot find the cloning code at the named location. Update the guide to reference test/test-utils/db-helpers/attendee-seeding.ts.
AGENTS.md reference: AGENTS.md:L1093-L1097
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0864406. AGENTS.md now points at test/test-utils/db-helpers/attendee-seeding.ts, so the batching example is where the guide says it is.
Generated by Claude Code
| const { createTestGroup } = await import( | ||
| "#test-utils/db-helpers/groups.ts" | ||
| ); | ||
| const group = await createTestGroup({ isPackage: true }); | ||
| const { getDb } = await import("#shared/db/client.ts"); |
There was a problem hiding this comment.
Reuse the hidden-package fixture
This setup reimplements the existing createHiddenPackageGroup helper and then writes both groups and group_listings with raw SQL. Besides duplicating the package invariant, the raw UPDATE bypasses the group cache invalidation performed by the shared table helper, leaving the fixture with database and cached group state that production writes do not create. Use createHiddenPackageGroup and create the listing with its groupId instead.
AGENTS.md reference: AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0864406. The fixture now calls createHiddenPackageGroup() and creates the listing with its groupId — no raw SQL, and no bypassed cache invalidation. It is shorter than what it replaced, and it makes the same writes production does.
Generated by Claude Code
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/features/api/payment-processing/store-refund.test.ts`:
- Around line 129-136: Strengthen the test case “falls back to the
unexpected-error reason for anything else” by also asserting the customer-facing
reason returned in spec.reason. Use the expected unexpected-error fallback value
so mutations that alter or remove the fallback message fail.
- Around line 212-219: Strengthen the persisted placeholder assertions in
test/features/api/payment-processing/store-refund.test.ts:212-219 and
test/features/api/payment-processing/store-refund.test.ts:358-363. In the “keeps
the booking, holding no places” test, assert the stored attendee row’s quantity
is 0 in addition to asserting one row exists; in the over-capacity test, assert
the third persisted row’s quantity is 0 in addition to asserting three rows
exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bb75599c-1681-4250-92e4-946531a999d2
📒 Files selected for processing (9)
TODO.mdscripts/mutation/equivalent-mutants.txttest/features/admin/attendee-page.test.tstest/features/admin/attendees-list.test.tstest/features/admin/listing-page-data/loaders.test.tstest/features/admin/listing-page-data/roster-filter.test.tstest/features/api/payment-processing/store-refund.test.tstest/integration/server/attendees-list.test.tstest/test-utils/db-helpers/attendee-seeding.ts
…tions Records the child-names ?? equivalent and updates the TODO ledger. NOT GREEN as a finished job: the final listing-page-data mutation run is still in flight and the attendee-page figure needs re-checking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
listing-page-data and attendee-page confirmed at 100% after the last fixtures and the recorded equivalents; the TODO ledger now matches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
The placeholder tests hand-rolled untyped listing fixtures behind no-explicit-any and cast the results with as never. test-utils already has testListingWithCount, so the fixtures are now properly typed ValidatedItem values and every cast and lint suppression is gone. Score unchanged at 100%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EdDZCWRXwjqsidfRJFarXC
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@TODO.md`:
- Around line 1748-1750: Clarify the sentence in TODO.md by explicitly stating
“100% coverage” when describing the gate’s requirement, while preserving the
existing meaning and surrounding wording.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 71f39c8a-299f-4542-a34b-63df80672016
📒 Files selected for processing (6)
AGENTS.mdTODO.mdscripts/mutation/equivalent-mutants.txttest/features/admin/listing-page-data/loaders.test.tstest/features/admin/listing-page-data/roster-filter.test.tstest/features/api/payment-processing/store-refund.test.ts
| Every one of them is now at the 100% the gate demands, so a branch touching | ||
| any of them can pass without first writing the tests that should already have | ||
| existed. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify that this refers to 100% coverage.
“Now at the 100% the gate demands” is grammatically incomplete. Use “now at the 100% coverage the gate demands” or equivalent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@TODO.md` around lines 1748 - 1750, Clarify the sentence in TODO.md by
explicitly stating “100% coverage” when describing the gate’s requirement, while
preserving the existing meaning and surrounding wording.
We have a checker that measures how good our tests are: it makes small changes to a piece of code and sees whether any test complains. If nothing complains, that change could have shipped unnoticed.
The checker refuses to run at all on a file that has no test of its own. Four files were in that state, so any change to them — however small — could not be checked. All four now have one, and all four are fully covered.
What the four files do
Where they ended up
Three changes cannot be caught by any test — two ways of writing the same fallback, and one flag on a key nothing can look at — and each is written down with the reasoning that proves no test could tell the difference.
Some of what the tests now pin down. Which tabs each role may open, and that a manager is not even shown a link to the money pages they cannot enter. That an attendee holding no places gets a "No quantity" note instead of a ticket link that would fail to open. That the spreadsheet export keeps reading until it has every booking, not just the first hundred. That a payment we cannot honour still records what was bought — holding no places — and tells the customer the truth about their refund either way.
Worth knowing. Chasing the checker's complaints turned up several of my own tests that passed for the wrong reason. Checking that a page "mentions" a web address proves nothing when the menu down the side of every page mentions the same address; nor does comparing two things that could never have matched anyway. Those now check the exact thing on the page.
The review caught more of the same, including one test whose fixture never made the listing full enough for the rule it claimed to protect, and a stray edit of mine that deleted an unrelated note from the backlog. Both fixed, and the deleted note is restored.
A helper that quickly seeds a lot of attendees now lives in the shared test toolbox rather than inside one suite, so two suites share one copy.
deno task precommitpasses.Summary by CodeRabbit
Bug Fixes
Documentation
Tests