Move 104 test files to their sources' mirror locations - #1736
Conversation
The unit-tests-report counts a source file as unit-tested only when its test lives at the mirror location (test/<src-path-without-root>). Most of these tests sat under legacy roots (test/lib/, test/templates/, test/routes/) or carried a joined name (dates-long-label) instead of the directory-suite form (dates/long-label). Each move was made only when the owning source was unambiguous: the test imports the source and matches it by exact legacy path, by basename, by parent directory name, or by name prefix. Request-driving tests were only moved when their owner is an HTTP-layer (src/features/) module, so integration-style suites stay put for a later pass. Relative imports in moved files now use the #test/ alias, except the prune and refund-ledger helpers, which moved into their suites' directories alongside their only importers. Report before: 224/855 sources with a mirrored test, 620 orphan tests. Report after: 290/855 with a mirrored test, 514 orphans. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR adds extensive automated coverage across admin features, shared modules, database workflows, payment helpers, forms, client-side behavior, and UI templates. It also strengthens assertions, standardizes test helper imports, centralizes reusable test utilities, and updates TODO references. ChangesTest coverage and test-wiring updates
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 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/admin/attendee-form-model.test.ts`:
- Around line 635-650: Replace the partial toMatchObject assertion in the “keeps
a no-quantity line as a quantity-0 desired line” test with toEqual, asserting
the complete expected desired-line shape including fields such as date,
durationDays, key, packageGroupId, and parentListingId, consistent with sibling
tests in the same describe block.
In `@test/features/wallet/google.test.ts`:
- Around line 31-33: Update ensureCreds to use or return the credentials
produced by generateGoogleTestCreds instead of discarding the result; adjust its
return type and callers as needed so the helper has a meaningful effect.
- Around line 153-246: Refactor the four validation tests around the shared
handleRequest/expectFlashRedirect setup into deterministic table-driven cases,
following the pattern used by the sibling secret.test.ts “advanced redirect”
tests. Define each case with its test name, request field overrides, and
expected flash message, then iterate them from a single test structure while
preserving the existing login, request, redirect, and assertion behavior.
In `@test/shared/config/env.test.ts`:
- Around line 261-265: Update the test named “returns bunny for any other
configured value” to use a non-“turso” value that differs from the expected
result, such as “other-provider,” and assert that getDefaultDbProvider() returns
that configured value. Keep the test focused on verifying the fallback/echo
behavior for arbitrary providers rather than relying on the literal “bunny.”
In `@test/shared/db/attendees/booking-slot.test.ts`:
- Around line 133-148: Remove the redundant conditional spread and simplify the
test case in hasDuplicateBookingSlot to use an object that directly omits
parentListingId, or remove the test if it duplicates the earlier
undefined-parent coverage. Do not retain branches based on a compile-time
undefined value.
In `@test/shared/db/attendees/create.test.ts`:
- Around line 162-164: Extract the repeated result-narrowing logic into a shared
helper that accepts the result, throws “expected ok” for “sold-out” or
unsuccessful results, and returns the narrowed successful result. Replace the
duplicated checks in all five call sites, including the tests around the
existing result handling, with this helper.
- Around line 130-133: Update the SQL query in the attendee creation test to
alias listing_attendees with a descriptive singular alias using AS, and
reference that alias in the selected column. Keep the existing queryOne call and
result type unchanged.
In `@test/shared/db/backup.test.ts`:
- Around line 147-155: Replace the weak truthiness checks in the manifest
assertions with meaningful expectations: compare manifest.latestUpdate exactly
against the imported LATEST_UPDATE constant, and validate manifest.timestamp
against a concrete expected format or invariant such as an ISO-8601 timestamp.
Keep the existing schemaHash and tables.listings assertions unchanged.
In `@test/shared/db/backup/restore.test.ts`:
- Around line 26-27: The duplicate listingCount helper should be centralized in
a shared `#test-utils` module. Add the byte-identical helper there, export it,
remove the local definitions from the restore and refund-ledger tests, and
import listingCount from `#test-utils` in both files.
In `@test/shared/db/client/batch.test.ts`:
- Around line 14-21: Extract the duplicated emptyResultSet helper from
batch.test.ts and busy.test.ts into a shared `#test-utils` helper, then import and
use that shared helper in both tests, removing their local definitions.
In `@test/shared/db/client/busy.test.ts`:
- Around line 16-23: Extract the duplicated emptyResultSet helper from
busy.test.ts and batch.test.ts into a shared `#test-utils` module, preserving its
ResultSet shape and type import. Remove both local definitions and import
emptyResultSet from the shared module in each test file.
In `@test/shared/db/prune/helpers.ts`:
- Around line 210-234: Deduplicate the repeated setter calls in
clearAllLastPruned and setAllLastPruned by defining one shared array of the ten
lastPruned setter functions, then iterating over it with the appropriate value.
Have clearAllLastPruned delegate using an empty string and setAllLastPruned
delegate using its value parameter, preserving async behavior and TypeScript
typing.
In `@test/shared/forms/saved-data.test.ts`:
- Around line 220-231: Replace the real 20ms setTimeout yield inside the
concurrent request scopes test with a deterministic microtask-based interleave,
such as awaiting a resolved promise, while preserving the ordering needed to
exercise both runWithSavedFormContext calls. Keep the existing request helper
and assertions validating Alice and Bob isolation.
In `@test/shared/logger/request-id.test.ts`:
- Around line 113-122: Make “different requests get different IDs” deterministic
by stubbing crypto.getRandomValues using the existing pattern in the test file,
supplying distinct fixed values for each invocation, then assert the resulting
IDs are distinct. Restore the stub after the test and retain the
runWithRequestId/getRequestId coverage.
In `@test/ui/client/admin/address-lookup/coords-diff.test.ts`:
- Around line 123-157: Extract the duplicated form/selector utilities from
formSpec and searchAndChoose in the address-lookup tests into shared helpers
under `#test-utils`. Update both coords-diff.test.ts and client.test.ts to import
and use the shared formSpec and one-style selector helper, preserving existing
behavior and removing the local duplicate implementations.
- Around line 159-171: Update the “choosing a located address fills lat/lng and
fires input events” test to register an input listener on both the lat and lng
elements, track each event independently, and assert that both counters equal
one after dispatching the change event. Use the existing lat, lng, and select
references in the test.
In `@test/ui/templates/admin/settings/boolean-settings-section.test.tsx`:
- Around line 32-40: Strengthen the assertions in the “checks the yes radio when
the state is on and the no radio when off” test by verifying that the radio with
value="true" is checked for enabled: true and the radio with value="false" is
checked for enabled: false, while the opposite radios are not checked. Scope
each checked-state assertion to its corresponding value so swapped selections
cannot pass.
- Line 6: Update the assertions in the boolean settings tests using TestState to
inspect each radio input’s actual checked attribute, verifying the true-valued
option is checked when enabled and the false-valued option is checked when
disabled, rather than only asserting that “checked” appears in the rendered
output.
In `@test/ui/templates/admin/users/agents.test.ts`:
- Around line 52-58: Rename the local DisplayUser object in the affected test
block to a distinct name, such as displayAgentUser, and update all references
within that block, preserving the outer agentUser factory function name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e2bc8d2b-f0c3-403f-b29f-66c4607faba7
📒 Files selected for processing (108)
TODO.mdtest/features/admin/attendee-form-model.test.tstest/features/admin/attendees-csv.test.tstest/features/admin/attribute-page-data.test.tstest/features/admin/calendar-csv.test.tstest/features/admin/entity-pages.test.tstest/features/admin/listings-csv.test.tstest/features/admin/settings-helpers/clearable.test.tstest/features/admin/settings-helpers/create.test.tstest/features/admin/settings-helpers/secret.test.tstest/features/admin/settings-helpers/toggle.test.tstest/features/api/request-schemas.test.tstest/features/public/custom-css.test.tstest/features/public/ticket-form.test.tstest/features/public/ticket-payment/packages.test.tstest/features/request-body.test.tstest/features/tickets/token-utils.test.tstest/features/wallet/google.test.tstest/shared/accounting/backfill.test.tstest/shared/booking/build-tree.test.tstest/shared/booking/build-tree/children.test.tstest/shared/booking/capacity-tree.test.tstest/shared/booking/fold-tree.test.tstest/shared/booking/order-lines.test.tstest/shared/booking/package-cap.test.tstest/shared/booking/package-cap/limits.test.tstest/shared/booking/page-packages.test.tstest/shared/booking/price-tree.test.tstest/shared/booking/signed-metadata.test.tstest/shared/capacity-rules.test.tstest/shared/checkout-pricing/consistency.test.tstest/shared/column-order.test.tstest/shared/columns/attendee-columns.test.tstest/shared/config/bunny-cdn.test.tstest/shared/config/env.test.tstest/shared/crypto/define-signed-token.test.tstest/shared/dates/long-label.test.tstest/shared/dates/pinned-values.test.tstest/shared/db/attendees/booking-slot.test.tstest/shared/db/attendees/create.test.tstest/shared/db/attendees/kind.test.tstest/shared/db/attendees/pii.test.tstest/shared/db/backup.test.tstest/shared/db/backup/restore.test.tstest/shared/db/built-sites.test.tstest/shared/db/capacity.test.tstest/shared/db/client/batch.test.tstest/shared/db/client/busy.test.tstest/shared/db/client/invalidation.test.tstest/shared/db/logistics/runsheet.test.tstest/shared/db/migrations/2026-06-18_contact_preferences.test.tstest/shared/db/migrations/2026-06-22_backfill_transfers.test.tstest/shared/db/migrations/2026-06-22_drop_listing_income.test.tstest/shared/db/migrations/2026-06-22_drop_modifiers_total_revenue.test.tstest/shared/db/migrations/2026-07-05_first_class_images.test.tstest/shared/db/name-registry.test.tstest/shared/db/processed-payments/staleness.test.tstest/shared/db/prune/helpers.tstest/shared/db/prune/payments.test.tstest/shared/db/prune/scheduler.test.tstest/shared/db/prune/tables.test.tstest/shared/db/query-log.test.tstest/shared/db/settings/bunny-cdn.test.tstest/shared/email.test.tstest/shared/forms/components.test.tstest/shared/forms/define-form.test.tstest/shared/forms/rendering.test.tstest/shared/forms/saved-data.test.tstest/shared/forms/validate-form.test.tstest/shared/i18n.test.tstest/shared/logger/formatting.test.tstest/shared/logger/log-output.test.tstest/shared/logger/redact-path.test.tstest/shared/logger/request-id.test.tstest/shared/payment-helpers/build-items.test.tstest/shared/payment-helpers/dispatch.test.tstest/shared/payment-helpers/metadata.test.tstest/shared/refund-ledger.test.tstest/shared/refund-ledger/batch.test.tstest/shared/refund-ledger/groups.test.tstest/shared/refund-ledger/helpers.tstest/shared/square/client.test.tstest/shared/square/payment-link-validation.test.tstest/shared/square/payment-link.test.tstest/shared/square/rest-transport.test.tstest/shared/square/webhook.test.tstest/shared/validation/coordinates.test.tstest/ui/client/admin/address-lookup/client.test.tstest/ui/client/admin/address-lookup/coords-diff.test.tstest/ui/client/admin/child-compat.test.tstest/ui/client/admin/child-required.test.tstest/ui/client/admin/child-selection.test.tstest/ui/client/admin/custom-question-visibility.test.tstest/ui/client/admin/logistics-map.test.tstest/ui/client/admin/markdown-editor.test.tstest/ui/client/admin/order-gallery.test.tstest/ui/templates/admin/backup.test.tstest/ui/templates/admin/builder.test.tstest/ui/templates/admin/detail-rows.test.tstest/ui/templates/admin/guide/anchor-links.test.tstest/ui/templates/admin/guide/schema.test.tstest/ui/templates/admin/listings/edit-panel.test.tstest/ui/templates/admin/listings/form-pages.test.tstest/ui/templates/admin/settings/boolean-settings-section.test.tsxtest/ui/templates/admin/users/agents.test.tstest/ui/templates/components/address-lookup.test.tstest/ui/templates/public/reservations/og.test.tstest/ui/templates/public/reservations/ticket-page.test.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 19
🤖 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/admin/attendee-form-model.test.ts`:
- Around line 635-650: Replace the partial toMatchObject assertion in the “keeps
a no-quantity line as a quantity-0 desired line” test with toEqual, asserting
the complete expected desired-line shape including fields such as date,
durationDays, key, packageGroupId, and parentListingId, consistent with sibling
tests in the same describe block.
In `@test/features/wallet/google.test.ts`:
- Around line 31-33: Update ensureCreds to use or return the credentials
produced by generateGoogleTestCreds instead of discarding the result; adjust its
return type and callers as needed so the helper has a meaningful effect.
- Around line 153-246: Refactor the four validation tests around the shared
handleRequest/expectFlashRedirect setup into deterministic table-driven cases,
following the pattern used by the sibling secret.test.ts “advanced redirect”
tests. Define each case with its test name, request field overrides, and
expected flash message, then iterate them from a single test structure while
preserving the existing login, request, redirect, and assertion behavior.
In `@test/shared/config/env.test.ts`:
- Around line 261-265: Update the test named “returns bunny for any other
configured value” to use a non-“turso” value that differs from the expected
result, such as “other-provider,” and assert that getDefaultDbProvider() returns
that configured value. Keep the test focused on verifying the fallback/echo
behavior for arbitrary providers rather than relying on the literal “bunny.”
In `@test/shared/db/attendees/booking-slot.test.ts`:
- Around line 133-148: Remove the redundant conditional spread and simplify the
test case in hasDuplicateBookingSlot to use an object that directly omits
parentListingId, or remove the test if it duplicates the earlier
undefined-parent coverage. Do not retain branches based on a compile-time
undefined value.
In `@test/shared/db/attendees/create.test.ts`:
- Around line 162-164: Extract the repeated result-narrowing logic into a shared
helper that accepts the result, throws “expected ok” for “sold-out” or
unsuccessful results, and returns the narrowed successful result. Replace the
duplicated checks in all five call sites, including the tests around the
existing result handling, with this helper.
- Around line 130-133: Update the SQL query in the attendee creation test to
alias listing_attendees with a descriptive singular alias using AS, and
reference that alias in the selected column. Keep the existing queryOne call and
result type unchanged.
In `@test/shared/db/backup.test.ts`:
- Around line 147-155: Replace the weak truthiness checks in the manifest
assertions with meaningful expectations: compare manifest.latestUpdate exactly
against the imported LATEST_UPDATE constant, and validate manifest.timestamp
against a concrete expected format or invariant such as an ISO-8601 timestamp.
Keep the existing schemaHash and tables.listings assertions unchanged.
In `@test/shared/db/backup/restore.test.ts`:
- Around line 26-27: The duplicate listingCount helper should be centralized in
a shared `#test-utils` module. Add the byte-identical helper there, export it,
remove the local definitions from the restore and refund-ledger tests, and
import listingCount from `#test-utils` in both files.
In `@test/shared/db/client/batch.test.ts`:
- Around line 14-21: Extract the duplicated emptyResultSet helper from
batch.test.ts and busy.test.ts into a shared `#test-utils` helper, then import and
use that shared helper in both tests, removing their local definitions.
In `@test/shared/db/client/busy.test.ts`:
- Around line 16-23: Extract the duplicated emptyResultSet helper from
busy.test.ts and batch.test.ts into a shared `#test-utils` module, preserving its
ResultSet shape and type import. Remove both local definitions and import
emptyResultSet from the shared module in each test file.
In `@test/shared/db/prune/helpers.ts`:
- Around line 210-234: Deduplicate the repeated setter calls in
clearAllLastPruned and setAllLastPruned by defining one shared array of the ten
lastPruned setter functions, then iterating over it with the appropriate value.
Have clearAllLastPruned delegate using an empty string and setAllLastPruned
delegate using its value parameter, preserving async behavior and TypeScript
typing.
In `@test/shared/forms/saved-data.test.ts`:
- Around line 220-231: Replace the real 20ms setTimeout yield inside the
concurrent request scopes test with a deterministic microtask-based interleave,
such as awaiting a resolved promise, while preserving the ordering needed to
exercise both runWithSavedFormContext calls. Keep the existing request helper
and assertions validating Alice and Bob isolation.
In `@test/shared/logger/request-id.test.ts`:
- Around line 113-122: Make “different requests get different IDs” deterministic
by stubbing crypto.getRandomValues using the existing pattern in the test file,
supplying distinct fixed values for each invocation, then assert the resulting
IDs are distinct. Restore the stub after the test and retain the
runWithRequestId/getRequestId coverage.
In `@test/ui/client/admin/address-lookup/coords-diff.test.ts`:
- Around line 123-157: Extract the duplicated form/selector utilities from
formSpec and searchAndChoose in the address-lookup tests into shared helpers
under `#test-utils`. Update both coords-diff.test.ts and client.test.ts to import
and use the shared formSpec and one-style selector helper, preserving existing
behavior and removing the local duplicate implementations.
- Around line 159-171: Update the “choosing a located address fills lat/lng and
fires input events” test to register an input listener on both the lat and lng
elements, track each event independently, and assert that both counters equal
one after dispatching the change event. Use the existing lat, lng, and select
references in the test.
In `@test/ui/templates/admin/settings/boolean-settings-section.test.tsx`:
- Around line 32-40: Strengthen the assertions in the “checks the yes radio when
the state is on and the no radio when off” test by verifying that the radio with
value="true" is checked for enabled: true and the radio with value="false" is
checked for enabled: false, while the opposite radios are not checked. Scope
each checked-state assertion to its corresponding value so swapped selections
cannot pass.
- Line 6: Update the assertions in the boolean settings tests using TestState to
inspect each radio input’s actual checked attribute, verifying the true-valued
option is checked when enabled and the false-valued option is checked when
disabled, rather than only asserting that “checked” appears in the rendered
output.
In `@test/ui/templates/admin/users/agents.test.ts`:
- Around line 52-58: Rename the local DisplayUser object in the affected test
block to a distinct name, such as displayAgentUser, and update all references
within that block, preserving the outer agentUser factory function name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e2bc8d2b-f0c3-403f-b29f-66c4607faba7
📒 Files selected for processing (108)
TODO.mdtest/features/admin/attendee-form-model.test.tstest/features/admin/attendees-csv.test.tstest/features/admin/attribute-page-data.test.tstest/features/admin/calendar-csv.test.tstest/features/admin/entity-pages.test.tstest/features/admin/listings-csv.test.tstest/features/admin/settings-helpers/clearable.test.tstest/features/admin/settings-helpers/create.test.tstest/features/admin/settings-helpers/secret.test.tstest/features/admin/settings-helpers/toggle.test.tstest/features/api/request-schemas.test.tstest/features/public/custom-css.test.tstest/features/public/ticket-form.test.tstest/features/public/ticket-payment/packages.test.tstest/features/request-body.test.tstest/features/tickets/token-utils.test.tstest/features/wallet/google.test.tstest/shared/accounting/backfill.test.tstest/shared/booking/build-tree.test.tstest/shared/booking/build-tree/children.test.tstest/shared/booking/capacity-tree.test.tstest/shared/booking/fold-tree.test.tstest/shared/booking/order-lines.test.tstest/shared/booking/package-cap.test.tstest/shared/booking/package-cap/limits.test.tstest/shared/booking/page-packages.test.tstest/shared/booking/price-tree.test.tstest/shared/booking/signed-metadata.test.tstest/shared/capacity-rules.test.tstest/shared/checkout-pricing/consistency.test.tstest/shared/column-order.test.tstest/shared/columns/attendee-columns.test.tstest/shared/config/bunny-cdn.test.tstest/shared/config/env.test.tstest/shared/crypto/define-signed-token.test.tstest/shared/dates/long-label.test.tstest/shared/dates/pinned-values.test.tstest/shared/db/attendees/booking-slot.test.tstest/shared/db/attendees/create.test.tstest/shared/db/attendees/kind.test.tstest/shared/db/attendees/pii.test.tstest/shared/db/backup.test.tstest/shared/db/backup/restore.test.tstest/shared/db/built-sites.test.tstest/shared/db/capacity.test.tstest/shared/db/client/batch.test.tstest/shared/db/client/busy.test.tstest/shared/db/client/invalidation.test.tstest/shared/db/logistics/runsheet.test.tstest/shared/db/migrations/2026-06-18_contact_preferences.test.tstest/shared/db/migrations/2026-06-22_backfill_transfers.test.tstest/shared/db/migrations/2026-06-22_drop_listing_income.test.tstest/shared/db/migrations/2026-06-22_drop_modifiers_total_revenue.test.tstest/shared/db/migrations/2026-07-05_first_class_images.test.tstest/shared/db/name-registry.test.tstest/shared/db/processed-payments/staleness.test.tstest/shared/db/prune/helpers.tstest/shared/db/prune/payments.test.tstest/shared/db/prune/scheduler.test.tstest/shared/db/prune/tables.test.tstest/shared/db/query-log.test.tstest/shared/db/settings/bunny-cdn.test.tstest/shared/email.test.tstest/shared/forms/components.test.tstest/shared/forms/define-form.test.tstest/shared/forms/rendering.test.tstest/shared/forms/saved-data.test.tstest/shared/forms/validate-form.test.tstest/shared/i18n.test.tstest/shared/logger/formatting.test.tstest/shared/logger/log-output.test.tstest/shared/logger/redact-path.test.tstest/shared/logger/request-id.test.tstest/shared/payment-helpers/build-items.test.tstest/shared/payment-helpers/dispatch.test.tstest/shared/payment-helpers/metadata.test.tstest/shared/refund-ledger.test.tstest/shared/refund-ledger/batch.test.tstest/shared/refund-ledger/groups.test.tstest/shared/refund-ledger/helpers.tstest/shared/square/client.test.tstest/shared/square/payment-link-validation.test.tstest/shared/square/payment-link.test.tstest/shared/square/rest-transport.test.tstest/shared/square/webhook.test.tstest/shared/validation/coordinates.test.tstest/ui/client/admin/address-lookup/client.test.tstest/ui/client/admin/address-lookup/coords-diff.test.tstest/ui/client/admin/child-compat.test.tstest/ui/client/admin/child-required.test.tstest/ui/client/admin/child-selection.test.tstest/ui/client/admin/custom-question-visibility.test.tstest/ui/client/admin/logistics-map.test.tstest/ui/client/admin/markdown-editor.test.tstest/ui/client/admin/order-gallery.test.tstest/ui/templates/admin/backup.test.tstest/ui/templates/admin/builder.test.tstest/ui/templates/admin/detail-rows.test.tstest/ui/templates/admin/guide/anchor-links.test.tstest/ui/templates/admin/guide/schema.test.tstest/ui/templates/admin/listings/edit-panel.test.tstest/ui/templates/admin/listings/form-pages.test.tstest/ui/templates/admin/settings/boolean-settings-section.test.tsxtest/ui/templates/admin/users/agents.test.tstest/ui/templates/components/address-lookup.test.tstest/ui/templates/public/reservations/og.test.tstest/ui/templates/public/reservations/ticket-page.test.ts
🛑 Comments failed to post (19)
test/features/admin/attendee-form-model.test.ts (1)
635-650: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Prefer
toEqualovertoMatchObjectfor exact-shape assertions.
toMatchObjectonly checks the listed keys and silently ignores drift in unasserted fields (date,durationDays,key,packageGroupId,parentListingId), unlike the sibling tests in this samedescribeblock that assert the full desired-line shape withtoEqual.♻️ Proposed fix
expect(desired).toHaveLength(1); - expect(desired[0]).toMatchObject({ exists: true, quantity: 0 }); + expect(desired[0]).toEqual({ + date: null, + durationDays: 1, + exists: true, + key: "1|", + listingId: 1, + packageGroupId: 0, + parentListingId: 0, + quantity: 0, + });As per path instructions,
test/**/*.{ts,tsx}should "avoid ... weak assertions; assert exact observable behavior and meaningful invariants."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.test("keeps a no-quantity line as a quantity-0 desired line", () => { const desired = toDesiredLines( parsedBase({ lines: [ line({ existingBooking: bookingRow({ listing_id: 1 }), key: "1|", noQuantity: true, quantity: 0, }), ], }), ); expect(desired).toHaveLength(1); expect(desired[0]).toEqual({ date: null, durationDays: 1, exists: true, key: "1|", listingId: 1, packageGroupId: 0, parentListingId: 0, quantity: 0, }); });🤖 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 `@test/features/admin/attendee-form-model.test.ts` around lines 635 - 650, Replace the partial toMatchObject assertion in the “keeps a no-quantity line as a quantity-0 desired line” test with toEqual, asserting the complete expected desired-line shape including fields such as date, durationDays, key, packageGroupId, and parentListingId, consistent with sibling tests in the same describe block.Source: Path instructions
test/features/wallet/google.test.ts (2)
31-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
ensureCredsdiscards the value it generates.
generateGoogleTestCreds()'s return value is dropped; if the call has no side effect beyond returning creds, this helper does nothing useful, and if it does have a side effect, that contract isn't documented here.🤖 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 `@test/features/wallet/google.test.ts` around lines 31 - 33, Update ensureCreds to use or return the credentials produced by generateGoogleTestCreds instead of discarding the result; adjust its return type and callers as needed so the helper has a meaningful effect.
153-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Four near-identical validation tests could be table-driven.
"requires Issuer ID", "requires Service Account Email", "requires private key on initial setup", and "rejects invalid PEM private key" repeat the same request/assert scaffolding, differing only in which field is blanked/invalid and the expected message — the same pattern already used for "advanced redirect" in the sibling
secret.test.tsfile (Lines 168-178 there).As per coding guidelines, use deterministic table-driven test cases and eliminate duplicated code with shared helpers.
🤖 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 `@test/features/wallet/google.test.ts` around lines 153 - 246, Refactor the four validation tests around the shared handleRequest/expectFlashRedirect setup into deterministic table-driven cases, following the pattern used by the sibling secret.test.ts “advanced redirect” tests. Define each case with its test name, request field overrides, and expected flash message, then iterate them from a single test structure while preserving the existing login, request, redirect, and assertion behavior.Source: Coding guidelines
test/shared/config/env.test.ts (1)
261-265: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Test name promises broader coverage than the assertion provides.
"returns bunny for any other configured value"only exercisesDEFAULT_DB_HOST: "bunny", i.e. the exact expected output value, not an arbitrary non-"turso"value (e.g."other-provider"). It doesn't actually prove the "any other value" branch beyond echoing the literal"bunny"string back.♻️ Proposed fix
test("returns bunny for any other configured value", () => { - withEnv({ DEFAULT_DB_HOST: "bunny" }, () => { + withEnv({ DEFAULT_DB_HOST: "some-unrecognized-value" }, () => { expect(getDefaultDbProvider()).toBe("bunny"); }); });As per path instructions,
test/**/*.{ts,tsx}should "assert exact observable behavior and meaningful invariants" rather than weak/tautological checks.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.test("returns bunny for any other configured value", () => { withEnv({ DEFAULT_DB_HOST: "some-unrecognized-value" }, () => { expect(getDefaultDbProvider()).toBe("bunny"); }); });🤖 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 `@test/shared/config/env.test.ts` around lines 261 - 265, Update the test named “returns bunny for any other configured value” to use a non-“turso” value that differs from the expected result, such as “other-provider,” and assert that getDefaultDbProvider() returns that configured value. Keep the test focused on verifying the fallback/echo behavior for arbitrary providers rather than relying on the literal “bunny.”Source: Path instructions
test/shared/db/attendees/booking-slot.test.ts (1)
133-148: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Redundant test with a dead conditional branch.
undefinedParentis a fixedundefinedliteral, soundefinedParent !== undefinedis always false — the spread’s true branch is unreachable, and the resulting object is identical to simply omittingparentListingId, which the earlier "undefined parentListingId is treated as 0" test (Lines 123-131) already covers.Based on learnings, avoid introducing branches in test files that can never be exercised, since this repo's CI enforces branch coverage.
♻️ Proposed simplification
- test("explicit parentListingId 0 and undefined are the same slot", () => { - // undefined and 0 both normalise to 0 in the key — same slot, still a duplicate. - const undefinedParent: number | undefined = undefined; - expect( - hasDuplicateBookingSlot([ - { - date: "2026-07-01", - listingId: 7, - ...(undefinedParent !== undefined - ? { parentListingId: undefinedParent } - : {}), - }, - { date: "2026-07-01", listingId: 7, parentListingId: 0 }, - ]), - ).toBe(true); - }); + test("explicit parentListingId 0 and undefined are the same slot", () => { + // undefined and 0 both normalise to 0 in the key — same slot, still a duplicate. + expect( + hasDuplicateBookingSlot([ + { date: "2026-07-01", listingId: 7 }, + { date: "2026-07-01", listingId: 7, parentListingId: 0 }, + ]), + ).toBe(true); + });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.test("explicit parentListingId 0 and undefined are the same slot", () => { // undefined and 0 both normalise to 0 in the key — same slot, still a duplicate. expect( hasDuplicateBookingSlot([ { date: "2026-07-01", listingId: 7 }, { date: "2026-07-01", listingId: 7, parentListingId: 0 }, ]), ).toBe(true); });🤖 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 `@test/shared/db/attendees/booking-slot.test.ts` around lines 133 - 148, Remove the redundant conditional spread and simplify the test case in hasDuplicateBookingSlot to use an object that directly omits parentListingId, or remove the test if it duplicates the earlier undefined-parent coverage. Do not retain branches based on a compile-time undefined value.Source: Learnings
test/shared/db/attendees/create.test.ts (2)
130-133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Alias the queried table.
As per coding guidelines, SQL tables should be aliased with a descriptive singular word via
AS(e.g.listing_attendees AS attendee).💡 Proposed fix
- "SELECT ledger_event_group FROM listing_attendees WHERE attendee_id = ?", + "SELECT ledger_event_group FROM listing_attendees AS attendee WHERE attendee_id = ?",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const row = await queryOne<{ ledger_event_group: string }>( "SELECT ledger_event_group FROM listing_attendees AS attendee WHERE attendee_id = ?", [attendeeId], );🤖 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 `@test/shared/db/attendees/create.test.ts` around lines 130 - 133, Update the SQL query in the attendee creation test to alias listing_attendees with a descriptive singular alias using AS, and reference that alias in the selected column. Keep the existing queryOne call and result type unchanged.Source: Coding guidelines
162-164: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract the repeated "expect ok" result-narrowing block.
The
if (result === "sold-out" || !result.success) throw new Error("expected ok");pattern is duplicated across five tests. As per coding guidelines, eliminate duplicated code with shared helpers.♻️ Proposed helper
+const expectBookingOk = ( + result: Awaited<ReturnType<typeof createBookingAtomic>>, +) => { + if (result === "sold-out" || !result.success) { + throw new Error("expected ok"); + } + return result; +};Then at each call site, e.g. line 162:
- if (result === "sold-out" || !result.success) { - throw new Error("expected ok"); - } - const attendeeId = result.attendees[0]!.id; + const { attendees } = expectBookingOk(result); + const attendeeId = attendees[0]!.id;Also applies to: 247-249, 273-274, 307-308, 400-402
🤖 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 `@test/shared/db/attendees/create.test.ts` around lines 162 - 164, Extract the repeated result-narrowing logic into a shared helper that accepts the result, throws “expected ok” for “sold-out” or unsuccessful results, and returns the narrowed successful result. Replace the duplicated checks in all five call sites, including the tests around the existing result handling, with this helper.Source: Coding guidelines
test/shared/db/backup.test.ts (1)
147-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Weak
toBeTruthy()assertions — assert exact/expected values instead.
manifest.latestUpdatecan be compared exactly to the already-importedLATEST_UPDATEconstant, just likeschemaHashis checked againstSCHEMA_HASHon the line above.manifest.timestampcould at least assert a concrete format.🧪 Proposed fix for exact assertions
expect(manifest.schemaHash).toBe(SCHEMA_HASH); - expect(manifest.latestUpdate).toBeTruthy(); - expect(manifest.timestamp).toBeTruthy(); + expect(manifest.latestUpdate).toBe(LATEST_UPDATE); + expect(manifest.timestamp).toMatch(/^\d{4}-\d{2}-\d{2}T/); expect(manifest.tables.listings).toBe(1);As per coding guidelines: "Avoid tautological, presence-only, truthiness-only, compound-boolean, and weak assertions; assert exact observable behavior and meaningful invariants."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// Manifest has correct schema hash const manifest: BackupManifest = JSON.parse( new TextDecoder().decode(files["manifest.json"]!), ); expect(manifest.schemaHash).toBe(SCHEMA_HASH); expect(manifest.latestUpdate).toBe(LATEST_UPDATE); expect(manifest.timestamp).toMatch(/^\d{4}-\d{2}-\d{2}T/); expect(manifest.tables.listings).toBe(1);🤖 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 `@test/shared/db/backup.test.ts` around lines 147 - 155, Replace the weak truthiness checks in the manifest assertions with meaningful expectations: compare manifest.latestUpdate exactly against the imported LATEST_UPDATE constant, and validate manifest.timestamp against a concrete expected format or invariant such as an ISO-8601 timestamp. Keep the existing schemaHash and tables.listings assertions unchanged.Source: Coding guidelines
test/shared/db/backup/restore.test.ts (1)
26-27: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate
listingCounthelper — extract to shared test util.This is byte-identical to the
listingCounthelper already defined intest/shared/refund-ledger.test.ts((await queryOne<{ n: number }>("SELECT COUNT(*) AS n FROM listings"))!.n). Move it into a shared#test-utilshelper and import it from both files.As per coding guidelines, "Use helpers from
#test-utilsinstead of defining duplicate local test utilities" and "Eliminate duplicated code with shared helpers or currying."🤖 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 `@test/shared/db/backup/restore.test.ts` around lines 26 - 27, The duplicate listingCount helper should be centralized in a shared `#test-utils` module. Add the byte-identical helper there, export it, remove the local definitions from the restore and refund-ledger tests, and import listingCount from `#test-utils` in both files.Source: Coding guidelines
test/shared/db/client/batch.test.ts (1)
14-21: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate
emptyResultSethelper — same as inbusy.test.ts.Identical to the helper in
test/shared/db/client/busy.test.ts. Extract both into a shared#test-utilshelper.As per coding guidelines: "Use helpers from
#test-utilsinstead of defining duplicate local test utilities."🤖 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 `@test/shared/db/client/batch.test.ts` around lines 14 - 21, Extract the duplicated emptyResultSet helper from batch.test.ts and busy.test.ts into a shared `#test-utils` helper, then import and use that shared helper in both tests, removing their local definitions.Source: Coding guidelines
test/shared/db/client/busy.test.ts (1)
16-23: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate
emptyResultSethelper — extract to#test-utils.This is byte-identical to the
emptyResultSethelper intest/shared/db/client/batch.test.ts. Move it into a shared#test-utilsmodule (e.g.test/test-utils/db-helpers/result-set.ts) and import it from both files.As per coding guidelines: "Eliminate duplicated code with shared helpers or currying" and "Use helpers from
#test-utilsinstead of defining duplicate local test utilities."♻️ Proposed fix
// test/test-utils/db-helpers/result-set.ts import type { ResultSet } from "`@libsql/client`"; /** A minimal libsql ResultSet for stubbed batch/execute calls. */ export const emptyResultSet = (): ResultSet => ({ columns: [], columnTypes: [], lastInsertRowid: undefined, rows: [], rowsAffected: 0, toJSON: () => ({}), });-import type { Client, ResultSet } from "`@libsql/client`"; +import type { Client } from "`@libsql/client`"; import { expect } from "`@std/expect`"; import { afterEach, describe, it as test } from "`@std/testing/bdd`"; import { FakeTime } from "`@std/testing/time`"; import { DatabaseBusyError, execute, setDb } from "`#shared/db/client.ts`"; +import { emptyResultSet } from "`#test-utils/db-helpers/result-set.ts`";🤖 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 `@test/shared/db/client/busy.test.ts` around lines 16 - 23, Extract the duplicated emptyResultSet helper from busy.test.ts and batch.test.ts into a shared `#test-utils` module, preserving its ResultSet shape and type import. Remove both local definitions and import emptyResultSet from the shared module in each test file.Source: Coding guidelines
test/shared/db/prune/helpers.ts (1)
210-234: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Deduplicate
clearAllLastPruned/setAllLastPruned.Both functions repeat the same 10 setter calls, differing only in the value passed. Consolidate into a single array of setters and iterate.
♻️ Proposed refactor
+const LAST_PRUNED_SETTERS = [ + settings.update.lastPrunedPayments, + settings.update.lastPrunedSessions, + settings.update.lastPrunedLogins, + settings.update.lastPrunedTokens, + settings.update.lastPrunedSumup, + settings.update.lastPrunedStrings, + settings.update.lastPrunedContacts, + settings.update.lastPrunedAddresses, + settings.update.lastPrunedInvites, + settings.update.lastPrunedOrphans, +]; + -export const clearAllLastPruned = async (): Promise<void> => { - await settings.update.lastPrunedPayments(""); - await settings.update.lastPrunedSessions(""); - await settings.update.lastPrunedLogins(""); - await settings.update.lastPrunedTokens(""); - await settings.update.lastPrunedSumup(""); - await settings.update.lastPrunedStrings(""); - await settings.update.lastPrunedContacts(""); - await settings.update.lastPrunedAddresses(""); - await settings.update.lastPrunedInvites(""); - await settings.update.lastPrunedOrphans(""); -}; +export const setAllLastPruned = (value: string): Promise<void> => + Promise.all(LAST_PRUNED_SETTERS.map((setter) => setter(value))).then(); -export const setAllLastPruned = async (value: string): Promise<void> => { - await settings.update.lastPrunedPayments(value); - await settings.update.lastPrunedSessions(value); - await settings.update.lastPrunedLogins(value); - await settings.update.lastPrunedTokens(value); - await settings.update.lastPrunedSumup(value); - await settings.update.lastPrunedStrings(value); - await settings.update.lastPrunedContacts(value); - await settings.update.lastPrunedAddresses(value); - await settings.update.lastPrunedInvites(value); - await settings.update.lastPrunedOrphans(value); -}; +export const clearAllLastPruned = (): Promise<void> => setAllLastPruned("");As per coding guidelines, "Eliminate duplicated code with shared helpers or currying; use
jscpd:ignoreonly for import blocks or unavoidable infrastructure."🤖 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 `@test/shared/db/prune/helpers.ts` around lines 210 - 234, Deduplicate the repeated setter calls in clearAllLastPruned and setAllLastPruned by defining one shared array of the ten lastPruned setter functions, then iterating over it with the appropriate value. Have clearAllLastPruned delegate using an empty string and setAllLastPruned delegate using its value parameter, preserving async behavior and TypeScript typing.Source: Coding guidelines
test/shared/forms/saved-data.test.ts (1)
220-231: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace real
setTimeoutsleep with a non-timer interleave.As per coding guidelines, tests must not sleep; timing here should not depend on a real 20ms timer to force concurrent interleaving.
♻️ Suggested fix using a microtask yield instead of a real timer
test("concurrent request scopes do not leak saved form data", async () => { const request = (name: string) => runWithSavedFormContext(async () => { const form = new FormParams(`name=${name}`); setSavedFormData(form); - await new Promise((r) => setTimeout(r, 20)); + await Promise.resolve(); // yield to interleave with the other scope without a real timer return getSavedFormData()?.getString("name"); }); const [a, b] = await Promise.all([request("Alice"), request("Bob")]); expect(a).toBe("Alice"); expect(b).toBe("Bob"); });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.test("concurrent request scopes do not leak saved form data", async () => { const request = (name: string) => runWithSavedFormContext(async () => { const form = new FormParams(`name=${name}`); setSavedFormData(form); await Promise.resolve(); // yield to interleave with the other scope without a real timer return getSavedFormData()?.getString("name"); }); const [a, b] = await Promise.all([request("Alice"), request("Bob")]); expect(a).toBe("Alice"); expect(b).toBe("Bob"); });🤖 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 `@test/shared/forms/saved-data.test.ts` around lines 220 - 231, Replace the real 20ms setTimeout yield inside the concurrent request scopes test with a deterministic microtask-based interleave, such as awaiting a resolved promise, while preserving the ordering needed to exercise both runWithSavedFormContext calls. Keep the existing request helper and assertions validating Alice and Bob isolation.Source: Coding guidelines
test/shared/logger/request-id.test.ts (1)
113-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Non-deterministic uniqueness assertion.
This test relies on real randomness and only asserts
unique.size > 1across 10 draws — it's flaky by construction rather than deterministic. The file already demonstrates stubbingcrypto.getRandomValues(lines 33-44) for deterministic IDs; the same pattern could drive this test with fixed, distinct inputs instead of relying on chance.As per coding guidelines, "Maintain 100% deterministic test coverage; cover important branches with direct in-process unit tests rather than incidental subprocess or end-to-end coverage."
🤖 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 `@test/shared/logger/request-id.test.ts` around lines 113 - 122, Make “different requests get different IDs” deterministic by stubbing crypto.getRandomValues using the existing pattern in the test file, supplying distinct fixed values for each invocation, then assert the resulting IDs are distinct. Restore the stub after the test and retain the runWithRequestId/getRequestId coverage.Source: Coding guidelines
test/ui/client/admin/address-lookup/coords-diff.test.ts (2)
123-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Duplicated form/selector helpers across sibling test files.
formSpecand theoneselector helper here closely mirror the same helpers intest/ui/client/admin/address-lookup/client.test.ts(per graph context). Consider extracting a sharedformSpec/onehelper into#test-utilsfor both files to consume.As per coding guidelines, "Eliminate duplicated code with shared helpers or currying; use
jscpd:ignoreonly for import blocks or unavoidable infrastructure."🤖 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 `@test/ui/client/admin/address-lookup/coords-diff.test.ts` around lines 123 - 157, Extract the duplicated form/selector utilities from formSpec and searchAndChoose in the address-lookup tests into shared helpers under `#test-utils`. Update both coords-diff.test.ts and client.test.ts to import and use the shared formSpec and one-style selector helper, preserving existing behavior and removing the local duplicate implementations.Source: Coding guidelines
159-171: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Event-firing assertion covers only
lat, notlng.The test name promises "fires input events" but only registers a listener on
lat;lng's event firing is unverified.✅ Proposed addition
test("choosing a located address fills lat/lng and fires input events", async () => { - let inputs = 0; + let latInputs = 0; + let lngInputs = 0; const { form, lat, lng } = await searchAndChoose(LOCATED); // Listen late is fine — re-choose to observe the events. lat.addEventListener("input", () => { - inputs += 1; + latInputs += 1; }); + lng.addEventListener("input", () => { + lngInputs += 1; + }); expect(lat.value).toBe("51.503396"); expect(lng.value).toBe("-0.127640"); const select = form.querySelector("[data-address-results]")!; select.dispatch("change"); - expect(inputs).toBe(1); + expect(latInputs).toBe(1); + expect(lngInputs).toBe(1); });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.test("choosing a located address fills lat/lng and fires input events", async () => { let latInputs = 0; let lngInputs = 0; const { form, lat, lng } = await searchAndChoose(LOCATED); // Listen late is fine — re-choose to observe the events. lat.addEventListener("input", () => { latInputs += 1; }); lng.addEventListener("input", () => { lngInputs += 1; }); expect(lat.value).toBe("51.503396"); expect(lng.value).toBe("-0.127640"); const select = form.querySelector("[data-address-results]")!; select.dispatch("change"); expect(latInputs).toBe(1); expect(lngInputs).toBe(1); });🤖 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 `@test/ui/client/admin/address-lookup/coords-diff.test.ts` around lines 159 - 171, Update the “choosing a located address fills lat/lng and fires input events” test to register an input listener on both the lat and lng elements, track each event independently, and assert that both counters equal one after dispatching the change event. Use the existing lat, lng, and select references in the test.test/ui/templates/admin/settings/boolean-settings-section.test.tsx (2)
6-6: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash sed -n '1,16p' test/ui/templates/admin/settings/boolean-settings-section.test.tsxRepository: chobbledotcom/tickets
Length of output: 668
🏁 Script executed:
#!/bin/bash nl -ba test/ui/templates/admin/settings/boolean-settings-section.test.tsx | sed -n '1,120p'Repository: chobbledotcom/tickets
Length of output: 195
🏁 Script executed:
#!/bin/bash awk '{printf "%4d %s\n", NR, $0}' test/ui/templates/admin/settings/boolean-settings-section.test.tsx | sed -n '1,220p'Repository: chobbledotcom/tickets
Length of output: 2449
Assert the specific checked radio.
checkedappearing in both outputs doesn’t prove thetrueandfalsestates map to the right inputs; inspect the actual radio attributes instead.🤖 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 `@test/ui/templates/admin/settings/boolean-settings-section.test.tsx` at line 6, Update the assertions in the boolean settings tests using TestState to inspect each radio input’s actual checked attribute, verifying the true-valued option is checked when enabled and the false-valued option is checked when disabled, rather than only asserting that “checked” appears in the rendered output.
32-40: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Weak assertion doesn't verify which radio is checked.
Both
onHtmlandoffHtmlrender bothvalue="true"andvalue="false"radios per the comment. AssertingtoContain("checked")on each only proves some radio is checked somewhere in the markup — it would still pass if the checked state were swapped (e.g.enabled: trueincorrectly checking thefalseradio). This doesn't test the behavior the test name claims to verify.As per coding guidelines: "Avoid tautological, presence-only, truthiness-only, compound-boolean, and weak assertions; assert exact observable behavior and meaningful invariants."
🐛 Proposed fix — assert checked state against the correct value
test("checks the yes radio when the state is on and the no radio when off", () => { const onHtml = String(section({ enabled: true })); const offHtml = String(section({ enabled: false })); - expect(onHtml).toContain('value="true"'); - expect(offHtml).toContain('value="false"'); - // Both render both options; the checked state is what differs. - expect(onHtml).toContain("checked"); - expect(offHtml).toContain("checked"); + // Both render both options; the checked state is what differs. + expect(onHtml).toMatch(/value="true"[^>]*checked/); + expect(onHtml).not.toMatch(/value="false"[^>]*checked/); + expect(offHtml).toMatch(/value="false"[^>]*checked/); + expect(offHtml).not.toMatch(/value="true"[^>]*checked/); });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.test("checks the yes radio when the state is on and the no radio when off", () => { const onHtml = String(section({ enabled: true })); const offHtml = String(section({ enabled: false })); // Both render both options; the checked state is what differs. expect(onHtml).toMatch(/value="true"[^>]*checked/); expect(onHtml).not.toMatch(/value="false"[^>]*checked/); expect(offHtml).toMatch(/value="false"[^>]*checked/); expect(offHtml).not.toMatch(/value="true"[^>]*checked/); });🤖 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 `@test/ui/templates/admin/settings/boolean-settings-section.test.tsx` around lines 32 - 40, Strengthen the assertions in the “checks the yes radio when the state is on and the no radio when off” test by verifying that the radio with value="true" is checked for enabled: true and the radio with value="false" is checked for enabled: false, while the opposite radios are not checked. Scope each checked-state assertion to its corresponding value so swapped selections cannot pass.Source: Coding guidelines
test/ui/templates/admin/users/agents.test.ts (1)
52-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Local
agentUsershadows the outeragentUserfactory.This block-scoped
agentUserobject shadows the top-levelagentUserfactory function (Line 21) with a different type/purpose. It works today, but the duplicate name invites future confusion if these blocks are ever merged or refactored.♻️ Suggested rename
describe("adminUserAgentsPage", () => { - const agentUser: DisplayUser = { + const targetAgent: DisplayUser = { activated: true, adminLevel: "agent", id: 7, inviteExpired: false, username: "driver", }; test("pre-checks the agents already assigned", () => { - const html = adminUserAgentsPage(agentUser, AGENTS, new Set([2]), SESSION); + const html = adminUserAgentsPage(targetAgent, AGENTS, new Set([2]), SESSION);🤖 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 `@test/ui/templates/admin/users/agents.test.ts` around lines 52 - 58, Rename the local DisplayUser object in the affected test block to a distinct name, such as displayAgentUser, and update all references within that block, preserving the outer agentUser factory function name.
Applies the small, clearly-valid suggestions from the PR #1736 review (the moved test content was reviewed as if new): - attendee-form-model: assert the full desired-line shape with toEqual - config/env: prove the "any other value" branch with a non-echo input - booking-slot: drop the never-true conditional spread - attendees/create: shared expectBookingOk narrowing helper; alias the listing_attendees query per SQL style - backup: exact LATEST_UPDATE match and a real ISO-8601 timestamp check - saved-data: microtask yield instead of a real 20ms sleep - request-id: deterministic distinct-IDs test via a getRandomValues stub - address-lookup coords-diff: assert input events on both lat and lng - boolean-settings-section: pin checked state to the right radio value - users/agents: rename the fixture that shadowed the agentUser factory - prune helpers: one setter list drives clear/set of last-pruned stamps - client tests: shared emptyResultSet in #test-utils (toJSON omitted — nothing calls it, and test-utils files are coverage-measured) - wallet/google: drop ensureCreds, call the once()-cached generator Two bigger refactors (table-driven wallet validation tests, shared address-lookup DOM helpers) are recorded in TODO.md. The suggested listingCount extraction was a false positive — refund-ledger.test.ts's listingCount is a numeric constant, not a duplicate query helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
|
Addressed the CodeRabbit review in 005c25b (the inline threads failed to post, so replying here in one place). All findings concern pre-existing test content that this PR relocated; the small, clearly-valid ones are fixed, two bigger refactors are recorded in TODO.md, and one was a false positive: Fixed in 005c25b
Deferred to TODO.md (valid, but bigger than this rename-only PR's remit — see the "Deferred CodeRabbit suggestions from PR #1736" entry)
Not actionable
Full Generated by Claude Code |
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 `@test/shared/logger/request-id.test.ts`:
- Around line 128-132: Replace the index-based loop in the request ID test with
a collection-based mapping operation using the repository’s curried `#fp` utility
(or equivalent), invoking runWithRequestId for each of ten collection elements
and collecting the returned IDs into ids while preserving deterministic
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 32268ca0-dd1e-48e6-b8e1-88abca5c2846
📒 Files selected for processing (16)
TODO.mdtest/features/admin/attendee-form-model.test.tstest/features/wallet/google.test.tstest/shared/config/env.test.tstest/shared/db/attendees/booking-slot.test.tstest/shared/db/attendees/create.test.tstest/shared/db/backup.test.tstest/shared/db/client/batch.test.tstest/shared/db/client/busy.test.tstest/shared/db/prune/helpers.tstest/shared/forms/saved-data.test.tstest/shared/logger/request-id.test.tstest/test-utils/db-helpers/result-set.tstest/ui/client/admin/address-lookup/coords-diff.test.tstest/ui/templates/admin/settings/boolean-settings-section.test.tsxtest/ui/templates/admin/users/agents.test.ts
The loop counter was never used; Array.from maps ten elements through runWithRequestId (which returns its callback's result) directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
![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
tickets/test/ui/client/admin/address-lookup/coords-diff.test.ts
Lines 10 to 14 in 478939b
Give address-diff its own mirrored test
This file lives under test/ui/client/admin/address-lookup/, so the report assigns the whole suite to src/ui/client/admin/address-lookup.ts, but these imports and the first two describe blocks directly test src/ui/client/admin/address-diff.ts. Move the diffAddressWords/renderAddressDiff cases to test/ui/client/admin/address-diff.test.ts; otherwise the address-diff module remains uncredited while address-lookup gets credit for tests that are not exercising it.
Mirror logistics-map-loader tests separately
Because this path mirrors src/ui/client/admin/logistics-map.ts, the loader cases at the bottom are counted as logistics-map coverage even though they exercise src/ui/client/admin/logistics-map-loader.ts. Move the initLogisticsMapLoader tests into test/ui/client/admin/logistics-map-loader.test.ts so the loader source gets credited and the map suite only reports coverage for the map module.
tickets/test/ui/client/admin/markdown-editor.test.ts
Lines 23 to 31 in 478939b
Split markdown helper modules into mirrored suites
This suite now mirrors only src/ui/client/admin/markdown-editor.ts, but it directly imports and tests the loader, setup, and toolbar modules too. Under the mirror report those cases are all credited to markdown-editor.ts, leaving markdown-editor-loader.ts, markdown-editor-setup.ts, and markdown-editor-toolbar.ts without their own mirrored tests; split those describes/constants into matching test/ui/client/admin/markdown-editor-*.test.ts files.
Move attendee-link tests to the attendee-links mirror
This suite now mirrors src/shared/db/attendees/kind.ts, but the second describe block directly exercises attendeeAdminPath from src/shared/attendee-links.ts. Move those link-routing cases to test/shared/attendee-links.test.ts; otherwise the report credits the kind helper for tests of the link builder while attendee-links.ts remains without a mirrored unit test.
ℹ️ 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".
| @@ -139,8 +137,8 @@ describeWithEnv("ticket view google wallet link", { db: true }, () => { | |||
| }); | |||
|
|
|||
| describeWithEnv("POST /admin/settings/google-wallet", { db: true }, () => { | |||
There was a problem hiding this comment.
When unit-tests-report runs, this path is owned by src/features/wallet/google.ts, so the admin settings cases starting here are credited to the /gwallet/:token route even though /admin/settings/google-wallet is wired through src/features/admin/settings.ts and handled in settings-wallets.ts. Split these POST/settings-page/host-config cases into a mirrored admin settings or settings-wallet suite; otherwise the report overstates coverage for the wallet route while the real settings handler still appears to lack its own unit test.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 00e1abd — the POST /admin/settings/google-wallet describe (validation, save, clear, and settings-page display cases) plus the two host-env settings-page label tests moved to test/features/admin/settings-wallets.test.ts, mirroring src/features/admin/settings-wallets.ts. The wallet config fixtures both suites need (configureGoogleWallet, setGoogleWalletEnvVars) now live in #test-utils/google-wallet.ts, so google.test.ts keeps only the /gwallet/:token route, ticket-link, and config-fallback coverage.
Generated by Claude Code
| @@ -8,7 +8,7 @@ import { | |||
| } from "#templates/public/errors.tsx"; | |||
There was a problem hiding this comment.
This file now mirrors src/ui/templates/public/reservations/og.ts, but this import brings in #templates/public/errors.tsx and the later describe blocks test the public error pages. Under the mirror rule those lines are counted as OG coverage instead of giving src/ui/templates/public/errors.tsx its own mirrored test, so move the error-page cases to test/ui/templates/public/errors.test.tsx and leave only buildOgTags here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 00e1abd — the notFoundPage/temporaryErrorPage/migrationInProgressPage/siteNotActivatedPage describes moved to test/ui/templates/public/errors.test.ts (.ts rather than .tsx since the tests contain no JSX), and og.test.ts now imports and covers only buildOgTags.
Generated by Claude Code
| @@ -2,13 +2,12 @@ import { expect } from "@std/expect"; | |||
| import { describe, it as test } from "@std/testing/bdd"; | |||
| import { adminListingRecalculatePage } from "#templates/admin/listings/aggregates.tsx"; | |||
There was a problem hiding this comment.
This file is now credited to src/ui/templates/admin/listings/form-pages.tsx, but the first describe block tests adminListingRecalculatePage from src/ui/templates/admin/listings/aggregates.tsx. Move that block into test/ui/templates/admin/listings/aggregates.test.tsx; otherwise the aggregates template still appears untested while form-pages receives credit for coverage that belongs elsewhere.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 00e1abd — the adminListingRecalculatePage describe moved to test/ui/templates/admin/listings/aggregates.test.ts (.ts rather than .tsx since the test contains no JSX), mirroring src/ui/templates/admin/listings/aggregates.tsx; form-pages.test.ts now covers only the form-pages templates.
Generated by Claude Code
Codex flagged suites whose path credited one module while their tests exercised others, leaving those modules uncredited by unit-tests-report: - address-diff tests move out of address-lookup/coords-diff.test.ts into test/ui/client/admin/address-diff.test.ts (diffSpec shared via #test-utils/address-lookup-dom.ts) - initLogisticsMapLoader tests move to logistics-map-loader.test.ts - markdown-editor.test.ts splits into markdown-editor-setup.test.ts, markdown-editor-toolbar.test.ts, and markdown-editor-loader.test.ts, with the shared happy-dom harness in #test-utils/markdown-editor-dom.ts - attendeeAdminPath tests move from db/attendees/kind.test.ts to test/shared/attendee-links.test.ts - the /admin/settings/google-wallet cases move from wallet/google.test.ts to test/features/admin/settings-wallets.test.ts (wallet config fixtures shared via #test-utils/google-wallet.ts) - adminListingRecalculatePage tests move from listings/form-pages.test.ts to listings/aggregates.test.ts - public error-page tests move from reservations/og.test.ts to test/ui/templates/public/errors.test.ts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
Main's new provider-batch test imports the helpers at their pre-move path (refund-ledger-helpers.ts); this branch relocated them to refund-ledger/helpers.ts, so typecheck failed after the merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/shared/db/attendees/kind.test.ts (1)
35-49: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse a reachable input for the type-guard branch
test/shared/db/attendees/kind.test.ts:35-49—kindis alwaysSERVICING_KIND, so theelsepath never runs and will remain uncovered underdeno task test:coverage. Reuse the existingcasestable here, or split this into a true-branch narrowing test plus the false cases.🤖 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 `@test/shared/db/attendees/kind.test.ts` around lines 35 - 49, Update the isServicing type-guard test to exercise both reachable outcomes instead of assigning kind only to SERVICING_KIND; reuse the existing cases table or separate true-branch narrowing from false-case assertions. Preserve the compile-time narrowing proof for SERVICING_KIND while ensuring the false branch is executed and covered.Source: Learnings
🤖 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/test-utils/google-wallet.ts`:
- Around line 22-31: Remove the discarded generateGoogleTestCreds() call at the
start of setGoogleWalletEnvVars. Keep the
generateGoogleTestCreds().serviceAccountKey call used for
GOOGLE_WALLET_SERVICE_ACCOUNT_KEY unchanged.
---
Outside diff comments:
In `@test/shared/db/attendees/kind.test.ts`:
- Around line 35-49: Update the isServicing type-guard test to exercise both
reachable outcomes instead of assigning kind only to SERVICING_KIND; reuse the
existing cases table or separate true-branch narrowing from false-case
assertions. Preserve the compile-time narrowing proof for SERVICING_KIND while
ensuring the false branch is executed and covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 47a30b49-e556-4a8c-bab8-f50581c45ad0
📒 Files selected for processing (21)
TODO.mdtest/features/admin/settings-wallets.test.tstest/features/wallet/google.test.tstest/shared/attendee-links.test.tstest/shared/db/attendees/kind.test.tstest/test-utils/address-lookup-dom.tstest/test-utils/google-wallet.tstest/test-utils/markdown-editor-dom.tstest/ui/client/admin/address-diff.test.tstest/ui/client/admin/address-lookup/coords-diff.test.tstest/ui/client/admin/logistics-map-loader.test.tstest/ui/client/admin/logistics-map.test.tstest/ui/client/admin/markdown-editor-loader.test.tstest/ui/client/admin/markdown-editor-setup.test.tstest/ui/client/admin/markdown-editor-toolbar.test.tstest/ui/client/admin/markdown-editor.test.tstest/ui/templates/admin/listings/aggregates.test.tstest/ui/templates/admin/listings/form-pages.test.tstest/ui/templates/public/errors.test.tstest/ui/templates/public/reservations/og.test.tstest/ui/templates/public/reservations/ticket-page.test.ts
💤 Files with no reviewable changes (4)
- test/ui/templates/admin/listings/form-pages.test.ts
- TODO.md
- test/ui/templates/public/reservations/og.test.ts
- test/ui/client/admin/logistics-map.test.ts
The isServicing narrowing test only ever took the true branch, leaving the false branch unreachable under branch coverage; it now maps both kinds through the guard. setGoogleWalletEnvVars pre-warmed the once()- cached credentials one line before reading them — dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
Both branches appended a deferred-follow-ups section to TODO.md at the same spot; the resolution keeps this branch's PR #1736 notes and main's PR #1729 notes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
|
Re the four Codex review findings that had no inline threads — all fixed in 00e1abd:
Generated by Claude Code |
The four Google Wallet settings-validation tests moved to settings-wallets.test.ts in the mirror split, so the TODO entry now names that file instead of google.test.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
Main's helper unification (#1735) rewrote the two payment-link tests to use #test-utils/checkout.ts and relative fixture imports valid at their old test/lib/square location; this branch had moved the tests to test/shared/square, so the resolution keeps main's new imports with the #test/lib/square alias paths that resolve from the new location. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce88b7b006
ℹ️ 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".
| describeWithEnv( | ||
| "googleWallet.getHostConfig", |
There was a problem hiding this comment.
Fresh evidence after the split is that this googleWallet.getHostConfig block still lives under test/features/wallet/google.test.ts, whose mirror owner is src/features/wallet/google.ts, but these assertions exercise only settings.googleWallet.hostConfig and never the /gwallet/:token route. That means unit-tests-report still credits settings coverage to the wallet route while src/shared/wallets/google-wallet-settings.ts remains without its own mirrored coverage; move these host-config settings cases to the wallet-settings/settings mirror instead.
Useful? React with 👍 / 👎.
| }); | ||
| }); | ||
|
|
||
| describeWithEnv("ticket view google wallet link", { db: true }, () => { |
There was a problem hiding this comment.
This block is still owned by the /gwallet/:token route mirror, but these tests request /t/:token (or fetchAliceTicketPageBody) and only assert whether the ticket page renders the Google Wallet link. That behavior is wired in src/features/tickets/index.ts when it passes settings.googleWallet.hasConfig into ticketViewPage, so leaving these cases here overstates coverage for src/features/wallet/google.ts while the ticket route/template path does not receive the mirrored coverage; move the ticket-page link cases (including the env-var variant later in this file) to the ticket-page mirror.
Useful? React with 👍 / 👎.
| describe("buildOrderLines", () => { | ||
| // Exhaustive unit-price precedence lives in test/lib/price-tree.test.ts; here | ||
| // Exhaustive unit-price precedence lives in test/shared/booking/price-tree.test.ts; here |
There was a problem hiding this comment.
In this renamed mirror file, the buildOrderLines block is now credited to src/features/public/ticket-payment.ts, but the cases import and exercise buildOrderLines from src/shared/booking/order-lines.ts rather than any ticket-payment export. This inflates ticket-payment's mirrored coverage and leaves the shared order-lines suite as the owner of logic tested elsewhere; move this block into test/shared/booking/order-lines.test.ts or keep only route-level package context cases here.
Useful? React with 👍 / 👎.
Resolves modify/delete conflicts on 3 more #1693-only files that main updated (inputs.ts, packages.ts, rows.ts) — kept them deleted, our structure replaces them. Also fixes a stale import: test/ui/templates/public/reservation-rows.test.ts (moved by #1736) imported soldOutLabel from reservations/rows.ts (deleted). Exported soldOutLabel from listing-rows.ts (where the sold-out rendering lives) and updated the 3 inline sites to use it. All checks green: typecheck PASS, src jscpd 0 clones, test jscpd 0 clones, lint PASS.
Resolves all conflicts with main including the test file moves from #1736 (Move 104 test files to their sources' mirror locations): - 3 modify/delete on #1693-only files (inputs.ts, packages.ts, rows.ts) removed — our structure replaces them - test/ui/templates/public/reservations/og.test.ts: content conflict resolved by taking main's file location (moved by #1736) but updating the import from reservations/og.ts (#1693, deleted) to reservations/og-tags.ts (ours) - test/ui/templates/public/reservation-rows.test.ts: stale import of soldOutLabel from reservations/rows.ts (deleted) fixed to import from reservations/listing-rows.ts; soldOutLabel exported and the 3 inline sold-out spans refactored to use it - test/templates/public/og-and-status-pages.test.ts: deleted by main's #1736 split (status-page tests moved to errors.test.ts) All checks green: typecheck PASS, src jscpd 0 clones, test jscpd 0 clones, lint PASS.
2 participants
What changed
The unit-tests-report only counts a source file as unit-tested when its test sits at the mirror location (
test/<same path as src>). Lots of real unit tests were sitting under older folder names —test/lib/,test/templates/,test/routes/— or used a joined name likedates-long-label.test.tsinstead of the folder formdates/long-label.test.ts. This moves 104 of them to where the report looks for them. No test logic changed; only file locations and a few import paths.How each move was decided
A test was only moved when its owning source file was unambiguous:
test/lib/i18n.test.ts→src/shared/i18n.ts), or<source>-<topic>for exactly one imported source, so it becomes<source>/<topic>.test.ts.Tests that drive HTTP requests were only moved when they belong to a route (
src/features/) file — integration-style suites that exercise many modules at once stayed put for a future pass.Housekeeping
#test/alias, which the repo already uses for cross-folder test imports.prune-helpers.tsandrefund-ledger-helpers.tsmoved into their suites' new folders, since only those suites use them.TODO.mdand a code comment) were updated.Report impact
🤖 Generated with Claude Code
https://claude.ai/code/session_01AnAupwn2rCio7DENdnWh2w
Generated by Claude Code
Summary by CodeRabbit