Commit Graph

4 Commits

Author SHA1 Message Date
Levi Neuwirth 8d93fc6978 review round 2: MIXED must not exit 0, and its assumed trigger is wrong
P2 --- the MIXED arm said "OK" and exited 0 while its likeliest cause
is vacuity. Reaching it means cargo failed, at least one `test result:`
line exists, and none says FAILED --- i.e. the named tests may have run
CLEAN against the old tree, the vacuous outcome, with an unrelated
failure supplying the exit status. The message admitted as much while
the exit code contradicted it, so a scripted caller, or a framing doc
quoting "bite exited 0", would record a certified bite. It now exits 4
and is labelled INCONCLUSIVE rather than OK. The exit ladder in the
header gains the code.

While exercising that arm I found the review's stated trigger --- and
my own comment repeating it --- is WRONG, which is worth more than the
fix. `--test A --test B`, where B's swapped file will not build, does
NOT produce MIXED: cargo builds every named target before running any,
so B's build failure stops A from running too and the swapped run
emits no summary at all. Verified by running exactly that shape
against an ancient `tests/m4_acceptance.rs`; it lands in COMPILE, as
it should. What genuinely reaches MIXED is a run that prints summaries
and *then* fails --- doc-tests failing to compile after the lib tests
pass, or a harness dying after its summary line.

So the arm stays as defence but is documented as defensive, and the
ledger lists it as the one path NOT reproduced rather than implying
the validation was complete. Manufacturing a doc-test failure to reach
it would test the arm at the cost of a contrived fixture; saying which
paths were executed and which were not is the more useful record.

Two wording fixes from the same round. The COMPILE message said the
file "does not build against the current tree", but the no-summary
condition also covers a harness that built and then died before
printing one; it now says "produced no test summary (build failure, or
a harness that died before printing one)" so the label never overstates
the observation. And the colour comment no longer overclaims: exporting
CARGO_TERM_COLOR=never defeats an ambient variable, but an explicit
`--color always` in the passed-through args still beats the
environment. Self-inflicted and fail-closed, and now said so.
2026-07-29 11:05:55 -04:00
Levi Neuwirth 1485f7610b review round 1: classify from libtest, and drop a dangling doc citation
P1 --- the classifier could mislabel a genuine bite, and this repo
manufactures the trigger. It grepped the swapped run's output for
`^error\[E...\]` / `^error: could not compile`, but a failing test can
print those lines itself: `tests/compile_mode_acceptance.rs:572` has a
fixture that emits `error[E0308]: mismatched types` at column 0, and
any suite that dumps buffer or PTY text on failure does the same. A
clean assertion failure --- the strong evidence --- would then be
stamped `OK (COMPILE)`, "weaker evidence", while the new handoff text
tells the reader to trust the label. The verdict was never wrong; the
label was.

Classification now reads libtest's own summary instead:
`test result: FAILED` present means the harness ran and tests failed;
no `test result:` line at all means nothing ran. A third `MIXED` arm
covers "some target ran clean but cargo still failed", which the
two-way split silently folded into the wrong side. This also removes
two latent portability bugs for free: `\|` alternation in a BRE is a
GNU extension, so on BSD grep every failure would have labelled as
assertion; and cargo's coloured `error:` output would have evaded the
grep entirely. libtest's summary lines stay uncoloured when piped, and
`CARGO_TERM_COLOR=never` is now exported so `--color always` in the
passed-through args cannot break the parse either --- that would
otherwise have zeroed `count_passed` and produced a spurious
NO CONTROL.

Verified rather than reasoned: a test made to print BOTH
`error[E0308]: mismatched types` and ``error: could not compile
`probe` `` at column 0, then failed by swap, is still classified
`OK (assertion)`.

P2 --- the ledger cited a file that is not in the tree. The lane
scoped itself by `TEST_IMPROVEMENT.md` §8, which is untracked and
machine-local: not on `main`, not in this PR's tree. That is a
dangling reference for any other machine, which is the failure this
ledger exists to prevent. The entry is now self-contained and says so;
if that audit is to scope later lanes it needs committing in its own
PR first.

P3 --- a comment records that both runs are captured rather than
streamed, and why, so nobody "fixes" it back to streaming and breaks
the parser. Double blank line before the parked lane collapsed.

Also, on review's prompting: the #165 wiped-round incident is now
UNEXPLAINED, and the handoff says so rather than leaving a gap the
next reader will fill by re-deriving the false mechanism. Work was
lost; not by the cause this file blamed. SIGKILL bypassing the trap
and a stash collision in the same round are both candidates, and the
note declines to pick one.
2026-07-29 10:55:42 -04:00
Levi Neuwirth 29d02ae1b0 tooling: give scripts/bite a positive control, and correct the handoff
Lane 6 of the test-improvement arc, pulled ahead of its own priority
list because every later lane's evidence is bite-shaped. A verifier
that cannot fail is worth no more than the claims it certifies.

The defect. The script ran only the SWAPPED tree, so "the tests failed
against the old file" was the only thing it ever checked. It could not
distinguish a load-bearing fix from a broken test: a typo, an unrelated
compile break, or a filter matching nothing all make the swapped run
fail, and each printed `bite: OK`.

The control now asserts two things, because passing alone is not
enough --- `cargo test` with a filter matching zero tests exits 0, and
a compile error in the old tree would then still read as OK:

  * the named tests PASS against the working tree, and
  * at least one test actually RAN,

exiting 3 as NO CONTROL otherwise. The swapped run additionally
distinguishes `OK (assertion)` from `OK (COMPILE)`, since an old file
that will not build against the current tree is much weaker evidence:
the tests may never have run at all.

Validated on all three paths rather than the happy one alone. A
zero-match filter reports NO CONTROL. A genuine bite --- PR #190's
`rd1`, which is known to fail against `main` --- reports `control OK
--- 1 test(s) pass` then `OK (assertion)`. A test deliberately broken
so it fails on BOTH trees reports NO CONTROL at exit 3; the previous
script printed `bite: OK` for exactly that case.

Handoff correction, verified rather than inherited. §5 claimed the
script "restores by `git checkout --`, which reverts the file to HEAD",
destroying uncommitted work, and cited a wiped review round during
#165. The mechanism description is false: the script copies the file to
a mktemp path before swapping and restores from that copy under an
`EXIT INT TERM` trap, touching no git state beyond a read-only
`git show`. The commit-before-gating rule is kept --- it stands on
gate-results-describe-the-pushed-tree --- but the false mechanism is
removed, because a reader who believes bite eats uncommitted work will
reach for `git stash` to protect themselves, which is the repo-global
trap the script exists to avoid.

No product code. No tests changed.
2026-07-29 10:12:38 -04:00
Levi Neuwirth 3284b89a5b dev: scripts/bite — machine-checkable bite-verification
A trap-guarded one-file swap over read-only git show: run the named
tests against an older version of one file and exit 0 iff they FAIL
there (the fix bites); exit 1 flags a vacuous fix. The file is
restored on every exit path including interrupts; the ./ pathspec
prefix makes it work from any directory inside the repo. Never
touches git stash — the stash namespace is repo-global, shared
across worktrees and with humans (a #111-round mishap popped a
PR-#17-era user stash; hence the tool).

Dogfooded three ways against the merged #111 history: the round-1
fix commit bites (exit 0), the same tests against HEAD read as
vacuous (exit 1), and a docs/-relative invocation restores cleanly.

Handoff: section 1 snapshot (editops #111 merged, worktree folded),
section 5 lessons (repo-global stash; worktree-per-lane for
parallel work).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vF4gQVozBWi38y1SJiGfQ
2026-07-12 17:09:41 +01:00