Move admin debug tests and add a template rendering test - #1875
Conversation
- Move test/lib/server-debug.test.ts to test/features/admin/debug/server.test.ts, extracting makeDebugState and the owner session into test/test-utils/debug.ts. - Move test/lib/server-debug-sentry.test.ts unchanged to test/features/admin/debug/sentry.test.ts. - Add test/ui/templates/admin/debug/rendering.test.tsx covering the navigation/section structure, missing and supplied values, badge variants for site/database/availability/storage, overridden limits, and the SENTRY_TEST_FORM_ID constant. This targets current main, so the prune state stays at Never across all fields and the existing Database pruning section test in the server test is preserved unchanged.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds shared admin debug test utilities and expands coverage for debug-page rendering and Sentry endpoint behavior, including configuration states, submission outcomes, read-only mode, request counts, and manager authorization. It also documents checkout-stage and debug-test follow-ups. ChangesAdmin debug testing
Checkout stage deletion follow-ups
Estimated code review effort: 3 (Moderate) | ~20 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: 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/admin/debug/sentry.test.ts`:
- Around line 63-68: Strengthen the “sends a tagged test error and confirms
delivery” test by replacing the shared request-count-only stub with a local
fetch recorder, then inspect the captured Sentry envelope to assert the expected
test-error message and tags source=admin-debug and test=true. Retain the
request-count assertion and existing redirect/flash checks, using the recorder
to validate the emitted request body details.
In `@test/ui/templates/admin/debug/rendering.test.tsx`:
- Around line 38-40: Update the assertions in the debug rendering test around
the href check to validate semantic debug section headings or table labels
instead of counting “prose” and “table-scroll” CSS-class occurrences. Remove the
hard-coded class-count expectations while preserving the existing admin debug
link assertion.
🪄 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: 7a642699-17c9-427c-a5ec-72e2adf89c47
📒 Files selected for processing (4)
test/features/admin/debug/sentry.test.tstest/features/admin/debug/server.test.tstest/test-utils/debug.tstest/ui/templates/admin/debug/rendering.test.tsx
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: 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/admin/debug/sentry.test.ts`:
- Around line 63-68: Strengthen the “sends a tagged test error and confirms
delivery” test by replacing the shared request-count-only stub with a local
fetch recorder, then inspect the captured Sentry envelope to assert the expected
test-error message and tags source=admin-debug and test=true. Retain the
request-count assertion and existing redirect/flash checks, using the recorder
to validate the emitted request body details.
In `@test/ui/templates/admin/debug/rendering.test.tsx`:
- Around line 38-40: Update the assertions in the debug rendering test around
the href check to validate semantic debug section headings or table labels
instead of counting “prose” and “table-scroll” CSS-class occurrences. Remove the
hard-coded class-count expectations while preserving the existing admin debug
link assertion.
🪄 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: 7a642699-17c9-427c-a5ec-72e2adf89c47
📒 Files selected for processing (4)
test/features/admin/debug/sentry.test.tstest/features/admin/debug/server.test.tstest/test-utils/debug.tstest/ui/templates/admin/debug/rendering.test.tsx
🛑 Comments failed to post (2)
test/features/admin/debug/sentry.test.ts (1)
63-68: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Verify the diagnostic payload, not only its request count.
This test does not prove that the emitted event is tagged or even that it is the intended test error. Use a local fetch recorder to assert the Sentry envelope contains the expected message and
source=admin-debug/test=truetags; the shared stub is only appropriate for call-count checks.As per coding guidelines, tests must use strong mutation-resistant assertions. Based on learnings, retain/create a local fetch mock when per-request URL, method, or body details must be asserted.
🤖 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/debug/sentry.test.ts` around lines 63 - 68, Strengthen the “sends a tagged test error and confirms delivery” test by replacing the shared request-count-only stub with a local fetch recorder, then inspect the captured Sentry envelope to assert the expected test-error message and tags source=admin-debug and test=true. Retain the request-count assertion and existing redirect/flash checks, using the recorder to validate the emitted request body details.Sources: Coding guidelines, Learnings
test/ui/templates/admin/debug/rendering.test.tsx (1)
38-40: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert semantic debug sections instead of CSS-class counts.
Lines 39-40 couple this test to presentation classes and hard-coded wrapper totals; adding/removing a layout wrapper can fail it without changing page behavior. Assert the expected section headings or table labels 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/debug/rendering.test.tsx` around lines 38 - 40, Update the assertions in the debug rendering test around the href check to validate semantic debug section headings or table labels instead of counting “prose” and “table-scroll” CSS-class occurrences. Remove the hard-coded class-count expectations while preserving the existing admin debug link assertion.Source: Coding guidelines
|
Thanks for the review. Both findings are valid observations but are out of scope for this PR's brief, so I've recorded them in
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0235d0ae34
ℹ️ 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".
# Conflicts: # TODO.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
TODO.md (2)
1330-1341: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the exact Sentry assertion values.
Specify the literal expected message and tag values so the follow-up cannot be implemented with weaker or incorrect assertions.
🤖 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 1330 - 1341, Update the TODO guidance for the “sends a tagged test error and confirms delivery” test to require literal assertions for the envelope message emitted by the POST /admin/debug/sentry handler, plus source=admin-debug and test=true; derive the exact message literal from that handler rather than describing it generically, while retaining request-count, redirect, and flash assertions.
1342-1355: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the semantic test oracle independent of
DEBUG_SECTIONS.Deriving expected headings from the production list can allow a missing or renamed rendered section to pass if the list changes with it. Use an explicit expected heading set or separately maintained fixture, while retaining the
/admin/debuglink assertion.🤖 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 1342 - 1355, Update the “keeps the debug navigation and section structure” test to assert an explicitly maintained set of expected debug section headings, independent of DEBUG_SECTIONS or production rendering data. Keep the /admin/debug link assertion and ensure the test detects added, removed, or renamed rendered sections without relying on CSS-class counts.
🤖 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.
Outside diff comments:
In `@TODO.md`:
- Around line 1330-1341: Update the TODO guidance for the “sends a tagged test
error and confirms delivery” test to require literal assertions for the envelope
message emitted by the POST /admin/debug/sentry handler, plus source=admin-debug
and test=true; derive the exact message literal from that handler rather than
describing it generically, while retaining request-count, redirect, and flash
assertions.
- Around line 1342-1355: Update the “keeps the debug navigation and section
structure” test to assert an explicitly maintained set of expected debug section
headings, independent of DEBUG_SECTIONS or production rendering data. Keep the
/admin/debug link assertion and ensure the test detects added, removed, or
renamed rendered sections without relying on CSS-class counts.
Codex review of PR #1875 pointed out that "runtime-marker" is not a member of the RuntimeInfo.runtime union ("bunny" | "deno" | "node" | "unknown"). The test nonetheless passed because test/**/*.tsx is outside the typecheck glob and the suite runs with --no-check, so an invalid literal could slip through and would resurface as a real type error the moment TSX tests were added to the typecheck. Use "bunny" instead.
CodeRabbit's 13:58 review on PR #1875 left two advisory comments on the TODO.md section that records the deferred follow-ups. Both are good points about the future follow-up's quality: - The Sentry-envelope follow-up should name the literal expected message and tag values ("Test Sentry notification from the admin debug page." and source=admin-debug / test=true) so it cannot be implemented with weaker assertions than intended. Incorporated by reading sendSentryTest in src/shared/sentry.ts and pinning the literal values into the TODO. - The semantic-sections follow-up should maintain the expected heading set as an explicit literal list inside the test, not derive it from DEBUG_SECTIONS: deriving the oracle from the same source list the template renders against lets a removed or renamed section pass undetected when both the rendering and the oracle shift in lockstep. The guidance now says so explicitly.
|
Addressed both advisory comments from CodeRabbit's 13:58 review (the ones flagged as outside-diff-range on
These are guidance improvements for the future follow-up PR; the current PR's code is unchanged. |
Resolved conflicts in 10 files: - TODO.md: kept both entries (PR #1872 extraction deferral and PR #1875 admin debug follow-ups). - e2e-payments/src/providers/stripe.ts: kept main's restructured imports and our per-endpoint try/catch in the cleanup loop. - test/integration/code-quality.test.ts: combined main's #test/ alias imports with our detectRelativeImport import from relative-import.ts. - test/integration/server-balance-webhook.test.ts, test/integration/server/public/ticket-csrf-and-capacity.test.ts, test/integration/server/reservation-edge-cases.test.ts, test/integration/server/reservation-no-provider.test.ts, test/integration/server/webhooks/can-pay-more-multi-ticket.test.ts, test/integration/stripe-mock-ports.test.ts, test/integration/webhook-price-signature-trusted-and-mismatch.test.ts: took main's versions — Stripe SDK replacement dropped resetStripeClient imports, file moves changed #test/lib/ aliases to relative paths.
Two test-only follow-ups from the PR #1875 review: The admin Sentry test now records the envelope the page actually sends and checks the test message and its source/test tags, instead of only counting that one request happened. The debug page structure test now asserts the exact ordered list of section headings — an independent literal list, so a dropped, renamed, or added section is a conscious test edit — instead of counting presentation classes. Writing that list surfaced the Email section the class counts said nothing about. Closes the "Admin debug test coverage follow-ups" TODO section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ehWXhDVAmSZFkKWVo4grA
1 participant
This is a test-only change. It does not touch
src/or any user-facing copy.What changed
test/lib/server-debug.test.tstotest/features/admin/debug/server.test.tsand extracted the localmakeDebugStatehelper plus the owner session into a sharedtest/test-utils/debug.ts. The moved server test now imports these shared exports and still keeps its existing "Database pruning section" test.test/lib/server-debug-sentry.test.tsbyte-for-byte totest/features/admin/debug/sentry.test.ts.test/ui/templates/admin/debug/rendering.test.tsxthat renders the admin debug template directly and asserts the navigation/section structure, missing and supplied values, badge variants for site/database/availability/storage, overridden limits, and the stableSENTRY_TEST_FORM_ID.TODO.mdunder "Admin debug test coverage follow-ups" — both are valid observations about the future follow-up work (inspect the Sentry envelope body, not just the request count; assert semantic section headings rather than CSS-class counts), kept here so the suggestions are not lost between this PR and whoever picks them up. No production code is touched.Why
These tests had outlived their home under
test/lib/. They now sit alongside the rest of the admin feature tests undertest/features/admin/debug/, and the sharedmakeDebugState/debugOwnerSessionhelpers are reusable for the new direct-rendering test.Current-main contracts
Because this targets current
main, the prune state stays atNeveracrossaddresses,logins,payments,sessions, andstrings, and the debug page still renders its prune section. The new rendering test asserts the matching counts for current main: 3class="prose"and 13class="table-scroll".Verification
The diff contains the two moves, the new
test/test-utils/debug.tsandtest/ui/templates/admin/debug/rendering.test.tsx, plus aTODO.mdsection recording two out-of-scope review suggestions for the future follow-up work they describe.