Move the inline tests out of native_outstandings/wire.rs and db/tally_write_store.rs - #442
Merged
Merged
Conversation
…_write_store.rs docs/proposed-rust-module-conventions.md item 2, continuing #440 and #441. Three inline test modules move beside their parents, each kept as the same child module via #[path]: bridge-tally-protocol native_outstandings/wire.rs currency_tests -> wire_currency_tests.rs group_tests -> wire_group_tests.rs bridge db/tally_write_store.rs tests -> tally_write_store_tests.rs wire.rs goes from 1,796 to 1,262 lines; tally_write_store.rs from 1,794 to 1,259. Neither has production code after its test blocks. Checked, not asserted: - Both parents rebuild byte-identical to HEAD from the new parent plus the children re-indented, leaving the eight multi-line string literals (3 + 1 + 4) untouched rather than re-indented. - rustfmt changed whitespace only in wire_currency_tests.rs and tally_write_store_tests.rs: no code token changed and every literal is identical. The parents and wire_group_tests.rs were unchanged by it. - Moved tests, counted from the blocks' own test attributes and matching their exact module paths: 23 in wire.rs, 6 in tally_write_store.rs. All pass; wire.rs's under both default and all features. - Whole-lib test lists are identical before and after: bridge under default (968), voucher-scan (986), live-calibration-harness (988) and lab-writes (1,068); bridge-tally-protocol under default (73) and all features (112). The only compiler warnings are the two existing unused variables in tally/runtime.rs under voucher-scan. - The moved wire.rs tests' include_bytes! paths use concat!(env!(...)) and the one relative include_str! resolves from the same directory. - Text readers: two fixture provenance notes name wire.rs, both in the past tense about a hand-authored constant these captures replaced, so they stay accurate. check-fixture-provenance.mjs walks fixture directories, not source; it fails identically on origin/master (51 undocumented fixtures), before and after this change. - Frontend suite passes (239 node, 113 vitest, 10 Playwright). check-tally-live-read-boundary.mjs passes. - clippy -D warnings is clean for CI's portable-crate step, its protocol-evidence-features step, and bridge --lib --tests. - Resealing changed exactly two pinned hashes; the coverage report counts three newly unsealed test-only modules and lists nothing else. The three new test files are unpinned, as in #440 and #441. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
lamemustafa
added a commit
that referenced
this pull request
Sep 16, 2026
…ot by name #440, #441 and #442 moved inline `#[cfg(test)] mod tests { ... }` blocks into sibling files loaded by `#[cfg(test)] #[path = "..."] mod tests;`. check-tally-request-builder-hazards.mjs skipped only the inline form, so it began reading those files as production code. Results did not change, but a test that builds a hazard-shaped string as an expectation would have raised a false alarm. Skipping any file named *_tests.rs was rejected: a safety scanner must not exempt production code for its file name. Instead a file is skipped only when it is actually loaded as a test module: - Edges: a brace-depth-0 out-of-line `mod name;` whose attribute group has a cfg implying test (cfg(test), or test as an argument of cfg(all(...))), with #[path] or bare in a crate root or mod.rs; plus any such declaration made by a file already test-only, iterated to a fixed point. - Vetoes, from every .rs file in the repository and matched by basename case-insensitively: any other `mod name;` at any depth (including macro bodies and inline modules), any `path = "..."` or include!-family string not part of an edge (covering cfg_attr), and every Cargo crate root. - Files whose braces underflow or do not end balanced contribute no edges. - Everything unhandled -- cfg(any(...)), #[path] inside an inline module, r#name -- leaves the file scanned, so an error is a false alarm, never a missed hazard. The design was critiqued before implementation. The critique found four ways the first draft could skip a production file -- a non-test declaration it did not recognise, a Cargo target, a declaration outside the scan roots, case variants -- and that it would still have scanned 24 of the 74 extracted files: 22 declared by other test files, one under cfg(all(test, ...)), one bare in a mod.rs. The veto set, the fixed point and those two cfg/bare shapes are the response. Checked, not asserted: on this repository the skipped set is exactly the 74 files the rustc-validated module graph from #436 marks test-only, path for path; the 10 pinned violations are unchanged. The skipped count is now pinned too, so a lexer regression that quarantines too much fails the gate. Main now runs under `import.meta.main` so the functions can be imported; a test runs the real script as a child process so a main that silently does nothing cannot pass. 15 fixture tests; 15 mutations of the scanner (trust file names, accept any cfg mentioning test, drop each veto, trust unbalanced files, accept nested declarations, drop propagation, drop the crate-root veto, case-sensitive veto, #[path] under the stem, main never running, no bare-mod detection, strings or comments not skipped, only the first attribute, no raw-string path) each fail a named test. One of those first survived because a fixture's production declaration masked the lexer; the fixture was fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lamemustafa
added a commit
that referenced
this pull request
Sep 16, 2026
docs/proposed-rust-module-conventions.md item 2, continuing #440-#442, for the last large inline test block among pinned files: the `tests` module of tools/bridge-tally-compatibility/src/lib.rs -- the tool that validates the compatibility surface -- moves to lib_tests.rs beside it via #[path]. lib.rs goes from 2,853 to 1,686 lines. No production code followed the block. Checked, not asserted: - lib.rs rebuilds byte-identical to HEAD from the new parent plus the re-indented child. - rustfmt's only non-whitespace change is one trailing comma; all 169 string and char literals are identical. - Test lists are identical before and after: 24 lib tests by default, 30 with bills-native-outstandings-probe-receipt. The full crate passes under both, with no warnings. - Before resealing, real_tree_has_complete_migration_and_report_surface_coverage failed with surface_file_changed -- the expected result of changing a pinned file -- and passes after; reseal --verify exits 0 and exactly one pinned hash changed. - clippy -D warnings is clean for CI's tools-workspace step and its probe-receipt feature step. - Text readers: check-tally-live-read-boundary.mjs allowlists this lib.rs for probe identifiers; the moved tests contain none, and it passes. The request-builder hazard scanner now reads lib_tests.rs as production (see #443) and still reports exactly its 10 pinned violations. The other scanning scripts pass, and the frontend suite passes (239 node, 113 vitest, 10 Playwright) on a clean tree -- the merge-driver test refuses to run with uncommitted surface changes. Coverage, stated rather than assumed. lib_tests.rs is unpinned, following #416's rule and #440-#442: tests decide nothing about what Bridge posts or emits. For this file the loss is sharper than before, because real_tree_has_complete_migration_and_report_surface_coverage holds the only executable check of the reserve bound, `MAX_SURFACE_FILES - files.len() <= RESERVED_SURFACE_FILES`. The cap itself stays enforced in the pinned lib.rs. Editing or deleting that assertion no longer changes a pinned hash. The one #[cfg(windows)] test, normalise_surface_path_uses_forward_slashes, could not run on the macOS host that did this move; CI's windows-latest native job runs the tools workspace tests and covers it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lamemustafa
added a commit
that referenced
this pull request
Sep 16, 2026
…446) docs/proposed-rust-module-conventions.md item 2, continuing #440-#442. The inline `tests` module of tools/bridge-tally-compatibility/src/lib.rs -- the tool that validates the compatibility surface -- moves to lib_tests.rs beside it via #[path]. lib.rs goes from 2,853 to 1,686 lines. No production change: before rustfmt, lib.rs rebuilt byte-identical to the base from the new parent plus the re-indented child; rustfmt then changed layout only, its one non-whitespace change a trailing comma, with every literal identical. Test lists are identical before and after (24 lib tests by default, 30 with bills-native-outstandings-probe-receipt), all pass, and clippy is clean for both CI tools steps. The #[cfg(windows)] test compiles for x86_64-pc-windows-msvc and runs in CI's windows-latest native job. lib_tests.rs is unpinned, following #416's rule and #440-#442. That unseals the only executable check of the reserve bound, `MAX_SURFACE_FILES - files.len() <= RESERVED_SURFACE_FILES`; the cap itself stays enforced by validate_shape in the pinned lib.rs. Inline tests are not done: 70 pinned files still hold 18,384 lines of top-level inline #[cfg(test)] modules, led by source_draft/catalog.rs (1,779) and tally/connector.rs (1,317). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues #440 and #441 under
docs/proposed-rust-module-conventions.mditem 2, which extracts inline test modules first. There is no production change.bridge-tally-protocolnative_outstandings/wire.rswire_currency_tests.rs(283),wire_group_tests.rs(250)bridgedb/tally_write_store.rstally_write_store_tests.rs(534)Each block stays the same child module via
#[cfg(test)] #[path = "…"] mod <name>;. Neither parent has production code after its test blocks.Checked, not asserted
No production change. Both parents rebuild byte-identical to
HEADfrom the new parent plus the re-indented children. The eight multi-line string literals (3 + 1 + 4) were left as-is rather than re-indented.rustfmt changed only whitespace in
wire_currency_tests.rsandtally_write_store_tests.rs. No code token changed, and every literal is identical. It left the parents andwire_group_tests.rsunchanged.Moved tests were counted from the blocks' own test attributes, and the counts match their exact module paths: 23 in
wire.rsand 6 intally_write_store.rs. All pass.wire.rs's tests pass under both default and all features.No test lost. Whole-lib test lists are identical before and after:
bridge: default (968),voucher-scan(986),live-calibration-harness(988),lab-writes(1,068).bridge-tally-protocol: default (73) and all features (112).The only compiler warnings are the two existing unused variables in
tally/runtime.rsundervoucher-scan.Include paths. The moved
wire.rstests useinclude_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), …)). The one relativeinclude_str!resolves from the same directory as before.Text readers, checked before moving anything:
wire.rs, both in the past tense about a hand-authored constant these captures replaced, so they stay accurate.check-fixture-provenance.mjswalks fixture directories, not source. It fails onorigin/master(be660567) with identical output (51 undocumented fixtures), before and after this change.check-tally-live-read-boundary.mjspasses.Frontend suite passes: 239 node, 113 vitest and 10 Playwright tests.
Clippy
-D warningsis clean for CI's portable-crate step, its protocol-evidence-features step, andbridge --lib --tests.Reseal: exactly 2 pinned hashes changed, and
--verifyexits 0. The coverage report countsnewly unsealed test-only modules: 3and lists nothing else.The three new test files are unpinned, as in #440 and #441.
🤖 Generated with Claude Code