test(stack): red the suite when a domain file contributes no assertions (#1400) - #1416
test(stack): red the suite when a domain file contributes no assertions (#1400)#1416VijitSingh97 wants to merge 2 commits into
Conversation
…ns (#1400) run.sh runs under `set -uo pipefail` with no `-e` and computes its verdict from PASS/FAIL alone, so a domain file that fails to source contributes nothing to either counter: the suite prints "0 passed, 0 failed" for that whole domain, exits 0, and nothing goes red. 25 stanzas were unchecked. The sharpest case is the last one, test-lifecycle.sh, which sits immediately above the verdict. Each of the 24 domain stanzas now captures the counters, sources, and calls domain_ran either way: _d0=$((PASS + FAIL)) && source "$HERE/x.sh" && domain_ran x.sh "$_d0" "$?" || domain_ran x.sh "$_d0" "$?" The guard is a POSITIVE control -- did this file move the counters -- not a check of `source`'s exit status, which answers a different question: it reads 0 for a top-level `return` (the failure mode that leaves no diagnostic at all) and non-zero for a healthy file that merely ended on a cleanup that failed. The status is carried into the message only, never into the verdict. lib.sh's own stanza stays bare on purpose. If lib.sh fails to source, PASS is unbound and the first guarded stanza's arithmetic aborts the shell under `set -u` -- that case already fails loud, not green. Line-neutral in both files it touches. run.sh is 3841 against a 3841 ceiling, so it has to be: 24 directive comments gain `disable=SC2015` in place and 24 source lines are replaced one-for-one. `a; b; c` and brace groups were measured and rejected -- `shfmt -i 4 -d`, which is half of `make lint`, rewrites both into three lines each and would take run.sh to 3889. shfmt preserves `&&`/`||` chains. lib.sh goes 232 -> 259, still under TARGET_LINES so it stays unrowed. SC2015 warns that C may run when A is true. It cannot see that domain_ran ends in an explicit `return 0`, which is why that line is load-bearing rather than decoration: a non-zero return would run the second branch too and count the same failure twice. The suppression rides the directive comment that was already there, so it costs no lines and is scoped to exactly these 24 stanzas. Limit, stated rather than implied: this proves a domain contributed at least one assertion, not that it ran to completion. A file that dies half way through still passes. Covering that needs a per-domain expected count, which collides with the nondeterminism in #1325.
|
CI is RED on What broke
This PR moves The fix belongs in The second defect, which is pre-existing
A guard whose fallback condition is "the set is empty" is defeated by one surviving member. The check needed here is a floor, not a non-empty test — the sourced count and the domain-file count are both knowable, and they should be required to agree. Third, smaller: the drift loop reports the first offender and exits, so the CI message named one file ( Why this is worth stating plainlyThis PR adds a guard against a domain file being silently skipped. It broke the existing guard against a domain file being silently skipped — and that guard's own anti-false-green fallback failed open. Both halves are the shape this repo keeps filing, and I would rather record that than quietly patch a regex. The second and third findings are |
The domain-file drift check found sourced files with a pattern anchored to `source` opening the line. The source-stanza guard put a prefix ahead of every one of them, so the pattern matched 1 of 25 and the whole `SOURCED` set collapsed to `lib.sh`, whose stanza is bare by design. CI then reported the other 24 files as on disk but sourced by nothing. The anchor exists to stop a commented-out `# source "$HERE/x.sh"` reading as a live one, so it is relaxed rather than removed: nothing before the `source` may be a `#`. A commented-out stanza is still rejected, indented or not, and so is a `source` sitting after a trailing comment on a live line. Measured at the head, not reasoned: 25 of 25 stanzas match, no duplicates, and the three commented shapes are all still refused. `bash tests/inventory.sh` exits 0.
tests/stack/run.shruns underset -uo pipefailwith no-eand computes its verdict fromPASS/FAILalone. A domain file that fails to source contributes nothing to either counter, sothe suite prints its green line and exits 0 with an entire domain never having run. 25 stanzas were
unchecked. The sharpest case is the last one,
test-lifecycle.sh, which sits atrun.sh:3832immediately above the verdict.
This is prevention, not a live bug, and that is measured rather than assumed: no domain file has
a top-level
returntoday. The sweep was run with a positive control first — it fires on a plantedreturn 0— so the empty result is evidence rather than a pattern that cannot match.The change
Each of the 24 domain stanzas becomes:
lib.sh's own stanza stays bare on purpose: iflib.shfails to source,PASSis unbound and thefirst guarded stanza's arithmetic aborts the shell under
set -u. Probed directly — a script fileexits 1 with
PASS: unbound variable. That case already fails loud, not green.One thing worth stating because it is invisible by reading
The design rests on
_d0=$((PASS + FAIL))returning 0 when the sum is zero, which it does: theexit status of an assignment is not the value of its arithmetic expansion. The near-identical
arithmetic command form
(( PASS + FAIL ))returns 1 on a zero result. Written that way, thevery first stanza would short-circuit past its own
sourceon every clean run. It would fail closed— a loud false RED naming the file, not a silent skip — but the distinction does not survive a
reading, so it is written down.
Why a positive control and not
source x || domain_fail xThe issue proposes the status form. It answers a different question:
sourcereturns the status ofthe file's LAST command. Measured on five fixtures under run.sh's real options, with a healthy
control:
Neither form dominates. The shipped form wins row R, the only failure mode that leaves no
diagnostic anywhere, and carries no false-positive surface. The declined form wins row P — but its
extra catch is inseparable from its false positive on row C, because both are just "the last command
returned non-zero". Row H green in BOTH columns is what makes this a strictly better instrument on
the rows that matter rather than a blanket inversion that reds more and looks stricter.
Reviewer call, offered rather than decided silently. A combined
delta == 0 || rc != 0checkwould catch strictly more, and its false-positive surface is measurably EMPTY today — an instrumented
full-suite run recorded every one of the 24 domains returning
sourcerc 0, including all four filesthat end in
rm -f. I did not ship it because it invents an undeclared contract on 24 files ("yourlast top-level command must succeed") that the next lane appending a cleanup line would trip with no
way to find out why. Overrule me if you disagree; the measurement is in the repo history of this PR.
Line-neutrality, and the constraint that shaped the form
run.shis 3841 lines against a 3841 ceiling, so the change had to be line-neutral. The obvious form—
_d0=...; source ...; domain_ran ...— is NOT:shfmt -i 4 -dis half ofmake lint-shandrewrites every
;-separated line and every brace group onto separate lines, which takes run.sh to3889. Measured across candidate forms:
a; b; csplits to 3,a; bto 2,{ a; b; }to 4;a && b,a || banda && b || care preserved. The line-neutral solution space here iscontrol-operator chains, not statement separators.
So: 24 directive comments gain
disable=SC2015in place, and 24 source lines are replacedone-for-one.
run.sh3841 -> 3841.lib.sh232 -> 259, still underTARGET_LINES, so it staysunrowed and
docs/dev/file-budget.tsvis untouched.Structural check on the result: all 25 stanzas accounted for — 24 guarded,
lib.shbare, everydisable=SC2015directive adjacent to its own stanza, and each stanza naming the same filename inall three positions (directive,
source, bothdomain_rancalls). No mismatches.On the SC2015 suppression
Both
||branches are textually identical because the call must happen either way, and$?carriessource's status correctly into both.
domain_ranends in an explicitreturn 0, which isload-bearing rather than decoration: a non-zero return would run the second branch too and count the
same failure twice. SC2015 is the heuristic that cannot see that guarantee.
It is suppressed on the directive comment that was already there, so it costs no lines and is scoped
to exactly these stanzas. It is deliberately NOT left for
--severity=warningto filter: shipping 24findings that pass only because of a severity filter is a shape this repo files.
Verified narrow with a negative control — a fixture whose source target is MISSING still emits
SC1091 and exits 1 under the combined directive, so
disable=SC2015suppresses only SC2015 and thesource=half still resolves. Run at full severity with shellcheck 0.11.0, the version CI pins.What was RUN
BOTH. 3233 log lines both, 272 section headers both, same SET and same ORDER (The #1105 split proof recipe cannot see execution reordering — two merged cuts reordered, one unexamined #1398).
Sorted-multiset diff is exactly 2 lines: the two halves of one ed25519 fingerprint (Two suite verdict lines are nondeterministic, weakening the split-proof multiset check #1325).
#1400lines in the after-proof. The guard does not fire on a healthytree.
can never fire is not a guard; this one is satisfiable by every domain that exists.
make lint-shrc 0, Makefile's exact pair,PATHset so shellcheck resolves the CI-pinned0.11.0. Not banked on its own — it returned in under two minutes, so it was checked with a
negative control the repo already predicts: drop
os/overlay/pithead-bootandrun.shalone mustred with three SC2034s. It did (
BOOT_DOCTOR_JSON,OS_INFLIGHT,OS_STATE_DIR, rc 1).lint-file-budget,lint-topology,lint-operator-strings,lint-docs-voice,lint-jsallrc 0, each rc captured separately rather than read off a filtered tail.
Mutation battery — every class on
test-lifecycle.shIt is the last stanza before the verdict, so nothing can cascade off the mutation and the run always
reaches its verdict line. Each tree is a full
git clone— notgit archive, which has no.gitand carries a 6-failure floor that reads exactly like cascade damage. Every mutation wasverified applied, distinct, and correctly positioned before running, and
tests/in the clones isbyte-identical to the pushed tip, so every row exercises the shipped code.
return-topis the decisive row. Three REDs look interchangeable; they are not. It is the onlyone where
sourcereturns 0, which is exactly what the declined|| domain_failform reads as ahealthy file. That row is what makes the design choice defensible rather than merely stated.
syntax-bottomis the survivor, and it is demonstrated rather than asserted. Its verdict isidentical to the control's — 2922 passed, 0 failed, rc 0, zero
#1400lines — so on the verdictalone the row would prove nothing. What makes it evidence is that the mutation provably fired: the
log carries
test-lifecycle.sh: line 369: syntax error: unexpected end of file, and the same grepagainst the
nonecontrol is empty. So the suite went green with a genuinely broken domain file,which is exactly the limit this guard has.
The
syntax-top/syntax-bottompair isolates position as the only variable: byte-identicalinjected text, same syntax error at the same reported line, opposite outcomes — RED with all 48
assertions lost when it precedes them, GREEN with all 48 intact when it follows them.
Each mutation produced exactly one failure, and it is the assertion the mutation was aimed at —
so all three parts of the standard are met rather than asserted: the mutation applied, something
died, and the right thing died. No cascade, no bystander kill.
deletealso reconciles independently:2922 - 48 = 2874, where 48 is the assertion delta the instrumented run measured for that domain. Two
instruments agree.
Limits, stated rather than implied
that dies half way through still passes — that is row P above, and the
syntax-bottommutation isin the battery to demonstrate the survivor rather than assert it. Covering it needs a per-domain
expected count, which collides with the nondeterminism in Two suite verdict lines are nondeterministic, weakening the split-proof multiset check #1325.
cleanly and simply contained no assertions. No such file exists today (deltas are 9 to 207), and
the RED names the file either way, so the hint is imprecise rather than wrong — flagged here rather
than silently churning a verified tree.
Over-engineering pass
Run by hand on merit. The repo's gate keys its sentinel to the pane's branch rather than the PR's and
clears on a second invocation regardless, so its silence is not a record of anything — this is what
was actually looked at, findings either way.
st="${3:-0}"indomain_ranhas a default nocaller uses. All 48 call sites (24 stanzas x 2 branches) pass three arguments; there is no
two-argument call anywhere. So the default is unreachable today, and if a future stanza were
written with two arguments it would silently report
source returned 0, whose hint reads "atop-level return" — a confident wrong diagnosis. Plain
"$3"would abort underset -uinstead,which is the better failure. I did not change it:
domain_ranis the code the mutation batteryabove exercises, and editing it would break the byte-identity between the tested clones and the
shipped tree for the sake of a dead default. Reviewer's call — say the word and I will change it
and re-run the battery.
why a positive control rather than
source's status (stops a future "simplification" back to theform measured worse above), why
return 0must stay (removing it double-counts the failure), andthe limit demonstrated by
syntax-bottom.domain_rancall in both branches is forced, not chosen. The single-call formneeds a
;chain or a brace group, andshfmtsplits both — see line-neutrality above.Not run
make test's dashboard halves (pytest, node). This change touchestests/stack/plus one line ofdocs/dev/repo-map.md; the stack suite is the after-proof above. CI is the check for the rest.The branch is 3 commits behind
develop-v2. Those commits touch onlydashboard/andos/rootfs/Dockerfile— notests/stack/file and notdocs/dev/file-budget.tsv— so theafter-proof still describes this code and the 3841 ceiling has not moved on the base.