From cb2429719740e7269331e9d1defa4cb3fb5de4b6 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Fri, 18 Sep 2026 10:21:40 -0600 Subject: [PATCH 1/3] test: every suite that records must know its major (#1121) Eleven suites recorded every check against the literal string `unknown`. `pgc_record` writes `${PGC_MAJOR:-unknown}` and PGC_MAJOR is set inside `pgc_setup`, which none of them calls. A ledger row claiming `unknown` matches no run: the gate considers a row only where its majors intersect the majors the run observed, and no run ever observes it. So these checks were structurally unseedable. smoke 9 audit 31 objstore_stash_recovery 17 phase2 42 phase3 32 phase4 38 phase5 36 phase6 43 decode_interrupts 29 hilbert_curve 184 wal_envelope 20 ---- 481 of 481 records named no major ---- Same one-line fix as #1109, which reached concurrency, unique_conc and update_conc. THE THREE THAT TAKE NO PG_CONFIG read `$1`. The runner passes the pg_config to every suite, including those needing no cluster, and `pgc_major_of` yields empty on a path it cannot run -- so a bad path degrades to today's `unknown` rather than to a WRONG major. A guessed major would seed a row claiming a major the check was never observed on. And a static sweep in selftest 400 over the REGISTERED suites, carrying both wrong pattern spellings as fixtures: .*lib\.sh matches portlib.sh -- false positives .*/lib\.sh"?$ misses `. ".../lib.sh" || {` -- false negative, 184 records .*/lib\.sh" correct Both failures silent, in opposite directions, from one character. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- test/audit.sh | 13 +++ test/decode_interrupts.sh | 13 +++ test/hilbert_curve.sh | 13 +++ test/objstore_stash_recovery.sh | 13 +++ test/phase2.sh | 13 +++ test/phase3.sh | 13 +++ test/phase4.sh | 13 +++ test/phase5.sh | 13 +++ test/phase6.sh | 13 +++ .../400-a-check-result-must-be-machine.sh | 80 +++++++++++++++++++ test/smoke.sh | 13 +++ test/wal_envelope.sh | 13 +++ 12 files changed, 223 insertions(+) diff --git a/test/audit.sh b/test/audit.sh index 0c886d5c..3a971384 100755 --- a/test/audit.sh +++ b/test/audit.sh @@ -73,6 +73,19 @@ fi' EXIT . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" BINDIR="$("$PG_CONFIG" --bindir)" PORT="${PGC_PORT:-$(pgc_pick_port)}" SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" diff --git a/test/decode_interrupts.sh b/test/decode_interrupts.sh index 37d975d9..2f160c12 100755 --- a/test/decode_interrupts.sh +++ b/test/decode_interrupts.sh @@ -34,6 +34,19 @@ set -uo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "${1:-}")" + # No cluster needed; pgc_setup is skipped deliberately. Provide the counters the # shared check() helper expects. PGC_CHECKS=0 diff --git a/test/hilbert_curve.sh b/test/hilbert_curve.sh index 09a6215d..1e814886 100755 --- a/test/hilbert_curve.sh +++ b/test/hilbert_curve.sh @@ -211,6 +211,19 @@ set -uo pipefail exit 1 } +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "${1:-}")" + # No cluster, so pgc_setup is skipped deliberately -- the shape wal_envelope.sh # uses. lib.sh already zeroes the counters; they are restated so a reader can see # this suite keeps them itself and so pgc_summary's reconciliation is meaningful. diff --git a/test/objstore_stash_recovery.sh b/test/objstore_stash_recovery.sh index 1def6b88..84068ed7 100755 --- a/test/objstore_stash_recovery.sh +++ b/test/objstore_stash_recovery.sh @@ -35,6 +35,19 @@ set -uo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" SUITE="$SRCDIR/test/objstore_module.sh" diff --git a/test/phase2.sh b/test/phase2.sh index 78c05797..d6dbdbeb 100755 --- a/test/phase2.sh +++ b/test/phase2.sh @@ -21,6 +21,19 @@ set -euo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" BINDIR="$("$PG_CONFIG" --bindir)" PORT="${PGC_PORT:-$(pgc_pick_port)}" SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" diff --git a/test/phase3.sh b/test/phase3.sh index 034e223a..a9eb7376 100755 --- a/test/phase3.sh +++ b/test/phase3.sh @@ -23,6 +23,19 @@ set -euo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" BINDIR="$("$PG_CONFIG" --bindir)" PORT="${PGC_PORT:-$(pgc_pick_port)}" SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" diff --git a/test/phase4.sh b/test/phase4.sh index 313a9670..0ed03e70 100755 --- a/test/phase4.sh +++ b/test/phase4.sh @@ -26,6 +26,19 @@ set -euo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" BINDIR="$("$PG_CONFIG" --bindir)" PORT="${PGC_PORT:-$(pgc_pick_port)}" SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" diff --git a/test/phase5.sh b/test/phase5.sh index 1e54d953..de533396 100755 --- a/test/phase5.sh +++ b/test/phase5.sh @@ -29,6 +29,19 @@ set -euo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" BINDIR="$("$PG_CONFIG" --bindir)" PORT="${PGC_PORT:-$(pgc_pick_port)}" SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" diff --git a/test/phase6.sh b/test/phase6.sh index 7ce69396..32aa2f99 100755 --- a/test/phase6.sh +++ b/test/phase6.sh @@ -33,6 +33,19 @@ set -euo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" BINDIR="$("$PG_CONFIG" --bindir)" PORT="${PGC_PORT:-$(pgc_pick_port)}" SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" diff --git a/test/selftest/400-a-check-result-must-be-machine.sh b/test/selftest/400-a-check-result-must-be-machine.sh index 510c689a..6e1b53cb 100644 --- a/test/selftest/400-a-check-result-must-be-machine.sh +++ b/test/selftest/400-a-check-result-must-be-machine.sh @@ -608,3 +608,83 @@ check "premise: and that population is the suite corpus" \ _pl_hits="$(_pipeloop_sites "$PGC_TESTDIR"/*.sh "$PGC_TESTDIR"/selftest/*.sh 2>/dev/null | grep -c . || true)" [ "${_pl_hits:-0}" = 0 ] || _pipeloop_sites "$PGC_TESTDIR"/*.sh "$PGC_TESTDIR"/selftest/*.sh | sed 's/^/ /' check "no suite calls a check inside a piped loop" "${_pl_hits:-0}" "0" + +# ---- a suite that records must know its major (#1121, #1109) ----------------- +# +# `pgc_record` writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside +# `pgc_setup`. A suite that sources lib.sh -- so `check` exists and records -- but +# never calls `pgc_setup` writes every record against the literal string `unknown`. +# +# A LEDGER ROW CLAIMING `unknown` MATCHES NO RUN. The gate considers a row only +# where its majors intersect the majors the run observed, and no run ever observes +# `unknown`, so those checks are structurally unseedable: they cannot be recorded +# and could never be matched again if they were. +# +# #1109 fixed three of these -- concurrency, unique_conc, update_conc -- found by +# reading a failure. Eight more were found the same way by @jdatcmd (#1121), and +# this sweep found the remaining three. ELEVEN, and the population is why this is a +# sweep rather than three more one-line fixes: a run-based check ("no record says +# unknown") is silent about a suite that did not run, and every one of these was +# found only when something else happened to fail. +# +# STATIC, SO IT COVERS A SUITE NOBODY DISPATCHED. Decidable without running +# anything, which is the property that makes it a guard rather than an observation. +# +# THE PATTERN COST THREE ITERATIONS and each earlier one was wrong differently: +# +# .*lib\.sh matches portlib.sh -- six files, all false positives +# .*/lib\.sh"?$ misses `. ".../lib.sh" || {` -- hilbert_curve, a false +# negative, and it is the largest suite in the set at 184 +# records +# .*/lib\.sh" correct: a literal slash excludes portlib, no end-anchor +# admits a trailing `|| { ... }` +# +# Both failures were silent in the direction that matters: the first inflated the +# set with files that were fine, the second dropped a file that was not. +# THE POPULATION IS THE REGISTERED SUITES, not every file in test/. Only a +# registered suite produces a log the ledger reads, and sweeping the directory +# instead put `run_all_versions.sh` in the offender list -- whose only matches are +# the word "check" inside its own echo strings. +_maj_s="$(grep -n '^SUITES=(' "$_rv" | cut -d: -f1)" +_maj_e="$(awk -v s="$_maj_s" 'NR>s && /^\)/{print NR; exit}' "$_rv")" +_maj_reg="$PGC_WORKDIR/registered.txt" +sed -n "$((_maj_s + 1)),$((_maj_e - 1))p" "$_rv" | sed 's/#.*//' | tr -s ' \t' '\n' \ + | grep -E '^[a-z][a-z0-9_]*$' | LC_ALL=C sort -u > "$_maj_reg" + +_maj_src='^[[:space:]]*\.[[:space:]].*/lib\.sh"' +_maj_bad="" +_maj_seen=0 +while read -r _maj_n; do + _maj_f="$PGC_TESTDIR/$_maj_n.sh" + [ -f "$_maj_f" ] || continue + grep -qE "$_maj_src" "$_maj_f" || continue + _maj_seen=$((_maj_seen + 1)) + # HERESTRINGS, NOT `printf | grep -q` (#486, selftest part 080). The piped form + # lets the reader exit early, printf takes EPIPE, and pipefail calls the whole + # pipeline failed -- so `&& continue` never fired for a suite that DOES call + # pgc_setup and `|| continue` fired for one that records. Measured: the piped + # draft of this very sweep both added three files that were fine and dropped + # phase3, which is not. The guard one part over exists for this and caught it. + _maj_body="$(grep -vE '^[[:space:]]*#' "$_maj_f")" + grep -qE '(^|[[:space:];&|])pgc_setup[[:space:]]' <<<"$_maj_body" && continue + grep -qE '^[[:space:]]*PGC_MAJOR=' <<<"$_maj_body" && continue + # A file that records nothing has no record to mis-label. + grep -qE '(^|[[:space:];&|])(check|check_num|check_text|check_skip|pgc_record)[[:space:]]' \ + <<<"$_maj_body" || continue + _maj_bad="$_maj_bad $_maj_n" +done < "$_maj_reg" +check "premise: the sweep read the suites, so an empty result means something" \ + "$([ "$_maj_seen" -ge 200 ] && echo yes || echo no)" "yes" +check "every suite that records also learns which major it ran on" \ + "${_maj_bad# }" "" + +# AND THE PATTERN ITSELF, since it is the part that was wrong twice. Two fixtures +# in the workdir rather than a claim about the tree: one that sources portlib and +# must NOT match, one that sources lib.sh with a trailing `|| {` and must. +_maj_d="$PGC_WORKDIR/major"; rm -rf "$_maj_d"; mkdir -p "$_maj_d" +printf '. "$(dirname "${BASH_SOURCE[0]}")/portlib.sh"\n' > "$_maj_d/port.sh" +printf '. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" || {\n\techo no\n}\n' > "$_maj_d/trail.sh" +check "the source pattern does not mistake portlib.sh for lib.sh" \ + "$(grep -cE "$_maj_src" "$_maj_d/port.sh")" "0" +check "and does match a lib.sh source with a trailing brace" \ + "$(grep -cE "$_maj_src" "$_maj_d/trail.sh")" "1" diff --git a/test/smoke.sh b/test/smoke.sh index 49b43809..ee5bc44b 100755 --- a/test/smoke.sh +++ b/test/smoke.sh @@ -21,6 +21,19 @@ set -euo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" + +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" BINDIR="$("$PG_CONFIG" --bindir)" PORT="${PGC_PORT:-$(pgc_pick_port)}" SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" diff --git a/test/wal_envelope.sh b/test/wal_envelope.sh index ebc8bdd4..37a53722 100755 --- a/test/wal_envelope.sh +++ b/test/wal_envelope.sh @@ -32,6 +32,19 @@ set -uo pipefail . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# WHICH MAJOR THIS SUITE RAN ON (#1121, the wider half of #1109). `pgc_record` +# writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside `pgc_setup` -- which +# this suite does not call, deliberately. Without this line every record it emits +# says `unknown`, and a ledger row claiming `unknown` matches no run, so none of +# these checks could ever be seeded or matched again. +# +# The runner passes the pg_config as $1 to EVERY suite, including those that need +# no cluster, so it is available here. `pgc_major_of` returns empty on a path it +# cannot run, which degrades to exactly today's `unknown` rather than to a WRONG +# major -- a guessed major would seed a row claiming a major the check was never +# observed on, which is worse than saying nothing. +PGC_MAJOR="$(pgc_major_of "${1:-}")" + # No cluster needed; pgc_setup is skipped deliberately. Provide the counters the # shared check() helper expects. PGC_CHECKS=0 From ecb6d3ceb5a693ddb4d1709e6d477bc2da1b4843 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 18 Sep 2026 10:21:57 -0600 Subject: [PATCH 2/3] test: refuse a run whose records name no major (#1121) The complement to the static sweep in the commit below: that one is decidable without running anything and so covers a suite nobody dispatched; this one reads what was actually recorded and so does not care which pattern finds which file. Neither subsumes the other, and this issue is the argument for both -- the population was derived wrongly three times from static reasoning, and a reader of the records would have been right every time about the suites that ran. `run_all_versions.sh` gains `pgc_unknown_major_records`, and refuses a major whose logs carry such a record, NAMING the suites rather than counting them. A STATIC RULE CANNOT DECIDE WHICH SUITES RECORD, and two attempts failed differently: "defines no check() of its own" misses `audit`, whose own check() body calls pgc_record; "the file contains pgc_record" misses `objstore_stash_recovery`, which uses lib.sh's check() so the string never appears. Whether a suite records is a runtime property. Removal proof, four mutations, each mutant asserted to parse: reader matches the word anywhere, not field 6 1 arm red reader stops at the first offending record 2 arms red guard prints but does not fail the major 1 arm red message stops naming the suites 1 arm red control 1062 passed + 0 failed THE THIRD MUTATION CAUGHT A VACUOUS ARM OF MY OWN. "fails the major rather than only printing" scanned from the guard's test to the next `verfail=1` ANYWHERE below, and the runner has many -- so deleting the guard's own line left the arm green, satisfied by a different block. It is bounded to the block's closing `fi` now, and the reason is recorded where the pattern is. Both harnesses: selftest part 530 evals the reader out of the runner, test/pytest/test_record_names_its_major.py parses the record format in Python. Neither names the other. Ledger rows for the fourteen new arms derived from five majors on one frozen snapshot, all uniform 15;16;17;18;19: records 1062, namehash 7f49bcd6f9ca and verdicthash 9290ce012d69 identical across all five legs. checks_never_observed_red re-derived 1349 -> 1363. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- CHANGELOG.md | 43 ++++++ test/check_ledger.tsv | 14 ++ test/check_ledger_budget.txt | 13 +- test/pytest/TESTS.md | 44 ++++++ test/pytest/expected_tests.txt | 8 +- test/pytest/test_harness_deps.py | 3 + test/pytest/test_record_names_its_major.py | 130 ++++++++++++++++++ test/run_all_versions.sh | 51 +++++++ .../530-a-record-must-name-its-major.sh | 109 +++++++++++++++ test/selftest/parts.manifest | 1 + 10 files changed, 414 insertions(+), 2 deletions(-) create mode 100644 test/pytest/test_record_names_its_major.py create mode 100644 test/selftest/530-a-record-must-name-its-major.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c51b1f2..c2458f65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,49 @@ true until the next version shipped. layout already prunes as tightly. The same misattributed query reports 1.000; a 5 percent range on the sort key still reports 0.050 against a 50 percent range at 0.500. +- Eleven suites recorded every check against a major that is not a major (#1121). + + `pgc_record` writes `${PGC_MAJOR:-unknown}` and PGC_MAJOR is set inside + `pgc_setup`. A suite that records but never calls `pgc_setup` wrote every check + against the literal string `unknown`. + + THE GATE MATCHES A LEDGER ROW ONLY WHERE ITS MAJORS INTERSECT THE RUN'S, and no + run ever observes `unknown`. So none of those checks could be seeded, and a row + for one could never be matched again. Measured on PG17 before the fix: + + smoke 9 audit 31 objstore_stash_recovery 17 phase2 42 phase3 32 + phase4 38 phase5 36 phase6 43 decode_interrupts 29 + hilbert_curve 184 wal_envelope 20 + ---- 481 of 481 records named no major ---- + + Same defect and same one-line fix as #1109, which reached `concurrency`, + `unique_conc` and `update_conc`. After the fix all eleven record their real major + and every record count is unchanged. + + THE THREE THAT TAKE NO PG_CONFIG read it from `$1`. The runner passes the + pg_config to every suite, including those needing no cluster, and `pgc_major_of` + yields empty on a path it cannot run -- so a bad path degrades to today's + `unknown` rather than to a WRONG major. A guessed major would seed a row claiming + a major the check was never observed on, which is worse than saying nothing. + + TWO GUARDS, BECAUSE NEITHER SUBSUMES THE OTHER. `run_all_versions.sh` refuses a + major whose logs carry such a record, naming the suites: that reads what was + actually written, but is silent about a suite nothing dispatched. Selftest 400 + sweeps the registered suites statically: that is decidable without running + anything, but models how a suite gets its major rather than observing it. + + THE POPULATION WAS WRONG TWICE AND BOTH ERRORS WERE STATIC ONES. A sweep keyed on + "defines no `check()` of its own" misses `audit`, whose own `check()` body calls + `pgc_record`. One keyed on "the file contains `pgc_record`" misses + `objstore_stash_recovery`, which uses lib.sh's `check()`. And excluding files that + match `pgc_setup` dropped `decode_interrupts`, `hilbert_curve` and `wal_envelope` + -- the three whose comments say pgc_setup is SKIPPED deliberately, matching the + same grep a call would. The text explaining the behaviour was read as the + behaviour. Selftest 400 carries both wrong regex spellings as fixtures. + + `pg_upgrade` meets several of those static tests and is NOT affected: it carries + its own `check()` that never calls `pgc_record`, so it emits no records at all. + Checked by running it. - A `CONFLICTING` badge on a changelog entry is GitHub, not git (#1116). diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index e6e0d68a..8936d866 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -813,6 +813,7 @@ harness_selftest 400-a-check-result-must-be-machine an unrunnable check emits ex harness_selftest 400-a-check-result-must-be-machine an unrunnable check still prints its old line 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine and a different major reaches the same field, so it is read not hardcoded 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine and a harness that set no major says unknown rather than an empty field 15;16;17;18;19 never - +harness_selftest 400-a-check-result-must-be-machine and does match a lib.sh source with a trailing brace 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine and it is counted 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine and it is counted, so checks run: reports it 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine and its major field carries the server major it ran under 15;16;17;18;19 never - @@ -850,6 +851,7 @@ harness_selftest 400-a-check-result-must-be-machine check_text on an empty side harness_selftest 400-a-check-result-must-be-machine check_text's empty-side line is unchanged 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine control: a well-formed record still reconciles 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine control: unknown IS a major the reconciler accepts, or the four above prove nothing 15;16;17;18;19 never - +harness_selftest 400-a-check-result-must-be-machine every suite that records also learns which major it ran on 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine fewer records than counted checks is not described as a subshell 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine lib.sh bumps PGC_CHECKS in exactly one place 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine more records than counted checks names the cause, not just the arithmetic 15;16;17;18;19 never - @@ -868,6 +870,7 @@ harness_selftest 400-a-check-result-must-be-machine premise: the fixtures carry harness_selftest 400-a-check-result-must-be-machine premise: the piped-loop sweep read every file it was offered 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine premise: the probe ran every helper shape once 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine premise: the sweep classified a corpus of check-calling files 15;16;17;18;19 never - +harness_selftest 400-a-check-result-must-be-machine premise: the sweep read the suites, so an empty result means something 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine premise: while a file that calls no check is not its business 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine the accounting line reconciles four outcomes against the count 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine the major [18.2] is not a major, so that record does not reconcile 15;16;17;18;19 never - @@ -884,6 +887,7 @@ harness_selftest 400-a-check-result-must-be-machine the runner calls the record harness_selftest 400-a-check-result-must-be-machine the runner defines the record reconciliation 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine the same ratio check, ENABLED, emits one record and passes 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine the same timing check, ENABLED, emits one record and passes 15;16;17;18;19 never - +harness_selftest 400-a-check-result-must-be-machine the source pattern does not mistake portlib.sh for lib.sh 15;16;17;18;19 never - harness_selftest 400-a-check-result-must-be-machine the sweep finds a check inside a PIPED loop 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red --mutation across two failing checks in one run is refused 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red --prune REFUSES the whole prune when any orphan carries history 15;16;17;18;19 never - @@ -1269,6 +1273,16 @@ harness_selftest 520-a-merged-row-must-cover-the-majors premise: the single-majo harness_selftest 520-a-merged-row-must-cover-the-majors seeding a ledger with no prevailing set is not warned 15;16;17;18;19 never - harness_selftest 520-a-merged-row-must-cover-the-majors the gate's printed recipe names one log per gated major 15;16;17;18;19 never - harness_selftest 520-a-merged-row-must-cover-the-majors the warning does not fail the merge 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major a log whose records all name a major counts none, while a mixed one counts its own 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major a log with no records at all is not an offender 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major and a suite that names no major fails the major, rather than only printing 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major and it counts every offending record, not just the first 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major and the message names the offending suites 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major premise: an unreadable log is indistinguishable from a clean one, so the caller must guard it 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major premise: it is callable 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major premise: the runner defines the reader this part evals 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major the runner calls the reader over every suite's log 15;16;17;18;19 never - +harness_selftest 530-a-record-must-name-its-major the word in a REASON is not a record that names no major 15;16;17;18;19 never - index_fetch_penalty_crossover index_fetch_penalty_crossover a 50000-row correlated range uses the custom scan, not a fetching index 15;16;17;18;19 never - index_fetch_penalty_crossover index_fetch_penalty_crossover a selective point lookup still uses the index 15;16;17;18;19 never - index_fetch_penalty_crossover index_fetch_penalty_crossover both paths return the same aggregate at 50000 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index d9f0bac6..9044ff3d 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -99,4 +99,15 @@ suites_not_covered 249 # two under the 0.5 causation mutation, one under rel->rows / rel->tuples). # Re-derived by COUNTING, not by adding two to a number from another tree: # awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l -checks_never_observed_red 1377 +# 1349 -> 1363: fourteen arms across two guards for #1121 -- ten in selftest part 530 +# over the runner's record reader, four in part 400 over the static sweep. Derived by +# the command above on this tree, and by the gate's census in the same run. +# RESEATED onto main carrying #1117. Main states 1368, this branch stated 1363, and +# the merged tree is neither. The ledger auto-merged silently and was checked by KEY: +# 0 main keys lost, 14 added. Re-derived by the command above. +# RESEATED onto main carrying #1107. Main states 1377, this branch stated 1382, +# and the merged tree is neither. The ledger auto-merged SILENTLY while this file +# and TESTS.md conflicted loudly -- the usual pairing -- so the union was checked +# by KEY: 0 keys lost from either side, 14 added, 0 duplicates. Re-derived by the +# command above. +checks_never_observed_red 1391 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index eafc8f69..d0dbb4ba 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -97,6 +97,7 @@ behaviour, the source of that number is named. - [49. test_residual_is_counted.py: a residual must be counted, not subtracted](#49-test_residual_is_countedpy-a-residual-must-be-counted-not-subtracted) - [50. test_collation_pinned.py: comm's inputs must be sorted the same way](#50-test_collation_pinnedpy-comms-inputs-must-be-sorted-the-same-way) - [51. test_projection_scan_cost.py: a covering projection is not priced at half](#51-test_projection_scan_costpy-a-covering-projection-is-not-priced-at-half) +- [52. test_record_names_its_major.py: a record must name its major](#52-test_record_names_its_majorpy-a-record-must-name-its-major) ## 1. How to read a test in here @@ -4595,3 +4596,46 @@ names match. | test | what it asserts | | --- | --- | | `test_projection_scan_cost` | the table and covering projection exist; the tight and loose plans use that projection; without the GUC they are base columnar scans; every compared scan has a positive run cost; a tight covering projection is cheaper relative to the base than a loose one; the two ratios are not both 0.5; a non-sort-key restriction does not cheapen a covering projection | +## 52. test_record_names_its_major.py: a record must name its major + +#1121. `pgc_record` writes `${PGC_MAJOR:-unknown}` and `PGC_MAJOR` is set inside +`pgc_setup`, so a suite that records but never calls `pgc_setup` writes every +check against the literal string `unknown`. + +The gate matches a ledger row only where its majors intersect the majors the run +observed, and **no run ever observes `unknown`**. Such a check cannot be seeded, +and a row for it could never be matched again. Measured on PG 17 before the fix: + +``` +smoke 9/9 audit 31/31 objstore_stash_recovery 17/17 phase2 42/42 +phase3 32/32 phase4 38/38 phase5 36/36 phase6 43/43 +---- 248 of 248 records named no major ---- +``` + +**A static rule cannot do this job, and two attempts failed in different +directions.** "Defines no `check()` of its own" finds 5 and misses `audit`, whose +own `check()` body calls `pgc_record`. "The file contains the string +`pgc_record`" finds 7 and misses `objstore_stash_recovery`, which uses `lib.sh`'s +`check()` so the string never appears in it. The truth was 8 both times. Whether +a suite records is a runtime property, so the guard reads the records. + +Public seam: the RESULT record format and the runner's text. Read independently +of `test/selftest/530-a-record-must-name-its-major.sh`, which evals the shell +reader out of the runner while this parses the format directly. Neither file +names the other. + +### Every arm + +| test | what it holds | +| --- | --- | +| `test_a_clean_log_counts_none_while_a_mixed_one_counts_its_own` | asserted as a pair, because zero is also what a wrong field number and a broken parser produce | +| `test_every_offending_record_is_counted_not_just_the_first` | a suite can record some checks before `pgc_setup` and some after | +| `test_a_log_with_no_records_is_not_an_offender` | carrying no records is not naming a bad major | +| `test_the_word_in_a_reason_field_is_not_an_offending_record` | field six, not the line -- with the control that the same word in field six IS caught | +| `test_a_short_record_does_not_crash_or_count` | a truncated line has no field six | +| `test_the_runner_reads_every_suites_log_and_fails_the_major` | the wiring, bounded to the guard's own block | + +The load-bearing assertion is the last one's bound. The runner has many later +`verfail=1` lines, so an unbounded search finds one whatever the guard does -- +which is exactly how the shell twin's first version of that arm stayed green +against the line removed. Mutation testing caught it. diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index cf5fded5..c3998f05 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -193,7 +193,13 @@ # 372; re-derived by collection on this tree: `373 tests collected`. This key has now # been re-derived four times on one branch, each time against a different base, and the # arithmetic would have been right by luck twice and wrong twice. -guard_tests 374 +# 373 -> 379 when test_record_names_its_major.py landed (#1121): six arms over the +# RESULT record's major field and the runner's guard, parsed in Python rather than by +# evalling the shell reader, so the two harnesses fail differently. +# Re-derived by collection, never by adding six. +# RESEATED onto main carrying #1117. Main states 374, this branch stated 379; neither +# is the merged truth. Re-derived by collection: `380 tests collected`. +guard_tests 380 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index ef7faf16..6c92e8e7 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -125,6 +125,9 @@ # cluster, no driver -- reading the same corpus the shell part reads, with its # own implementation. "test_collation_pinned.py", + # #1121's file. Parses the RESULT record format in Python and reads the runner's + # text -- no connection, no cluster, no driver. + "test_record_names_its_major.py", ] diff --git a/test/pytest/test_record_names_its_major.py b/test/pytest/test_record_names_its_major.py new file mode 100644 index 00000000..b290b042 --- /dev/null +++ b/test/pytest/test_record_names_its_major.py @@ -0,0 +1,130 @@ +"""A RESULT record that names no major is a row nothing can seed. + +#1121. `pgc_record` writes ``${PGC_MAJOR:-unknown}`` and `PGC_MAJOR` is set inside +`pgc_setup`, so a suite that records but never calls `pgc_setup` writes every check +against the literal string ``unknown``. + +The gate matches a ledger row only where its majors intersect the majors the run +observed, and **no run ever observes ``unknown``**. So such a check cannot be seeded, +and a row for it could never be matched again. Measured on PG 17 before the fix:: + + smoke 9/9 audit 31/31 objstore_stash_recovery 17/17 phase2 42/42 + phase3 32/32 phase4 38/38 phase5 36/36 phase6 43/43 + ---- 248 of 248 records named no major ---- + +#1109 had already fixed three more the same way; these eight were not in that sweep. + +A STATIC RULE CANNOT DO THIS JOB, and two attempts failed in different directions: +"defines no `check()` of its own" misses `audit`, whose own `check()` body calls +`pgc_record`; "the file contains the string pgc_record" misses +`objstore_stash_recovery`, which uses lib.sh's `check()` so the string never appears. +Whether a suite records is a runtime property, so the guard reads the records. + +Read INDEPENDENTLY of `test/selftest/530-a-record-must-name-its-major.sh`: that part +evals the shell reader out of the runner, this one parses the record format directly +in Python. Neither file names the other. +""" + +import pathlib + +REPO = pathlib.Path(__file__).resolve().parents[2] +RUNNER = REPO / "test" / "run_all_versions.sh" + +# The producer writes tab-separated fields and the major is the SIXTH: +# RESULT suite part name verdict major reason +MAJOR_FIELD = 5 # zero-based + + +def _unknown_major_records(text): + """Records whose MAJOR field is the literal `unknown`. + + Field six, not "the word appears on the line": the reason field is free text and + may legitimately contain it. + """ + out = [] + for line in text.splitlines(): + f = line.split("\t") + if f and f[0] == "RESULT" and len(f) > MAJOR_FIELD and f[MAJOR_FIELD] == "unknown": + out.append(line) + return out + + +GOOD = ("RESULT\tdemo\tpart1\tone\tPASS\t17\t\n" + "RESULT\tdemo\tpart1\ttwo\tPASS\t17\t\n" + "checks run: 2\n") + +MIXED = ("RESULT\tdemo\tpart1\tone\tPASS\tunknown\t\n" + "RESULT\tdemo\tpart1\ttwo\tPASS\tunknown\t\n" + "RESULT\tdemo\tpart1\tthree\tPASS\t17\t\n" + "checks run: 3\n") + + +def test_a_clean_log_counts_none_while_a_mixed_one_counts_its_own(expect): + """Asserted as a PAIR. + + Zero is also what a wrong field number, an empty input and a broken parser all + produce, so the clean case is never asserted alone. + """ + expect.text((len(_unknown_major_records(GOOD)), len(_unknown_major_records(MIXED))), + (0, 2), + "a clean log yields none and a mixed one yields its two") + + +def test_every_offending_record_is_counted_not_just_the_first(expect): + """A suite can record some checks before `pgc_setup` and some after.""" + expect.num(len(_unknown_major_records(MIXED)), 2, + "both offending records are found, not just the first") + + +def test_a_log_with_no_records_is_not_an_offender(expect): + expect.num(len(_unknown_major_records("checks run: 0\n")), 0, + "a log carrying no records names no bad major") + + +def test_the_word_in_a_reason_field_is_not_an_offending_record(expect): + """Reasons are free text. Matching the line rather than the field counts them.""" + line = "RESULT\tdemo\tpart1\tfour\tPASS\t17\tthe major was unknown at first\n" + expect.num(len(_unknown_major_records(line)), 0, + "the word in a reason is not a record that names no major") + # And the control: the same text in the MAJOR field is caught, so the arm above + # is about the field and not about the word being absent. + caught = "RESULT\tdemo\tpart1\tfour\tPASS\tunknown\tthe major was unknown at first\n" + expect.num(len(_unknown_major_records(caught)), 1, + "while the same word in field six is") + + +def test_a_short_record_does_not_crash_or_count(expect): + """A truncated line has no field six. Indexing it blindly raises.""" + expect.num(len(_unknown_major_records("RESULT\tdemo\tpart1\n")), 0, + "a record too short to have a major is not counted") + + +# ---- the wiring ------------------------------------------------------------- + + +def test_the_runner_reads_every_suites_log_and_fails_the_major(expect): + """Proving a parser correct says nothing about whether the runner acts on it.""" + code = [l for l in RUNNER.read_text().splitlines() + if not l.lstrip().startswith("#")] + text = "\n".join(code) + + expect.num(text.count('pgc_unknown_major_records "$builddir/${s}.log"'), 1, + "the runner calls the reader over every suite's log") + + # BOUNDED TO THE GUARD'S OWN BLOCK. The runner has many later `verfail=1` lines, + # so an unbounded search finds one whatever this block does -- which is exactly + # how the shell twin's first version of this arm passed against the line removed. + block, seen = [], False + for l in code: + if '_unk_total" != 0' in l: + seen = True + if seen: + block.append(l) + if l == "\tfi": + break + expect.text("found" if block else "absent", "found", + "premise: the guard's block was located") + expect.num(sum(1 for l in block if "verfail=1" in l), 1, + "and a suite naming no major fails the major rather than only printing") + expect.num(sum(1 for l in block if "${_unk_suites}" in l), 1, + "and the message names the offending suites rather than counting them") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index da247986..c6d74c28 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -1137,6 +1137,22 @@ pgc_accounted_among() { # pgc_accounted_among NAMEFILE WIDEFILE -> names LC_ALL=C comm -12 <(LC_ALL=C sort "$1") <(LC_ALL=C sort "$2") } +# How many of a log's records name no major (#1121). +# +# A STATIC RULE CANNOT DO THIS JOB, and two attempts got it wrong in different +# directions before this reader existed. "Does this suite record?" is a RUNTIME +# property: `audit.sh` defines its own `check()` whose body calls `pgc_record`, so a +# rule keyed on defining a local check() misses it; `objstore_stash_recovery.sh` +# uses lib.sh's check() directly, so the string `pgc_record` never appears in the +# file at all and a rule keyed on that misses it too. And a third, excluding files +# that match `pgc_setup`, dropped the three suites whose COMMENTS say they skip it +# deliberately -- one of them the largest in the set at 184 records. +# +# The three predicates found 5, 7 and 8 suites. The truth was ELEVEN. Read the +# records: this reader does not care which pattern finds which file. +pgc_unknown_major_records() { # pgc_unknown_major_records LOGFILE -> count + awk -F'\t' '$1 == "RESULT" && $6 == "unknown"' "$1" 2>/dev/null | grep -c . || true +} pgc_log_shows_any_accounting() { # pgc_log_shows_any_accounting LOGFILE -> yes|no # Did this suite count its checks AT RUNTIME, by any mechanism the log shows? @@ -1510,6 +1526,41 @@ pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE [ "$(grep -c '^RESULT ' "$builddir/${s}.log" || true)" -ne 0 ] \ && _led_logs="$_led_logs $builddir/${s}.log" done + # A RECORD THAT NAMES NO MAJOR IS A ROW NOTHING CAN SEED (#1121). + # + # `pgc_record` writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside + # `pgc_setup`. A suite that sources lib.sh -- so pgc_record exists and runs -- + # but never calls pgc_setup records every check against the literal `unknown`. + # + # THE GATE CONSIDERS A ROW ONLY WHERE ITS MAJORS INTERSECT THE RUN'S, and no run + # ever observes `unknown`, so such a check cannot be seeded and a row for it could + # never be matched again. Eight suites were in that state, 248 of 248 records, + # until #1121; #1109 had already fixed three more the same way. + # + # CHECKED HERE because this is the only place that holds every log of a real run. + # The static version -- "a suite sourcing lib.sh must set PGC_MAJOR" -- is in the + # selftest and catches it earlier; this one catches it for the records that were + # actually written, which is the claim that matters. + _unk_suites="" + _unk_total=0 + for s in "${SUITES[@]}"; do + [ -s "$builddir/${s}.log" ] || continue + _unk_n="$(pgc_unknown_major_records "$builddir/${s}.log")" + if [ "$_unk_n" -ne 0 ]; then + _unk_suites="$_unk_suites $s($_unk_n)" + _unk_total=$((_unk_total + _unk_n)) + fi + done + if [ "$_unk_total" != 0 ]; then + # NAMED, NOT COUNTED. The count says something is wrong; the names say which + # suite to add the one line to. + echo " PG$major: $_unk_total record(s) name no major, so no ledger row for them" + echo " could ever be seeded -- the gate matches a row only where its majors" + echo " intersect the run's, and no run observes 'unknown':${_unk_suites}" + echo " Set PGC_MAJOR in each, as #1109 did. That is not a pass." + verfail=1 + fi + if [ -z "$_led_logs" ]; then echo " no suite emitted a check record on PG$major, so the ledger has nothing to gate" verfail=1 diff --git a/test/selftest/530-a-record-must-name-its-major.sh b/test/selftest/530-a-record-must-name-its-major.sh new file mode 100644 index 00000000..041f1c81 --- /dev/null +++ b/test/selftest/530-a-record-must-name-its-major.sh @@ -0,0 +1,109 @@ +# ---- a record that names no major is a row nothing can seed (#1121) --------- +# +# `pgc_record` writes `${PGC_MAJOR:-unknown}`, and PGC_MAJOR is set inside +# `pgc_setup`. A suite that records but never calls `pgc_setup` writes every check +# against the literal string `unknown`. +# +# THE GATE MATCHES A ROW ONLY WHERE ITS MAJORS INTERSECT THE RUN'S, and no run ever +# observes `unknown`. So such a check cannot be seeded, and a row for it could never +# be matched again. Measured on PG17 before the fix: +# +# smoke 9/9 audit 31/31 objstore_stash_recovery 17/17 phase2 42/42 +# phase3 32/32 phase4 38/38 phase5 36/36 phase6 43/43 +# ---- 248 of 248 records named no major ---- +# +# #1109 had already fixed three more the same way (concurrency, unique_conc, +# update_conc); these eight were simply not in that sweep. +# +# A STATIC RULE CANNOT DO THIS JOB AND I TRIED TWICE. +# +# * "sources lib.sh and defines no check() of its own" finds 5 and misses +# `audit`, whose own `check()` body calls `pgc_record`. Defining a local check +# says nothing about whether it records. +# * "the file contains the string pgc_record" finds 7 and misses +# `objstore_stash_recovery`, which uses lib.sh's `check()` directly, so the +# string never appears in the file. +# +# The truth was 8 both times. Whether a suite records is a RUNTIME property, so the +# guard reads the records. That is the #545 rule again: define a population by what +# it DOES, not by what it is named or what it looks like. +# +# The reader is evalled out of the runner rather than restated here, per selftest +# 320: a check that recomputes a rule tests the world instead of the code. +# --------------------------------------------------------------------------- + +_rv530="$PGC_TESTDIR/run_all_versions.sh" + +check "premise: the runner defines the reader this part evals" \ + "$(grep -c '^pgc_unknown_major_records()' "$_rv530")" "1" + +eval "$(sed -n '/^pgc_unknown_major_records()/,/^}/p' "$_rv530")" + +check "premise: it is callable" \ + "$(type -t pgc_unknown_major_records)" "function" + +_d530="$(mktemp -d)" + +# A record's major is field 6. The fixtures are written as the producer writes them, +# tab separated, so a change to that layout breaks this part rather than passing it. +printf 'RESULT\tdemo\tpart1\tone\tPASS\t17\t\n' >"$_d530/good.log" +printf 'RESULT\tdemo\tpart1\ttwo\tPASS\t17\t\n' >>"$_d530/good.log" +printf 'checks run: 2\n' >>"$_d530/good.log" + +printf 'RESULT\tdemo\tpart1\tone\tPASS\tunknown\t\n' >"$_d530/bad.log" +printf 'RESULT\tdemo\tpart1\ttwo\tPASS\tunknown\t\n' >>"$_d530/bad.log" +printf 'RESULT\tdemo\tpart1\tthree\tPASS\t17\t\n' >>"$_d530/bad.log" +printf 'checks run: 3\n' >>"$_d530/bad.log" + +printf 'checks run: 0\n' >"$_d530/norecords.log" + +# ASSERTED AS A PAIR. "0 unknown" is also what an absent reader, an unreadable file +# and a wrong field number all produce, so the clean case is never asserted alone. +check "a log whose records all name a major counts none, while a mixed one counts its own" \ + "$(pgc_unknown_major_records "$_d530/good.log")/$(pgc_unknown_major_records "$_d530/bad.log")" \ + "0/2" + +# THE MIXED LOG IS THE POINT. A suite can record some checks before `pgc_setup` and +# some after, and a reader that stopped at the first record would report 1 or 0. +check "and it counts every offending record, not just the first" \ + "$(pgc_unknown_major_records "$_d530/bad.log")" "2" + +check "a log with no records at all is not an offender" \ + "$(pgc_unknown_major_records "$_d530/norecords.log")" "0" + +# A MISSING FILE MUST NOT READ AS CLEAN. The runner skips empty logs before calling +# this, but a reader that answers 0 for a file it could not open is one that reports +# success having asked nothing. +check "premise: an unreadable log is indistinguishable from a clean one, so the caller must guard it" \ + "$(pgc_unknown_major_records "$_d530/does-not-exist.log")" "0" + +# FIELD 6, NOT "anywhere in the line". A reason field containing the word would +# otherwise be counted, and reasons are free text. +printf 'RESULT\tdemo\tpart1\tfour\tPASS\t17\tthe major was unknown at first\n' >"$_d530/reason.log" +check "the word in a REASON is not a record that names no major" \ + "$(pgc_unknown_major_records "$_d530/reason.log")" "0" + +# ---- the wiring, not only the reader --------------------------------------- +# +# Proving the reader correct says nothing about whether the runner calls it or acts +# on the answer. +_w530() { grep -vE '^[[:space:]]*#' "$_rv530"; } + +check "the runner calls the reader over every suite's log" \ + "$(_w530 | grep -c 'pgc_unknown_major_records "\$builddir/\${s}.log"')" "1" + +# BOUNDED TO THE GUARD'S OWN BLOCK. The first version of this arm scanned from the +# `_unk_total` test to the next `verfail=1` ANYWHERE below it, and the runner has many +# later ones -- so deleting the guard's own `verfail=1` left the arm green, finding a +# different block's. Mutation testing caught it: removing the line reddened nothing. +# The window now stops at the block's closing `fi`, so only that block can satisfy it. +check "and a suite that names no major fails the major, rather than only printing" \ + "$(_w530 | awk '/_unk_total" != 0/{f=1} f{print} f && /^\tfi$/{exit}' \ + | grep -c 'verfail=1')" "1" + +# NAMED, NOT COUNTED. The count says something is wrong; the names say which suite +# needs the one line. +check "and the message names the offending suites" \ + "$(_w530 | grep -c '\${_unk_suites}')" "1" + +rm -rf "$_d530" diff --git a/test/selftest/parts.manifest b/test/selftest/parts.manifest index eafc7e7f..e61bfc96 100644 --- a/test/selftest/parts.manifest +++ b/test/selftest/parts.manifest @@ -49,3 +49,4 @@ 500-the-gate-takes-a-prior-the-caller-names.sh 510-a-residual-must-be-counted.sh 520-a-merged-row-must-cover-the-majors.sh +530-a-record-must-name-its-major.sh From a8ae9e549388954da46bee03375eac886a8b1fa3 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 18 Sep 2026 10:38:50 -0600 Subject: [PATCH 3/3] test: declare the twin's reach into the shell harness (#1121) `test_harness_deps.py` keeps an inventory of the pytest files that read the shell harness and asserts it is EXACTLY what the corpus does, in both directions. The new twin reads `run_all_versions.sh` to check the guard is called and acts on its answer, so it belongs in that inventory and the arm named it. Second time this has caught a file of mine on the same day (#1110 was the first), which is the arm doing its job rather than a nuisance: a twin that reaches into the other harness and does not say so is the thing the inventory exists to surface. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- test/pytest/test_harness_deps.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index 6c92e8e7..8272201f 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -1173,6 +1173,11 @@ def test_the_job_installs_no_database_driver(expect): "test_mutation_ledger.py": "executes the matrix runner with its list flag to get the registered suite " "list, which is the same mechanism the entry above uses", + "test_record_names_its_major.py": + "reads the matrix runner's text to assert that the guard it defines is " + "actually called and actually fails the major -- the reader can be correct " + "while nothing acts on its answer, which is the half a removal proof over " + "the reader alone cannot see", "test_residual_is_counted.py": "reads the matrix runner's text to take out its two set readers and the " "summary block that prints the suite breakdown, then executes them -- so the "