ci: build the binary's tests, which nothing did - #67
Merged
Conversation
`cargo test` did not compile on main. The `sync ls --json` schema test in `src/main.rs` builds a `SyncEntryStatus` literally, and #61 added a `sweep` field to that struct without updating it, so the bin test target failed with E0063. CI stayed green through all of it, because the workflow runs `--lib` and three named integration targets and never builds the binary's own tests. So the gap is not the missing field. The gap is that a test could stop COMPILING and every check still passed. FIXING THE COMPILE ERROR THEN EXPOSED A REAL FAILURE, which is the point. `sync_ls_json_schema_exposes_counts_and_drift` pins the exact JSON of `fabric sync ls --json`. #61 added `sweep` to that output. The test that exists to catch a wire-schema change did not catch it, because it had not run since before the change. I WATCHED THE NEW CHECK FAIL. `cargo test --bins` failed on the compile error, then failed again on the schema assertion with `sweep` present on the left and absent on the right, then passed once the expectation carried the field. That is the positive control, in this order, on this machine. Two changes, and they belong together because either alone leaves the hole open: the expectation now carries `sweep`, and the workflow now runs `cargo test --locked --bins`. Verified on macOS 15 arm64. 12 bin tests pass. Linux is CI's to confirm. Agent: Silber.fabric
myobie
added a commit
that referenced
this pull request
Aug 24, 2026
Nathan says fabric uses too much CPU. Nobody can currently say which part of a sync pass spends it, and three attributions were reported and withdrawn in one evening because of that. Each guess cost more than measuring would have. This adds the measurement. TWO COUNTERS, AND THE FIRST ONE IS A BUG FIX AS MUCH AS AN INSTRUMENT. `sync_passes` counts calls to `sync_once`. `full_scans` does NOT: it counts TWO per call, because `sync_once` scans before the peer step and again after. Reading `full_scans` as a pass rate doubles it, and dividing a per-pass cost by it halves the answer. That is exactly how a guard effectiveness figure of 58 percent was reported against the live fleet and then withdrawn; the honest number was at most about 15 percent. `scan_micros`, `materialize_micros`, `persist_micros` and `reconcile_micros` are cumulative microseconds per phase. Cumulative, not per-pass, so a reader takes two samples and divides. A total on its own describes the past. All of it is exposed through `fabric sync ls` and its `--json` form, next to the counters already there. WHY CUMULATIVE COUNTERS RATHER THAN LOG LINES. The existing observability counters are read by sampling `fabric sync ls` twice, and a log line would need a parser and a retention policy to answer the same question. TESTS, AND I WATCHED BOTH FAIL. `full_scans_counts_two_per_pass_and_sync_passes_counts_one` pins the 2:1 ratio as a property of the function. Mutated `sync_passes` to increment by two: "left: 10, right: 5". `phase_timers_follow_the_work_and_not_the_pass` pins that a phase timer is wired to the work it names, not to the pass. Every pass scans; only a changed pass writes. So `scan_micros` must keep rising while `persist_micros` stays flat across no-op passes. Moved the persist timer outside the guard that skips the write, and it failed. Neither test mirrors the change. Both would fail if the timers were attached to the wrong call sites. THE NEW FIELDS BROKE THE `sync ls --json` SCHEMA TEST, AS THEY SHOULD. That test only runs at all because #67 added `cargo test --bins` an hour ago. Before that it did not compile and CI could not see it. This is the first change it has caught. WHAT THIS DOES NOT DO. It fixes no CPU. It makes the next fix aimable. The walk is already known not to be the answer: the watched tree is 16.6 MB across 17,175 files, a full stat pass over it costs about 36 ms, and one `sync_once` costs roughly 1.8 s. Agent: Silber.fabric
This was referenced Aug 24, 2026
myobie
added a commit
that referenced
this pull request
Aug 25, 2026
The adversarial delete matrix was added to `tests/folder_sync.rs` on 2026-08-25 and no step was added here, so the two tests that reproduce a file-loss incident ran only on the author's laptop. CI stayed green throughout and told nobody. That is the SECOND time. #67 fixed the same shape for the binary's own tests: `cargo test --locked --lib` plus a few named integration targets means a whole file can be invisible, and the suite reports success while the new tests never execute. TWO CHANGES. `--test folder_sync` now runs, so the guards against last night's loss are checked by something other than me remembering. And a step that FAILS when a `tests/*.rs` target is neither run above nor named in the list of known gaps. Adding a test file and nothing else now breaks the build, which is the point. Forgetting should be loud. I RAN THE GUARD BOTH WAYS BEFORE SHIPPING IT. Against the current tree it passes and prints what it accounted for. With a throwaway extra target in `tests/` it fails and names it. A guard that cannot fail is decoration. THE KNOWN GAPS ARE NAMED RATHER THAN HIDDEN: `lifecycle`, `pathwatch_slice` and `shell` still do not run here. All three pass on macOS locally and none has been verified on Linux CI, so I am not switching them on in the same change that fixes the reporting. The list is meant to get shorter. Agent: Silber.fabric
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.
What this changes
cargo testdoes not compile onmaintoday. Thesync ls --jsonschematest in
src/main.rsbuilds aSyncEntryStatusliterally, and #61 added asweepfield to that struct without updating it. The bin test target fails withE0063: missing field sweep.Every check stayed green through all of it. The workflow runs
--libplusthree named integration targets, and never builds the binary's own tests.
So the defect is not the missing field. The defect is that a test could stop
compiling and CI could not tell you.
Fixing the compile error then exposed a real failure, which is the whole
point.
sync_ls_json_schema_exposes_counts_and_driftpins the exact JSON offabric sync ls --json. #61 addedsweepto that output. The test that existsto catch a wire-schema change did not catch it, because it had not run since
before the change landed.
Two changes, together, because either one alone leaves the hole open:
sweepcargo test --locked --binsWhat was verified, and what was not
Platform: macOS 15 on arm64 (Silber). Linux is CI's to confirm, and this PR is
the first run that will actually build these tests there.
I watched the new check fail, in this order:
cargo test --binsfailed to compile —E0063: missing field sweep.sweep: "disabled"present on the left, absent on the right.
I also confirmed the compile error pre-existed my work rather than assuming it:
I stashed my changes, ran
cargo teston cleanmain, and reproduced the sameE0063before restoring them.Not verified: whether any other target is unbuilt by this workflow. I fixed
the one I tripped over and did not audit the rest.