Move eleven more misplaced unit tests to their source mirror paths - #1769
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (11)
⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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/shared/demo/mode.test.ts
Line 3 in cbc2a48
Keep payment-provider tests out of demo mode's mirror
This suite was moved to test/shared/demo/mode.test.ts because it imports setDemoModeForTest, but that import is only used for teardown while every assertion exercises POST /admin/settings/payment-provider. With the file at the direct mirror for src/shared/demo/mode.ts, the unit-test report now credits demo-mode code to payment-provider route tests, and a mutation run for demo mode would not run the real demo-mode assertions that still live elsewhere. Move these cases under the settings handler's mirror (or split actual mode assertions into this file) so the source-to-test bookkeeping stays accurate.
ℹ️ 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".
A second batch of the same low-risk cleanup: eleven test files each exercise exactly one source but lived under test/lib or another folder that mirrors nothing, so the coverage report counted their sources as untested. Moving each to its source's mirror location lines the test tree up with src/ and flips eleven sources from untested to mirrored (316 -> 327). All imports are alias-based, so the moves are pure renames with no code changes: admin-image-upload.test.ts -> test/features/admin/image-upload.test.ts question-forms.test.ts -> test/features/admin/questions.test.ts packages.test.ts -> test/features/api/payment-processing/package-pricing.test.ts pair-entries-by-listing.test.ts -> test/features/api/payment-processing/create.test.ts server-listings/with-cookie... -> test/features/response.test.ts column-order/columns.test.ts -> test/shared/columns/listing-columns.test.ts csv/generate.test.ts -> test/shared/csv/index.test.ts server-settings/payment-provider -> test/features/admin/settings-general/payment-provider.test.ts render-date-selector.test.ts -> test/ui/templates/public/reservations/controls.test.ts reservation-rows.test.ts -> test/ui/templates/public/reservations/listing-rows.test.ts reservations/og.test.ts -> test/ui/templates/public/reservations/og-tags.test.ts The payment-provider suite imports demo/mode only for afterEach teardown while every assertion drives POST /admin/settings/payment-provider, so it belongs at the settings-general handler's mirror, not demo/mode's. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q8LbdSHz7eSXyqY3GBSAgw
cbc2a48 to
e66a4b7
Compare
|
@codex Good catch — fixed in e66a4b7. You're right that this suite only imports Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Move the demo-mode test to its actual mirror
This file still imports exactly one source, #shared/demo/mode.ts, but it was moved under test/features/admin/settings-general/ even though there is no matching src/features/admin/settings-general/payment-provider.ts. Under the repo's mirror rule, src/shared/demo/mode.ts maps to test/shared/demo/mode.test.ts or a file inside test/shared/demo/mode/, so the unit-test report will continue to count this as a misplaced orphan and will not credit the demo-mode source as intended.
ℹ️ 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".
Summary
Testing
|
|
@codex Thanks — and agreed with your follow-up summary that no further change is needed. To close the loop on the intermediate P2 ("will not credit the demo-mode source"): The placement is correct, and I verified it against the actual report. There are two independent mechanisms in play:
So the source-to-test bookkeeping is accurate where it matters. Leaving as placed. Generated by Claude Code |
Summary
Testing
|
2 participants
What changed
A second batch of the same low-risk cleanup as #1765. Eleven unit-test files each check a single piece of code in
src/, but sat undertest/lib/(or another folder that mirrors nothing), so the coverage report treated those pieces of source as having no test at all — even though a working test existed.Each test moves to sit alongside the code it checks:
admin-image-upload.test.tstest/features/admin/image-upload.test.tsquestion-forms.test.tstest/features/admin/questions.test.tspayment-processing/packages.test.tstest/features/api/payment-processing/package-pricing.test.tspayment-processing/pair-entries-by-listing.test.tstest/features/api/payment-processing/create.test.tsserver-listings/with-cookie.test.tstest/features/response.test.tscolumn-order/columns.test.tstest/shared/columns/listing-columns.test.tscsv/generate.test.tstest/shared/csv/index.test.tsserver-settings/payment-provider.test.tstest/features/admin/settings-general/payment-provider.test.tsrender-date-selector.test.tstest/ui/templates/public/reservations/controls.test.tsreservation-rows.test.tstest/ui/templates/public/reservations/listing-rows.test.tsreservations/og.test.tstest/ui/templates/public/reservations/og-tags.test.tsWhy it's safe
Every one of these files imports its code through project aliases (
#shared/...,#routes/...), not relative paths, so moving the file changes nothing about how it runs — they are pure renames with no edits to any test code.A note on the payment-provider suite
The automated misplaced-test report flagged
payment-provider.test.tsagainstsrc/shared/demo/mode.ts, because that's the only source it imports. But that import (setDemoModeForTest) is used only forafterEachteardown — every assertion drivesPOST /admin/settings/payment-provider, whose handler lives insrc/features/admin/settings-general.ts. Filing it at demo-mode's mirror would have mis-credited demo-mode coverage to a route test and skewed that file's mutation bookkeeping. It's therefore placed at the settings-general handler's mirror instead, which is the source it actually exercises. Thanks to the Codex reviewer for catching this.Result
The unit-test report now recognises eleven more sources as having a mirrored test (316 → 327), and eleven fewer as untested. All eleven moved tests were run in their new locations and pass (0 failures); typecheck and lint are clean.
As with the first batch, these are the safe subset of the
deno task unit-tests-reportmisplaced-tests list — each carries no shared fixtures, isn't imported by any other test, and lands on a free mirror path. Tests that share fixtures or use relative imports (e.g. thebooking/modelandstripegroups) were left out of this round.🤖 Generated with Claude Code
https://claude.ai/code/session_01Q8LbdSHz7eSXyqY3GBSAgw