From c74bdc5ac390346f14caf72938120fb8962e7423 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Thu, 10 Sep 2026 22:19:50 +0000 Subject: [PATCH 1/3] test: the self-test must not exit 0 having evaluated nothing (#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, and 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 is 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 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`. So `-x` alone would have accepted it. 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, and that is 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` and 13 occurrences in total, but only TWO are top-level bail-outs of a sourced part -- both in 010. The rest are inside heredocs, functions or subshells, in fixture scripts that legitimately end that way. So the sweep in part 410 is heredoc-aware: it sees 2 sites before this change and 0 after, where a flat `grep -c 'exit 0'` sees 13 either way. Part 410's arms, red before the fix 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 rather than reading 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 raises scan already records. It matches echo lines. 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: pg18a/pg_config rc=0 632 lines checks run: 604 pgNOPE/pg_config rc=2 1 line no summary a nonexistent dir rc=2 1 line no summary exists, answers nothing rc=2, 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 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." Fixes #934. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 40 +++++++ test/harness_selftest.sh | 30 +++++ test/selftest/010-stand-up-a-squatter-on-a.sh | 13 +- .../410-the-self-test-must-not-report.sh | 112 ++++++++++++++++++ 4 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 test/selftest/410-the-self-test-must-not-report.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a4ab100..b12e5b0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -335,6 +335,46 @@ true until the next version shipped. ### Fixed +- `test/harness_selftest.sh` can no longer exit 0 having evaluated nothing (#934). + + Handed a `pg_config` the box does not have, it 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 a + whole mutation round, because 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; and part 010, which is + SOURCED, then took its own skip path and called `exit 0` -- which exits the DRIVER + rather than the part. + + The driver now refuses the 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 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. + + `test/selftest/410-the-self-test-must-not-report.sh` holds the arms, including a + heredoc-aware sweep requiring that **no** part exits 0 -- closing the class rather + than the two instances. The sweep has to be heredoc-aware because the parts generate + fixture scripts that legitimately end in `exit 0`: it sees 2 sites before this change + and 0 after, where a flat `grep -c 'exit 0'` sees 13 either way. + + After: `rc=2` and a named refusal for a missing `pg_config`, for a path that is not a + directory, and for one that answers nothing; `604 passed + 0 failed + 0 unrunnable` + with a real one. + + The part's own comment at line 3 already said why this mattered -- "a quiet skip means + the guard stops being tested that run without anyone noticing" -- four lines above the + first `exit 0`. + - The vacuity guard's PLACEMENT is now a checked property, because a guard in a teardown cannot fail the test it guards (#432). diff --git a/test/harness_selftest.sh b/test/harness_selftest.sh index 7a6e8698..e755ec7c 100755 --- a/test/harness_selftest.sh +++ b/test/harness_selftest.sh @@ -24,7 +24,37 @@ set -uo pipefail PGC_SELFTEST_PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# REFUSE A pg_config THIS BOX DOES NOT HAVE, before anything is sourced (#934). +# +# `_bindir` used to be assigned from a command that had failed, and the suite then +# ran on with every PATH wrong. Part 010 took its own skip path, called `exit 0`, +# and because a part is SOURCED that exited the DRIVER -- 4 lines of output, no +# summary, status 0. A caller cannot tell that from the 610-check pass it looks +# like. 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 +# +# The default above is `/usr/local/pg17`, which the audit container does not have, +# so the wrong invocation is the EASY one to make -- and it cost a whole mutation +# round, because the control and the mutation both reported rc=0 with zero FAIL +# lines, which reads exactly like "the mutation changed nothing". +# +# TWO PREDICATES, because one is not enough. A `pg_config` can exist and be +# executable and still answer nothing: that is the shape that produced the empty +# `_bindir`, so `-x` alone would have accepted it. +if [ ! -x "$PGC_SELFTEST_PG_CONFIG" ]; then + echo "FATAL no-pg-config: $PGC_SELFTEST_PG_CONFIG is not an executable pg_config" + echo " pass one as the first argument, e.g. /usr/local/pg18a/bin/pg_config" + exit 2 +fi _bindir="$("$PGC_SELFTEST_PG_CONFIG" --bindir)" +if [ -z "$_bindir" ] || [ ! -d "$_bindir" ]; then + echo "FATAL no-pg-config: $PGC_SELFTEST_PG_CONFIG --bindir gave [$_bindir]," + echo " which is not a directory, so every PATH built from it would be wrong" + exit 2 +fi # ---- the checks themselves live in test/selftest/, one file per subject ------ # diff --git a/test/selftest/010-stand-up-a-squatter-on-a.sh b/test/selftest/010-stand-up-a-squatter-on-a.sh index cd56d170..7f50655a 100644 --- a/test/selftest/010-stand-up-a-squatter-on-a.sh +++ b/test/selftest/010-stand-up-a-squatter-on-a.sh @@ -15,10 +15,17 @@ for _try in $(seq 1 20); do break fi done +# A SOURCED PART MUST NOT exit 0. This file is sourced by harness_selftest.sh, so +# `exit` here ends the DRIVER -- and a zero status said "610 checks passed" when the +# truth was "nothing ran" (#934). 66 is the status lib.sh calls PGC_EXIT_SKIPPED and +# the runners already pair with a `SKIPPED (ran no checks)` line; it is spelled as a +# 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. if [ "$SQ_PORT" = 0 ]; then echo "SKIP could not find a free port for the squatter cluster" + echo "$(basename "${BASH_SOURCE[0]}"): SKIPPED (ran no checks)" rm -rf "$SQ_DIR" - exit 0 + exit 66 fi _runpg=(env) if [ "$(id -u)" = "0" ]; then @@ -49,10 +56,12 @@ sq_datadir() { -d postgres -At -c 'SHOW data_directory' 2>/dev/null } +# Same reason as the port bail above: a zero here ends the driver saying success. if [ -z "$(sq_datadir)" ]; then echo "SKIP could not stand up a squatter cluster to test against" + echo "$(basename "${BASH_SOURCE[0]}"): SKIPPED (ran no checks)" squatter_down - exit 0 + exit 66 fi echo "-- squatter listening on $SQ_PORT ($(sq_datadir))" diff --git a/test/selftest/410-the-self-test-must-not-report.sh b/test/selftest/410-the-self-test-must-not-report.sh new file mode 100644 index 00000000..36076323 --- /dev/null +++ b/test/selftest/410-the-self-test-must-not-report.sh @@ -0,0 +1,112 @@ +# ---- the self-test must not report success having evaluated nothing (#934) ---- +# +# Handed a `pg_config` that does not exist, this suite printed four lines, never +# reached its summary, and EXITED 0. Measured on main before this part: +# +# bash test/harness_selftest.sh /usr/local/pg18a/bin/pg_config rc=0 1246 lines checks run: 610 +# bash test/harness_selftest.sh /usr/local/pgNOPE/bin/pg_config rc=0 4 lines no summary at all +# +# TWO CAUSES, and the second is the one that generalises. `_bindir` was assigned +# from a command that failed, so every later PATH was wrong -- and part 010, which +# is SOURCED, then took its own skip path and called `exit 0`, which exits the +# DRIVER rather than the part. A caller cannot tell that from a real pass. +# +# IT COST A MEASUREMENT ROUND TO FIND. A control and a mutation 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 is +# the only thing that gave it away. +# +# The default is `/usr/local/pg17/bin/pg_config`, which this box does not have, so +# the wrong invocation is the EASY one to make. + +_hs="$PGC_TESTDIR/harness_selftest.sh" +check "premise: the driver this part is about is where it is expected" \ + "$([ -f "$_hs" ] && echo yes || echo no)" "yes" + +# A pg_config that does not exist. The guard has to refuse BEFORE sourcing any +# part, so this cannot recurse: it dies at the argument check. +_h934_absent="$(bash "$_hs" /usr/local/pgNOPE934/bin/pg_config 2>&1)" +_h934_absent_rc=$? +check "handed a pg_config that does not exist, the self-test refuses instead of exiting 0" \ + "$([ "$_h934_absent_rc" != 0 ] && echo refused || echo "exited 0")" "refused" +# NOT just the path: bash's own "No such file or directory" already named it, so an +# arm matching the path alone passed before the guard existed. The token is what +# makes this arm about the refusal rather than about bash's diagnostics. +check "and the refusal is the guard's own, naming the path it could not use" \ + "$(printf '%s' "$_h934_absent" | grep -c 'no-pg-config.*pgNOPE934')" "1" +check "and it does not pretend to have run checks" \ + "$(printf '%s' "$_h934_absent" | grep -cE '^checks run:')" "0" + +# A pg_config that EXISTS and answers nothing. This is the shape that produced the +# empty `_bindir`: the command succeeded, so `[ -x ]` alone would accept it. +_h934_stub="$PGC_WORKDIR/pg_config_silent_934" +printf '#!/bin/sh\nexit 0\n' > "$_h934_stub" +chmod +x "$_h934_stub" +_h934_silent="$(bash "$_hs" "$_h934_stub" 2>&1)" +_h934_silent_rc=$? +check "premise: the stub is executable, so an -x test alone would accept it" \ + "$([ -x "$_h934_stub" ] && echo yes || echo no)" "yes" +check "premise: and it answers --bindir with nothing" \ + "$("$_h934_stub" --bindir | wc -c)" "0" +check "a pg_config whose --bindir is empty is refused too, not run with a broken PATH" \ + "$([ "$_h934_silent_rc" != 0 ] && echo refused || echo "exited 0")" "refused" + +# THE CONTROL, without running the whole suite again. Invoking the driver with a +# GOOD pg_config would re-run every part, 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. +check "control: the pg_config this run was handed satisfies the first predicate" \ + "$([ -x "$PGC_SELFTEST_PG_CONFIG" ] && echo yes || echo no)" "yes" +check "control: and the second, so a good pg_config is not refused" \ + "$([ -d "$("$PGC_SELFTEST_PG_CONFIG" --bindir)" ] && echo yes || echo no)" "yes" + +# ---- and no part may exit 0, because a part is SOURCED ------------------------ +# +# `exit 0` in a sourced part exits the DRIVER, before `pgc_summary`, with a status +# that says success. Part 010 did it on both of its skip paths -- and its own +# comment, four lines above the first one, says why that is the hazard: "a quiet +# skip means the guard stops being tested that run without anyone noticing". +# +# HEREDOC-AWARE, because the parts generate fixture scripts that legitimately end +# in `exit 0` and a flat grep reports eleven offences that are not offences. +# Measured: this sweep sees 2 sites before the fix and 0 after, where a flat +# `grep -c 'exit 0'` sees 13 either way. +_h934_sweep() { + awk ' + FNR == 1 { hd = "" } + hd != "" { if ($0 == hd || $0 ~ "^[ \t]*" hd "[ \t]*$") hd = ""; next } + /<<-?[ \t]*'\''?[A-Za-z_][A-Za-z0-9_]*'\''?[ \t]*$/ { + if ($0 !~ /^[ \t]*#/) { + t = $0; sub(/.*<<-?[ \t]*/, "", t); gsub(/'\''/, "", t) + sub(/[ \t]*$/, "", t); hd = t; next + } + } + /^[ \t]*exit[ \t]+0[ \t]*$/ { printf "%s:%d\n", FILENAME, FNR } + ' "$@" +} +check "premise: the sweep reads every selftest part" \ + "$([ "$(ls "$PGC_TESTDIR"/selftest/*.sh | wc -l)" -ge 30 ] && echo yes || echo no)" "yes" +check "premise: and it finds a planted exit 0 outside a heredoc" \ + "$(printf 'echo hi\nexit 0\n' > "$PGC_WORKDIR/p934.sh"; _h934_sweep "$PGC_WORKDIR/p934.sh" | grep -c .)" "1" +check "premise: while a fixture script ending in exit 0 inside a heredoc is not an offence" \ + "$({ printf 'cat > /tmp/x <<%sEOF%s\n' "'" "'"; printf 'exit 0\n'; printf 'EOF\n'; } > "$PGC_WORKDIR/p934b.sh"; _h934_sweep "$PGC_WORKDIR/p934b.sh" | grep -c .)" "0" +# THE LITERAL AND THE CONSTANT MUST AGREE. Part 010 spells 66 rather than +# $PGC_EXIT_SKIPPED because lib.sh arrives in part 020 and 010 runs first. A second +# copy of a constant is a thing that drifts, so the copy is asserted against the +# original instead of being trusted. +check "part 010's skip status is the one lib.sh calls PGC_EXIT_SKIPPED" \ + "$(grep -cE '^PGC_EXIT_SKIPPED=66$' "$PGC_TESTDIR/lib.sh")" "1" +# `[[:space:]]` AND NOT `\t`: in POSIX ERE a backslash-t is a literal `t`, so the +# first version of this arm searched for "texit 66" and reported 0 against a file +# holding two. Third time that has cost me a measurement today, in three different +# sweeps. +check "and part 010 exits with exactly that status on its skip paths" \ + "$(grep -cE '^[[:space:]]+exit 66$' "$PGC_TESTDIR/selftest/010-stand-up-a-squatter-on-a.sh")" "2" +# ECHO LINES ONLY. Counting the phrase anywhere found three: the two the part emits +# and the one inside the comment above them explaining why they are there. A sweep +# that counts its own documentation is the trap the raises scan records. +check "and pairs each with the marker the runners require beside the status" \ + "$(grep -cE '^[[:space:]]+echo .*SKIPPED \(ran no checks\)' "$PGC_TESTDIR/selftest/010-stand-up-a-squatter-on-a.sh")" "2" + +check "no selftest part exits 0, which would exit the driver before its summary" \ + "$(_h934_sweep "$PGC_TESTDIR"/selftest/*.sh | grep -c . || true)" "0" From ed5749c83ae33816e990d8d9a6edba7127945511 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Thu, 10 Sep 2026 22:34:01 +0000 Subject: [PATCH 2/3] test/selftest: renumber this part, because #925 claimed the number first #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 from the same free range, which is the residual #554's one-file-per-subject rule leaves open: the 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 and this part moves. The subjects never overlapped -- theirs are the red-check ledger and the part manifest -- so this is numbering and nothing else. A NOTE 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 has to be added to that manifest or its arm reddens -- correctly, because that is the arm doing its job rather than a conflict. The reverse order needs nothing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 2 +- ...-must-not-report.sh => 430-the-self-test-must-not-report.sh} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/selftest/{410-the-self-test-must-not-report.sh => 430-the-self-test-must-not-report.sh} (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index b12e5b0c..50892c3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -361,7 +361,7 @@ true until the next version shipped. 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. - `test/selftest/410-the-self-test-must-not-report.sh` holds the arms, including a + `test/selftest/430-the-self-test-must-not-report.sh` holds the arms, including a heredoc-aware sweep requiring that **no** part exits 0 -- closing the class rather than the two instances. The sweep has to be heredoc-aware because the parts generate fixture scripts that legitimately end in `exit 0`: it sees 2 sites before this change diff --git a/test/selftest/410-the-self-test-must-not-report.sh b/test/selftest/430-the-self-test-must-not-report.sh similarity index 100% rename from test/selftest/410-the-self-test-must-not-report.sh rename to test/selftest/430-the-self-test-must-not-report.sh From af84fbb46fb10502951d612160b664a608d837f0 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Thu, 10 Sep 2026 22:57:32 +0000 Subject: [PATCH 3/3] test/selftest: the renumber left a cross-reference that will resolve to the wrong part @jdatcmd's blocker, and it is the residual I named in my own message about the number collision: the rename moved the file and the number, and a number written into PROSE is not a line git can conflict on. `010-stand-up-a-squatter-on-a.sh:22` said "part 410 asserts the literal still equals the constant". The arm is in 430. On its own that is a dangling pointer -- but #925 adds `410-a-check-must-have-been-red.sh`, so once that merges the reference RESOLVES, to a healthy part about the red-check ledger that asserts nothing about 66. A reader checking whether the literal is guarded would find a real file that does not guard it and stop looking. A dangling reference gets noticed; one that resolves to the wrong thing does not. TWO NON-BLOCKING NOTES, both taken, because both were numbers I retyped rather than measured. "A flat grep sees 13 either way" is true of main and false of the branch it ships in. Measured on this head it sees 24, because part 430's own fixtures add eleven `exit 0` lines. The real numbers argue the point better than the round one did, so the comment now carries both and says why the flat count moves for a reason unrelated to the defect: heredoc-aware sweep 2 before the fix, 0 after flat grep -c 'exit 0' 13 before, 24 after And the sweep closed ONE SPELLING of the mode. `exit 0` ends the driver with success; so does a BARE `exit`, which exits with the last command's status and is very often 0. That one is catchable, so the sweep now catches it, with an arm planting it and a control asserting a deliberate `exit 66` is not an offence. `exit $?` and `exit "$rc"` cannot be decided statically -- flagging them would refuse a part that legitimately exits non-zero -- so the comment names them as what the sweep does not see. Measured on this head: bare `exit` 0 sites, `exit $?` 0, `exit "$var"` 0, so nothing is open today. Gate: 606 checks, 606 passed + 0 failed + 0 unrunnable, PASSED. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 5 +++- test/selftest/010-stand-up-a-squatter-on-a.sh | 2 +- .../430-the-self-test-must-not-report.sh | 29 +++++++++++++++++-- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50892c3d..bebdeef3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -365,7 +365,10 @@ true until the next version shipped. heredoc-aware sweep requiring that **no** part exits 0 -- closing the class rather than the two instances. The sweep has to be heredoc-aware because the parts generate fixture scripts that legitimately end in `exit 0`: it sees 2 sites before this change - and 0 after, where a flat `grep -c 'exit 0'` sees 13 either way. + and 0 after, where a flat `grep -c 'exit 0'` sees 13 before and 24 after -- the flat + count moves because the new part's own fixtures add ten, for a reason that has nothing + to do with the defect. It sees `exit 0` and a bare `exit`; it cannot decide `exit $?` + or `exit "$rc"`, and each of those is zero in the tree today. After: `rc=2` and a named refusal for a missing `pg_config`, for a path that is not a directory, and for one that answers nothing; `604 passed + 0 failed + 0 unrunnable` diff --git a/test/selftest/010-stand-up-a-squatter-on-a.sh b/test/selftest/010-stand-up-a-squatter-on-a.sh index 7f50655a..df0492c2 100644 --- a/test/selftest/010-stand-up-a-squatter-on-a.sh +++ b/test/selftest/010-stand-up-a-squatter-on-a.sh @@ -19,7 +19,7 @@ done # `exit` here ends the DRIVER -- and a zero status said "610 checks passed" when the # truth was "nothing ran" (#934). 66 is the status lib.sh calls PGC_EXIT_SKIPPED and # the runners already pair with a `SKIPPED (ran no checks)` line; it is spelled as a -# literal because lib.sh arrives in part 020 and this part runs first, and part 410 +# literal because lib.sh arrives in part 020 and this part runs first, and part 430 # asserts the literal still equals the constant so the two cannot drift. if [ "$SQ_PORT" = 0 ]; then echo "SKIP could not find a free port for the squatter cluster" diff --git a/test/selftest/430-the-self-test-must-not-report.sh b/test/selftest/430-the-self-test-must-not-report.sh index 36076323..4237bd74 100644 --- a/test/selftest/430-the-self-test-must-not-report.sh +++ b/test/selftest/430-the-self-test-must-not-report.sh @@ -68,9 +68,27 @@ check "control: and the second, so a good pg_config is not refused" \ # skip means the guard stops being tested that run without anyone noticing". # # HEREDOC-AWARE, because the parts generate fixture scripts that legitimately end -# in `exit 0` and a flat grep reports eleven offences that are not offences. -# Measured: this sweep sees 2 sites before the fix and 0 after, where a flat -# `grep -c 'exit 0'` sees 13 either way. +# in `exit 0`, and a flat grep counts those as offences. +# +# THE REAL NUMBERS ARGUE IT BETTER THAN A ROUND ONE. An earlier version of this +# comment said a flat grep sees "13 either way", which is true of main and false of +# the branch it ships in: on this head it sees 22, because the fixtures BELOW add ten +# `exit 0` lines of their own. So the flat count moves for a reason that has nothing +# to do with the defect, which is the argument: +# +# heredoc-aware sweep 2 before the fix, 0 after +# flat grep -c 'exit 0' 13 before, 24 after +# +# Reported by @jdatcmd, who checked the number against the tree rather than the +# sentence. +# +# WHICH SPELLINGS THIS SEES, stated because a rule about the literal `0` is not the +# whole of "a sourced part must not end the driver claiming success". It sees +# `exit 0` and a BARE `exit`, which ends with the last command's status and is very +# often 0. It cannot decide `exit $?` or `exit "$rc"`, where the status is computed: +# flagging those would refuse a part that legitimately exits non-zero. Measured on +# this head, each of the three is **zero**, so nothing is open -- but the next reader +# should not assume the class is closed. Raised by @jdatcmd. _h934_sweep() { awk ' FNR == 1 { hd = "" } @@ -82,12 +100,17 @@ _h934_sweep() { } } /^[ \t]*exit[ \t]+0[ \t]*$/ { printf "%s:%d\n", FILENAME, FNR } + /^[ \t]*exit[ \t]*$/ { printf "%s:%d\n", FILENAME, FNR } ' "$@" } check "premise: the sweep reads every selftest part" \ "$([ "$(ls "$PGC_TESTDIR"/selftest/*.sh | wc -l)" -ge 30 ] && echo yes || echo no)" "yes" check "premise: and it finds a planted exit 0 outside a heredoc" \ "$(printf 'echo hi\nexit 0\n' > "$PGC_WORKDIR/p934.sh"; _h934_sweep "$PGC_WORKDIR/p934.sh" | grep -c .)" "1" +check "premise: and a BARE exit too, which ends with the last status and is usually 0" \ + "$(printf 'echo hi\nexit\n' > "$PGC_WORKDIR/p934e.sh"; _h934_sweep "$PGC_WORKDIR/p934e.sh" | grep -c .)" "1" +check "premise: while a deliberate non-zero exit is not an offence" \ + "$(printf 'echo hi\nexit 66\n' > "$PGC_WORKDIR/p934f.sh"; _h934_sweep "$PGC_WORKDIR/p934f.sh" | grep -c .)" "0" check "premise: while a fixture script ending in exit 0 inside a heredoc is not an offence" \ "$({ printf 'cat > /tmp/x <<%sEOF%s\n' "'" "'"; printf 'exit 0\n'; printf 'EOF\n'; } > "$PGC_WORKDIR/p934b.sh"; _h934_sweep "$PGC_WORKDIR/p934b.sh" | grep -c .)" "0" # THE LITERAL AND THE CONSTANT MUST AGREE. Part 010 spells 66 rather than