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.
This commit is contained in:
parent
29d02ae1b0
commit
1485f7610b
|
|
@ -524,12 +524,18 @@ has **no branch and no framing yet**.
|
||||||
## Test-improvement arc, lane 6 — `scripts/bite` positive control
|
## Test-improvement arc, lane 6 — `scripts/bite` positive control
|
||||||
|
|
||||||
- Portable branch: `githubsucks/bite-positive-control`, worktree
|
- Portable branch: `githubsucks/bite-positive-control`, worktree
|
||||||
`../pmacs-bite`, based on `main` @ `300cbc4`. First lane of the
|
`../pmacs-bite`, based on `main` @ `300cbc4`. Tooling and
|
||||||
test-improvement arc scoped by `TEST_IMPROVEMENT.md` §8. Tooling and
|
|
||||||
documentation only — **no product code, no tests changed.**
|
documentation only — **no product code, no tests changed.**
|
||||||
- Sequenced **first**, ahead of the arc's own priority list, because
|
- **Self-contained deliberately.** An earlier draft of this entry
|
||||||
every later lane's evidence is bite-shaped. A verifier that cannot
|
scoped the lane by citing a `TEST_IMPROVEMENT.md` at the repo root.
|
||||||
fail is not worth more than the claims it certifies.
|
That file is **untracked and machine-local** — not on `main`, not in
|
||||||
|
this PR's tree — so the citation was a dangling reference for any
|
||||||
|
other machine, which is exactly what this ledger exists to prevent.
|
||||||
|
The lane is described here on its own terms instead. If that audit
|
||||||
|
is to scope later lanes it needs committing first, in its own PR.
|
||||||
|
- Sequenced ahead of the rest of the testing work because every later
|
||||||
|
lane's evidence is bite-shaped. A verifier that cannot fail is not
|
||||||
|
worth more than the claims it certifies.
|
||||||
- **The defect:** the script ran only the swapped tree, so a failing
|
- **The defect:** the script ran only the swapped tree, so a failing
|
||||||
swapped run was the only thing it checked. A test that fails
|
swapped run was the only thing it checked. A test that fails
|
||||||
everywhere — a typo, an unrelated compile break — therefore printed
|
everywhere — a typo, an unrelated compile break — therefore printed
|
||||||
|
|
@ -556,7 +562,6 @@ has **no branch and no framing yet**.
|
||||||
`git fetch githubsucks && git worktree add ../pmacs-bite
|
`git fetch githubsucks && git worktree add ../pmacs-bite
|
||||||
-b bite-positive-control githubsucks/bite-positive-control`.
|
-b bite-positive-control githubsucks/bite-positive-control`.
|
||||||
|
|
||||||
|
|
||||||
## Parked lane: kill-ring browser + persistence
|
## Parked lane: kill-ring browser + persistence
|
||||||
|
|
||||||
- Portable branch: `githubsucks/kill-ring-browser`
|
- Portable branch: `githubsucks/kill-ring-browser`
|
||||||
|
|
|
||||||
|
|
@ -1597,7 +1597,14 @@ round-trip cannot detect a discriminant shift.
|
||||||
a commit splits worktree from branch — but do not repeat the
|
a commit splits worktree from branch — but do not repeat the
|
||||||
destroys-your-work claim, which will push the next reader toward
|
destroys-your-work claim, which will push the next reader toward
|
||||||
`git stash` to protect themselves, straight into the repo-global
|
`git stash` to protect themselves, straight into the repo-global
|
||||||
trap above. Corollary for a NEW file: the swap-over-`git show` mode
|
trap above. **The #165 incident itself is now unexplained**, and
|
||||||
|
that is recorded rather than papered over: work really was lost, but
|
||||||
|
not by the mechanism this file blamed. A `SIGKILL` bypasses the trap
|
||||||
|
and would leave the swapped file in place, which is one candidate;
|
||||||
|
so is a stash collision, given the same round. Do not invent a
|
||||||
|
mechanism to close the gap — an unexplained incident is safer than a
|
||||||
|
confident wrong cause, which is what produced this correction.
|
||||||
|
Corollary for a NEW file: the swap-over-`git show` mode
|
||||||
does not apply at all, so its claims must be bitten by hand-editing.
|
does not apply at all, so its claims must be bitten by hand-editing.
|
||||||
- **A CONFLICTING PR silently runs no CI at all.** GitHub builds
|
- **A CONFLICTING PR silently runs no CI at all.** GitHub builds
|
||||||
`pull_request` workflow runs against the PR's **merge ref**, which it
|
`pull_request` workflow runs against the PR's **merge ref**, which it
|
||||||
|
|
|
||||||
44
scripts/bite
44
scripts/bite
|
|
@ -35,6 +35,16 @@
|
||||||
# an assertion failure; treat a compile failure as a prompt to narrow
|
# an assertion failure; treat a compile failure as a prompt to narrow
|
||||||
# the swap to a file that compiles both ways.
|
# the swap to a file that compiles both ways.
|
||||||
#
|
#
|
||||||
|
# That classification reads libtest's own `test result:` summary and
|
||||||
|
# deliberately does NOT grep for compiler text. A test may PRINT
|
||||||
|
# compiler output on failure --- `compile_mode_acceptance` has a
|
||||||
|
# fixture that emits `error[E0308]: mismatched types` at column 0, and
|
||||||
|
# the compile-mode and editops suites are this script's primary
|
||||||
|
# consumers --- so a `^error[E...]` grep would stamp "weaker evidence"
|
||||||
|
# on exactly the strong result it was meant to identify. `test result:`
|
||||||
|
# is emitted whenever the harness ran, is absent when the swapped file
|
||||||
|
# did not build, and stays uncolored when piped.
|
||||||
|
#
|
||||||
# Why this exists: bite-verification is step 4 of the working method,
|
# Why this exists: bite-verification is step 4 of the working method,
|
||||||
# and the obvious shortcut --- git stash --- is a trap here. The stash
|
# and the obvious shortcut --- git stash --- is a trap here. The stash
|
||||||
# namespace is REPO-GLOBAL: shared across every worktree and with
|
# namespace is REPO-GLOBAL: shared across every worktree and with
|
||||||
|
|
@ -60,6 +70,17 @@ if [ ! -f "$path" ]; then
|
||||||
exit 2
|
exit 2
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# Both cargo runs are CAPTURED rather than streamed, because their
|
||||||
|
# output is parsed (passed-counts for the control, `test result:` for
|
||||||
|
# the classification). A long acceptance suite therefore prints
|
||||||
|
# nothing until it finishes --- that is the trade, not a bug. Do not
|
||||||
|
# "fix" it back to streaming without giving the parser another source.
|
||||||
|
#
|
||||||
|
# Colour is pinned off on both runs: `--color always` in the
|
||||||
|
# passed-through args would otherwise wrap the summary lines in escape
|
||||||
|
# codes, and every anchored match here would silently stop matching.
|
||||||
|
export CARGO_TERM_COLOR=never
|
||||||
|
|
||||||
# Sum the `N passed` figures across every `test result:` line, so a
|
# Sum the `N passed` figures across every `test result:` line, so a
|
||||||
# multi-target invocation is counted correctly rather than only its
|
# multi-target invocation is counted correctly rather than only its
|
||||||
# last binary.
|
# last binary.
|
||||||
|
|
@ -110,12 +131,23 @@ if [ "$swapped_status" -eq 0 ]; then
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# Distinguish the two ways the swapped run can fail. An assertion
|
# Distinguish the ways the swapped run can fail. An assertion failure
|
||||||
# failure is the evidence we want; a compile failure only says the old
|
# is the evidence we want; a build failure only says the old file does
|
||||||
# file does not build here.
|
# not compile here, and the tests may never have run at all.
|
||||||
if printf '%s\n' "$swapped_out" | grep -q '^error\[E[0-9]*\]\|^error: could not compile'; then
|
#
|
||||||
|
# Classified from libtest's summary line, NOT from compiler text --- a
|
||||||
|
# failing test can print `error[E0308]: ...` at column 0 itself (see
|
||||||
|
# the header). `test result: FAILED` means the harness ran and tests
|
||||||
|
# failed; no `test result:` line at all means nothing ran.
|
||||||
|
if printf '%s\n' "$swapped_out" | grep -q '^test result: FAILED'; then
|
||||||
|
echo "bite: OK (assertion) --- tests fail against $ref:$path (the fix bites)"
|
||||||
|
elif printf '%s\n' "$swapped_out" | grep -q '^test result:'; then
|
||||||
|
# Some target ran to completion without a failure, yet cargo still
|
||||||
|
# exited non-zero --- typically a *different* target in the same
|
||||||
|
# invocation failed to build. Neither clean result applies.
|
||||||
|
echo "bite: OK (MIXED) --- some tests ran and none failed, but the run still failed." >&2
|
||||||
|
echo "bite: narrow the invocation to one target before trusting this." >&2
|
||||||
|
else
|
||||||
echo "bite: OK (COMPILE) --- $ref:$path does not build against the current tree." >&2
|
echo "bite: OK (COMPILE) --- $ref:$path does not build against the current tree." >&2
|
||||||
echo "bite: weaker evidence than an assertion failure --- the tests may never have run." >&2
|
echo "bite: weaker evidence than an assertion failure --- the tests may never have run." >&2
|
||||||
else
|
|
||||||
echo "bite: OK (assertion) --- tests fail against $ref:$path (the fix bites)"
|
|
||||||
fi
|
fi
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue