Repository navigation
broker: split broker/tests.rs into schema / validation / client modules #6746
Description
Follow-up from review of #6742 (broker action contract, PR 1 of the broker stack). Quality review approved the PR at 5e2879a72 and flagged one non-blocking item: crates/buzz-sdk/src/broker/tests.rs is now 1,693 lines, past the point where its section headers compensate for file size.
This is a mechanical split, not a test change. It must land before the next broker test addition — i.e. before PR 2 of the stack (broker client + buzz agents CLI) grows the file further. Do not do it on ss-dev-00/broker-action-contract; that head is frozen for review.
What to do
Split broker/tests.rs into focused child modules along the existing section boundaries, moving code verbatim. Current sections and their sizes at 5e2879a72:
| Lines | Section | Goes to |
|---|---|---|
| 1–163 | fixtures + helpers (pubkey, signed_message, all_error_codes, action_fixtures, outcome_fixtures, prepared, keys_of, the three consts) |
tests/mod.rs (shared) |
| 164–195 | Coverage | tests/schema.rs |
| 196–463 | Envelope round-trip | tests/schema.rs |
| 464–565 | Envelope rejection | tests/schema.rs |
| 566–881 | Wire schemas: the enforceable no-secret invariant | tests/schema.rs |
| 882–986 | Argument validation | tests/validation.rs |
| 987–1189 | Reads carry verifiable provenance | tests/validation.rs |
| 1190–1385 | Results | tests/validation.rs |
| 1386–1455 | Retry is identical bytes | tests/client.rs |
| 1456–1693 | Client trait (DoubleBroker, WireBroker, block_on) |
tests/client.rs |
Resulting sizes: schema ≈ 718, validation ≈ 504, client ≈ 308, plus a shared mod.rs of ≈ 163 — all comfortably under the repo's 1,000-line cap.
Constraints
- Keep it in-crate.
tests.rsis a#[cfg(test)] mod tests;child ofbroker(crates/buzz-sdk/src/broker/mod.rs:838) and starts withuse super::*. The strictness guards it exercises live inDeserializeimpls behind private wire intermediaries —WireResponse(broker/mod.rs:623) andStrictEvent(broker/actions/outcomes.rs:30) — andDispatch's field is private to the client module. Converting these tocrates/buzz-sdk/tests/integration tests would change what they can reach and what they prove. Structure the split asbroker/tests/{mod,schema,validation,client}.rs. - Tables preserved unchanged. The exact-key-set schema table,
all_error_codes(), the status↔error-code table (which deliberately spells the table a second time rather than consultingmay_be_failed()/may_be_indeterminate()), and the read-limit boundary table are the load-bearing part of this suite. Copy them; do not rewrite, dedupe, parameterize, or "tidy" them. - No behavioral coverage added in the same change. Review explicitly did not ask for more tests here. A split that also adds or edits assertions cannot be reviewed as a no-op.
- Guard against silent loss: the test count must be identical before and after (
cargo test -p buzz-sdkreports 289 at5e2879a72).fixtures_cover_every_actionstays as the guard that a new action cannot go untested, so it belongs wherever the fixtures do or must remain reachable from there.
Verification
cargo test -p buzz-sdk (same pass count, no scoped module run), cargo clippy --workspace --all-targets -- -D warnings, cargo fmt --check, and the repo file-size check.
Activity
baxen commentedon Aug 25, 2026
Correcting one line of this issue's rationale so the record matches the PR (#6759).
This issue says the split must stay in-crate because the strictness guards sit behind private intermediaries (
WireResponse,StrictEvent), so an out-of-crate test would pass while proving less. That is not true, and I confirmed it rather than assuming either way: I copied all four modules verbatim intocrates/buzz-sdk/tests/, changed only the import preamble, and all 29 tests compiled and passed. They name only public API and reach those private types through the publicDeserializeimpls — the same door a host author uses.The real reason to keep it in-crate is narrower: the file already lived there, so moving it out would mix a pure move with a scope change. The one guard that genuinely depends on privacy is the dispatch token —
Dispatch's field is private toclient, so nothing outside that module can forge it and bypassexecute; the tests implementsendand never call it.Practical consequence for anything downstream: in-crate placement does not buy test reach.
test(buzz-sdk): split broker/tests.rs into schema/validation/client m…
baxen commentedon Aug 25, 2026
Rebase note for this issue, since it pins a number that has moved.
The pinned baseline of 291
buzz-sdktests is stale — PR #6742 took four review revisions that added four tests, so the baseline at its current head559eb306eis 295. The split PR (#6759) now gates against 295. Anyone reading this issue as the acceptance criterion should use 295, not 291.One other correction to the plan sketched here: the split is five modules, not four.
tests.rsgrew from 1,900 to 2,221 lines with those revisions, and with the wire-schema section still insideschema.rsthat file lands at 1,045 — over the very cap the split exists to satisfy. The wire-schema section is a self-contained 364 lines behind its own// ── ──header, so it becametests/wire_schema.rs. No test moved across a concern boundary, and the reconstruction is still byte-identical totests.rsat559eb306e(c7d07544954bcc0afa6538342348fdb3f558d2811c50f7af08b230311f5ad0bb).test(buzz-sdk): split broker/tests.rs into six concern modules
baxen commentedon Aug 25, 2026
Follow-up correction to my earlier note: the baseline test count moved again with #6742's second review revision, 291 → 295 → 299. The acceptance criterion for this issue should read 299, not the 291 pinned in the description.
The split also lands as six modules rather than the five I reported last time. #6742's latest revision added a 415-line identity-canonicalization section; leaving it inside
validation.rswould put that file at roughly 1,130 lines — the same self-defeating outcome that already forcedwire_schema.rsout ofschema.rs. Final shape: mod 184 / schema 689 / wire_schema 361 / validation 714 / identities 423 / client 312, all under the cap.Done as #6759, still draft until #6742 merges. Reconstruction proof is unchanged in kind: concat-minus-scaffolding reproduces
tests.rsat7c853e0fbwith a single one-token exception (member_pathsbecomespub(super), since it is now shared by two modules), and the same 37 broker tests exist before and after with module paths normalized away.