fix(ci): classify the merged regression tests explicitly in the layout inventories - #5075
Conversation
…t inventories The regex seeds place a conventionally named file, so a regression test can sit in the tree, run in CI, and still be absent from the authoritative table. That is how the regression tests for #5050, #5051 and #5055 landed without ever entering scripts/test-layout/layout.json or tests/fixtures/test-layout-expected.json (#5059). The two inventories are two copies of one table and the membership oracle already compares them, so both sides get the same three entries. A new test names the three files so they cannot fall out again silently, and checks that each one actually sits in the directory its registration claims. No repository-wide explicit-registration policy is introduced here; the seeds keep carrying brand-new files as designed.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds explicit inventory entries for three regression tests, updates the expected layout fixture, and adds a membership-oracle regression test that verifies ownership and file placement. ChangesTest layout inventory
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The inventory, fixture, and membership checks are aligned, so this change is ready to merge with normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Summary
The regression tests that landed with #5050, #5051 and #5055 are not in the authoritative
test-layout inventories. They run today only because the regex seeds in
scripts/test-layout/layout.jsonplace a conventionally named file, which is exactly what theseeds are for — carrying a brand-new file until someone classifies it. Nobody ever did, so three
merged regression tests have no recorded owner and nothing would notice if a rename or a move
quietly dropped them (#5059).
layout.explicitandtests/fixtures/test-layout-expected.jsonare two copies of one table andthe membership oracle already asserts they are identical, so both sides get the same three
entries:
tests/ci-workflows/ci-structure-gate.test.tsci-workflowstests/responses/responses-code-mode-patch-compile.test.tsresponsestests/gui/gui-codex-usage-score-parity.test.tsguiA test in
tests/test-layout-tooling.test.tsnames the three so they cannot fall out againwithout failing, and checks that each one actually sits in the directory its registration
claims. A registration pointing at a path the tree does not have is a claim, not a fact.
What this deliberately does not do is introduce a repository-wide "every test must be explicit"
rule. That would change the existing seed policy for 1364 entries and is a separate decision;
this change fixes the three files the issue is about.
Closes #5059
Verification
No local suite was run; this repository verifies on hosted CI. The change is inventory data plus
one oracle test, and the existing guards bind it:
membership oracle > the live tree and the fixture agree entry by entrycompareslayout.explicitagainst the fixture and fails on anydivergence, and
tests/test-layout.test.tsfails if a registered file does not sit in itsdomain. Both inventories were re-read after the edit and parse to identical maps of 1367 entries.
Checklist
Summary by CodeRabbit