test: the self-test must not exit 0 having evaluated nothing (#934)

[OffgridwithJD]

Fixes #934.

Handed a pg_config the box does not have, test/harness_selftest.sh printed four lines, never reached its summary, and exited 0. Measured on main:

/usr/local/pg18a/bin/pg_config    rc=0  1246 lines  checks run: 610
/usr/local/pgNOPE/bin/pg_config   rc=0     4 lines  no summary at all

A caller cannot tell the second from the first. It is not hypothetical — it cost me a whole mutation round. The control and the mutated arm both reported rc=0 with zero FAIL lines, which reads exactly like "the mutation changed nothing": the conclusion the run existed to test. The log being 4 lines instead of 1246 was the only thing that gave it away.

The default argument is /usr/local/pg17/bin/pg_config, which the audit container does not have, so the wrong invocation is the easy one to make.

Two causes, and the second is the one that generalises

_bindir was assigned from a command that had failed, so every later PATH was wrong. Part 010, which is sourced, then took its own skip path and called exit 0 — which exits the driver rather than the part, with a status that says success.

The driver now refuses its argument before sourcing anything, on two predicates because one is not enough: a pg_config can exist and be executable and still answer nothing, which is exactly the shape that produced the empty _bindir.

Both of part 010's skip paths now exit 66 — the status lib.sh calls PGC_EXIT_SKIPPED — paired with the SKIPPED (ran no checks) line the runners already require beside it. Two independent signals, which is what lib.sh's own comment says 66 needs. The literal is spelled rather than the constant because lib.sh arrives in part 020 and 010 runs first; part 410 asserts the literal still equals the constant so the copy cannot drift.

A driver-level EXIT trap would not have worked

Worth recording, because it was my first design. Part 010 arms trap squatter_down EXIT for its own cleanup, and bash has one EXIT trap — the part would have clobbered the driver's guard. The static sweep closes the class instead.

Measured before building

Seven parts contain exit 0, 13 occurrences in total, but only two are top-level bail-outs of a sourced part — both in 010. The rest sit inside heredocs, functions or subshells, in fixture scripts that legitimately end that way.

So part 410's sweep is heredoc-aware: it sees 2 sites before this change and 0 after, where a flat grep -c 'exit 0' sees 13 either way.

The arms, red before and green after

handed a pg_config that does not exist, it refuses instead of exiting 0
and the refusal is the guard's own, naming the path
and it does not pretend to have run checks
a pg_config whose --bindir is empty is refused too
control: the pg_config THIS run was handed satisfies both predicates
part 010's skip status is the one lib.sh calls PGC_EXIT_SKIPPED
and part 010 exits with exactly that status on its skip paths
and pairs each with the marker the runners require
no selftest part exits 0, which would exit the driver before its summary

The control does not re-run the suite. Invoking the driver with a good pg_config would source every part again, so the control is the two predicates the guard tests applied to the pg_config this run was given — plus the existence of this run, which is the guard having accepted it.

Two of my own arms were wrong, both caught by running them

• grep -cE '^\texit 66$' reported 0 against a file holding two, because in POSIX ERE a backslash-t is a literal t. Third time that has cost me a measurement today, in three different sweeps. It is [[:space:]] now, with the reason written beside it.
• The marker count found three where the part emits two — the third was inside my own comment explaining why the other two are there. A sweep that counts its own documentation is the trap the pytest.raises scan already records. It matches echo lines now.

And one arm could not discriminate: "it names the path" passed before the guard existed, satisfied by bash's own No such file or directory naming the same path. It now requires the guard's own no-pg-config token.

After

argument	rc	lines	summary
pg18a/bin/pg_config	0	632	checks run: 604
pgNOPE/bin/pg_config	2	1	none
a nonexistent directory	2	1	none
exists, answers nothing	2	2	none, naming the empty --bindir

Gate

harness_selftest   604 checks, 604 passed + 0 failed + 0 unrunnable, PASSED
pytest corpus      253 passed with a cluster on pg18a
shellcheck -S error over test/*.sh and test/selftest/*.sh   rc=0

Based on main 79882992. Part numbered 410 rather than 400, because #923 claims 400 and a collision there is the #554 conflict this directory numbers parts to avoid.

The part's own comment said why this mattered, four lines above the first exit 0: "a quiet skip means the guard stops being tested that run without anyone noticing."

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

[OffgridwithJD]

Renumbered, and there is a merge-order note.

#925 adds 410-a-check-must-have-been-red.sh and 420-a-deleted-part-must-be-visible.sh, and #923 under it adds 400-a-check-result-must-be-machine.sh. Two of us picked the same numbers out of the same free range — the residual #554's one-file-per-subject rule leaves open, since that rule stops two additions touching the same line and a number is not a line.

#925 is approved and answering blockers, so it keeps the number. The subjects never overlapped — theirs are the red-check ledger and the part manifest — so this is numbering and nothing else.

For whoever merges second: #925 also adds test/selftest/parts.manifest and a part comparing it against the glob, because a glob cannot notice a deletion. If #925 lands first, this part must be added to that manifest or its arm reddens — correctly, which is the arm doing its job rather than a conflict. The reverse order needs nothing.

Re-gated after the rename: 604 passed + 0 failed + 0 unrunnable on #940's branch and 601 passed + 0 failed + 0 unrunnable on #941's, 40 parts each.

[jdatcmd]

Requesting changes at exact head 47c80514837787ff6c12556bb7fcd4d5bdc517ff on one line of comment, and it is the renumber that caused it. Everything else below reproduced.

The blocker: a cross-reference the renumber left pointing at nothing, and soon at the wrong thing

test/selftest/010-stand-up-a-squatter-on-a.sh:22

# literal because lib.sh arrives in part 020 and this part runs first, and part 410
# asserts the literal still equals the constant so the two cannot drift.

There is no part 410 in this tree. The arm it names is in 430, lines 97 and 104. That alone is only a dangling pointer — but #925 adds 410-a-check-must-have-been-red.sh, so after that merges the reference resolves to a real part about the red-check ledger, which asserts nothing about 66. A reader checking whether the literal is guarded will find a healthy part that does not guard it, and stop looking. A dangling reference gets noticed; one that resolves to the wrong file does not.

The PR body says 410 twice as well.

This is the residual you named in your own message about the collision: the rename moved the file and the number, and a number written into prose is not a line git can conflict on.

Everything else reproduced, on the head, by removal

The premise is real. I ran the driver on main 79882992 myself rather than reading your table:

main   pg_config that does not exist       rc=0   4 lines   no summary
main   exists but answers nothing          rc=0   3 lines   no summary
head   pg_config that does not exist       rc=2   2 lines   no summary

The removal proof, and it needs BOTH halves. Neutering the driver's two predicates alone gives rc=66, not rc=0 — part 010's other fix catches it. So the change is defence in depth, and only removing both restores the bug:

both predicates -> false, part 010's two `exit 66` -> `exit 0`   rc=0    the bug is back

Each mutation asserted its anchor present, bash -n confirmed the file still parsed before running, and the tree was checked back to zero modified files after each.

My first attempt at that proof was void and I nearly filed it. I cut the guard by byte offsets, the file stopped parsing, and both arms reported rc=2 — which I could have read as "the guard is not load-bearing". It was bash refusing the file. rc=2 from a syntax error and rc=2 from exit 2 are the same number; the mutation above changes the predicate and leaves the syntax whole, which is why it can fail.

The sweep discriminates, measured on main rather than on the branch:

heredoc-aware sweep on main    2 sites, both in 010: lines 21 and 55
after the fix                  0
flat grep -c 'exit 0' on main  13

FNR == 1 { hd = "" } resets the heredoc state per file, so this one does not have the awk-globals-leak-across-files bug that has cost this repo measurements before. I checked because that is where I expected it to be wrong.

Two notes, neither blocking

1. "13 either way" is true of main and false of the branch it ships in. Part 430's comment says a flat grep sees 13 either way. On this head it sees 22, because part 430 itself adds ten exit 0 lines in its own fixtures. The real numbers argue your point better than the retyped one does: flat grep 13 before and 22 after, heredoc-aware 2 before and 0 after — the flat count moves for a reason that has nothing to do with the defect.

2. The sweep closes one spelling of the mode. exit 0 ends the driver with success; so do exit (bare, which exits with the last status, very often 0), exit $?, and exit "$rc". Measured on this head, each of those is zero, so nothing is open and this is not a hole today — but the subject is "a sourced part must not end the driver claiming success", and the rule is a claim about the literal 0. One line in the comment saying which spellings it does and does not see would keep the next reader from assuming the class is closed.

Recorded

Your "a driver-level EXIT trap would not have worked, because part 010 arms its own and bash has one" is the kind of rejected design worth keeping in the PR. It is not obvious, and without it the next person tries the trap first.

[jdatcmd]

Approving at exact head af84fbb46fb10502951d612160b664a608d837f0. All three re-verified by running them, not by reading the diff.

The cross-reference. 010-stand-up-a-squatter-on-a.sh:22 now says part 430, that part exists here, and grep -rn 'part 410' over test/ and CHANGELOG.md returns 0. So the string is gone rather than moved.

The corrected count. I measured the flat count myself rather than taking 24 from the comment: grep -h 'exit 0' test/selftest/*.sh | wc -l on this head is 24. 13 before, 24 after, and the comment now says why the flat number moves for a reason unrelated to the defect.

The bare-exit extension, proved by removal.

shipped sweep over the tree                      0 offences
bare-exit plant, both rules                      1     (the arm wants 1)
bare-exit plant, exit-0 rule only                0     <- the arm goes red
control `exit 66`, both rules                    0     (the arm wants 0)

The control is the half that matters: without it, widening the rule to any exit at all would have passed the plant and silently condemned part 010's own skip status.

And the guard still does its job on this head, including the full removal proof rather than only the happy path:

pg_config that does not exist        rc=2   no summary
exists but answers nothing           rc=2   no summary
both predicates false AND part 010's two `exit 66` back to `exit 0`    rc=0   the bug returns

bash -n confirmed the file still parsed before that last run, which is the thing my first attempt got wrong — and the tree checked back to zero modified files after.

Two checks are still pending at this SHA, suites (PG 17) and suites (PG 18); the other 11 are green. Ready to merge once those land, on the owner's word.