diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f2676c4f..52c3178b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -472,7 +472,7 @@ jobs: - name: Run the suite matrix on PG ${{ matrix.pg }} run: | set -euo pipefail - # PGC_SKIP_TIMING drops the three wall-clock suites: a shared runner + # PGC_SKIP_TIMING drops the four wall-clock suites: a shared runner # cannot hold a ratio still, and a gate that reds for reasons unrelated # to the change is worse than one that does not run. They stay in the # local matrix, which is where those numbers mean anything. diff --git a/CHANGELOG.md b/CHANGELOG.md index ee62d1c1..a0fc9251 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,41 @@ true until the next version shipped. ### Added +- Every check result is machine-readable, and counting a check is the same + operation as recording it (#917). + + `check`, `check_num` and `check_text` printed `PASS` or `FAIL` and nothing + else, so proving that a mutation reddened one NAMED check meant grepping prose + and retyping the result. That is also how a reverted guard once reported plain + green while the check count fell from 190 to 186: the suite passed, and the only + evidence anything had changed was a number nobody compared. + + `lib.sh` had eleven places that bumped `PGC_CHECKS`, each with its own outcome + line beside it, which is eleven chances to add a twelfth and forget the line. + `projections.sh` did exactly that with an `expect_fail` at ten call sites, for + as long as it existed. There is now one, `pgc_record`, so a helper cannot report + an outcome without being counted and cannot be counted without reporting one. + Every human line is byte-identical; 3,762 call sites is past what a careful + refactor can be trusted on, so both harnesses pin the exact strings. + + Each record names the suite, the part, the check, the verdict and the reason. + The part matters because `harness_selftest` sources 40-odd parts into one shell + and phrases its premises to be copied, so a key of suite and name is a key of + check NAMES rather than of checks: 583 records give 579 distinct pairs against + 582 distinct triples. It is derived from `BASH_SOURCE`, not from a convention. + + A skipped wall-clock check is a fourth counted outcome. Under + `PGC_SKIP_TIMING`, `check_timing` and `check_ratio_needs_quiet_machine` printed + a `SKIP` line a reader sees while leaving the count at zero and emitting no + record, in branches no arm reached. `checks run:` now reports the checks a suite + encountered rather than the ones it evaluated, and the summary reconciles four + counters against it. A suite that skipped every check reports `SKIPPED` rather + than `PASSED`, which the old zero-check condition caught only by accident. + + The matrix reconciles each suite's records against the count its log states, and + names the cause rather than the arithmetic: more records than counted is a check + that ran in a subshell, fewer is a counter bumped outside `pgc_record`. + - Exact zone-map boundary coverage now lives in matching shell and pytest tests (#831). diff --git a/bench/build_citus.sh b/bench/build_citus.sh index 0a4b2711..aa648f6f 100755 --- a/bench/build_citus.sh +++ b/bench/build_citus.sh @@ -28,7 +28,9 @@ echo "== building Citus $TAG against $("$PG_CONFIG" --version) ($PG_CONFIG)" # Benchmark arms must run on a non-assert PostgreSQL. An assert build's numbers # are invalid, so refuse one the way the rest of the bench tooling does. -if "$PG_CONFIG" --configure | grep -q -- '--enable-cassert'; then +# grep -c, not grep -q; see test/lib.sh's pgc_is_columnar_scan. +_cfg="$("$PG_CONFIG" --configure 2>/dev/null || true)" +if [ "$(grep -c -- '--enable-cassert' <<<"$_cfg" || true)" != 0 ]; then echo "REFUSING: $PG_CONFIG is an assert build; benchmark numbers from it are invalid" >&2 exit 1 fi diff --git a/bench/build_timescaledb.sh b/bench/build_timescaledb.sh index c975b0f0..e1f32bda 100755 --- a/bench/build_timescaledb.sh +++ b/bench/build_timescaledb.sh @@ -28,7 +28,9 @@ echo "== building TimescaleDB $VERSION against $("$PG_CONFIG" --version) ($PG_CO # Benchmark arms must run on a non-assert PostgreSQL. An assert build's numbers # are invalid, so refuse one the way the rest of the bench tooling does. -if "$PG_CONFIG" --configure | grep -q -- '--enable-cassert'; then +# grep -c, not grep -q; see test/lib.sh's pgc_is_columnar_scan. +_cfg="$("$PG_CONFIG" --configure 2>/dev/null || true)" +if [ "$(grep -c -- '--enable-cassert' <<<"$_cfg" || true)" != 0 ]; then echo "REFUSING: $PG_CONFIG is an assert build; benchmark numbers from it are invalid" >&2 exit 1 fi diff --git a/bench/provision.sh b/bench/provision.sh index b1c7a1f1..a94fcea7 100755 --- a/bench/provision.sh +++ b/bench/provision.sh @@ -76,7 +76,9 @@ pg_present() { [ -x "$PREFIX_ROOT/$1/bin/pg_config" ]; } # built with cassert would produce benchmark numbers that are quietly wrong, and # nothing else on the box would notice. pg_is_assert() { - "$PREFIX_ROOT/$1/bin/pg_config" --configure 2>/dev/null | grep -q -- '--enable-cassert' + # grep -c, not grep -q; see test/lib.sh's pgc_is_columnar_scan. + _cfg="$("$PREFIX_ROOT/$1/bin/pg_config" --configure 2>/dev/null || true)" + [ "$(grep -c -- '--enable-cassert' <<<"$_cfg" || true)" != 0 ] } check_pg() { diff --git a/test/analyze_differential.sh b/test/analyze_differential.sh index 16eaa8e7..443e09a8 100755 --- a/test/analyze_differential.sh +++ b/test/analyze_differential.sh @@ -54,9 +54,8 @@ ROWS=${PGC_ANALYZE_DIFF_ROWS:-50000} # The major is asserted first so an unreadable version is not mistaken for an old # one and reported as "supported, skipped". if ! pgc_is_number "${PGC_MAJOR:-}"; then - echo "FAIL could not read the server major, so the gate below cannot be trusted: got [${PGC_MAJOR:-}]" - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)) - PGC_FAIL=1 + pgc_fail "could not read the server major, so the gate below cannot be trusted" \ + "got [${PGC_MAJOR:-}]" pgc_summary fi if [ "$PGC_MAJOR" -lt 18 ]; then diff --git a/test/analyze_function.sh b/test/analyze_function.sh index d491e1ef..10a133f4 100755 --- a/test/analyze_function.sh +++ b/test/analyze_function.sh @@ -63,9 +63,8 @@ ROWS=${PGC_ANALYZE_ROWS:-500000} # The major is asserted first. An unreadable version must not be mistaken for an # old one, or a broken environment would report SKIP and look supported. if ! pgc_is_number "${PGC_MAJOR:-}"; then - echo "FAIL could not read the server major, so the gate below cannot be trusted: got [${PGC_MAJOR:-}]" - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)) - PGC_FAIL=1 + pgc_fail "could not read the server major, so the gate below cannot be trusted" \ + "got [${PGC_MAJOR:-}]" pgc_summary fi if [ "$PGC_MAJOR" -lt 18 ]; then diff --git a/test/arrow_import.sh b/test/arrow_import.sh index afee56bf..b38afad0 100755 --- a/test/arrow_import.sh +++ b/test/arrow_import.sh @@ -579,9 +579,12 @@ idx_count() { # force an index scan SELECT count(*) FROM ix_tgt WHERE id BETWEEN 100 AND 199;" | tail -1 } idx_plan_is_index_scan() { - q "$IDX_SETUP - EXPLAIN (COSTS OFF) SELECT count(*) FROM ix_tgt WHERE id BETWEEN 100 AND 199;" \ - | grep -qi 'Index.*Scan' && echo yes || echo no + # grep -c on a captured value, not a pipe into grep -q; see lib.sh's + # pgc_is_columnar_scan for the mechanism and the measurement. + local _plan + _plan="$(q "$IDX_SETUP + EXPLAIN (COSTS OFF) SELECT count(*) FROM ix_tgt WHERE id BETWEEN 100 AND 199;")" + [ "$(grep -ci 'Index.*Scan' <<<"$_plan" || true)" != 0 ] && echo yes || echo no } seq_count() { # force a sequential scan q "SET enable_indexscan = off; SET enable_bitmapscan = off; diff --git a/test/concurrency.sh b/test/concurrency.sh index a1fe004d..1bd2848c 100755 --- a/test/concurrency.sh +++ b/test/concurrency.sh @@ -67,7 +67,11 @@ LOGFILE="$WORKDIR/server.log" # postmaster. port_is_free() { # port -> 0 if nothing is listening on it if command -v ss >/dev/null 2>&1; then - ! ss -Htln "sport = :$1" 2>/dev/null | grep -q ":$1" + # grep -c on a captured value, not a pipe into grep -q. A spurious + # EPIPE here answers "nothing is listening" for a port that IS taken, + # and the suite then starts a cluster on an occupied port. + _pif="$(ss -Htln "sport = :$1" 2>/dev/null || true)" + [ "$(grep -c ":$1" <<<"$_pif" || true)" = 0 ] else # fall back to a connect probe: a refused connection means free ! (exec 3<>"/dev/tcp/127.0.0.1/$1") 2>/dev/null diff --git a/test/entry_point_privilege.sh b/test/entry_point_privilege.sh index 78bbd981..7c89c58e 100755 --- a/test/entry_point_privilege.sh +++ b/test/entry_point_privilege.sh @@ -307,9 +307,12 @@ as_ep() { # as_ep -> the SQLSTATE, or the literal noerror local out out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U t_ep -d "$PGC_DB" \ -At -v VERBOSITY=sqlstate -v ON_ERROR_STOP=0 -c "$1" 2>&1)" - printf '%s\n' "$out" | sed -n 's/^.*ERROR:[[:space:]]*\([0-9A-Z]\{5\}\).*$/\1/p' | head -1 \ - | grep -q . && printf '%s\n' "$out" | sed -n 's/^.*ERROR:[[:space:]]*\([0-9A-Z]\{5\}\).*$/\1/p' | head -1 \ - || echo noerror + # Extract once into a variable rather than twice through a pipeline whose + # STATUS is the answer: `... | grep -q .` reports "no sqlstate" whenever the + # writer takes EPIPE, and this function's answer is a SQLSTATE. + local _sqlstate + _sqlstate="$(sed -n 's/^.*ERROR:[[:space:]]*\([0-9A-Z]\{5\}\).*$/\1/p' <<<"$out" | head -1)" + if [ -n "$_sqlstate" ]; then printf '%s\n' "$_sqlstate"; else echo noerror; fi } # The premise that makes every SQLSTATE arm below mean anything: this role can diff --git a/test/lib.sh b/test/lib.sh index 3ea4d61f..3228f904 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -28,7 +28,17 @@ # failure and continue. PGC_FAIL=0 +# The suite's own name, resolved ONCE at load rather than per check: pgc_record +# runs at every one of 3,762 call sites, and a basename fork at each of them is +# 3,762 forks a suite does not need. +PGC_SUITE="$(basename "$0" .sh)" PGC_CHECKS=0 +# A FOURTH OUTCOME. A check deliberately not asked -- a wall-clock measurement on +# a shared runner under PGC_SKIP_TIMING -- is not a pass, not a failure, and not +# unrunnable. It is counted, so `checks run:` reports the checks a suite +# ENCOUNTERED rather than the ones it managed to evaluate, and pgc_summary +# reconciles four counters against that count instead of three. +PGC_SKIPPED=0 # The status pgc_summary uses for "ran no checks". # @@ -969,17 +979,104 @@ psql_file() { # only supported way to add a check from outside this file, and selftest part 320 # sweeps for direct PGC_CHECKS writes so the next expect_fail is caught when it # is written rather than when it reddens something. -pgc_pass() { # pgc_pass NAME +# ---- one place that counts a check, and it is the same place that records it - +# +# lib.sh had ELEVEN sites bumping PGC_CHECKS, each with its own outcome line +# beside it. That is eleven chances to add a twelfth and forget the line, which +# is exactly what projections.sh's expect_fail did with ten call sites for as +# long as it existed. +# +# Counting and recording are therefore ONE operation. A helper cannot report an +# outcome without being counted, and cannot be counted without reporting one, +# because there is no code path that does either alone. `checks run: N` and the +# N record lines are the same increment seen twice. +# +# DISPLAY is passed whole rather than composed here, so every existing human line +# stays byte-identical: suites, selftests and CI all grep `^PASS` and `^FAIL`, +# and 3,762 call sites is far past what a careful refactor can be trusted on. +# +# The record is tab separated, five columns after the RESULT marker: +# +# RESULT suite part name verdict reason +# +# so a check name containing spaces survives. The reason carries the REASON_CODE +# #915 introduced, which is what makes this more than a reformat: an unrunnable +# check is distinguishable from a passing one without parsing prose. The verdict +# is one of PASS, FAIL, UNRUN or SKIP. +# +# There is no mutation column here. That one belongs to the LEDGER (#918), which +# keys on (suite, part, name) and records which mutation reddened a check; a +# record is one observation, not a history. +pgc_record() { # pgc_record VERDICT NAME DISPLAY [REASON] + local _v="$1" _name="$2" _display="$3" _reason="${4:-}" PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_PASSED=$((PGC_PASSED + 1)) - echo "PASS $1" + case "$_v" in + PASS) PGC_PASSED=$((PGC_PASSED + 1)) ;; + FAIL) PGC_FAILED=$((PGC_FAILED + 1)); PGC_FAIL=1 ;; + UNRUN) PGC_UNRUN=$((PGC_UNRUN + 1)) ;; + SKIP) PGC_SKIPPED=$((PGC_SKIPPED + 1)) ;; + *) + # An unknown verdict is a failure of the harness, not a check to + # drop. Dropping it would leave PGC_CHECKS bumped with no outcome + # recorded, which is the reconciliation failure pgc_summary refuses. + PGC_FAILED=$((PGC_FAILED + 1)); PGC_FAIL=1 + _display="FAIL $_name: pgc_record was given the verdict [$_v], which is not PASS, FAIL, UNRUN or SKIP" + _v=FAIL + ;; + esac + # WHICH PART asked this question, derived from the call stack. + # + # The suite is not enough. harness_selftest sources 40-odd parts into one + # shell, and its premises are phrased to be COPIED -- "premise: the pytest + # layer is where THIS PART thinks it is" says "this part" precisely so the + # same sentence works in any of them. main carries two copies of that one and + # two of another, and the number grows with every part anyone adds. + # + # So a key of (suite, name) is not a key of checks, it is a key of check + # NAMES, and they differ by however many parts share a boilerplate premise. + # One of them going red would then mark every sharer as observed red -- a + # claim about a check nothing attacked. Found by OffgridwithJD, who noticed + # that all six of their own branches added more. + # + # BASH_SOURCE, not a convention change, so the next part written the same way + # is keyed correctly without anyone remembering. Parameter expansion only: no + # basename fork, at 3,762 call sites. + local _part="" _bs + for _bs in "${BASH_SOURCE[@]}"; do + case "$_bs" in */lib.sh|lib.sh) continue ;; esac + _part="${_bs##*/}"; _part="${_part%.sh}" + break + done + + printf '%s\n' "$_display" + # Tabs in a field would split it. Nothing in the tree puts one in a check + # name, and this makes that true rather than assumed. + # + # PARAMETER EXPANSION, not `printf | tr` in a command substitution. The first + # version paid four forks per record -- two subshells and two tr processes -- + # in the function that runs at every one of 3,762 check sites, and whose own + # comment hoists PGC_SUITE out of the body on exactly that ground. Measured on + # an idle box, 2,000 calls, identical output on every input including a real + # tab: 3.1577 ms per call against 0.0096 ms, 331x, or 11.9 seconds of pure + # fork overhead across a full suite against 36 ms. Reported by OffgridwithJD. + printf 'RESULT\t%s\t%s\t%s\t%s\t%s\n' \ + "${PGC_SUITE:-unknown}" \ + "${_part:-${PGC_SUITE:-unknown}}" \ + "${_name//$'\t'/ }" \ + "$_v" \ + "${_reason//$'\t'/ }" +} + +pgc_pass() { # pgc_pass NAME + pgc_record PASS "$1" "PASS $1" } pgc_fail() { # pgc_fail NAME DETAIL - PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_FAILED=$((PGC_FAILED + 1)) - PGC_FAIL=1 - if [ -n "${2:-}" ]; then echo "FAIL $1: $2"; else echo "FAIL $1"; fi + if [ -n "${2:-}" ]; then + pgc_record FAIL "$1" "FAIL $1: $2" + else + pgc_record FAIL "$1" "FAIL $1" + fi } # A check that could not be evaluated is a third state, not a pass. @@ -998,30 +1095,23 @@ pgc_fail() { # pgc_fail NAME DETAIL # FAILED, because the failure is the more urgent fact. check_unrunnable() { # check_unrunnable NAME REASON_CODE DETAIL local name="$1" reason="${2:-}" detail="${3:-}" - PGC_CHECKS=$((PGC_CHECKS + 1)) case " $PGC_UNRUN_REASONS " in *" $reason "*) ;; *) - echo "FAIL $name: unrunnable reason [$reason] is not one of: $PGC_UNRUN_REASONS" - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) + pgc_record FAIL "$name" \ + "FAIL $name: unrunnable reason [$reason] is not one of: $PGC_UNRUN_REASONS" return ;; esac - PGC_UNRUN=$((PGC_UNRUN + 1)) - echo "UNRUN $name: $reason: $detail" + pgc_record UNRUN "$name" "UNRUN $name: $reason: $detail" "$reason" } check() { local name="$1" got="$2" want="$3" - PGC_CHECKS=$((PGC_CHECKS + 1)) if [ "$got" = "$want" ]; then - PGC_PASSED=$((PGC_PASSED + 1)) - echo "PASS $name" + pgc_record PASS "$name" "PASS $name" else - echo "FAIL $name: got [$got] want [$want]" - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) + pgc_record FAIL "$name" "FAIL $name: got [$got] want [$want]" fi } @@ -1071,11 +1161,8 @@ pgc_is_number() { # $1 -> 0 when $1 is a number check_text() { local name="$1" got="$2" want="$3" if [ -z "$got" ] || [ -z "$want" ]; then - PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) - echo "FAIL $name: a side is empty, so nothing was compared:" \ - "got [$got] want [$want]" + pgc_record FAIL "$name" \ + "FAIL $name: a side is empty, so nothing was compared: got [$got] want [$want]" return 1 fi check "$name" "$got" "$want" @@ -1085,11 +1172,8 @@ check_text() { check_num() { local name="$1" got="$2" want="$3" if ! pgc_is_number "$got" || ! pgc_is_number "$want"; then - PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) - echo "FAIL $name: not a measurement, so nothing was compared:" \ - "got [$got] want [$want]" + pgc_record FAIL "$name" \ + "FAIL $name: not a measurement, so nothing was compared: got [$got] want [$want]" return 1 fi check "$name" "$got" "$want" @@ -1121,30 +1205,20 @@ check_ratio() { # $1 label, $2 a, $3 b, $4 max local name="$1" a="$2" b="$3" max="$4" ratio if ! pgc_is_number "$a" || ! pgc_is_number "$b" || ! pgc_is_number "$max"; then - PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) - echo "FAIL $name: not a measurement, so no ratio was formed:" \ - "a=[$a] b=[$b] max=[$max]" + pgc_record FAIL "$name" \ + "FAIL $name: not a measurement, so no ratio was formed: a=[$a] b=[$b] max=[$max]" return 1 fi if [ "$(awk -v x="$a" -v y="$b" 'BEGIN { print (x + 0 == 0 || y + 0 == 0) ? "yes" : "no" }')" = yes ]; then - PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) - echo "FAIL $name: a side of the ratio is zero, so nothing was measured:" \ - "a=[$a] b=[$b]" + pgc_record FAIL "$name" \ + "FAIL $name: a side of the ratio is zero, so nothing was measured: a=[$a] b=[$b]" return 1 fi ratio="$(awk -v a="$a" -v b="$b" 'BEGIN { printf "%.2f", a / b }')" - PGC_CHECKS=$((PGC_CHECKS + 1)) if [ "$(awk -v r="$ratio" -v m="$max" 'BEGIN { print (r <= m) ? "yes" : "no" }')" = yes ]; then - PGC_PASSED=$((PGC_PASSED + 1)) - echo "PASS $name (${ratio}x, bound ${max}x, from a=$a b=$b)" + pgc_record PASS "$name" "PASS $name (${ratio}x, bound ${max}x, from a=$a b=$b)" else - echo "FAIL $name: ${ratio}x exceeds the ${max}x bound (a=$a b=$b)" - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) + pgc_record FAIL "$name" "FAIL $name: ${ratio}x exceeds the ${max}x bound (a=$a b=$b)" fi } @@ -1157,10 +1231,8 @@ pgc_require_tools() { command -v "$t" >/dev/null 2>&1 || missing="$missing $t" done if [ -n "$missing" ]; then - echo "FAIL the tools this suite measures with are missing:$missing" - PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) + pgc_record FAIL "the tools this suite measures with are missing" \ + "FAIL the tools this suite measures with are missing:$missing" return 1 fi return 0 @@ -1185,7 +1257,12 @@ check_timing() { local name="$1" got="$2" want="$3" if [ "${PGC_SKIP_TIMING:-0}" = 1 ]; then - echo "SKIP $name (PGC_SKIP_TIMING: wall-clock measurement)" + # Counted and recorded, like every other outcome. It printed a line a + # reader sees; leaving PGC_CHECKS at zero made that outcome invisible to + # the count and to the records both (#917, found by @linuxhikerpm). + pgc_record SKIP "$name" \ + "SKIP $name (PGC_SKIP_TIMING: wall-clock measurement)" \ + "PGC_SKIP_TIMING" return 0 fi check "$name" "$got" "$want" @@ -1227,7 +1304,9 @@ check_timing() { # same-run ratio) and nothing said it here, where it is decided (#787). check_ratio_needs_quiet_machine() { # if [ "${PGC_SKIP_TIMING:-0}" = 1 ]; then - echo "SKIP $1 (PGC_SKIP_TIMING: wall-clock ratio)" + pgc_record SKIP "$1" \ + "SKIP $1 (PGC_SKIP_TIMING: wall-clock ratio)" \ + "PGC_SKIP_TIMING" return 0 fi check_ratio "$@" @@ -1249,9 +1328,17 @@ check_ratio_needs_quiet_machine() { # # EXPLAIN without ANALYZE is enough: the line comes from the plan rather than the # run, so the assertion costs a plan and does not execute the query. pgc_is_columnar_scan() { # query -> yes|no - env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ - -d "$PGC_DB" -At -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null \ - | grep -q 'Columnar Projected Columns' && echo yes || echo no + # grep -c, NOT grep -q. grep -q exits the moment it matches, psql is still + # writing, and under a suite's `set -o pipefail` the pipeline reports failure + # though the pattern WAS present -- so this helper answers "no" for a plan that + # contains the line. Latent rather than live at EXPLAIN size (0 wrong in 200 + # trials at 1.9KB) but with no floor in the mechanism: measured 1/200 wrong at + # 8.9KB and 40/40 at 289KB, on a loaded machine. grep -c reads to EOF. + local _plan + _plan="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -At -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null)" + [ "$(grep -c 'Columnar Projected Columns' <<<"$_plan" || true)" != 0 ] \ + && echo yes || echo no } # Does this query's plan drive the per-row fetch path? @@ -1278,9 +1365,12 @@ pgc_is_columnar_scan() { # query -> yes|no # report "no" for a query that does exercise the cache. Measured: an UPDATE over # 2,000 rows made 20,366 fetch_row calls under a Custom Scan plan (#797). pgc_uses_row_fetch() { # setup query -> yes|no - env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ - -d "$PGC_DB" -At -c "$1" -c "EXPLAIN (COSTS OFF) $2" 2>/dev/null \ - | grep -q 'Index Scan using' && echo yes || echo no + # grep -c, not grep -q; see pgc_is_columnar_scan above for the measurement. + local _plan + _plan="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -At -c "$1" -c "EXPLAIN (COSTS OFF) $2" 2>/dev/null)" + [ "$(grep -c 'Index Scan using' <<<"$_plan" || true)" != 0 ] \ + && echo yes || echo no } # Order-independent set hash of an arbitrary query's result. The row is cast to @@ -1468,10 +1558,7 @@ pgc_skip() { # pgc_skip echo "SKIP $2 (waived by $allow_one or PGC_ALLOW_MISSING)" pgc_summary fi - PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_FAIL=1 - PGC_FAILED=$((PGC_FAILED + 1)) - echo "FAIL $2" + pgc_record FAIL "$2" "FAIL $2" echo " A missing dependency is an environment defect, not a pass. Install" echo " it, or set $allow_one=1 to run knowingly without this coverage." pgc_summary @@ -1496,7 +1583,7 @@ pgc_skip() { # pgc_skip # did one level up for how many VERSIONS actually ran. pgc_summary() { local _failed=$PGC_FAILED - local _sum=$((PGC_PASSED + PGC_FAILED + PGC_UNRUN)) + local _sum=$((PGC_PASSED + PGC_FAILED + PGC_UNRUN + PGC_SKIPPED)) echo echo "checks run: $PGC_CHECKS" echo "checks unrunnable: $PGC_UNRUN" @@ -1504,7 +1591,7 @@ pgc_summary() { # go missing, and this harness has 3,762 check sites -- far past what anyone # notices by reading. If this line does not add up the harness is lying about # its own arithmetic, so it is a failure rather than a note. - echo "accounting: $PGC_PASSED passed + $_failed failed + $PGC_UNRUN unrunnable = $PGC_CHECKS" + echo "accounting: $PGC_PASSED passed + $_failed failed + $PGC_UNRUN unrunnable + $PGC_SKIPPED skipped = $PGC_CHECKS" # A MEASUREMENT, not an identity. The failed count is its own counter rather # than CHECKS - PASSED - UNRUN, because a derived third term makes # P + (N-P-U) + U = N true for ANY values: a helper that counts a check and @@ -1514,9 +1601,9 @@ pgc_summary() { # line could not see it. Three counters maintained independently, reconciled # against a fourth, is the only version of it that can fail. if [ "$_sum" != "$PGC_CHECKS" ]; then - echo "FAIL the summary does not reconcile: $PGC_PASSED passed + $PGC_FAILED failed + $PGC_UNRUN unrunnable = $_sum, but $PGC_CHECKS checks ran" + echo "FAIL the summary does not reconcile: $PGC_PASSED passed + $PGC_FAILED failed + $PGC_UNRUN unrunnable + $PGC_SKIPPED skipped = $_sum, but $PGC_CHECKS checks ran" echo " A check was counted whose outcome nothing recorded. Find the helper" - echo " that bumps PGC_CHECKS without touching PGC_PASSED, PGC_FAILED or PGC_UNRUN." + echo " that bumps PGC_CHECKS without touching PGC_PASSED, PGC_FAILED, PGC_UNRUN or PGC_SKIPPED." PGC_FAIL=1 fi if [ "$PGC_FAIL" != "0" ]; then @@ -1552,7 +1639,11 @@ pgc_summary() { fi exit 1 fi - if [ "$PGC_CHECKS" = "0" ]; then + # EVALUATED nothing, not ENCOUNTERED nothing. Before the fourth counter a + # skipped check left PGC_CHECKS at zero, so this branch caught the all-skipped + # suite by accident; now it is counted, and the suite would report PASSED with + # nothing behind it. The condition is what it always meant. + if [ "$((PGC_PASSED + PGC_FAILED + PGC_UNRUN))" = "0" ]; then echo "$(basename "$0"): SKIPPED (ran no checks)" exit $PGC_EXIT_SKIPPED fi diff --git a/test/native_groupagg.sh b/test/native_groupagg.sh index c46c86b0..85b3dbbf 100755 --- a/test/native_groupagg.sh +++ b/test/native_groupagg.sh @@ -45,9 +45,13 @@ groupvec_off() { psql_run "ALTER DATABASE $PGC_DB SET pgcolumnar.enable_group_ve # "Columnar Vectorized Group Keys"; no other node emits it, so a positive grep is # proof of the node rather than an absence test that a fallback would also pass. pgc_is_groupvec() { # query -> yes|no - env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ - -d "$PGC_DB" -At -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null \ - | grep -q 'Columnar Vectorized Group Keys' && echo yes || echo no + # grep -c on a captured plan, not a pipe into grep -q; see lib.sh's + # pgc_is_columnar_scan for the mechanism and the measurement. + local _plan + _plan="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -At -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null)" + [ "$(grep -c 'Columnar Vectorized Group Keys' <<<"$_plan" || true)" != 0 ] \ + && echo yes || echo no } # toggle_diff LABEL "QUERY on t_col": same query, path off vs on, byte-exact. diff --git a/test/native_groupagg_batch.sh b/test/native_groupagg_batch.sh index 0149a972..72aa57d7 100755 --- a/test/native_groupagg_batch.sh +++ b/test/native_groupagg_batch.sh @@ -59,7 +59,11 @@ q1() { q "$1" | tail -1; } # Keys" is emitted by no other node, so a positive grep proves the node rather # than an absence test a fallback would also satisfy. is_groupvec() { # query -> yes|no - q "EXPLAIN (COSTS OFF) $1" | grep -q 'Columnar Vectorized Group Keys' \ + # grep -c on a captured value, not a pipe into grep -q; see lib.sh's + # pgc_is_columnar_scan for the mechanism and the measurement. + local _plan + _plan="$(q "EXPLAIN (COSTS OFF) $1")" + [ "$(grep -c 'Columnar Vectorized Group Keys' <<<"$_plan" || true)" != 0 ] \ && echo yes || echo no } @@ -89,8 +93,7 @@ agree_in() { # agree_in TABLE LABEL "SELECT ... FROM %T ..." # md5 of empty input is a fixed string, so require the columnar arm produced # rows at all before trusting the comparison. if [ -z "$(q "${tmpl//%T/$tbl}" | head -1)" ]; then - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)); PGC_FAIL=1 - echo "FAIL $label: the columnar arm returned no rows, so nothing was compared" + pgc_fail "$label" "the columnar arm returned no rows, so nothing was compared" return 1 fi check_text "$label" "$col" "$heap" diff --git a/test/native_repack.sh b/test/native_repack.sh index b8ef2bd0..ef671131 100755 --- a/test/native_repack.sh +++ b/test/native_repack.sh @@ -54,9 +54,7 @@ srv="$(q 'SHOW server_version_num')" # on an older major depends on it being 0. Asserting the premise must not destroy # the skip it guards. if ! pgc_is_number "$srv"; then - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)) - PGC_FAIL=1 - echo "FAIL the server did not answer 'SHOW server_version_num': got [$srv]" + pgc_fail "the server did not answer 'SHOW server_version_num'" "got [$srv]" pgc_summary fi if [ "$srv" -lt 190000 ]; then diff --git a/test/native_vecskip.sh b/test/native_vecskip.sh index 48af9bb2..cadb9798 100755 --- a/test/native_vecskip.sh +++ b/test/native_vecskip.sh @@ -42,7 +42,12 @@ explain_of() { -d "$PGC_DB" -At -c "EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) $1" 2>/dev/null } is_scalar_scan() { - explain_of "$1" | grep -q 'Columnar Projected Columns' && echo yes || echo no + # grep -c, not grep -q: see lib.sh's pgc_is_columnar_scan for why and for the + # measurement. This file holds the worst instance of the shape -- the arm at + # "premise: and it is not the scalar scan" WANTS "no", so a spurious EPIPE + # answer makes that premise pass for the wrong reason. Vacuity, not a red. + [ "$(explain_of "$1" | grep -c 'Columnar Projected Columns' || true)" != 0 ] \ + && echo yes || echo no } # The node, before any counter is read out of it. @@ -99,13 +104,16 @@ explain_agg() { # this query falls back to the scalar scan -- which DOES print the line, so the # check below would pass while testing nothing at all. check "premise: the aggregate arm really is a vectorized aggregate" \ - "$(explain_agg "$AGGQ" | grep -q 'Columnar Vectorized Aggregates' && echo yes || echo no)" \ + "$([ "$(explain_agg "$AGGQ" | grep -c 'Columnar Vectorized Aggregates' || true)" != 0 ] \ + && echo yes || echo no)" \ "yes" check "premise: and it is not the scalar scan" \ - "$(explain_agg "$AGGQ" | grep -q 'Columnar Projected Columns' && echo yes || echo no)" "no" + "$([ "$(explain_agg "$AGGQ" | grep -c 'Columnar Projected Columns' || true)" != 0 ] \ + && echo yes || echo no)" "no" check "the vectorized aggregate reports Columnar Vectors Skipped" \ - "$(explain_agg "$AGGQ" | grep -q 'Columnar Vectors Skipped' && echo yes || echo no)" \ + "$([ "$(explain_agg "$AGGQ" | grep -c 'Columnar Vectors Skipped' || true)" != 0 ] \ + && echo yes || echo no)" \ "yes" # Boundary and cross-vector ranges still return exactly the heap rows. diff --git a/test/objstore_module.sh b/test/objstore_module.sh index 7fb139ac..3aea7f2b 100755 --- a/test/objstore_module.sh +++ b/test/objstore_module.sh @@ -70,10 +70,8 @@ for stash in "$MOD.away" "$MOD.probe"; do # every check below would run against a broken installation and report the # confusing half of the truth, so stop here and say which file to look at. if ! stash_is_debris; then - echo "FAIL restored $stash to $MOD, but that is not a module either." - echo " This installation needs 'make install' before the suite can run." - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)) - PGC_FAIL=1 + pgc_fail "restored $stash to $MOD, but that is not a module either" \ + "this installation needs 'make install' before the suite can run" pgc_summary exit 1 fi diff --git a/test/objstore_s3_read.sh b/test/objstore_s3_read.sh index 28a95e51..9753f49e 100755 --- a/test/objstore_s3_read.sh +++ b/test/objstore_s3_read.sh @@ -166,7 +166,9 @@ pg_restart_env "AWS_ENDPOINT_URL='https://127.0.0.1:$S3_PORT'" \ "AWS_ACCESS_KEY_ID='$AKID'" "AWS_SECRET_ACCESS_KEY='$SECRET'" \ "AWS_REGION='$REGION'" MOD_SO="$(pgc_pg "$PGC_BINDIR/pg_config --pkglibdir" | tail -1)/pgcolumnar_objstore.so" -if pgc_pg "ldd '$MOD_SO'" 2>/dev/null | grep -q libssl; then +# grep -c, not grep -q; see lib.sh's pgc_is_columnar_scan. +_ldd_out="$(pgc_pg "ldd '$MOD_SO'" 2>/dev/null || true)" +if [ "$(grep -c libssl <<<"$_ldd_out" || true)" != 0 ]; then HTTPS_WANT="08006" else HTTPS_WANT="0A000" diff --git a/test/objstore_stash_recovery.sh b/test/objstore_stash_recovery.sh index e0ef1906..81842b4e 100755 --- a/test/objstore_stash_recovery.sh +++ b/test/objstore_stash_recovery.sh @@ -52,13 +52,13 @@ echo "PG_CONFIG=$PG_CONFIG" if [ -z "${PGC_SKIP_BUILD:-}" ]; then echo "-- building" make -C "$SRCDIR" PG_CONFIG="$PG_CONFIG" >/dev/null || { - echo "FAIL build failed, so nothing below measures the guard" - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)); PGC_FAIL=1; pgc_summary + pgc_fail "build failed, so nothing below measures the guard" + pgc_summary } echo "-- installing" make -C "$SRCDIR" install PG_CONFIG="$PG_CONFIG" >/dev/null || { - echo "FAIL install failed, so nothing below measures the guard" - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)); PGC_FAIL=1; pgc_summary + pgc_fail "install failed, so nothing below measures the guard" + pgc_summary } fi diff --git a/test/parquet_export_stats.sh b/test/parquet_export_stats.sh index db83952b..f650d188 100755 --- a/test/parquet_export_stats.sh +++ b/test/parquet_export_stats.sh @@ -88,8 +88,7 @@ psql_run "SELECT pgcolumnar.export_parquet('es_c', '$PARQ');" if ! python3 "$STATS_PY" "$PARQ" > "$S" 2>"$PGC_WORKDIR/stats.err"; then echo "FAIL the footer parser could not read the exported file:" sed 's/^/ /' "$PGC_WORKDIR/stats.err" - PGC_FAIL=1 - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)) + pgc_fail "the statistics export failed" "see the indented output above" pgc_summary fi @@ -229,10 +228,7 @@ check_num "every bound is its physical width" \ check_float() { # check_float NAME GOT WANT local name="$1" got="$2" want="$3" if ! pgc_is_number "$got" || ! pgc_is_number "$want"; then - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)) - PGC_FAIL=1 - echo "FAIL $name: not a measurement, so nothing was compared:" \ - "got [$got] want [$want]" + pgc_fail "$name" "not a measurement, so nothing was compared: got [$got] want [$want]" return 1 fi check "$name" \ @@ -370,9 +366,8 @@ if python3 "$STATS_PY" "$NANQ" > "$NS" 2>&1; then check "a zero minimum is written as -0.0" \ "$(nf "$N_Z9" minhex)" "0000000000000080" else - echo "FAIL the footer parser could not read the NaN fixture:" + pgc_fail "the footer parser could not read the NaN fixture" "see the indented output below" sed 's/^/ /' "$NS" - PGC_FAIL=1; PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)) fi # ---- (f) column_orders, without which the bounds have no defined meaning ---- diff --git a/test/parquet_import.sh b/test/parquet_import.sh index aa99d567..aed0fb3d 100755 --- a/test/parquet_import.sh +++ b/test/parquet_import.sh @@ -167,9 +167,12 @@ idx_count() { # force an index scan SELECT count(*) FROM ix_tgt WHERE id BETWEEN 100 AND 199;" | tail -1 } idx_plan_is_index_scan() { - q "$IDX_SETUP - EXPLAIN (COSTS OFF) SELECT count(*) FROM ix_tgt WHERE id BETWEEN 100 AND 199;" \ - | grep -qi 'Index.*Scan' && echo yes || echo no + # grep -c on a captured value, not a pipe into grep -q; see lib.sh's + # pgc_is_columnar_scan for the mechanism and the measurement. + local _plan + _plan="$(q "$IDX_SETUP + EXPLAIN (COSTS OFF) SELECT count(*) FROM ix_tgt WHERE id BETWEEN 100 AND 199;")" + [ "$(grep -ci 'Index.*Scan' <<<"$_plan" || true)" != 0 ] && echo yes || echo no } seq_count() { # force a sequential scan q "SET enable_indexscan = off; SET enable_bitmapscan = off; diff --git a/test/pg19_vacuum_options.sh b/test/pg19_vacuum_options.sh index 6cafd0aa..3ee7f6d0 100755 --- a/test/pg19_vacuum_options.sh +++ b/test/pg19_vacuum_options.sh @@ -40,9 +40,7 @@ srv="$(q 'SHOW server_version_num')" # on an older major depends on it being 0. Asserting the premise must not destroy # the skip it guards. if ! pgc_is_number "$srv"; then - PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1)) - PGC_FAIL=1 - echo "FAIL the server did not answer 'SHOW server_version_num': got [$srv]" + pgc_fail "the server did not answer 'SHOW server_version_num'" "got [$srv]" pgc_summary fi if [ "$srv" -lt 190000 ]; then diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index f759f5b3..e683c53c 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -59,9 +59,11 @@ behaviour, the source of that number is named. - [11. test_zonemap_boundaries.py: exact boundaries](#11-test_zonemap_boundariespy-exact-boundaries) - [12. test_saop_element_pushdown.py: scattered set pruning](#12-test_saop_element_pushdownpy-scattered-set-pruning) - [13. test_hilbert_locality.py: what the Hilbert curve buys](#13-test_hilbert_localitypy-what-the-hilbert-curve-buys) -- [14. Adding a test](#14-adding-a-test) -- [15. What this corpus does NOT yet refuse](#15-what-this-corpus-does-not-yet-refuse) -- [16. Traps this corpus records](#16-traps-this-corpus-records) +- [14. test_suite_accounting.py: the matrix accounting for its own suites](#14-test_suite_accountingpy-the-matrix-accounting-for-its-own-suites) +- [15. test_check_results_are_machine_readable.py: one counter, one record](#15-test_check_results_are_machine_readablepy-one-counter-one-record) +- [16. Adding a test](#16-adding-a-test) +- [17. What this corpus does NOT yet refuse](#17-what-this-corpus-does-not-yet-refuse) +- [18. Traps this corpus records](#18-traps-this-corpus-records) ## 1. How to read a test in here @@ -1012,7 +1014,241 @@ pins the eight counts exactly. Its header records why -- for a CURVE change the digest pins upstream catch it first and the integers add nothing, so their real domain is a changed READER at an unchanged layout. -## 14. Adding a test +## 14. test_suite_accounting.py: the matrix accounting for its own suites + +`test_suite_accounting.py` holds the matrix runner to its own arithmetic. + +`run_all_versions.sh` prints `suites that ran: N of M` and never checks it, and twelve +registered suites exit 0 without ever calling `pgc_summary`. Measured with a pattern +tight enough to exclude `portlib.sh` -- a looser one matched it and gave both reviewers +of this change the same wrong answer: **none** of the twelve sources `test/lib.sh`. +Each defines its own `check()`, and ten keep no tally at all, so the harness cannot see +their checks. Counted among the suites that +"ran", they are the overcount #447 added that line to stop, one level further down. + +A count cannot close this. Two errors of opposite sign cancel, and an exempt list +maintained by hand makes the count agree by construction -- the check then measures +the list rather than the run. So membership is derived from a property each suite +carries, and the two readings are reconciled as SETS, in both directions: + +| reading | where it comes from | +| --- | --- | +| declared | the suite's own text calls `pgc_summary` | +| observed | the suite's log carries the `accounting:` line `pgc_summary` prints before every exit path | + +Neither is a number and neither is hand-maintained. A suite that stops calling +`pgc_summary` moves between the sets on its own. + +These tests drive the SHELL functions out of `run_all_versions.sh` rather than +reimplementing them in Python. A Python twin would be a second implementation and +would agree with itself; the house rule asks for two observers of one implementation. +They run under `set -o pipefail`, because `harness_selftest.sh` does. + +### `test_a_suite_that_calls_pgc_summary_declares_accounting` + +The property is the CALL. A comment mentioning `pgc_summary`, and a longer name +containing it, are both refused -- a claim satisfied by prose is the failure the whole +design exists to avoid. + +### `test_the_accounting_line_is_read_on_every_exit_path` + +Pass, failure, skip and incomplete all carry the line, which is what makes it the +runtime twin of the declaration rather than a synonym for PASSED. + +### `test_the_reconciliation_names_both_directions` + +Declared-but-not-accounted is a suite that died before reaching its summary; today +that reads PASS whenever the shell happened to exit 0. Accounted-but-not-declared is a +stale reading of the source, which a hand-maintained list can never report. + +### `test_opposite_errors_do_not_cancel` + +Both directions are reported from one run. One error masking the other is exactly what +a count cannot distinguish from correctness. + +### `test_the_driver_s_own_non_dispatch_record_excuses_only_what_it_names` + +`PGC_SKIP_TIMING` drops four suites on every CI run; they declare accounting and +correctly produce none. The driver records that decision where it makes it, rather +than leaving it to be inferred from the log the driver forges. The record excuses only +what it names, and a suite that both accounted and was recorded as never dispatched +fails. + +### `test_the_printed_identity_can_actually_fail` + +`inputs == sum(buckets)` is printed beside every reconciliation. Computing `inputs` +FROM the buckets makes the line true for any values and reddens nothing, which is why +it is counted from the two files by a separate route. Dropping the sort before `comm` +makes the totals diverge, and that is the fault the identity guards. + +### `test_the_reader_accepts_the_line_the_producer_actually_emits` + +Every other log in the file is a literal, and the shell half types the same four again, +and the format string lives a third time in `pgc_summary`. Three hand-written copies of +one line: a wording drift in the **producer** leaves both harnesses green while the +reader answers "no" for every real suite, reddening the whole matrix on both majors. +So this arm runs a real suite and feeds the reader its actual stdout, with a reworded +control to show it can fail. + +### `test_the_partition_over_the_registered_suites_adds_up` + +The readers run over the real registered suite list. No count is asserted: how many +suites are exempt is not a fact about correctness, and pinning it would be a second +copy of the list this design removes. What is asserted is that the partition covers +the population and that both buckets are occupied. + +### `test_the_accounted_reader_takes_either_runtime_mechanism` + +Two runtime-observable mechanisms exist: `pgc_summary`'s accounting line, used by 239 +suites, and a suite's own `checks run:` line, which `bench_guards` and `docs_style` +print from private counters without ever sourcing `lib.sh`. A reader that knew only the +first would call those two unaccounted, which is false. + +### `test_a_registered_suite_accounted_by_nothing_fails_by_name` + +The defect @linuxhikerpm blocked #922 on. `pgc_reconcile_accounting` takes the declared +and observed sets, both derived from the suites themselves, so a registered suite in +neither is **outside the universe it reconciles** — with all its inputs empty it reports +complete symmetry and returns 0, whatever `SUITES` holds. Treating absence of a +declaration as absence from the population preserves the overcount. + +`pgc_reconcile_population` takes the registered set as an input and puts every +registered suite in exactly one of four buckets: accounted, not dispatched, known debt, +or unaccounted — and unaccounted fails, by name. Each of the three ways out is asserted +to actually let a suite out, or the bucket would be a name for "always fails". + +### `test_the_debt_file_excuses_only_what_it_names` + +Debt is recorded by name rather than as a count, which is what makes it a burn-down: a +new unaccounted suite fails while the known ones are excused. Debt that is no longer +debt — a suite that now accounts, or one no longer registered — is reported, so the +burn-down cannot stall silently. Those two are reported rather than fatal: a gate that +reddens the moment someone *fixes* something teaches people not to fix things. + +### `test_the_population_partitions_and_prints_its_identity` + +`inputs == sum(buckets)` over the registered population, printed per the house rule. +Like the symmetry check's identity it **cannot** be false on the data — the four buckets +are built by successive subtraction from the registered set, so their sum equals it +identically, measured at 0 firings over 400 random four-set inputs while the real bucket +findings fired on 353. What it guards is `comm` reading unsorted input, which produces +buckets that are not a partition at all. + +### `test_the_debt_file_is_tracked_and_holds_only_registered_suites` + +`test/suites_without_accounting.txt` is tracked so that adding a name is a diff a +reviewer sees — the whole reason it is a file and not a number in the environment. Every +name in it must be a registered suite. + +### `test_the_declaration_reader_survives_pipefail_on_a_long_suite` + +A regression arm. The first implementation piped `sed` into `grep -q`; grep exits on +match, sed takes EPIPE, and `pipefail` reports the pipeline as failed. The reader +answered "no" for a suite that plainly calls `pgc_summary`. It is a race, so it +reproduces on long files and not short ones -- it passed every fixture and failed only +on the real population, naming two of the longest suites. Selftest 040 carries the same +story from #473 and #476. + +## 15. test_check_results_are_machine_readable.py: one counter, one record + +Check results were prose. `check`, `check_num` and `check_text` printed `PASS` or +`FAIL` and nothing else, so proving that a mutation reddened one **named** check meant +grepping text. That is how a reverted guard once reported plain green while the check +count fell from 190 to 186 -- the suite passed, and the only evidence anything had +changed was a number nobody was comparing. + +The fix is not a second emitter beside the counters. A second source of truth for how +many checks ran is the defect this family of issues exists to close, and `lib.sh` had +**eleven** places that bumped `PGC_CHECKS` -- eleven chances to add a twelfth and +forget the line beside it, which is exactly what `projections.sh`'s `expect_fail` did +with ten call sites for as long as it existed. + +So counting a check and recording it are **one operation**, `pgc_record`. A helper +cannot report an outcome without being counted, and cannot be counted without +reporting one, because no code path does either alone. `checks run: N` and the N +record lines are the same increment seen twice. + +The record is tab separated, five columns after the `RESULT` marker: + +``` +RESULT suite part name verdict reason +``` + +so a check name with spaces survives. The verdict is one of `PASS`, `FAIL`, `UNRUN` or +`SKIP`, and the reason carries the `REASON_CODE` from #915 — which is what makes this +more than a reformat: an unrunnable check is distinguishable from a passing one without +parsing prose. + +There is **no mutation column here**. That belongs to the ledger (#918), which keys on +`(suite, part, name)` and records which mutation reddened a check. A record is one +observation, not a history. + +### `test_lib_sh_counts_a_check_in_exactly_one_place` + +The structural arm, and the one that matters most. It stops the next `expect_fail` +from being written rather than catching it after a year of silent miscounting. + +### `test_each_verdict_emits_one_record_carrying_its_fields` + +PASS, FAIL and UNRUN each emit one record with the right verdict, and the name field +keeps its spaces. A reason code the enum does not hold is already a failure, and must +record the verdict it produced rather than the one it was asked for. + +### `test_every_helper_records_exactly_once` + +`check_text`, `check_num`, `check_ratio`, `pgc_pass` and `pgc_fail`, not a sample of +them. Each had its own counter bump and its own outcome line, and each was one place +the pair could come apart. + +### `test_the_human_lines_are_byte_identical` + +3,762 call sites, and suites, selftests and CI all grep `^PASS` and `^FAIL`. Adding a +record beside them is only safe if the prose did not move, so the exact strings are +pinned rather than the refactor trusted. + +### `test_the_record_count_equals_the_counter_the_summary_reports` + +One operation, so it cannot fail by drifting. It can fail if a helper is added that +prints an outcome without recording it, which is the `expect_fail` shape. + +### `test_a_skipped_timing_check_is_counted_and_recorded` + +`check_timing` and `check_ratio_needs_quiet_machine` under `PGC_SKIP_TIMING=1` printed a +human `SKIP` line and returned — **no count, no record**. Two outcomes a reader sees, +invisible to both, inside the part whose whole argument is that counting and recording +are one operation. Nothing reached those branches either: removing both emitters left +every other arm green. + +`SKIP` is now a fourth outcome, counted like the other three, so `checks run:` reports +the checks a suite **encountered** rather than the ones it managed to evaluate. + +It is deliberately **not** `check_unrunnable`. That third state exits the suite +`INCOMPLETE`, and CI sets `PGC_SKIP_TIMING` on every run — so every run would go red. A +wall-clock check deliberately not asked on a shared runner is a different thing from one +that could not be answered. + +### `test_the_accounting_line_reconciles_four_outcomes` + +Four counters against the count, which is the same shape as three against it. The +skipped term is printed even when zero: a term that disappears when empty is one a +reader cannot tell from a term that was never there. + +### `test_a_suite_that_evaluated_nothing_did_not_pass` + +Before the fourth counter, a skipped check left `PGC_CHECKS` at zero, so an all-skipped +suite hit the "ran no checks" branch **by accident**. Counting it would have made that +suite report `PASSED` with nothing behind it, so the condition now says what it always +meant: `PASSED + FAILED + UNRUN`, not `CHECKS`. + +### `test_the_runner_reconciles_records_against_the_stated_count` + +A suite's log states `checks run: N` and carries N records. Those are two artifacts of +the same run and they can genuinely disagree: a suite killed mid-way, a truncated log, +a helper that prints an outcome without recording it. A log with no count at all never +reached its summary -- a different fault from a miscount, and not a clean +reconciliation. + +## 16. Adding a test 0. **Write it twice.** Every test in this tree ships as a `.sh` suite and a pytest test **in the same change** (jd, 2026-09-09). Not ported later, not one or the @@ -1039,7 +1275,7 @@ domain is a changed READER at an unchanged layout. failed the selftest on both majors of the matrix, which is how it was found. A new directory under `test/` inherits every rule the old ones follow. -## 15. What this corpus does NOT yet refuse +## 17. What this corpus does NOT yet refuse `VACUITY_MODES.md` is the inventory: 79 ways a pytest harness can report a pass while asserting nothing, 73 of them demonstrated by an actual run. **This layer refuses 25 @@ -1051,7 +1287,7 @@ Read it before adding a test. The gaps most likely to affect a new test are that same family satisfies it, and that a write is not required to have written anything. Both are named there with the refusal each needs. -## 16. Traps this corpus records +## 18. Traps this corpus records Recorded because each one produced a confident wrong result before it was caught, and all are the same family as the defect the layer exists to prevent. diff --git a/test/pytest/test_check_results_are_machine_readable.py b/test/pytest/test_check_results_are_machine_readable.py new file mode 100644 index 00000000..20ba213f --- /dev/null +++ b/test/pytest/test_check_results_are_machine_readable.py @@ -0,0 +1,300 @@ +"""A check result must be machine-readable, and counted in ONE place. + +Check results were prose. `check`, `check_num` and `check_text` printed PASS or FAIL +and nothing else, so proving a mutation reddened one NAMED check meant grepping text. +That is how a reverted guard once reported plain green while the check count fell from +190 to 186: the suite passed, and the only evidence anything had changed was a number +nobody was comparing. + +The fix is not a second emitter beside the counters. A second source of truth for how +many checks ran is the defect this issue family exists to close, and `lib.sh` had +ELEVEN places that bumped `PGC_CHECKS` -- eleven chances to add a twelfth and forget +the line beside it, which is exactly what `projections.sh`'s `expect_fail` did with ten +call sites for as long as it existed. + +So counting a check and recording it are ONE operation, `pgc_record`. `checks run: N` +and the N record lines are the same increment seen twice. + +These tests drive the shell out of `lib.sh` rather than reimplementing it, for the same +reason `test_suite_accounting.py` does: a Python twin would agree with itself. +""" + +import pathlib +import subprocess + +REPO = pathlib.Path(__file__).resolve().parents[2] +LIB = REPO / "test" / "lib.sh" +RUNNER = REPO / "test" / "run_all_versions.sh" + + +def _sh(body): + """Run a snippet with lib.sh sourced, under the shell options the suites use.""" + script = f'set -uo pipefail\ncd "{LIB.parent}"\n. ./lib.sh >/dev/null 2>&1\n{body}\n' + return subprocess.run(["bash", "-c", script], capture_output=True, text=True).stdout + + +def _records(call): + return [l for l in _sh(call).splitlines() if l.startswith("RESULT\t")] + + +def _human(call): + return [l for l in _sh(call).splitlines() if not l.startswith("RESULT\t")] + + +def _extract(path, name): + out, keep = [], False + for line in path.read_text().splitlines(): + if line.startswith(f"{name}() "): + keep = True + if keep: + out.append(line) + if line == "}": + break + return "\n".join(out) + + +# ---- the structural arm ----------------------------------------------------- + + +def test_lib_sh_counts_a_check_in_exactly_one_place(expect): + """Eleven bump sites were eleven chances to add a twelfth and forget the outcome. + + This is the arm that stops the next `expect_fail` from being written, rather than + catching it after it has been silently miscounting for a year. + """ + text = LIB.read_text() + expect.num(text.count("PGC_CHECKS=$((PGC_CHECKS"), 1, + "lib.sh bumps PGC_CHECKS in exactly one place") + expect.num(_extract(LIB, "pgc_record").count("PGC_CHECKS=$((PGC_CHECKS"), 1, + "and that place is pgc_record") + + +# ---- the record line -------------------------------------------------------- + + +def test_each_verdict_emits_one_record_carrying_its_fields(tmp_path, expect): + """Tab separated -- suite, name, verdict, reason -- so a name with spaces survives. + + The reason carries the REASON_CODE, which is what makes this more than a reformat: + an unrunnable check is distinguishable from a passing one without parsing prose. + """ + for call, verdict in ((' check "a name" x x', "PASS"), + (' check "a name" x y', "FAIL")): + recs = _records(call) + expect.num(len(recs), 1, f"a {verdict} check emits exactly one record") + expect.text(recs[0].split("\t")[4], verdict, f"and its verdict field says {verdict}") + expect.text(_records(' check "a name" x x')[0].split("\t")[3], "a name", + "and the name field keeps its spaces") + + # WHICH PART asked it. The suite is not enough: harness_selftest sources + # 40-odd parts into one shell and its premises are phrased to be COPIED -- + # "premise: the pytest layer is where THIS PART thinks it is" says "this part" + # so the same sentence works in any of them. So (suite, name) is a key of + # check NAMES, not of checks, and one sharer going red would mark them all. + # Derived from BASH_SOURCE rather than from a convention. Found by + # OffgridwithJD, whose own six branches were each adding more. + # THESE TWO ARMS COULD NOT FAIL, and OffgridwithJD said so. The field is + # `${_part:-${PGC_SUITE:-unknown}}`, so deleting the derivation still yields a + # non-blank single value and both arms stayed green. "Exactly one, and not + # blank" is satisfied by the fallback. + # + # The property needs a case where the part and the suite DIFFER, which is the + # case the field exists for: harness_selftest sources 40-odd parts into one + # shell. So a fixture does the same -- an outer script that sources an inner + # one which calls the check -- and the record must name the INNER file. + outer = tmp_path / "outer_suite.sh" + inner = tmp_path / "inner_part.sh" + inner.write_text('check "a name" x x\n') + outer.write_text(f'. "{LIB}"\n. "{inner}"\n') + r = subprocess.run(["bash", "-c", f'set -uo pipefail\nbash "{outer}"'], + capture_output=True, text=True) + recs = [l for l in r.stdout.splitlines() if l.startswith("RESULT\t")] + expect.num(len(recs), 1, "premise: the sourced part emitted exactly one record") + suite, part = recs[0].split("\t")[1], recs[0].split("\t")[2] + expect.text(suite, "outer_suite", "the suite is the script that ran") + expect.text(part, "inner_part", "and the part is the file the check was asked from") + expect.text("different" if suite != part else "same", "different", + "which are not the same thing, and the fallback would make them so") + + recs = _records(' check_unrunnable "a name" MISSING_DEPENDENCY "no jq"') + expect.num(len(recs), 1, "an unrunnable check emits exactly one record") + expect.text(recs[0].split("\t")[4], "UNRUN", + "and its verdict is UNRUN, which is neither of the other two") + expect.text(recs[0].split("\t")[5], "MISSING_DEPENDENCY", + "and the REASON_CODE travels in the reason field, not in prose") + + # A reason the enum does not hold is already a FAIL. It must record the verdict it + # produced, not the one it was asked for. + expect.text(_records(' check_unrunnable "n" NOT_A_REASON "x"')[0].split("\t")[4], + "FAIL", "a bogus reason code records FAIL, not UNRUN") + + +def test_every_helper_records_exactly_once(expect): + """Not a sample. Each of these had its own counter bump and its own outcome line, + and each was one place the pair could come apart.""" + cases = { + 'check_text "n" "" "x"': "FAIL", + 'check_num "n" abc 1': "FAIL", + 'check_ratio "n" abc 1 2': "FAIL", + 'check_ratio "n" 0 1 2': "FAIL", + 'check_ratio "n" 1 1 2': "PASS", + 'pgc_pass "n"': "PASS", + 'pgc_fail "n" "d"': "FAIL", + } + for call, verdict in cases.items(): + recs = _records(" " + call) + expect.num(len(recs), 1, f"{call.split()[0]} emits exactly one record") + expect.text(recs[0].split("\t")[4], verdict, f"and records {verdict}") + + +def test_the_human_lines_are_byte_identical(expect): + """3,762 call sites, and suites, selftests and CI all grep `^PASS` and `^FAIL`. + + Adding a record beside them is only safe if the prose did not move, so the exact + strings are pinned rather than the refactor trusted. + """ + cases = { + ' check "a name" x x': "PASS a name", + ' check "a name" x y': "FAIL a name: got [x] want [y]", + ' check_unrunnable "a name" MISSING_DEPENDENCY "no jq"': + "UNRUN a name: MISSING_DEPENDENCY: no jq", + ' check_text "n" "" "x"': + "FAIL n: a side is empty, so nothing was compared: got [] want [x]", + ' check_num "n" abc 1': + "FAIL n: not a measurement, so nothing was compared: got [abc] want [1]", + } + for call, want in cases.items(): + expect.text("\n".join(_human(call)), want, f"{call.strip().split()[0]} prints its old line") + + +def test_the_record_count_equals_the_counter_the_summary_reports(expect): + """One operation, so it cannot fail by drifting -- but it CAN fail if a helper is + added that prints an outcome without recording it, which is the expect_fail shape.""" + out = _sh(' check a x x; check b x y; check_text c "" x; check_num d abc 1\n' + ' check_ratio e 1 1 2; pgc_pass f; check_unrunnable g MISSING_DEPENDENCY h\n' + ' echo "COUNTED $PGC_CHECKS"') + records = len([l for l in out.splitlines() if l.startswith("RESULT\t")]) + counted = int(next(l.split()[1] for l in out.splitlines() if l.startswith("COUNTED "))) + expect.num(records, 7, "premise: the probe ran every helper shape once") + expect.num(records, counted, "the record count equals the counter the summary reports") + + +# ---- the runner reconciles the two -------------------------------------------- + + +def test_the_runner_reconciles_records_against_the_stated_count(tmp_path, expect): + """A log states `checks run: N` and carries N records. Those are two artifacts of + the same run, and they can genuinely disagree: a suite killed mid-way, a truncated + log, a helper that prints an outcome without recording it.""" + body = _extract(RUNNER, "pgc_reconcile_records") + expect.at_least(len(body), 1, "premise: the runner defines the reconciliation") + + def run(text): + log = tmp_path / "s.log" + log.write_text(text) + script = f'set -uo pipefail\n{body}\npgc_reconcile_records "{log}"\n' + r = subprocess.run(["bash", "-c", script], capture_output=True, text=True) + return r.stdout, r.returncode + + ok = "RESULT\ts\ta\tPASS\t\nRESULT\ts\tb\tPASS\t\nchecks run: 2\n" + expect.num(run(ok)[1], 0, "a log whose records match its stated count reconciles") + + out, rc = run("RESULT\ts\ta\tPASS\t\nchecks run: 2\n") + expect.num(rc, 1, "a log with fewer records than it claims is caught") + expect.num(out.count("records=1"), 1, "and both numbers are named, not just the verdict") + + expect.num(run("RESULT\ts\ta\tPASS\t\nRESULT\ts\tb\tPASS\t\n" + "RESULT\ts\tc\tPASS\t\nchecks run: 2\n")[1], 1, + "a log with more records than it claims is caught too") + + # A log with no count at all never reached its summary. That is a different fault + # from a miscount and must not read as a clean reconciliation. + expect.num(run("RESULT\ts\ta\tPASS\t\n")[1], 1, + "a log that never stated a count is not silently accepted") + + +# ---- the timing helpers reported an outcome that nothing counted ------------ + + +def _timing(skip, call): + """Run one timing helper with PGC_SKIP_TIMING set or clear.""" + body = (f'PGC_SKIP_TIMING={skip}\n' + 'PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0; PGC_SKIPPED=0\n' + f'{call}\n' + 'echo "COUNTS $PGC_CHECKS/$PGC_PASSED/$PGC_SKIPPED"') + out = _sh(body) + recs = [l for l in out.splitlines() if l.startswith("RESULT\t")] + human = [l for l in out.splitlines() + if not l.startswith("RESULT\t") and not l.startswith("COUNTS ")] + counts = next((l.split()[1] for l in out.splitlines() if l.startswith("COUNTS ")), "") + return recs, human, counts + + +def test_a_skipped_timing_check_is_counted_and_recorded(expect): + """`check_timing` and `check_ratio_needs_quiet_machine` under PGC_SKIP_TIMING=1 + printed a human SKIP line and returned -- no count, no record. + + Two outcomes a reader sees, invisible to both the count and the records, in the + part whose whole argument is that those are one operation. Nothing reached those + branches either: removing both emitters left every other arm green. Found by + @linuxhikerpm. + + SKIP is a fourth outcome, counted like the other three, so `checks run:` reports + the checks a suite ENCOUNTERED rather than the ones it managed to evaluate. It is + deliberately not `check_unrunnable`: that state exits the suite INCOMPLETE, and CI + sets PGC_SKIP_TIMING on every run, so every run would go red. A wall-clock check + not asked on a shared runner is a different thing from one that could not be + answered. + """ + for call, tail in ((' check_timing "a timing check" 1 1', + "wall-clock measurement"), + (' check_ratio_needs_quiet_machine "a ratio check" 1 1 2', + "wall-clock ratio")): + name = call.split('"')[1] + recs, human, counts = _timing(1, call) + expect.num(len(recs), 1, f"{name}: skipped, emits exactly one record") + expect.text(recs[0].split("\t")[4], "SKIP", f"{name}: and its verdict is SKIP") + expect.text(counts, "1/0/1", f"{name}: and it is counted as a skip") + expect.text("\n".join(human), f"SKIP {name} (PGC_SKIP_TIMING: {tail})", + f"{name}: and its human line is unchanged") + + recs, _, counts = _timing(0, call) + expect.num(len(recs), 1, f"{name}: enabled, emits exactly one record") + expect.text(recs[0].split("\t")[4], "PASS", f"{name}: and passes") + expect.text(counts, "1/1/0", f"{name}: counted as a pass, not a skip") + + +def test_the_accounting_line_reconciles_four_outcomes(expect): + """Four counters against the count, which is the same shape as three against it. + + The skipped term is printed even when zero: a term that disappears when empty is + a term a reader cannot tell from a term that was never there. + """ + def acct(skip): + out = _sh(f'PGC_SKIP_TIMING={skip}\n' + ' check "an ordinary check" x x\n' + ' check_timing "a timing check" 1 1\n' + ' pgc_summary') + return next((l for l in out.splitlines() if l.startswith("accounting: ")), "") + + expect.text(acct(1), "accounting: 1 passed + 0 failed + 0 unrunnable + 1 skipped = 2", + "a skipped check appears in the accounting identity") + expect.text(acct(0), "accounting: 2 passed + 0 failed + 0 unrunnable + 0 skipped = 2", + "and the term is printed when zero, not omitted") + + +def test_a_suite_that_evaluated_nothing_did_not_pass(expect): + """Before the fourth counter a skipped check left PGC_CHECKS at zero, so the + all-skipped suite hit the "ran no checks" branch by accident. Counting it would + have made the suite report PASSED with nothing behind it, so the condition now + says what it always meant: PASSED + FAILED + UNRUN, not CHECKS. + """ + out = _sh('PGC_SKIP_TIMING=1\n' + ' check_timing "a timing check" 1 1\n' + ' pgc_summary') + verdicts = [l for l in out.splitlines() + if l.endswith(("PASSED", "FAILED", "INCOMPLETE")) + or l.endswith("SKIPPED (ran no checks)")] + expect.text("\n".join(v.split(": ", 1)[1] for v in verdicts), + "SKIPPED (ran no checks)", + "a suite whose every check was skipped did not pass") diff --git a/test/pytest/test_suite_accounting.py b/test/pytest/test_suite_accounting.py new file mode 100644 index 00000000..8b75119c --- /dev/null +++ b/test/pytest/test_suite_accounting.py @@ -0,0 +1,471 @@ +"""The matrix must reconcile the suites it registered against the ones that accounted. + +`run_all_versions.sh` prints "suites that ran: N of M" and never checks it, and twelve +registered suites exit 0 without ever calling `pgc_summary`. Measured with a pattern +tight enough to exclude `portlib.sh` -- a looser one matched it and gave both reviewers +of this change the same wrong answer: **none** of the twelve sources `test/lib.sh`. +Each defines its own `check()`, and ten keep no tally at all, so the harness cannot see +their checks. Counted among the suites that +"ran", they are the overcount #447 added that line to stop, one level further down. + +A count cannot close this. Two errors of opposite sign cancel, and an exempt list +maintained by hand makes the count agree by construction. So membership is derived +from a property each suite carries -- its own text calls `pgc_summary`, and its log +carries the `accounting:` line `pgc_summary` prints before every exit path -- and the +two readings are reconciled as SETS, in both directions. + +These tests drive the SHELL functions out of `run_all_versions.sh` rather than +reimplementing them in Python. A Python twin would be a second implementation and +would agree with itself; the house rule asks for two observers of one implementation. +""" + +import pathlib +import subprocess + +REPO = pathlib.Path(__file__).resolve().parents[2] +RUNNER = REPO / "test" / "run_all_versions.sh" + + +def _extract(name): + """The text of one shell function, taken from the runner itself.""" + out, keep = [], False + for line in RUNNER.read_text().splitlines(): + if line.startswith(f"{name}() "): + keep = True + if keep: + out.append(line) + if line == "}": + break + return "\n".join(out) + + +def _call(name, *args, mutate=None): + """Run one of the runner's functions and return (stdout, returncode).""" + body = _extract(name) + if mutate: + body = mutate(body) + # `set -o pipefail` is not decoration: harness_selftest.sh runs under it, and a + # pipeline inside one of these functions behaves differently with it on. Driving + # them without it would test a shell the harness never uses. + script = ("set -uo pipefail\n" + body + "\n" + + " ".join([name] + [f'"{a}"' for a in args]) + "\n") + r = subprocess.run(["bash", "-c", script], capture_output=True, text=True) + return r.stdout, r.returncode + + +def _write(tmp_path, name, text): + p = tmp_path / name + p.write_text(text) + return str(p) + + +# ---- the declaration reader ------------------------------------------------- + + +def test_a_suite_that_calls_pgc_summary_declares_accounting(tmp_path, expect): + """The property is the CALL. A mention is not one, and neither is a longer name. + + The easy wrong implementation is a bare grep, and it reads a suite that only + explains why it cannot account as though it does -- a claim satisfied by prose, + which is the failure this whole approach exists to refuse. + """ + calls = _write(tmp_path, "calls.sh", '. lib.sh\ncheck "x" a a\npgc_summary\n') + silent = _write(tmp_path, "silent.sh", ". lib.sh\necho hi\nexit 0\n") + noted = _write(tmp_path, "noted.sh", ". lib.sh\n# cannot call pgc_summary here\nexit 0\n") + longer = _write(tmp_path, "longer.sh", ". lib.sh\npgc_summary_of_something\n") + + expect.text(_call("pgc_suite_declares_accounting", calls)[0].strip(), "yes", + "a suite that calls pgc_summary declares accounting") + expect.text(_call("pgc_suite_declares_accounting", silent)[0].strip(), "no", + "one that never calls it does not") + expect.text(_call("pgc_suite_declares_accounting", noted)[0].strip(), "no", + "a comment mentioning it is not a declaration") + expect.text(_call("pgc_suite_declares_accounting", longer)[0].strip(), "no", + "a longer name containing it is not a declaration") + # An ABSENT file is its own answer. Reported by OffgridwithJD reviewing #922: + # folding it into "no" classifies a registered suite whose .sh has vanished as + # exempt, and the reconciliation then reads clean. + expect.text(_call("pgc_suite_declares_accounting", str(tmp_path / "gone.sh"))[0].strip(), + "absent", "an absent file is reported absent, not exempt") + + # The stripper follows the shell's rule -- a hash starts a comment at line + # start or after whitespace -- so a hash inside a word cannot hide the call. + inword = _write(tmp_path, "inword.sh", ". lib.sh\nX=a#b; pgc_summary\n") + trailing = _write(tmp_path, "trailing.sh", ". lib.sh\npgc_summary # trailing\n") + indented = _write(tmp_path, "indented.sh", ". lib.sh\n # pgc_summary here only\nexit 0\n") + expect.text(_call("pgc_suite_declares_accounting", inword)[0].strip(), "yes", + "a hash inside a word does not hide the call after it") + expect.text(_call("pgc_suite_declares_accounting", trailing)[0].strip(), "yes", + "a trailing comment after the call does not hide it") + expect.text(_call("pgc_suite_declares_accounting", indented)[0].strip(), "no", + "an indented comment is still a comment") + + +# ---- the observation reader ------------------------------------------------- + + +def test_the_accounting_line_is_read_on_every_exit_path(tmp_path, expect): + """`pgc_summary` prints it before pass, failure, skip and incomplete alike. + + That is what makes it the runtime twin of the declaration rather than a synonym + for PASSED: a suite that failed still reached its summary and still accounted. + """ + shapes = { + "pass": "accounting: 3 passed + 0 failed + 0 unrunnable + 0 skipped = 3\nx.sh: PASSED\n", + "fail": "accounting: 1 passed + 2 failed + 0 unrunnable + 0 skipped = 3\nx.sh: FAILED\n", + "skip": "accounting: 0 passed + 0 failed + 0 unrunnable + 0 skipped = 0\nx.sh: SKIPPED (ran no checks)\n", + "inc": "accounting: 2 passed + 0 failed + 1 unrunnable + 0 skipped = 3\nx.sh: INCOMPLETE\n", + } + for shape, text in shapes.items(): + log = _write(tmp_path, f"{shape}.log", text) + expect.text(_call("pgc_log_shows_accounting", log)[0].strip(), "yes", + f"a {shape} log shows accounting") + + bare = _write(tmp_path, "bare.log", "x.sh: PASSED\n") + prose = _write(tmp_path, "prose.log", "this suite prints accounting: in prose\n") + expect.text(_call("pgc_log_shows_accounting", bare)[0].strip(), "no", + "a PASSED claim without the line shows none") + expect.text(_call("pgc_log_shows_accounting", prose)[0].strip(), "no", + "and prose containing the word is not the line") + expect.text(_call("pgc_log_shows_accounting", str(tmp_path / "gone.log"))[0].strip(), + "no", "an absent log shows none rather than erroring") + + # THE ^ ANCHOR, which nothing above exercises: the prose fixture is refused by + # the regex SHAPE, not by the anchor, so removing ^ from the reader left every + # arm here green. Reported by OffgridwithJD. The distinguishing input is a + # well-formed accounting line that does not start its line. + indented = _write(tmp_path, "indented.log", + " accounting: 3 passed + 0 failed + 0 unrunnable + 0 skipped = 3\nx.sh: PASSED\n") + expect.num(pathlib.Path(indented).read_text() + .count("accounting: 3 passed + 0 failed + 0 unrunnable + 0 skipped = 3"), 1, + "premise: the fixture carries a well-formed line, just indented") + expect.text(_call("pgc_log_shows_accounting", indented)[0].strip(), "no", + "an accounting line that does not start its line is refused") + + +def test_the_reader_accepts_the_line_the_producer_actually_emits(tmp_path, expect): + """Every log above is a literal typed into this file, and the shell half types the + same four again, and the format string itself lives a third time in `pgc_summary`. + + Three hand-written copies of one line. A wording drift in the PRODUCER leaves both + harnesses green while the reader answers "no" for every real suite -- which would + redden the whole matrix on both majors, having passed its own tests. Found by + OffgridwithJD, who measured it: one realistic rewording of lib.sh flips the reader + on a real log with no arm going red. + + So run a REAL suite and feed the reader its actual stdout. This is the only arm in + the file that survives a change to the format. + """ + suite = tmp_path / "real.sh" + suite.write_text(f'. "{REPO / "test" / "lib.sh"}"\ncheck "x" a a\npgc_summary\n') + log = tmp_path / "real.log" + r = subprocess.run(["bash", str(suite)], capture_output=True, text=True) + log.write_text(r.stdout) + + expect.num(r.stdout.count("\naccounting: "), 1, + "premise: the real suite produced exactly one accounting line") + expect.text(_call("pgc_log_shows_accounting", str(log))[0].strip(), "yes", + "the reader accepts the line the producer actually emits") + + # The control that the arm is not simply insensitive. + drifted = tmp_path / "drifted.log" + drifted.write_text(r.stdout.replace("\naccounting: ", "\naccounting summary: ")) + expect.num(drifted.read_text().count("\naccounting: "), 0, + "premise: the drift changed the line the reader looks for") + expect.text(_call("pgc_log_shows_accounting", str(drifted))[0].strip(), "no", + "and a reworded producer line is refused, so the arm can fail") + + +# ---- the reconciliation ----------------------------------------------------- + + +def test_the_reconciliation_names_both_directions(tmp_path, expect): + """Declared-but-not-accounted and accounted-but-not-declared are opposite faults. + + The first is a suite that died before reaching its summary -- today that reads + PASS whenever the shell happened to exit 0. The second is a stale reading of the + source, which is the failure a hand-maintained exempt list can never report. + """ + d = _write(tmp_path, "d", "alpha\nbeta\ngamma\n") + o = _write(tmp_path, "o", "alpha\nbeta\ngamma\n") + expect.num(_call("pgc_reconcile_accounting", d, o)[1], 0, "equal sets reconcile") + + o = _write(tmp_path, "o", "alpha\nbeta\n") + out, rc = _call("pgc_reconcile_accounting", d, o) + expect.num(rc, 1, "a declared suite that never accounted is caught") + expect.num(out.count("declared but never accounted: gamma"), 1, "and is named") + + d = _write(tmp_path, "d", "alpha\nbeta\n") + o = _write(tmp_path, "o", "alpha\nbeta\ngamma\n") + out, rc = _call("pgc_reconcile_accounting", d, o) + expect.num(rc, 1, "an undeclared suite that DID account is caught") + expect.num(out.count("accounted but never declared: gamma"), 1, + "and is named as the opposite fault") + + +def test_opposite_errors_do_not_cancel(tmp_path, expect): + """One error masking the other is exactly what a count cannot distinguish.""" + d = _write(tmp_path, "d", "alpha\ndelta\n") + o = _write(tmp_path, "o", "alpha\ngamma\n") + out, rc = _call("pgc_reconcile_accounting", d, o) + expect.num(rc, 1, "a run wrong in both directions fails") + expect.num(out.count("declared but never accounted: delta") + + out.count("accounted but never declared: gamma"), 2, + "and both directions are reported, not one") + + +def test_the_driver_s_own_non_dispatch_record_excuses_only_what_it_names(tmp_path, expect): + """PGC_SKIP_TIMING drops four suites on every CI run. + + They declare accounting and correctly produce none, because nothing ran them. + Without a term for that the check goes red for the one reason that is not a + defect. The term is recorded by the branch that makes the decision rather than + inferred from the log that branch forges. + """ + d = _write(tmp_path, "d", "alpha\nbeta\ngamma\n") + o = _write(tmp_path, "o", "alpha\nbeta\n") + nd = _write(tmp_path, "nd", "gamma\n") + expect.num(_call("pgc_reconcile_accounting", d, o, nd)[1], 0, + "a suite the driver never dispatched reconciles") + expect.num(_call("pgc_reconcile_accounting", d, o)[1], 1, + "and without that record the same run is still caught") + + # A suite cannot both have reached its summary and not have been dispatched. + d = _write(tmp_path, "d", "alpha\nbeta\n") + o = _write(tmp_path, "o", "alpha\nbeta\n") + nd = _write(tmp_path, "nd", "beta\n") + out, rc = _call("pgc_reconcile_accounting", d, o, nd) + expect.num(rc, 1, "a suite both accounted and recorded as never dispatched fails") + expect.num(out.count("both accounted and recorded as never dispatched: beta"), 1, + "and is named as that fault rather than one of the other two") + + d = _write(tmp_path, "d", "alpha\n") + o = _write(tmp_path, "o", "alpha\n") + nd = _write(tmp_path, "nd", "zeta\n") + out, _ = _call("pgc_reconcile_accounting", d, o, nd) + expect.num(out.count("accounted but never declared: zeta"), 1, + "the record cannot introduce a suite the source never declared") + + +def test_the_printed_identity_can_actually_fail(tmp_path, expect): + """inputs == sum(buckets) is printed beside every reconciliation. Can it be false? + + Measured rather than argued. Computing inputs FROM the buckets makes the line + P + D + O == P + D + O and nothing reddens -- which is why inputs is counted from + the two files by a separate route. Dropping the sort before comm makes the buckets + garbage and the totals diverge, and that is the fault this identity guards: + selftest 070's subject arriving in a second place. + """ + unsorted = lambda b: (b + .replace('LC_ALL=C sort -u "$_decl" 2>/dev/null', + 'cat "$_decl" 2>/dev/null') + .replace('LC_ALL=C sort -u "$_obs" 2>/dev/null', + 'cat "$_obs" 2>/dev/null')) + + body = _extract("pgc_reconcile_accounting") + expect.num(body.count('LC_ALL=C sort -u "$_decl"'), 1, + "premise: the real function sorts its declared side") + expect.num(unsorted(body).count('LC_ALL=C sort -u "$_decl"'), 0, + "premise: the mutation applied -- the twin no longer does") + + d = _write(tmp_path, "d", "gamma\nbeta\nalpha\n") + o = _write(tmp_path, "o", "delta\ngamma\nbeta\n") + out_twin, _ = _call("pgc_reconcile_accounting", d, o, mutate=unsorted) + out_real, _ = _call("pgc_reconcile_accounting", d, o) + expect.num(out_twin.count("does not add up"), 1, + "the identity catches comm reading unsorted input") + expect.num(out_real.count("does not add up"), 0, + "and does not fire on the same input unmutated, so the arm is not noise") + + +# ---- the readers, over the real population ---------------------------------- + + +def test_the_partition_over_the_registered_suites_adds_up(expect): + """A reader that works on fixtures and not on the 251 registered suites has been + tested against the world it was written for. + + No count is asserted. How many suites are exempt is not a fact about correctness, + and pinning it here would be a second copy of the hand-maintained list this design + exists to remove. What is asserted is that the partition covers the population and + that both buckets are occupied -- a reader answering the same way for everything + would satisfy every fixture above. + """ + listed = subprocess.run(["bash", str(RUNNER), "--list-suites"], + capture_output=True, text=True).stdout.split() + expect.at_least(len(listed), 1, "premise: the runner listed its suites") + + body = _extract("pgc_suite_declares_accounting") + script = body + '\nfor f in "$@"; do pgc_suite_declares_accounting "$f"; done\n' + paths = [str(REPO / "test" / f"{s}.sh") for s in listed] + verdicts = subprocess.run(["bash", "-c", script, "_"] + paths, + capture_output=True, text=True).stdout.split() + + yes, no = verdicts.count("yes"), verdicts.count("no") + absent = verdicts.count("absent") + print(f" registered={len(listed)} | declares={yes}, does not={no}, " + f"absent={absent} | sum={yes + no + absent}") + # A COVERAGE check, and only that. It fails when the classification does not + # see every registered suite -- a dropped path, a list that changed between + # the two reads. It is NOT a check on the reader's correctness: the two arms + # below, which require both buckets occupied, are what catch a reader that + # answers the same way for everything. The population is counted from the + # runner's own list, a different route from the verdicts. + expect.num(yes + no + absent, len(listed), + "the partition covers every registered suite") + expect.num(absent, 0, "every registered suite has a file") + expect.at_least(no, 1, "the reader does not answer yes for every suite") + expect.at_least(yes, 1, "nor no for every one of them") + + +def test_the_declaration_reader_survives_pipefail_on_a_long_suite(tmp_path, expect): + """A regression arm for a bug this file's first implementation shipped. + + It piped `sed` into `grep -q`. grep -q exits the moment it matches, closing the + pipe while sed is still writing; sed takes EPIPE, and under `set -o pipefail` the + pipeline reports that failure even though grep matched. The reader then answered + "no" for a suite that plainly calls pgc_summary. + + It is a race, so it reproduces on long files and not short ones -- which is why it + passed every fixture above and failed only on the real population, naming the two + longest suites. Selftest 040 carries the same story from #473 and #476. + """ + big = tmp_path / "big.sh" + big.write_text(". lib.sh\npgc_summary\n" + "".join(f"echo padding {i}\n" for i in range(40000))) + expect.at_least(len(big.read_text().splitlines()), 10000, + "premise: the fixture is long enough to lose the race") + + expect.text(_call("pgc_suite_declares_accounting", str(big))[0].strip(), "yes", + "a long suite that calls pgc_summary still declares accounting") + + # The arm can fail: the grep -q shape is the one that gets this wrong. Restated + # here rather than extracted, because the wrong version is no longer in the tree. + twin = ("set -uo pipefail\n" + "sed 's/#.*$//' \"$1\" | grep -qE '(^|[^_[:alnum:]])pgc_summary([^_[:alnum:]]|$)'" + " && echo yes || echo no\n") + got = subprocess.run(["bash", "-c", twin, "_", str(big)], + capture_output=True, text=True).stdout.strip() + expect.text(got, "no", "the grep -q shape is the one that gets this wrong under pipefail") + + short = tmp_path / "short.sh" + short.write_text(". lib.sh\npgc_summary\n") + got_short = subprocess.run(["bash", "-c", twin, "_", str(short)], + capture_output=True, text=True).stdout.strip() + expect.text(got_short, "yes", + "and it agrees on a SHORT file, which is why it survived review") + + +# ---- the population, which the symmetry check cannot see -------------------- + + +def test_the_accounted_reader_takes_either_runtime_mechanism(tmp_path, expect): + """Two mechanisms exist in the tree, and both are runtime-observable. + + `pgc_summary` prints the accounting line and 239 suites use it. `bench_guards` + and `docs_style` keep private counters and print their own `checks run:`; they + never source `lib.sh`, so a reader that only knows the first would call them + unaccounted, which is false. Both are derived rather than declared, so a suite + that adopts either leaves the debt bucket on its own. + """ + lib = _write(tmp_path, "lib.log", + "accounting: 1 passed + 0 failed + 0 unrunnable + 0 skipped = 1\nx.sh: PASSED\n") + own = _write(tmp_path, "own.log", "checks run: 9\ndocs_style.sh: PASSED\n") + neither = _write(tmp_path, "none.log", "some output\nPASSED\n") + expect.text(_call("pgc_log_shows_any_accounting", lib)[0].strip(), "yes", + "a log carrying lib.sh's accounting line is accounted") + expect.text(_call("pgc_log_shows_any_accounting", own)[0].strip(), "yes", + "and a log carrying only its own checks-run line is too") + expect.text(_call("pgc_log_shows_any_accounting", neither)[0].strip(), "no", + "a log carrying neither is not accounted") + + +def test_a_registered_suite_accounted_by_nothing_fails_by_name(tmp_path, expect): + """The defect @linuxhikerpm blocked on, stated as its own arm. + + `pgc_reconcile_accounting` takes the declared and observed sets, both derived + from the suites themselves, so a registered suite in neither is outside the + universe it reconciles -- with all its inputs empty it reports complete + symmetry and returns 0, whatever SUITES holds. Treating absence of a + declaration as absence from the population preserves the overcount. + """ + reg = _write(tmp_path, "reg", "alpha\n") + acct = _write(tmp_path, "acct", "") + nd = _write(tmp_path, "nd", "") + debt = _write(tmp_path, "debt", "") + out, rc = _call("pgc_reconcile_population", reg, acct, nd, debt) + expect.num(rc, 1, "a registered suite accounted by nothing fails") + expect.num(out.count("registered but accounted by nothing: alpha"), 1, + "and is named, which the symmetry check could never do") + + # Each way out must actually let it out, or the bucket is a name for + # "always fails" and only the debt file is doing any work. + for label, f in (("accounted", acct), ("not dispatched", nd), ("known debt", debt)): + pathlib.Path(acct).write_text("") + pathlib.Path(nd).write_text("") + pathlib.Path(debt).write_text("") + pathlib.Path(f).write_text("alpha\n") + expect.num(_call("pgc_reconcile_population", reg, acct, nd, debt)[1], 0, + f"a suite that is {label} passes") + + +def test_the_debt_file_excuses_only_what_it_names(tmp_path, expect): + """Recording debt by NAME rather than as a count is what makes this a + burn-down: a new unaccounted suite must fail while the known ones are excused, + and debt that is no longer debt must be reported so it cannot stall.""" + reg = _write(tmp_path, "reg", "alpha\nbeta\n") + acct = _write(tmp_path, "acct", "") + nd = _write(tmp_path, "nd", "") + debt = _write(tmp_path, "debt", "alpha\n") + out, rc = _call("pgc_reconcile_population", reg, acct, nd, debt) + expect.num(out.count("registered but accounted by nothing: beta"), 1, + "a NEW unaccounted suite fails while the known debt is excused") + expect.num(out.count("registered but accounted by nothing: alpha"), 0, + "and the excused one is not named as a failure") + + reg = _write(tmp_path, "reg", "alpha\n") + acct = _write(tmp_path, "acct", "alpha\n") + debt = _write(tmp_path, "debt", "alpha\n") + out, _ = _call("pgc_reconcile_population", reg, acct, nd, debt) + expect.num(out.count("listed as debt but now accounts: alpha"), 1, + "a suite that now accounts but is still listed as debt is reported") + + debt = _write(tmp_path, "debt", "gone\n") + out, _ = _call("pgc_reconcile_population", reg, acct, nd, debt) + expect.num(out.count("listed as debt but not registered: gone"), 1, + "and debt naming a suite that is not registered is reported") + + +def test_the_population_partitions_and_prints_its_identity(tmp_path, expect): + """inputs == sum(buckets) over the REGISTERED population, printed per the house rule. + + It cannot be false on the data: the buckets are built by successive subtraction + from the registered set, so their sum equals it identically -- 0 firings over 400 + random four-set inputs while the real bucket findings fired on 353. What it guards + is `comm` reading unsorted input. The arm below it, on the unaccounted bucket, is + the one that carries weight here. + """ + reg = _write(tmp_path, "reg", "a\nb\nc\nd\n") + acct = _write(tmp_path, "acct", "a\n") + nd = _write(tmp_path, "nd", "b\n") + debt = _write(tmp_path, "debt", "c\n") + out, rc = _call("pgc_reconcile_population", reg, acct, nd, debt) + expect.num(out.count("registered=4 | accounted=1, not dispatched=1, " + "known debt=1, unaccounted=1 | sum=4"), 1, + "the population partitions and prints inputs == sum(buckets)") + expect.num(rc, 1, "and the one outside every excuse still fails") + + +def test_the_debt_file_is_tracked_and_holds_only_registered_suites(expect): + """It is a tracked file so that adding a name is a diff a reviewer sees -- + which is the whole reason it is not a number in the environment.""" + debt = REPO / "test" / "suites_without_accounting.txt" + expect.text("yes" if debt.exists() else "no", "yes", "the debt file is in the tree") + names = [l.strip() for l in debt.read_text().splitlines() + if l.strip() and not l.startswith("#")] + expect.at_least(len(names), 1, "premise: it names some debt to check") + + listed = subprocess.run(["bash", str(RUNNER), "--list-suites"], + capture_output=True, text=True).stdout.split() + strays = sorted(set(names) - set(listed)) + print(f" debt={len(names)} registered={len(listed)} strays={strays}") + expect.num(len(strays), 0, "every name in the debt file is a registered suite") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 084319d3..0aa2853a 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -782,6 +782,11 @@ for pgc in "${CONFIGS[@]}"; do # else would produce one and the run would be classified a failure. echo 66 >"$builddir/${s}.rc" echo "$s.sh: SKIPPED (ran no checks)" >"$builddir/${s}.log" + # Record the decision where it is made (#916). This suite calls + # pgc_summary and will produce no accounting line, because it was + # never run; the reconciliation needs that said by the driver rather + # than guessed from the log the driver just forged. + printf '%s\n' "$s" >>"$builddir/accounting.notdispatched" continue fi port=$((BASE_PORT++)) @@ -853,6 +858,334 @@ pgc_classify_suite_rc() { # pgc_classify_suite_rc RC LOGFILE -> PASS|SKIP|INCOMP # would leave every count at zero while every arm that drives this function # still passed -- the same shape of defect as the write-only flag, and just as # invisible to a green run. +# ---- accounting membership, derived rather than listed ----------------------- +# +# The matrix prints "suites that ran: N of M" and never checks it, and twelve +# registered suites exit 0 without ever calling pgc_summary. Measured, with a +# pattern tight enough to exclude portlib.sh -- a looser one matched it and gave +# both reviewers of this change the same wrong answer: NONE of the twelve sources +# test/lib.sh. Each defines its own check(), and ten of them keep no tally at all. +# So the harness cannot see their checks. The number is not written elsewhere on +# purpose; the reconciliation prints it at runtime. Counted among the +# suites that "ran", they are the same overcount #447 added that line to stop, +# one level further down. +# +# A COUNT cannot fix this. Two errors of opposite sign cancel, and an exempt list +# maintained by hand makes the count agree by construction -- the check then +# measures the list rather than the run. So membership is derived from a property +# each suite carries, and the two readings are reconciled as SETS, in both +# directions. + +pgc_suite_declares_accounting() { # pgc_suite_declares_accounting FILE -> yes|no + # A suite participates in check accounting exactly when it calls pgc_summary, + # which is the only thing that prints the accounting line and sets the status + # pgc_classify_suite_rc reads. + # + # Comments are stripped first. A suite that explains in prose why it cannot + # account would otherwise read as one that does, which is the failure mode + # this whole approach exists to avoid: a claim satisfied by a mention. + # grep -c, NOT grep -q, and the reason is the bug this harness has already + # paid for once. `grep -q` exits the moment it matches, which closes the pipe + # while sed is still writing; sed takes EPIPE and exits non-zero, and under + # `set -o pipefail` the PIPELINE reports that failure even though grep + # matched. It is a race between the two, so it reproduces on large files and + # not small ones, and it names DIFFERENT innocent suites each run. + # + # That is not a hypothetical. Selftest 040 carries the same story from #473 + # and #476, and the first version of this function reproduced it exactly: + # analyze_function and hilbert_curve -- two of the longest suites -- read as + # not declaring accounting inside the selftest and as declaring it outside. + # + # grep -c reads to EOF, so sed never sees a closed pipe. + # An ABSENT file is its own answer, not "does not declare accounting". + # Conflating them classifies a registered suite whose .sh has vanished as + # exempt, and the reconciliation then reads clean -- a suite disappearing + # from the matrix, inside the check whose whole subject is suites that go + # missing from the accounting. Reported by OffgridwithJD reviewing #922. + local _f="$1" _n + [ -f "$_f" ] || { echo absent; return 0; } + # Strip a `#` only where the shell would treat one as starting a comment: + # at the start of a line, or after whitespace. `sed 's/#.*$//'` strips from + # ANY hash, so a `#` inside a quoted string earlier on the line would hide a + # pgc_summary call after it. Measured across all 251 registered suites: three + # carry a line with both, and in every one the `#` starts the line, so no + # suite is misread today. The arm in selftest 390 keeps that true. + _n="$(sed 's/\(^\|[[:space:]]\)#.*$/\1/' "$_f" \ + | grep -cE '(^|[^_[:alnum:]])pgc_summary([^_[:alnum:]]|$)' || true)" + if [ "${_n:-0}" -gt 0 ]; then + echo yes + else + echo no + fi +} + +pgc_log_shows_accounting() { # pgc_log_shows_accounting LOGFILE -> yes|no + # The runtime twin of the declaration above. pgc_summary prints this line on + # EVERY exit path -- pass, failure, skip and incomplete -- before it decides + # the status, so its presence says "this suite reached its summary" and not + # "this suite passed". Anchored and fully shaped, so the word appearing in a + # suite's own prose cannot satisfy it. + # Unlike the declaration reader above, this one deliberately does NOT + # distinguish an absent log from a present one carrying no accounting line. + # Both mean the same thing here -- this suite did not reach its summary -- + # and a declared suite with no log at all is exactly the catch. The asymmetry + # between the two readers is intentional and is noted because it is the kind + # of thing that reads as an oversight later. + local _log="$1" + [ -f "$_log" ] || { echo no; return 0; } + if grep -qE '^accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable \+ [0-9]+ skipped = [0-9]+$' "$_log"; then + echo yes + else + echo no + fi +} + +pgc_reconcile_records() { # pgc_reconcile_records LOGFILE -> 0 ok, 1 mismatch + # A suite's log states `checks run: N` and carries N record lines. They are + # the same increment seen twice -- pgc_record does both -- so this cannot + # fail by drifting. It CAN fail, which is why it is asserted: a suite killed + # mid-way, a truncated log, or a helper that prints an outcome without + # recording it all separate the two. + # + # A log with no `checks run:` line at all never reached its summary. That is a + # different fault from a miscount, and it must not read as a clean + # reconciliation just because there is nothing to compare against. + local _log="$1" _records _stated + if [ ! -f "$_log" ]; then + echo " no log to reconcile records against: $_log" + return 1 + fi + _records="$(grep -c '^RESULT ' "$_log" || true)" + _stated="$(sed -n 's/^checks run: \([0-9][0-9]*\)$/\1/p' "$_log" | tail -1)" + if [ -z "$_stated" ]; then + echo " records=$_records but the log never stated a count, so it did not reach its summary" + return 1 + fi + if [ "$_records" != "$_stated" ]; then + echo " records=$_records but the log states checks run: $_stated" + # NAME THE CAUSE, not just the arithmetic. The two directions have + # different causes and a reader who has not met either has no route from + # a pair of numbers to the defect. Raised by OffgridwithJD. + if [ "$_records" -gt "$_stated" ]; then + echo " $((_records - _stated)) check(s) reported an outcome the count never saw:" + echo " a check ran in a subshell, so its counter bump died with it while its" + echo " outcome and record still reached the log. The usual shape is a check" + # The example is ASSEMBLED, not written out: spelling the shape here + # made the sweep in selftest 400 flag this very line. + printf ' inside a piped loop -- `cmd %s while read x; do check ...; done`.\n' '|' + else + echo " $((_stated - _records)) check(s) were counted without emitting a record:" + echo " something bumped PGC_CHECKS without going through pgc_record." + fi + return 1 + fi + return 0 +} + +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? + # + # Two exist in the tree. lib.sh's pgc_summary prints the accounting line, and + # 239 suites use it. bench_guards and docs_style keep private counters and + # print their own `checks run: N`; they never source lib.sh, so the first + # reader cannot see them, and calling them unaccounted would be false. + # + # Both are runtime-observable and derived rather than declared, so a suite + # that adopts either mechanism leaves the debt bucket on its own -- which is + # the property that keeps the debt file from becoming a permission slip. + # A HALF-MERGE WOULD BE CONFUSING RATHER THAN LOUD, and it is worth knowing + # which way. The `checks run:` alternative below answers YES to an accounting + # line of ANY shape, so it MASKS a change to that line: if the producer ever + # moved without this file, the population reconciliation would stay green + # while pgc_log_shows_accounting broke and the accounting reconciliation + # reddened. Two checks disagreeing about the same log is a worse signal than + # either failing. + # + # It cannot happen inside one tree -- producer and both readers move in the + # same commit -- so this is a note about what to look for, not a defect. + # Raised by OffgridwithJD while verifying the four-term shape change. + local _log="$1" + [ -f "$_log" ] || { echo no; return 0; } + if [ "$(grep -cE '^accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable \+ [0-9]+ skipped = [0-9]+$' "$_log" || true)" != 0 ] \ + || [ "$(grep -cE '^checks run: [0-9]+$' "$_log" || true)" != 0 ]; then + echo yes + else + echo no + fi +} + +pgc_reconcile_population() { # pgc_reconcile_population REGISTERED ACCOUNTED NOTDISPATCHED DEBT -> 0 ok, 1 unaccounted + # THE REGISTERED SET IS AN INPUT. pgc_reconcile_accounting reconciles the + # declared set against the observed one, and both are derived from the suites + # themselves -- so a registered suite in NEITHER is outside the universe it + # reconciles. Driven from that function with all its inputs empty: it prints + # `inputs=0 | both=0 ... sum=0` and returns 0, whatever SUITES holds. + # + # Reported by @linuxhikerpm, structurally: treating absence of a declaration + # as absence from the population preserves the overcount this change is named + # for. So the population is reconciled separately, over its own four buckets, + # and every registered suite must land in exactly one: + # + # accounted its log shows it counted its checks, by either mechanism + # not dispatched the driver recorded that it never ran it + # known debt named in the tracked debt file + # unaccounted none of the above -- FAILS, by name + # + # The debt file is DEBT, not an exemption: it is tracked, so adding a name is + # a diff a reviewer sees, and a name that starts accounting or stops being + # registered is reported so the burn-down cannot stall silently. + local _reg="$1" _acct="$2" _nd="${3:-}" _debt="${4:-}" _rc=0 + local _rf _af _ndf _df _unacc _stale_acct _stale_reg _n + local _nreg _nacc _nnd _ndebt _nunacc _sum + + _rf="$(mktemp)"; _af="$(mktemp)"; _ndf="$(mktemp)"; _df="$(mktemp)" + LC_ALL=C sort -u "$_reg" 2>/dev/null | sed '/^$/d' >"$_rf" + LC_ALL=C sort -u "$_acct" 2>/dev/null | sed '/^$/d' >"$_af" + [ -n "$_nd" ] && [ -f "$_nd" ] && LC_ALL=C sort -u "$_nd" | sed '/^$/d' >"$_ndf" + [ -n "$_debt" ] && [ -f "$_debt" ] && \ + grep -vE '^[[:space:]]*(#|$)' "$_debt" | LC_ALL=C sort -u >"$_df" + + # Buckets, in precedence order, so each registered name lands in exactly one. + local _t1 _t2 + _t1="$(mktemp)"; _t2="$(mktemp)" + LC_ALL=C comm -23 "$_rf" "$_af" >"$_t1" # registered, not accounted + LC_ALL=C comm -23 "$_t1" "$_ndf" >"$_t2" # ... nor not-dispatched + _unacc="$(LC_ALL=C comm -23 "$_t2" "$_df")" # ... nor known debt + + _nreg="$(grep -c . "$_rf" || true)" + _nacc="$(LC_ALL=C comm -12 "$_rf" "$_af" | grep -c . || true)" + _nnd="$(LC_ALL=C comm -12 "$_t1" "$_ndf" | grep -c . || true)" + _ndebt="$(LC_ALL=C comm -12 "$_t2" "$_df" | grep -c . || true)" + _nunacc="$(printf '%s' "$_unacc" | grep -c . || true)" + _sum=$(( _nacc + _nnd + _ndebt + _nunacc )) + + # Debt that is no longer debt. Reported rather than fatal: a burn-down that + # reddens the gate the moment someone FIXES something teaches people not to. + _stale_acct="$(LC_ALL=C comm -12 "$_df" "$_af")" + _stale_reg="$(LC_ALL=C comm -23 "$_df" "$_rf")" + rm -f "$_rf" "$_af" "$_ndf" "$_df" "$_t1" "$_t2" + + if [ "$_nunacc" != 0 ]; then + while IFS= read -r _n; do + [ -n "$_n" ] && echo " registered but accounted by nothing: $_n" + done <<<"$_unacc" + _rc=1 + fi + while IFS= read -r _n; do + [ -n "$_n" ] && echo " listed as debt but now accounts: $_n" + done <<<"$_stale_acct" + while IFS= read -r _n; do + [ -n "$_n" ] && echo " listed as debt but not registered: $_n" + done <<<"$_stale_reg" + + # registered == sum(buckets), printed beside every reconciliation per the + # house rule. BE PRECISE ABOUT WHAT IT CAN CATCH, because the next reader will + # go looking for a data case and there is not one: the four buckets are built + # by successive subtraction FROM the registered set, so their sum equals it + # identically. OffgridwithJD measured it -- 400 random four-set inputs, zero + # firings, while the real bucket findings fired on 353 of them. + # + # What can make it false is comm being fed unsorted input, which produces + # buckets that are not a partition at all. It is a comm tripwire, exactly like + # the one on pgc_reconcile_accounting, and selftest 390 drives it there with + # that mutation. + echo " population reconciliation: registered=$_nreg | accounted=$_nacc, not dispatched=$_nnd, known debt=$_ndebt, unaccounted=$_nunacc | sum=$_sum" + if [ "$_nreg" != "$_sum" ]; then + echo " the population does not add up: $_nreg registered, $_sum in the buckets" + _rc=1 + fi + return $_rc +} + +pgc_reconcile_accounting() { # pgc_reconcile_accounting DECLARED OBSERVED [NOTDISPATCHED] -> 0 ok, 1 asymmetric + # Set equality in both directions. The two directions catch opposite + # mistakes and neither can stand in for the other: + # + # declared but never accounted the suite died before reaching its + # summary. Today rc=0 makes that a PASS. + # accounted but never declared the reading of the source is stale. + # + # comm needs both sides sorted under the same collation; selftest 070 is the + # record of what an unsorted input costs here. + # The third list is the driver's own record of suites it chose not to + # dispatch -- PGC_SKIP_TIMING drops four on every CI run. Those suites DO + # declare accounting and correctly produced none, so without this term the + # reconciliation goes red for the one reason that is not a defect. + # + # It is recorded by the branch that makes the decision, not inferred from the + # log that branch forges. Inferring it would mean trusting a marker the driver + # wrote on the suite's behalf, which is the kind of claim this check exists to + # stop. + local _decl="$1" _obs="$2" _nd="${3:-}" _rc=0 + local _dfile _ofile _ndfile _obsonly _both _donly _oonly _clash + local _nboth _ndonly _noonly _nclash _inputs _sum _n + + _dfile="$(mktemp)"; _ofile="$(mktemp)"; _ndfile="$(mktemp)"; _obsonly="$(mktemp)" + LC_ALL=C sort -u "$_decl" 2>/dev/null | sed '/^$/d' >"$_dfile" + LC_ALL=C sort -u "$_obs" 2>/dev/null | sed '/^$/d' >"$_obsonly" + if [ -n "$_nd" ] && [ -f "$_nd" ]; then + LC_ALL=C sort -u "$_nd" 2>/dev/null | sed '/^$/d' >"$_ndfile" + fi + # The observed side is what accounted PLUS what was deliberately not run. + LC_ALL=C sort -u "$_obsonly" "$_ndfile" | sed '/^$/d' >"$_ofile" + + # A suite cannot both have reached its summary and not have been dispatched. + # If it is in both lists one of the two readings is wrong, and the union above + # would hide that by absorbing it. + _clash="$(LC_ALL=C comm -12 "$_obsonly" "$_ndfile")" + _nclash="$(printf '%s' "$_clash" | grep -c . || true)" + + _both="$(LC_ALL=C comm -12 "$_dfile" "$_ofile")" + _donly="$(LC_ALL=C comm -23 "$_dfile" "$_ofile")" + _oonly="$(LC_ALL=C comm -13 "$_dfile" "$_ofile")" + + _nboth="$(printf '%s' "$_both" | grep -c . || true)" + _ndonly="$(printf '%s' "$_donly" | grep -c . || true)" + _noonly="$(printf '%s' "$_oonly" | grep -c . || true)" + + # inputs is counted from the FILES, independently of the three buckets. A + # derived total makes the identity below true for any values. + # + # BUT BE PRECISE ABOUT WHAT IT CAN CATCH, because the next reader will go + # looking for a data case and there is not one: for sets, |D u O| always + # equals |D n O| + |D \ O| + |O \ D|. OffgridwithJD measured it -- 400 random + # set pairs, zero firings. What can make it false is comm being fed unsorted + # input, which produces buckets that are not a partition at all. It is a comm + # tripwire, and selftest 390 drives it with exactly that mutation. + _inputs="$(LC_ALL=C sort -u "$_dfile" "$_ofile" | grep -c . || true)" + _sum=$(( _nboth + _ndonly + _noonly )) + rm -f "$_dfile" "$_ofile" "$_ndfile" "$_obsonly" + + if [ "$_nclash" != 0 ]; then + while IFS= read -r _n; do + [ -n "$_n" ] && echo " both accounted and recorded as never dispatched: $_n" + done <<<"$_clash" + _rc=1 + fi + + if [ "$_ndonly" != 0 ]; then + while IFS= read -r _n; do + [ -n "$_n" ] && echo " declared but never accounted: $_n" + done <<<"$_donly" + _rc=1 + fi + if [ "$_noonly" != 0 ]; then + while IFS= read -r _n; do + [ -n "$_n" ] && echo " accounted but never declared: $_n" + done <<<"$_oonly" + _rc=1 + fi + + # Printed from the data on every path, green included, per the house rule + # that any list-derived claim shows inputs == sum(buckets). + echo " accounting reconciliation: inputs=$_inputs | both=$_nboth, declared only=$_ndonly, accounted only=$_noonly | sum=$_sum" + if [ "$_inputs" != "$_sum" ]; then + echo " the reconciliation does not add up: $_inputs names across both lists, $_sum in the buckets" + _rc=1 + fi + return $_rc +} + pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE local _name="$1" _verdict="$2" _log="$3" if [ "$_verdict" = PASS ]; then @@ -912,11 +1245,25 @@ pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE # PG16 would report PG15's incomplete suites in its own summary line and # still print PASS, because verfail is per major and this count was not. suites_incomplete=0 + _rec_bad=0 for s in "${SUITES[@]}"; do _rc="$(cat "$builddir/${s}.rc" 2>/dev/null)" _verdict="$(pgc_classify_suite_rc "$_rc" "$builddir/${s}.log")" pgc_tally_suite "$s" "$_verdict" "$builddir/${s}.log" + # Only a suite that reached its summary has a count to reconcile against + # (#917). One that was never dispatched, or that does not use lib.sh's + # accounting at all, has nothing to compare and is not a mismatch. + if [ "$(pgc_log_shows_accounting "$builddir/${s}.log")" = yes ]; then + if ! pgc_reconcile_records "$builddir/${s}.log"; then + echo " in $s" + _rec_bad=$((_rec_bad + 1)) + fi + fi done + if [ "$_rec_bad" != 0 ]; then + echo " $_rec_bad suite(s) on PG$major state a check count their records do not match" + verfail=1 + fi # How many suites actually asserted something, said out loud (#447). # @@ -925,7 +1272,72 @@ pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE # report a verdict without running a check when pyarrow is absent, and the old # per-version line counted them among the passes. A count that includes suites # nobody ran is the thing this project keeps having to unlearn. + # Reconcile what the suites SAY they account for against what this run + # OBSERVED (#916). Two readings taken from different places -- the suite's own + # text, and the log it produced -- so neither can satisfy the other by + # construction. A count over SUITES could not do this: the collect loop above + # visits every registered name, so any total derived from it is an identity. + _acc_declared="$builddir/accounting.declared" + _acc_observed="$builddir/accounting.observed" + _acc_notdisp="$builddir/accounting.notdispatched" + : >"$_acc_declared" + : >"$_acc_observed" + [ -f "$_acc_notdisp" ] || : >"$_acc_notdisp" + _acc_absent=0 + for s in "${SUITES[@]}"; do + _acc_verdict="$(pgc_suite_declares_accounting "$builddir/test/${s}.sh")" + case "$_acc_verdict" in + yes) printf '%s\n' "$s" >>"$_acc_declared" ;; + no) ;; + absent) echo " registered but has no file: $s.sh" + _acc_absent=$((_acc_absent + 1)) ;; + *) + # A default arm must not quietly name a real outcome. If the + # reader grows a fourth answer, this says so instead of filing + # it under "does not declare". + echo " pgc_suite_declares_accounting answered [$_acc_verdict] for $s, which is none of yes/no/absent" + _acc_absent=$((_acc_absent + 1)) + ;; + esac + [ "$(pgc_log_shows_accounting "$builddir/${s}.log")" = yes ] \ + && printf '%s\n' "$s" >>"$_acc_observed" + done + if [ "$_acc_absent" != 0 ]; then + echo " $_acc_absent registered suite(s) on PG$major have no file, which is not a pass" + verfail=1 + fi + if ! pgc_reconcile_accounting "$_acc_declared" "$_acc_observed" "$_acc_notdisp"; then + echo " PG$major cannot account for every registered suite, which is not a pass" + verfail=1 + fi + + # THE POPULATION, which the symmetry check above cannot see (#916, reported by + # @linuxhikerpm). Its inputs are both derived from the suites, so a registered + # suite in neither is outside the universe it reconciles. Here the registered + # set IS the input, and a name accounted by nothing fails by name. + _acc_registered="$builddir/accounting.registered" + _acc_accounted="$builddir/accounting.accounted" + printf '%s\n' "${SUITES[@]}" >"$_acc_registered" + : >"$_acc_accounted" + for s in "${SUITES[@]}"; do + [ "$(pgc_log_shows_any_accounting "$builddir/${s}.log")" = yes ] \ + && printf '%s\n' "$s" >>"$_acc_accounted" + done + if ! pgc_reconcile_population "$_acc_registered" "$_acc_accounted" \ + "$_acc_notdisp" "$builddir/test/suites_without_accounting.txt"; then + echo " PG$major has a registered suite nothing accounts for, which is not a pass" + verfail=1 + fi + + # How many of the suites counted as having RUN actually accounted for their + # checks (#916). Ten registered suites exit 0 having never called pgc_summary; + # counting them among the suites that ran is the overcount #447 added this + # line to stop, one level further down, and printing the total without this + # breakdown leaves it exactly where it was. Raised by OffgridwithJD, who was + # right that the drift detector alone does not close it. + _acc_ran="$(grep -c . "$_acc_observed" 2>/dev/null || true)" echo " suites that ran: $suites_ran of ${#SUITES[@]} (skipped: $suites_skipped, incomplete: $suites_incomplete)" + echo " of those, $_acc_ran accounted for their checks and $((suites_ran - _acc_ran)) did not" if [ "$suites_skipped" != 0 ]; then echo " skipped:${skipped_names}" fi diff --git a/test/selftest/070-and-comm-s-two-inputs-must.sh b/test/selftest/070-and-comm-s-two-inputs-must.sh index 391ade65..455d525d 100644 --- a/test/selftest/070-and-comm-s-two-inputs-must.sh +++ b/test/selftest/070-and-comm-s-two-inputs-must.sh @@ -19,7 +19,11 @@ check "premise: some suite still uses comm, or the check below is vacuous" \ _cm_unpinned="" for _f in $_cm_files; do # every `| sort` in a file that uses comm must carry LC_ALL=C - if grep -qE '\|[[:space:]]*sort' "$_f" && grep -E '\|[[:space:]]*sort' "$_f" | grep -qv 'LC_ALL=C'; then + # The second test is grep -c on a captured value, not a pipe into grep -qv; + # see selftest 080. The first reads a FILE and is not a pipeline at all. + _cm_sorts="$(grep -E '\|[[:space:]]*sort' "$_f" || true)" + if [ "$(grep -cE '\|[[:space:]]*sort' "$_f" || true)" != 0 ] \ + && [ "$(grep -cv 'LC_ALL=C' <<<"$_cm_sorts" || true)" != 0 ]; then _cm_unpinned="$_cm_unpinned $(basename "$_f")" fi done diff --git a/test/selftest/080-no-suite-pipes-a-captured-string.sh b/test/selftest/080-no-suite-pipes-a-captured-string.sh index 2db26fef..e19fedbd 100644 --- a/test/selftest/080-no-suite-pipes-a-captured-string.sh +++ b/test/selftest/080-no-suite-pipes-a-captured-string.sh @@ -68,10 +68,103 @@ check "control: piping a large string into grep -q reports a match as absent" \ # explanation of the rule, and the deliberate piped() demo above, both match the # pattern they exist to describe. Sweeping the file that enforces a rule for # instances of that rule is selftest 260's mistake once removed. +# test/selftest/ IS SCANNED TOO, AND IT WAS NOT, for the same reason bench/ was +# not: the directory scoping was never a decision, it fell out of writing +# "$TESTDIR"/*.sh. Three fragments held the exact shape this rule forbids -- +# 350's corpus membership test, 300's directory-coverage test, 340's Makefile +# sweep -- inside the directory that enforces the rule. +# +# NOT LATENT. 350's reddened #923's `suites (PG 17)` on a pytest name that +# exists, while PG 18 passed the same commit. At corpus size the writer is small +# enough to win the race on an idle machine, which is why it survived: measured +# at 170 names over 400 trials, 0 false absences idle and 6 under load, against 0 +# either way for the here-string form. +# +# The exemption is DERIVED, not listed. This file's own control below is inside a +# quoted heredoc, and so is any other deliberate demonstration of the shape; a +# line inside one is text being written to a file, not a pipeline this suite +# runs. A filename allowlist would have to be maintained, and this rule exists +# because things that must be maintained are not. _epipe_globs=("$TESTDIR"/*.sh) [ -d "$TESTDIR/../bench" ] && _epipe_globs+=("$TESTDIR"/../bench/*.sh) -_epipe_hits="$(grep -nE '(echo|printf)[^|]*\|[[:space:]]*grep -[a-zA-Z]*q' \ - "${_epipe_globs[@]}" 2>/dev/null | grep -v '/harness_selftest.sh:' || true)" +[ -d "$TESTDIR/selftest" ] && _epipe_globs+=("$TESTDIR"/selftest/*.sh) + +# file:line pairs that sit inside a quoted heredoc, computed from the files. +_epipe_heredoc_lines() { + awk ' + # Reset per FILE. awk keeps globals across inputs, so an unterminated + # heredoc in one file leaves the scanner inside one for every file after + # it -- and a later line that happens to equal the stale tag closes it in + # the wrong place. Measured: without this, 080s own line 26 fell OUTSIDE + # the exemption when the sweep ran over all 302 files, and inside it when + # the same function ran over that file alone. + FNR == 1 { inhd = 0; tag = "" } + !inhd && match($0, /<<[-]?'"'"'[A-Za-z_][A-Za-z0-9_]*'"'"'/) { + tag = substr($0, RSTART, RLENGTH) + gsub(/^<<[-]?'"'"'/, "", tag); gsub(/'"'"'$/, "", tag) + inhd = 1; next + } + inhd { + t = $0; gsub(/^[ \t]+|[ \t]+$/, "", t) + if (t == tag) { inhd = 0; next } + # FNR, not NR. NR is cumulative across inputs, so the second file + # onwards reports line numbers from a running total and no key ever + # matches the grep -n output it is compared against. It is the + # neighbouring question answered plausibly: single-file runs agree, + # because there NR == FNR. + print FILENAME ":" FNR ":" + } + ' "$@" +} +_epipe_hd="$(_epipe_heredoc_lines "${_epipe_globs[@]}" 2>/dev/null || true)" +# Comment lines are excluded first. A comment is not a pipeline this suite runs, +# and the rule's own explanation -- and the note beside every site that was fixed +# -- necessarily spells the shape out. A sweep that counts its own documentation +# is selftest 260's mistake. +# ANY PRODUCER, not just echo and printf. This reverses a scoping decision +# recorded above, so the reason is recorded too. +# +# The rule's own stated principle is a reader that exits early AND whose EXIT +# STATUS is the answer being read. That is producer-independent: the writer takes +# EPIPE whether it is a builtin, a psql, an ldd or an ss. Scoping to echo and +# printf was a narrower implementation than the principle, justified by "a +# pipeline out of psql or a file is a different question" -- which is true of +# `| head -1` used as TEXT, and false of `| grep -q` used as a VERDICT. +# +# Twenty-six sites were outside the old pattern, two of them in lib.sh and shared +# by every suite that asks whether a plan is a columnar scan. The worst was +# native_vecskip.sh's "premise: and it is not the scalar scan", which WANTS "no": +# a spurious EPIPE answer makes that premise pass for the wrong reason, so the +# race is vacuity there rather than a false red. +# +# NOT CLAIMED TO BE LYING TODAY. Measured by OffgridwithJD, 200 trials per size on +# a loaded box, match always on line one so the answer is knowably yes: +# +# 1,892 bytes (EXPLAIN-sized) 0/200 wrong +# 8,893 bytes 1/200 <- first observed lie +# 66,894 bytes 21/40 +# 288,894 bytes 40/40 +# control, match on the LAST line so grep reads to EOF: 0/200 at every size +# +# It is LATENT, and the mechanism has no floor -- the probability rises with size +# rather than crossing a threshold, which refuted a clean pipe-capacity +# hypothesis. Reasoning about "small enough" is how #473, #476 and selftest 350 +# each survived, so the rule sweeps instead. +# The leading [^|] excludes the `||` OPERATOR. Widening from the echo/printf +# form lost that exclusion for free: `[ "$rc" = 124 ] || grep -q PAT <<<"$out"` +# is a fallback branch reading a here-string -- no writer process, so no EPIPE -- +# and the first version of the widened pattern flagged both fuzz suites for it. +_epipe_hits="$(grep -nE '[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' \ + "${_epipe_globs[@]}" 2>/dev/null \ + | grep -v '/harness_selftest.sh:' \ + | grep -vE '^[^:]+:[0-9]+:[[:space:]]*#' || true)" +# Drop hits whose file:line is inside a quoted heredoc. +if [ -n "$_epipe_hd" ] && [ -n "$_epipe_hits" ]; then + _epipe_hits="$(printf '%s\n' "$_epipe_hits" | while IFS= read -r _eh; do + _ehk="${_eh%%:*}:$(printf '%s' "$_eh" | cut -d: -f2):" + [ "$(grep -cxF "$_ehk" <<<"$_epipe_hd" || true)" != 0 ] || printf '%s\n' "$_eh" + done)" +fi _epipe_count="$(printf '%s' "$_epipe_hits" | grep -c . || true)" [ -n "$_epipe_hits" ] || _epipe_count=0 check "no suite pipes a captured string into an early-exit reader" \ @@ -118,3 +211,67 @@ _epipe_bench="$(printf '%s' "$_epipe_files" | grep -c '/bench/' || true)" check "and bench/ was in the scan, which is the hole this rule had" \ "$([ "${_epipe_bench:-0}" -ge 1 ] && echo yes || echo "no (bench files read: $_epipe_bench)")" "yes" +# selftest/ gets its own arm for exactly the reason bench/ does: its glob is +# added conditionally, and the file-count premise above passes with it silently +# absent. +_epipe_self="$(printf '%s' "$_epipe_files" | grep -c '/selftest/' || true)" +check "and selftest/ was in the scan, which is the hole that reddened #923" \ + "$([ "${_epipe_self:-0}" -ge 5 ] && echo yes || echo "no (selftest files read: $_epipe_self)")" "yes" + +# The heredoc exemption must EXEMPT something and must not exempt everything. +# Without the first, the control below is unreachable and this file cannot hold +# its own rule; without the second, the sweep is switched off and reports zero. +_epipe_hd_count="$(printf '%s' "$_epipe_hd" | grep -c . || true)" +check "premise: the heredoc exemption found heredoc lines to exempt" \ + "$([ "${_epipe_hd_count:-0}" -ge 1 ] && echo yes || echo "no ($_epipe_hd_count)")" "yes" +# A PROPORTION, not a guessed ceiling. The first version of this arm used a bare +# 2000 and went red at 2,502 heredoc lines in a corpus that was entirely healthy +# -- a hand-written number failing for the reason hand-written numbers fail here. +_epipe_total_lines="$(cat "${_epipe_globs[@]}" 2>/dev/null | grep -c '' || true)" +echo " epipe sweep: total lines=$_epipe_total_lines, inside a quoted heredoc=$_epipe_hd_count" +check "premise: the sweep read lines to classify" \ + "$([ "${_epipe_total_lines:-0}" -ge 1000 ] && echo yes || echo "no ($_epipe_total_lines)")" "yes" +check "premise: and the exemption covers a minority of them, not the corpus" \ + "$([ "$((_epipe_hd_count * 2))" -lt "${_epipe_total_lines:-0}" ] && echo yes \ + || echo "no ($_epipe_hd_count of $_epipe_total_lines)")" "yes" + +# The rule must still SEE a violation that is not in a heredoc. A sweep whose +# exemption is too broad reports zero for the same reason a correct one does. +_epipe_probe="$PGC_WORKDIR/epipe_probe.sh" +# The shape is assembled from a fragment rather than written out, so these +# generator lines do not themselves match the sweep and need no exemption. +_epipe_shape='x() { printf "%s" "$1" | grep -%s PATTERN; }' +{ + printf '%s\n' "$(printf "$_epipe_shape" '%s' q)" + printf 'cat > /dev/null <<%sDEMO%s\n' "'" "'" + printf '%s\n' "$(printf "$_epipe_shape" '%s' q)" + printf 'DEMO\n' +} > "$_epipe_probe" +_epipe_probe_hd="$(_epipe_heredoc_lines "$_epipe_probe" 2>/dev/null || true)" +check "the sweep's pattern sees both lines of the probe" \ + "$(grep -cE '[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' "$_epipe_probe")" "2" +check "and the heredoc exemption covers the one inside the heredoc, not the other" \ + "$(printf '%s' "$_epipe_probe_hd" | grep -c ':3:' || true)" "1" +check "and does not cover the live one above it" \ + "$(printf '%s' "$_epipe_probe_hd" | grep -c ':1:' || true)" "0" + +# The widening itself, asserted. A producer that is neither echo nor printf must +# be caught, or the reversal above is prose and the 26 sites come back. +_epipe_wide="$PGC_WORKDIR/epipe_wide.sh" +printf 'ldd /bin/sh | grep -%s libc && echo yes || echo no\n' q > "$_epipe_wide" +check "the sweep catches a producer that is neither echo nor printf" \ + "$(grep -cE '[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' "$_epipe_wide")" "1" +check "premise: and the old echo/printf pattern did NOT catch it" \ + "$(grep -cE '(echo|printf)[^|]*\|[[:space:]]*grep -[a-zA-Z]*q' "$_epipe_wide")" "0" + +# `||` IS NOT A PIPE. A fallback branch reading a here-string has no writer +# process and cannot take EPIPE. The first version of the widened pattern flagged +# both fuzz suites for exactly that, so it is pinned here rather than left to be +# rediscovered the next time the pattern is touched. +_epipe_oror="$PGC_WORKDIR/epipe_oror.sh" +printf '[ "$rc" = 124 ] || grep -%s PAT <<<"$out"\n' q > "$_epipe_oror" +check "the sweep does not mistake the || operator for a pipe" \ + "$(grep -cE '[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' "$_epipe_oror")" "0" +check "premise: and the line really does hold the reader it must not flag" \ + "$(grep -c 'grep -q PAT' "$_epipe_oror")" "1" + diff --git a/test/selftest/300-a-test-script-must-be-runnable.sh b/test/selftest/300-a-test-script-must-be-runnable.sh index d8e6e438..c68d6835 100644 --- a/test/selftest/300-a-test-script-must-be-runnable.sh +++ b/test/selftest/300-a-test-script-must-be-runnable.sh @@ -63,7 +63,8 @@ _tsm_root="$(cd "$PGC_TESTDIR/.." && pwd)" _tsm_has_shebang() { # _tsm_has_shebang FILE - head -1 "$1" 2>/dev/null | grep -q '^#!' + # grep -c on a captured line, not a pipe into grep -q; see selftest 080. + [ "$(head -1 "$1" 2>/dev/null | grep -c '^#!' || true)" != 0 ] } # ok -- shebang and bit, or neither: internally consistent @@ -236,7 +237,9 @@ check_num "premise: the documents name a population of commands, not none" \ _tsm_uncovered="" for _tsm_d in "${_tsm_dirs[@]}"; do _tsm_b="$(basename "$_tsm_d")" - printf '%s\n' "$_tsm_named" | grep -q "^$_tsm_b/" \ + # grep -c on a here-string, not `printf | grep -q`; see selftest 080 and the + # note in 350. The piped form reports a present name as absent under load. + [ "$(grep -c "^$_tsm_b/" <<<"$_tsm_named" || true)" != 0 ] \ || _tsm_uncovered="$_tsm_uncovered $_tsm_b" done check "premise: and they name at least one command in every swept directory" \ diff --git a/test/selftest/320-a-check-that-could-not-run.sh b/test/selftest/320-a-check-that-could-not-run.sh index 82b62f75..ee6f23ce 100644 --- a/test/selftest/320-a-check-that-could-not-run.sh +++ b/test/selftest/320-a-check-that-could-not-run.sh @@ -87,8 +87,8 @@ check "one unrunnable check makes the suite INCOMPLETE, not passed" \ # the feature from its absence. Assert the accounting line instead, which only a # suite that recorded BOTH states can print. check "a failure outranks an unrunnable check, and both are still counted" \ - "$(_cur_out failunrun | grep -oE 'accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable')" \ - "accounting: 0 passed + 1 failed + 1 unrunnable" + "$(_cur_out failunrun | grep -oE 'accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable \+ [0-9]+ skipped')" \ + "accounting: 0 passed + 1 failed + 1 unrunnable + 0 skipped" # The distinction 66 cannot carry: this suite RAN a check. Reporting it as # "ran no checks" would merge "inert suite" with "could not evaluate one thing". @@ -125,12 +125,12 @@ check "an unrunnable reason outside the enum fails rather than being accepted" \ # Every state is in a total, or it is a state that can go missing. 3,762 check # sites is well past what anyone notices by reading. check "the summary reconciles the three states against the total" \ - "$(_cur_out passunrun | grep -oE 'accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable = [0-9]+')" \ - "accounting: 1 passed + 0 failed + 1 unrunnable = 2" + "$(_cur_out passunrun | grep -oE 'accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable \+ [0-9]+ skipped = [0-9]+')" \ + "accounting: 1 passed + 0 failed + 1 unrunnable + 0 skipped = 2" check "and a suite with no unrunnable checks reconciles too" \ - "$(_cur_out onlypass | grep -oE 'accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable = [0-9]+')" \ - "accounting: 1 passed + 0 failed + 0 unrunnable = 1" + "$(_cur_out onlypass | grep -oE 'accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable \+ [0-9]+ skipped = [0-9]+')" \ + "accounting: 1 passed + 0 failed + 0 unrunnable + 0 skipped = 1" # ---- the accounting must be a MEASUREMENT, not an identity ------------------ # @@ -155,8 +155,8 @@ _cur_make ratio 'check "a" ok ok check_ratio "a ratio well inside its bound" 10 100 1.0' check "a passing ratio check is counted as a pass, not a failure" \ - "$(_cur_out ratio | grep -oE 'accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable = [0-9]+')" \ - "accounting: 2 passed + 0 failed + 0 unrunnable = 2" + "$(_cur_out ratio | grep -oE 'accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable \+ [0-9]+ skipped = [0-9]+')" \ + "accounting: 2 passed + 0 failed + 0 unrunnable + 0 skipped = 2" check "and the suite that holds it still passes" \ "$(_cur_run ratio)" "0 PASSED" @@ -201,8 +201,15 @@ while IFS= read -r _cnt_l; do done < <(grep -rn 'PGC_CHECKS=\$((PGC_CHECKS' "$_cnt_dir"/*.sh "$_cnt_dir"/selftest/*.sh 2>/dev/null \ | grep -v '/lib\.sh:' | sort) -check "premise: the sweep finds the direct writes it is meant to police" \ - "$([ "${#_cnt_sites[@]}" -ge 5 ] && echo enough || echo "${#_cnt_sites[@]}")" "enough" +# The premise used to require FIVE direct writes to exist, which was a premise +# about the corpus rather than about the sweep -- and #917 converted the thirteen +# that used lib.sh's accounting, so it went red for the reason the change is FOR. +# A premise that fails when the thing it guards is fixed is the wrong premise. +# +# It now asserts the sweep read something, and the classifier is proven on a +# FIXTURE below rather than on whatever the corpus happens to contain. +check "premise: the sweep read the corpus and found sites to classify" \ + "$([ "${#_cnt_sites[@]}" -ge 1 ] && echo yes || echo "no (${#_cnt_sites[@]})")" "yes" # A file that keeps its OWN counters and never calls pgc_summary is not bound by # this invariant, because nothing reconciles it. Asserted rather than assumed: @@ -212,8 +219,9 @@ for _cnt_l in "${_cnt_sites[@]}"; do _cnt_f="${_cnt_l%%:*}" _cnt_ln="$(printf '%s' "$_cnt_l" | cut -d: -f2)" grep -q 'pgc_summary' "$_cnt_f" || continue - if ! sed -n "$((_cnt_ln > 3 ? _cnt_ln - 3 : 1)),$((_cnt_ln + 6))p" "$_cnt_f" \ - | grep -qE 'PGC_PASSED=|PGC_FAILED=|PGC_UNRUN='; then + # grep -c on a captured window, not a pipe into grep -q; see selftest 080. + _cnt_win="$(sed -n "$((_cnt_ln > 3 ? _cnt_ln - 3 : 1)),$((_cnt_ln + 6))p" "$_cnt_f")" + if [ "$(grep -cE 'PGC_PASSED=|PGC_FAILED=|PGC_UNRUN=' <<<"$_cnt_win" || true)" = 0 ]; then _cnt_n=$((_cnt_n + 1)) [ "$_cnt_n" -le 5 ] && _cnt_bad="$_cnt_bad ${_cnt_f##*/}:$_cnt_ln" fi @@ -222,6 +230,83 @@ done check "every direct write to PGC_CHECKS records an outcome too" \ "$(_tsm_fmt_cnt "$_cnt_n" "$_cnt_bad")" "[]" +# AND, SINCE #917, THERE MUST BE NONE AT ALL among files that use lib.sh's +# accounting. That is a stronger rule than the one above and it was not +# satisfiable before: counting a check and RECORDING it are now one operation, +# pgc_record, so a direct write to PGC_CHECKS counts a check that emits no +# RESULT line -- and pgc_reconcile_records then reports a records/checks mismatch +# on top of whatever the suite was actually failing for. +# +# The two rules disagreed until the thirteen sites were converted, which +# OffgridwithJD found: selftest 320 blessed a direct write with a nearby outcome, +# while the reconciler required a record. pgc_pass and pgc_fail exist precisely so +# a suite-local helper need not touch the counters, and lib.sh's own header +# already calls them "the only supported way to add a check from outside this +# file". This arm makes that a rule rather than a description. +# +# A file that keeps its OWN counters and never calls pgc_summary is exempt for the +# same reason as above, measured from the file rather than named: bench_guards +# reuses the variable name privately and never sources lib.sh. +_cnt_lib=0; _cnt_lib_bad="" +for _cnt_l in "${_cnt_sites[@]}"; do + _cnt_f="${_cnt_l%%:*}" + _cnt_ln="$(printf '%s' "$_cnt_l" | cut -d: -f2)" + [ "$(grep -c 'pgc_summary' "$_cnt_f" || true)" != 0 ] || continue + _cnt_lib=$((_cnt_lib + 1)) + [ "$_cnt_lib" -le 5 ] && _cnt_lib_bad="$_cnt_lib_bad ${_cnt_f##*/}:$_cnt_ln" +done +check "no suite that uses lib.sh's accounting writes PGC_CHECKS directly" \ + "$(_tsm_fmt_cnt "$_cnt_lib" "$_cnt_lib_bad")" "[]" + +# BOTH RULES, PROVEN ON A FIXTURE. With the corpus clean, two arms reading "[]" +# are satisfied by a sweep that classifies nothing, so the classifier is driven +# against files built to trip it. +_cnt_fx="$PGC_WORKDIR/cntfx"; mkdir -p "$_cnt_fx" + +# Uses lib.sh's accounting, bumps the counter, records no outcome nearby: both +# rules must see it. +# The bump is ASSEMBLED from the variable name rather than written out, so these +# generator lines do not themselves carry the shape the sweep looks for. Writing +# it literally made this file flag its own three fixtures -- the same mistake +# selftest 080's control avoids by living in a quoted heredoc. +_cnt_bump="$(printf '%s=$((%s + 1))' PGC_CHECKS PGC_CHECKS)" +_cnt_out="$(printf '%s=$((%s + 1))' PGC_FAILED PGC_FAILED)" +printf 'pgc_summary\n%s\necho hi\n' "$_cnt_bump" > "$_cnt_fx/bare.sh" +# Uses lib.sh's accounting, bumps the counter, DOES record an outcome: the old +# rule lets it through, the new one does not. That difference is the change. +printf 'pgc_summary\n%s\n%s\n' "$_cnt_bump" "$_cnt_out" > "$_cnt_fx/outcome.sh" +# Keeps its own counter and never calls pgc_summary: exempt from both. +printf '%s\necho "checks run: 1"\n' "$_cnt_bump" > "$_cnt_fx/private.sh" + +_cnt_fx_old() { # the ORIGINAL rule, applied to one file + local _f="$1" _l + [ "$(grep -c 'pgc_summary' "$_f" || true)" != 0 ] || { echo exempt; return; } + _l="$(grep -n 'PGC_CHECKS=\$((PGC_CHECKS' "$_f" | head -1 | cut -d: -f1)" + [ -n "$_l" ] || { echo none; return; } + if [ "$(sed -n "$((_l > 3 ? _l - 3 : 1)),$((_l + 6))p" "$_f" \ + | grep -cE 'PGC_PASSED=|PGC_FAILED=|PGC_UNRUN=' || true)" = 0 ]; then + echo flagged + else + echo allowed + fi +} +_cnt_fx_new() { # the STRONGER rule, applied to one file + local _f="$1" + [ "$(grep -c 'pgc_summary' "$_f" || true)" != 0 ] || { echo exempt; return; } + [ "$(grep -c 'PGC_CHECKS=\$((PGC_CHECKS' "$_f" || true)" != 0 ] && echo flagged || echo none +} + +check "premise: the fixtures carry the shapes these rules are about" \ + "$(grep -lc 'PGC_CHECKS=\$((PGC_CHECKS' "$_cnt_fx"/*.sh 2>/dev/null | grep -c . || true)" "3" +check "the original rule flags a bump that records no outcome" \ + "$(_cnt_fx_old "$_cnt_fx/bare.sh")" "flagged" +check "and allows one that does, which is what it was written to allow" \ + "$(_cnt_fx_old "$_cnt_fx/outcome.sh")" "allowed" +check "the stronger rule flags that same allowed bump, which is the change" \ + "$(_cnt_fx_new "$_cnt_fx/outcome.sh")" "flagged" +check "and both exempt a file that keeps its own counter without lib.sh" \ + "$(_cnt_fx_old "$_cnt_fx/private.sh")/$(_cnt_fx_new "$_cnt_fx/private.sh")" "exempt/exempt" + # ---- and the RUNNER must not report an INCOMPLETE suite as a pass ------------ # # lib.sh exiting 67 is only half the state. The runner decides what a status diff --git a/test/selftest/330-the-incomplete-path-must-run-whole.sh b/test/selftest/330-the-incomplete-path-must-run-whole.sh index ea51103f..6e2075c1 100644 --- a/test/selftest/330-the-incomplete-path-must-run-whole.sh +++ b/test/selftest/330-the-incomplete-path-must-run-whole.sh @@ -92,26 +92,40 @@ check "control fixture: a suite whose checks all ran exits 0" \ _e2e_txt_classify="$(sed -n '/^pgc_classify_suite_rc()/,/^}/p' "$_e2e_rv")" _e2e_txt_fails="$(sed -n '/^pgc_verdict_fails_major()/,/^}/p' "$_e2e_rv")" _e2e_txt_tally="$(sed -n '/^pgc_tally_suite()/,/^}/p' "$_e2e_rv")" +# TWO MORE, because the collect loop grew two callers and this part did not follow. +# +# #917 added a records-versus-count reconciliation inside the loop, guarded by +# pgc_log_shows_accounting. Neither was evalled here, and the guard is a COMMAND +# SUBSTITUTION: an undefined function yields "", the branch never runs, and the +# first absence hides the second. A real run printed `command not found` three +# times and still reported PASSED, 608 checks, 0 failures. +# +# So the part claiming to run "the runner's OWN collect loop" was running a loop +# whose reconciliation could not execute. Reported by OffgridwithJD. +_e2e_txt_anyacct="$(sed -n '/^pgc_log_shows_accounting()/,/^}/p' "$_e2e_rv")" +_e2e_txt_records="$(sed -n '/^pgc_reconcile_records()/,/^}/p' "$_e2e_rv")" -check "premise: all three runner functions were extracted, not empty ranges" \ - "$([ -n "$_e2e_txt_classify" ] && echo y || echo n)$([ -n "$_e2e_txt_fails" ] && echo y || echo n)$([ -n "$_e2e_txt_tally" ] && echo y || echo n)" \ - "yyy" +check "premise: all five runner functions were extracted, not empty ranges" \ + "$([ -n "$_e2e_txt_classify" ] && echo y || echo n)$([ -n "$_e2e_txt_fails" ] && echo y || echo n)$([ -n "$_e2e_txt_tally" ] && echo y || echo n)$([ -n "$_e2e_txt_anyacct" ] && echo y || echo n)$([ -n "$_e2e_txt_records" ] && echo y || echo n)" \ + "yyyyy" # A truncated extraction evals to a syntax error, not to nothing, so the closing # brace is asserted rather than assumed: the sed range stops at the first line # beginning with `}`, and a body containing one would yield a fragment. check "premise: and each extraction ends at its own closing brace" \ - "$(printf '%s\n%s\n%s\n' "$_e2e_txt_classify" "$_e2e_txt_fails" "$_e2e_txt_tally" | grep -c '^}$')" \ - "3" + "$(printf '%s\n%s\n%s\n%s\n%s\n' "$_e2e_txt_classify" "$_e2e_txt_fails" "$_e2e_txt_tally" \ + "$_e2e_txt_anyacct" "$_e2e_txt_records" | grep -c '^}$')" \ + "5" eval "$_e2e_txt_classify" eval "$_e2e_txt_fails" eval "$_e2e_txt_tally" +eval "$_e2e_txt_anyacct" +eval "$_e2e_txt_records" -check "premise: and all three are callable" \ - "$(type -t pgc_classify_suite_rc)/$(type -t pgc_verdict_fails_major)/$(type -t pgc_tally_suite)" \ - "function/function/function" - +check "premise: and all five are callable" \ + "$(type -t pgc_classify_suite_rc)/$(type -t pgc_verdict_fails_major)/$(type -t pgc_tally_suite)/$(type -t pgc_log_shows_accounting)/$(type -t pgc_reconcile_records)" \ + "function/function/function/function/function" check "the runner classifies the file that suite actually produced" \ "$(pgc_classify_suite_rc "$(cat "$_e2e_dir/e2e_incomplete.rc")" "$_e2e_dir/e2e_incomplete.log")" \ "INCOMPLETE" @@ -165,6 +179,22 @@ check "and reprints the suite's own UNRUN line beneath it" \ _e2e_txt_loop="$(awk '/^\tsuites_incomplete=/{f=1} f{print} f&&/^\tdone$/{exit}' "$_e2e_rv")" +# AND NOTHING THE LOOP CALLS MAY BE MISSING. The premise above names five because +# five is what the loop calls today; a sixth caller added later would reproduce +# exactly the failure this fixes, silently. So the loop's own text is swept for +# the functions it invokes, and every one must be defined here. +_e2e_loop_calls="$(printf '%s\n' "$_e2e_txt_loop" | grep -oE '(^|[^_[:alnum:]])pgc_[a-z_]+' \ + | grep -oE 'pgc_[a-z_]+' | sort -u)" +_e2e_undef=0 +while IFS= read -r _e2e_fn; do + [ -n "$_e2e_fn" ] || continue + [ "$(type -t "$_e2e_fn" 2>/dev/null)" = function ] || { + _e2e_undef=$((_e2e_undef + 1)); echo " the collect loop calls $_e2e_fn, which this part never evalled"; } +done <<<"$_e2e_loop_calls" +check "premise: the sweep found the loop's callers to check" \ + "$([ "$(printf '%s' "$_e2e_loop_calls" | grep -c .)" -ge 3 ] && echo yes || echo no)" "yes" +check "every function the collect loop calls is defined here" "$_e2e_undef" "0" + check "premise: the runner's collect loop was extracted, not an empty range" \ "$([ -n "$_e2e_txt_loop" ] && echo yes || echo no)" "yes" diff --git a/test/selftest/340-the-binary-must-be-built-from.sh b/test/selftest/340-the-binary-must-be-built-from.sh index ad34dd2a..584a01ca 100644 --- a/test/selftest/340-the-binary-must-be-built-from.sh +++ b/test/selftest/340-the-binary-must-be-built-from.sh @@ -112,7 +112,8 @@ while IFS= read -r _bd_var; do _bd_name="${_bd_val##*/}" [ -n "$_bd_name" ] || continue _bd_seen=$(( _bd_seen + 1 )) - printf '%s\n' "$_bd_covered" | grep -qx "$_bd_name" || \ + # grep -c on a here-string, not `printf | grep -qx`; see selftest 080. + [ "$(grep -cx "$_bd_name" <<<"$_bd_covered" || true)" != 0 ] || \ _bd_missing="$_bd_missing $_bd_name" done < "$_lc/tree/pgcolumnar.control" _lc_have="" for _lc_l in C C.utf8 en_US.utf8; do - locale -a 2>/dev/null | grep -qx "$_lc_l" && _lc_have="$_lc_have $_lc_l" + # grep -c, not grep -q; see selftest 080. locale -a lists hundreds of names. + [ "$(locale -a 2>/dev/null | grep -cx "$_lc_l" || true)" != 0 ] \ + && _lc_have="$_lc_have $_lc_l" done _lc_count="$(printf '%s\n' $_lc_have | grep -c .)" diff --git a/test/selftest/350-the-pytest-corpus-must-be.sh b/test/selftest/350-the-pytest-corpus-must-be.sh index 6835fe29..088f1bb5 100644 --- a/test/selftest/350-the-pytest-corpus-must-be.sh +++ b/test/selftest/350-the-pytest-corpus-must-be.sh @@ -196,7 +196,18 @@ _dcv_absent() { # _dcv_absent DIR DOC -> "[]" or "[n: a b c]" } | sort -u )" while IFS= read -r name; do [ -n "$name" ] || continue - printf '%s\n' "$ondisk" | grep -qxF "$name" && continue + # grep -cxF on a here-string, NOT `printf ... | grep -qxF`. grep -q exits + # the moment it matches, the printf takes EPIPE, and under this suite's + # `set -o pipefail` the pipeline reports failure though the name WAS + # present -- so a name in the corpus is reported absent. Selftest 080 + # states this rule and demonstrates it; its sweep is deliberately + # non-recursive and never looked in here. + # + # It is not latent. It reddened #923's `suites (PG 17)` on a name that + # exists, while PG 18 passed, and reproduces at this corpus size only + # under load: 170 names, 400 trials on a busy machine, 6 false absences + # piped and 0 on a here-string. + [ "$(grep -cxF "$name" <<<"$ondisk" || true)" != 0 ] && continue n=$((n + 1)); [ "$n" -le 6 ] && bad="$bad $name" done < <(grep -oE '`test_[A-Za-z0-9_]*(\.py)?`' "$doc" 2>/dev/null \ | tr -d '`' | sort -u) diff --git a/test/selftest/390-a-registered-suite-must-account.sh b/test/selftest/390-a-registered-suite-must-account.sh new file mode 100644 index 00000000..90f35a9b --- /dev/null +++ b/test/selftest/390-a-registered-suite-must-account.sh @@ -0,0 +1,625 @@ +# ---- a registered suite must account for its checks, or say it cannot ------- +# +# The matrix prints "suites that ran: N of M" and never checks it. Two different +# things hide behind that line. +# +# The first is arithmetic nobody does. ran + skipped + incomplete is printed +# beside M and never compared with it, so a suite whose verdict the tally loop +# drops leaves the sum short and the line still reads plausibly. +# +# The second is worse, because it is live today. pgc_classify_suite_rc maps rc=0 +# to PASS with no further question, and twelve registered suites exit 0 without +# ever calling pgc_summary. Measured, with a pattern tight enough to exclude +# portlib.sh -- a looser one matched it and gave both reviewers of this change the +# same wrong answer: NONE of the twelve sources test/lib.sh. Each defines its own +# check(), and ten of them keep no tally at all. So the harness cannot see their +# checks, and nothing says so. They are counted among the suites that "ran", which +# is the exact overcount #447 added that line to stop, one level further down. +# +# The number is deliberately not repeated in prose elsewhere. The reconciliation +# prints it at runtime, and a count in prose is the thing this repository keeps +# having to unlearn: nine collisions on one written number in a single day. +# +# So the fix is NOT a number. A count cannot do this job: two errors of opposite +# sign cancel, and an exempt list maintained by hand makes the count agree by +# construction -- the check then measures the list, not the run. +# +# Instead, derive MEMBERSHIP from a property each suite carries, and assert set +# equality in BOTH directions: +# +# declared the suite's own text calls pgc_summary +# observed the suite's log carries the "accounting:" line pgc_summary prints +# on every exit path, before it decides the status +# +# Neither is a number and neither is hand-maintained. A suite that stops calling +# pgc_summary moves between the sets on its own, and the two directions catch +# opposite mistakes: declared-but-not-observed is a suite that died before it +# could account, and observed-but-not-declared is a stale reading of the source. +# +# The functions are EVALLED OUT OF run_all_versions.sh, per selftest 320: a check +# that restates the rule tests the world instead of the code. +# --------------------------------------------------------------------------- + +_rv="$PGC_TESTDIR/run_all_versions.sh" + +check "premise: the runner defines the declaration reader this part evals" \ + "$(grep -c '^pgc_suite_declares_accounting()' "$_rv")" "1" +check "premise: the runner defines the observation reader this part evals" \ + "$(grep -c '^pgc_log_shows_accounting()' "$_rv")" "1" +check "premise: the runner defines the reconciliation this part evals" \ + "$(grep -c '^pgc_reconcile_accounting()' "$_rv")" "1" + +eval "$(sed -n '/^pgc_suite_declares_accounting()/,/^}/p' "$_rv")" +eval "$(sed -n '/^pgc_log_shows_accounting()/,/^}/p' "$_rv")" +eval "$(sed -n '/^pgc_reconcile_accounting()/,/^}/p' "$_rv")" +check "premise: the declaration reader evalled out of the runner is callable" \ + "$(type -t pgc_suite_declares_accounting)" "function" +check "premise: the observation reader evalled out of the runner is callable" \ + "$(type -t pgc_log_shows_accounting)" "function" +check "premise: the reconciliation evalled out of the runner is callable" \ + "$(type -t pgc_reconcile_accounting)" "function" + +_acc="$PGC_WORKDIR/acc"; mkdir -p "$_acc" + +# ---- the declaration reader ------------------------------------------------ + +printf '. "$(dirname "$0")/lib.sh"\ncheck "x" a a\npgc_summary\n' > "$_acc/declares.sh" +check "a suite that calls pgc_summary declares accounting" \ + "$(pgc_suite_declares_accounting "$_acc/declares.sh")" "yes" + +printf '. "$(dirname "$0")/lib.sh"\necho hi\nexit 0\n' > "$_acc/silent.sh" +check "a suite that never calls it does not" \ + "$(pgc_suite_declares_accounting "$_acc/silent.sh")" "no" + +# A MENTION is not a call. The easy wrong implementation is a bare grep, and it +# reads a suite that only explains why it cannot account as though it does. +printf '. "$(dirname "$0")/lib.sh"\n# this suite cannot call pgc_summary: it has no cluster\nexit 0\n' \ + > "$_acc/mentions.sh" +check "a comment mentioning pgc_summary is not a declaration" \ + "$(pgc_suite_declares_accounting "$_acc/mentions.sh")" "no" + +# Nor is a longer name that contains it. +printf '. "$(dirname "$0")/lib.sh"\npgc_summary_of_something\n' > "$_acc/prefix.sh" +check "a longer name containing pgc_summary is not a declaration" \ + "$(pgc_suite_declares_accounting "$_acc/prefix.sh")" "no" + +# An ABSENT file is its own answer. Reported by OffgridwithJD reviewing #922: +# folding it into "no" classifies a registered suite whose .sh has vanished as +# exempt, and the reconciliation then reads clean -- a suite disappearing from +# the matrix, inside the check whose subject is suites going missing from the +# accounting. +check "a file that does not exist is reported absent, not exempt" \ + "$(pgc_suite_declares_accounting "$_acc/absent.sh")" "absent" +check "and absent is distinguishable from a present file that does not declare" \ + "$([ "$(pgc_suite_declares_accounting "$_acc/absent.sh")" \ + = "$(pgc_suite_declares_accounting "$_acc/silent.sh")" ] && echo same || echo different)" \ + "different" + +# ---- the comment stripper follows the SHELL's rule ------------------------- +# +# `sed 's/#.*$//'` strips from ANY hash, so a `#` inside a quoted string earlier +# on the line hides a pgc_summary call after it. Also reported by OffgridwithJD. +# The stripper now only treats a hash at line start or after whitespace as a +# comment, which is what the shell does. +printf '. "$(dirname "$0")/lib.sh"\nX=a#b; pgc_summary\n' > "$_acc/hashinword.sh" +check "a hash inside a word does not hide the call after it" \ + "$(pgc_suite_declares_accounting "$_acc/hashinword.sh")" "yes" + +printf '. "$(dirname "$0")/lib.sh"\npgc_summary # and a trailing comment\n' > "$_acc/trailing.sh" +check "a trailing comment after the call does not hide it" \ + "$(pgc_suite_declares_accounting "$_acc/trailing.sh")" "yes" + +printf '. "$(dirname "$0")/lib.sh"\n # pgc_summary is only mentioned here\nexit 0\n' > "$_acc/indented.sh" +check "an indented comment is still a comment" \ + "$(pgc_suite_declares_accounting "$_acc/indented.sh")" "no" + +# The residual the shell rule does not cover is a hash after whitespace INSIDE a +# quoted string, and the arm for it must not be a second spelling of the hazard. +# +# The first version of this arm WAS that, and it was INVERTED: it required a +# non-whitespace character before the hash, which is a hash inside a WORD -- the +# shape the stripper handles correctly -- so it flagged the safe case and was +# blind to the dangerous one. Found by OffgridwithJD, who built the control: +# +# psql -c "SELECT 1 # note"; pgc_summary the reader answered NO, unflagged +# X=a#b; pgc_summary the reader answered YES, FLAGGED +# +# So the arm no longer restates the hazard. It compares the reader's INPUT with +# its OUTPUT: count the call in the raw file, count it again in the stripped +# text, and if the stripped count is lower the stripper hid a call. That detects +# it for any spelling, present or future, cannot be inverted, and does not depend +# on a measurement staying true -- it IS the measurement. +_hash_pat='(^|[^_[:alnum:]])pgc_summary([^_[:alnum:]]|$)' +_hidden=0 +while IFS= read -r _hs; do + _hf="$PGC_TESTDIR/${_hs}.sh" + [ -f "$_hf" ] || continue + _raw="$(grep -cE "$_hash_pat" "$_hf" || true)" + _str="$(sed 's/\(^\|[[:space:]]\)#.*$/\1/' "$_hf" | grep -cE "$_hash_pat" || true)" + if [ "$_str" -lt "$_raw" ]; then + # A call the stripper removed. Only a real one matters; a comment-only + # mention losing its line is the stripper working. + if [ "$(pgc_suite_declares_accounting "$_hf")" = no ] \ + && grep -qE "$_hash_pat" "$_hf"; then + _hidden=$((_hidden + 1)) + echo " the comment stripper hides a pgc_summary call in $_hs.sh" + fi + fi +done < <(listed_suites) +check "the stripper hides no pgc_summary call in any registered suite" "$_hidden" "0" + +# And prove that arm can fire, on a file built to trip it. Without this the zero +# above is satisfied by an arm that never looks at anything. +printf '. "$(dirname "$0")/lib.sh"\npsql -c "SELECT 1 # note"; pgc_summary\n' > "$_acc/hidden.sh" +_h_raw="$(grep -cE "$_hash_pat" "$_acc/hidden.sh" || true)" +_h_str="$(sed 's/\(^\|[[:space:]]\)#.*$/\1/' "$_acc/hidden.sh" | grep -cE "$_hash_pat" || true)" +check "premise: the fixture really does hide its call from the stripper" \ + "$([ "$_h_str" -lt "$_h_raw" ] && echo hidden || echo "raw=$_h_raw str=$_h_str")" "hidden" +check "and the reader answers no on it, which is the wrong answer the arm catches" \ + "$(pgc_suite_declares_accounting "$_acc/hidden.sh")" "no" + +# ---- the observation reader ------------------------------------------------ +# +# pgc_summary prints the accounting line before every exit path, so it is present +# on a pass, a failure, a skip and an incomplete alike. That is what makes it the +# runtime twin of the declaration rather than a synonym for PASSED. + +printf 'checks run: 3\nchecks unrunnable: 0\naccounting: 3 passed + 0 failed + 0 unrunnable + 0 skipped = 3\nx.sh: PASSED\n' > "$_acc/pass.log" +check "a passing log shows accounting" "$(pgc_log_shows_accounting "$_acc/pass.log")" "yes" + +printf 'accounting: 1 passed + 2 failed + 0 unrunnable + 0 skipped = 3\nx.sh: FAILED\n' > "$_acc/fail.log" +check "and so does a failing one, which is the point" \ + "$(pgc_log_shows_accounting "$_acc/fail.log")" "yes" + +printf 'accounting: 0 passed + 0 failed + 0 unrunnable + 0 skipped = 0\nx.sh: SKIPPED (ran no checks)\n' > "$_acc/skip.log" +check "and a skip, which reached the summary and counted zero" \ + "$(pgc_log_shows_accounting "$_acc/skip.log")" "yes" + +printf 'accounting: 2 passed + 0 failed + 1 unrunnable + 0 skipped = 3\nx.sh: INCOMPLETE\n' > "$_acc/inc.log" +check "and an incomplete" "$(pgc_log_shows_accounting "$_acc/inc.log")" "yes" + +printf 'x.sh: PASSED\n' > "$_acc/bare.log" +check "a log claiming PASSED without the accounting line shows none" \ + "$(pgc_log_shows_accounting "$_acc/bare.log")" "no" + +printf 'this suite prints the word accounting: in prose\n' > "$_acc/prose.log" +check "and prose containing the word does not count as the line" \ + "$(pgc_log_shows_accounting "$_acc/prose.log")" "no" + +# THE ^ ANCHOR, which nothing above exercises. The prose fixture is refused by the +# regex SHAPE, not by the anchor, so removing ^ from the reader left every arm +# green -- reported by OffgridwithJD. The distinguishing input is a well-formed +# accounting line that does NOT start the line, which is what a nested or indented +# suite run produces. Inert on real data today (0 non-line-start occurrences +# across 246 PG17 logs and 244 PG18), so this closes a coverage gap rather than a +# live defect. +printf ' accounting: 3 passed + 0 failed + 0 unrunnable + 0 skipped = 3\nx.sh: PASSED\n' \ + > "$_acc/indented_acc.log" +check "premise: the fixture carries a well-formed accounting line, just indented" \ + "$(grep -c 'accounting: 3 passed + 0 failed + 0 unrunnable + 0 skipped = 3' "$_acc/indented_acc.log")" "1" +check "an accounting line that does not start its line is refused" \ + "$(pgc_log_shows_accounting "$_acc/indented_acc.log")" "no" + +check "an absent log shows no accounting rather than erroring" \ + "$(pgc_log_shows_accounting "$_acc/absent.log")" "no" + +# ---- and the reader must be shown the PRODUCER's own output ----------------- +# +# Every log above is a literal typed into this file, and pytest types the same +# four again, and the format string itself lives a third time in lib.sh's +# pgc_summary. Three hand-written copies of one line: a wording drift in the +# PRODUCER leaves both harnesses green while the reader answers "no" for every +# real suite, which would redden the whole matrix on both majors having passed +# its own tests. +# +# That is the pipefail lesson surviving on the other reader, and it was found by +# OffgridwithJD, who measured it: one realistic rewording of lib.sh:1507 flips the +# reader to "no" on a real log with no arm going red. +# +# So run a REAL two-line suite and feed the reader its actual stdout. This is the +# only arm here that survives a change to the format. +_realsuite="$_acc/real.sh" +printf '. "%s/lib.sh"\ncheck "x" a a\npgc_summary\n' "$PGC_TESTDIR" > "$_realsuite" +_reallog="$_acc/real.log" +bash "$_realsuite" > "$_reallog" 2>&1 || true + +check "premise: the real suite ran and reached its summary" \ + "$(grep -c ': PASSED$' "$_reallog")" "1" +check "premise: and produced exactly one accounting line to be read" \ + "$(grep -c '^accounting: ' "$_reallog")" "1" +check "the reader accepts the line the producer actually emits" \ + "$(pgc_log_shows_accounting "$_reallog")" "yes" + +# The control that this arm is not simply insensitive: the same real log with its +# accounting line reworded must be refused. +sed 's/^accounting: /accounting summary: /' "$_reallog" > "$_acc/real_drifted.log" +check "premise: the drift changed the line the reader looks for" \ + "$(grep -c '^accounting: ' "$_acc/real_drifted.log")" "0" +check "and a reworded producer line is refused, so the arm can fail" \ + "$(pgc_log_shows_accounting "$_acc/real_drifted.log")" "no" + +# ---- the reconciliation, in both directions -------------------------------- + +_declared="$_acc/declared"; _observed="$_acc/observed" + +printf 'alpha\nbeta\ngamma\n' > "$_declared" +printf 'alpha\nbeta\ngamma\n' > "$_observed" +check "equal sets reconcile" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" >/dev/null 2>&1 && echo ok || echo asymmetric)" "ok" + +# The direction that catches the false green: a suite said it would account and +# no accounting line appeared, so it died before reaching pgc_summary. Today that +# reads PASS whenever the shell happened to exit 0. +printf 'alpha\nbeta\n' > "$_observed" +check "a declared suite that produced no accounting is caught" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" >/dev/null 2>&1 && echo ok || echo asymmetric)" "asymmetric" +check "and it is NAMED, so the reader does not have to diff two lists" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" 2>&1 | grep -c '^[[:space:]]*declared but never accounted: gamma$')" "1" + +# The opposite direction: an accounting line from a suite whose source says it +# cannot produce one. That means the reading of the source is stale, and it is +# the failure an exempt list maintained by hand can never report. +printf 'alpha\nbeta\n' > "$_declared" +printf 'alpha\nbeta\ngamma\n' > "$_observed" +check "an undeclared suite that DID account is caught too" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" >/dev/null 2>&1 && echo ok || echo asymmetric)" "asymmetric" +check "and it is named as the opposite fault, not the same one" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" 2>&1 | grep -c '^[[:space:]]*accounted but never declared: gamma$')" "1" + +# Both at once must report both. One error masking the other is how a count +# passes while two suites are wrong in opposite directions -- the exact failure +# a count cannot distinguish from correctness. +printf 'alpha\ndelta\n' > "$_declared" +printf 'alpha\ngamma\n' > "$_observed" +check "opposite errors do not cancel: both directions are reported" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" 2>&1 | grep -cE '^[[:space:]]*(declared but never accounted: delta|accounted but never declared: gamma)$')" "2" + +# inputs == sum(buckets), printed from the data, per the house rule. +printf 'alpha\nbeta\ngamma\n' > "$_declared" +printf 'beta\ngamma\ndelta\n' > "$_observed" +check "the reconciliation prints inputs == sum(buckets)" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" 2>&1 | grep -c 'inputs=4 .*both=2.*declared only=1.*accounted only=1.*sum=4')" "1" + +# ---- and the RUNNER must call it, not merely define it ---------------------- +# +# Selftest 320 records what testing a function and not its caller costs here: the +# classifier was right and the loop threw the answer away. So pin the call site +# and pin that its result can fail the major. + +# Count CALLS, not mentions. The first version of this arm matched the comment +# on the definition line as readily as the call below it -- the same +# mention-for-a-call mistake pgc_suite_declares_accounting exists to refuse, +# committed by the arm that asserts it. +check "the runner calls the reconciliation, not merely defines it" \ + "$(grep -c '[^_[:alnum:]]pgc_reconcile_accounting "' "$_rv")" "1" +check "premise: and that count excludes the definition line, which mentions it" \ + "$(grep -c '^pgc_reconcile_accounting()' "$_rv")" "1" +check "and a failed reconciliation sets the per-major failure flag" \ + "$(grep -A6 'pgc_reconcile_accounting "\$_acc_declared"' "$_rv" | grep -c 'verfail=1')" "1" + +# ---- the suites the driver deliberately never ran --------------------------- +# +# PGC_SKIP_TIMING drops four suites on every CI run. They call pgc_summary and +# correctly produce no accounting line, because nothing executed them. Without a +# term for that the reconciliation goes red for the one reason that is not a +# defect, and a check that cries wolf on every CI run is a check nobody reads. +# +# The driver records the decision where it makes it. These arms hold that the +# term EXCUSES only what the driver actually recorded, and cannot be used to +# excuse anything else. + +_notdisp="$_acc/notdispatched" + +printf 'alpha\nbeta\ngamma\n' > "$_declared" +printf 'alpha\nbeta\n' > "$_observed" +printf 'gamma\n' > "$_notdisp" +check "a declared suite the driver never dispatched reconciles" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" "$_notdisp" >/dev/null 2>&1 && echo ok || echo asymmetric)" "ok" + +# The same inputs WITHOUT the record must still be caught, or the term is not +# doing any work and the arm above is satisfied by a function that ignores it. +check "and without that record the same run is still caught" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" >/dev/null 2>&1 && echo ok || echo asymmetric)" "asymmetric" + +# A suite cannot both have reached its summary and not have been dispatched. +# Taking the union would absorb this silently, so it is asserted on its own. +printf 'alpha\nbeta\n' > "$_declared" +printf 'alpha\nbeta\n' > "$_observed" +printf 'beta\n' > "$_notdisp" +check "a suite recorded as never dispatched that DID account is caught" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" "$_notdisp" >/dev/null 2>&1 && echo ok || echo asymmetric)" "asymmetric" +check "and it is named as that fault, not as one of the other two" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" "$_notdisp" 2>&1 \ + | grep -c '^[[:space:]]*both accounted and recorded as never dispatched: beta$')" "1" + +# The record cannot excuse a suite that never declared accounting in the first +# place: that is still the stale-reading direction. +printf 'alpha\n' > "$_declared" +printf 'alpha\n' > "$_observed" +printf 'zeta\n' > "$_notdisp" +check "the record cannot introduce a suite the source never declared" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" "$_notdisp" 2>&1 \ + | grep -c '^[[:space:]]*accounted but never declared: zeta$')" "1" + +# ---- and the DRIVER must write that record --------------------------------- +# +# The term is only honest if the branch that decides not to run a suite is the +# thing that records it. Pin the write to that branch, beside the forged log it +# sits next to. + +check "the skip branch records the suite it did not dispatch" \ + "$(grep -A8 'echo "\$s.sh: SKIPPED (ran no checks)" >"\$builddir/\${s}.log"' "$_rv" \ + | grep -c 'accounting.notdispatched')" "1" +check "and the reconciliation is given that record" \ + "$(grep -c 'pgc_reconcile_accounting "\$_acc_declared" "\$_acc_observed" "\$_acc_notdisp"' "$_rv")" "1" + +# ---- and the identity must be able to FAIL --------------------------------- +# +# inputs == sum(buckets) is printed beside every reconciliation, per the house +# rule. Printing it is not the same as checking it, and asserting it is worth +# nothing unless something can make it false. +# +# Measured, not argued. Two mutations: +# +# compute _inputs from the buckets instead of from the files -- the arithmetic +# above becomes P + D + O == P + D + O, and NOTHING reddens. That is why +# _inputs is counted from the two files by a separate route. +# +# drop the sort before comm -- comm then reports garbage buckets, and the +# totals diverge. That is the fault this identity actually guards, and it is +# selftest 070's subject arriving in a second place. +# +# So the arm is the second mutation, applied to a twin evalled here. + +eval "$(sed -n '/^pgc_reconcile_accounting()/,/^}/p' "$_rv" \ + | sed 's|LC_ALL=C sort -u "$_decl" 2>/dev/null|cat "$_decl" 2>/dev/null|' \ + | sed 's|LC_ALL=C sort -u "$_obs" 2>/dev/null|cat "$_obs" 2>/dev/null|' \ + | sed 's/^pgc_reconcile_accounting()/pgc_reconcile_unsorted_twin()/')" + +check "premise: the unsorted twin is callable" \ + "$(type -t pgc_reconcile_unsorted_twin)" "function" +# A clean pass reads the same whether the code is load-bearing or the mutation +# never applied, so assert the twin really lost its sort. +check "premise: the mutation applied -- the twin no longer sorts its inputs" \ + "$(type pgc_reconcile_unsorted_twin | grep -c 'LC_ALL=C sort -u \"\$_decl\"')" "0" +check "premise: and the real function still does" \ + "$(type pgc_reconcile_accounting | grep -c 'LC_ALL=C sort -u \"\$_decl\"')" "1" + +printf 'gamma\nbeta\nalpha\n' > "$_declared" +printf 'delta\ngamma\nbeta\n' > "$_observed" +check "the identity catches comm reading unsorted input" \ + "$(pgc_reconcile_unsorted_twin "$_declared" "$_observed" 2>&1 | grep -c 'does not add up')" "1" +check "and the real function reconciles the same input, so the arm is not noise" \ + "$(pgc_reconcile_accounting "$_declared" "$_observed" 2>&1 | grep -c 'does not add up')" "0" + +# ---- the readers, run over the REAL population ------------------------------ +# +# Everything above uses fixtures. A reader that works on four synthetic files and +# not on the 251 registered suites has been tested against the world it was +# written for. So run the declaration reader over the actual list and print the +# partition, per the rule that a list-derived claim shows inputs == sum(buckets). +# +# No count is asserted. The number of exempt suites is not a fact about +# correctness, and pinning it here would make this arm a second copy of a +# hand-maintained list -- which is the thing the whole design removes. + +# THREE buckets, not two. Folding "absent" into "does not declare" is the +# conflation the reader was just fixed for, and repeating it here would leave the +# real population the one place it still happened. +# The population is counted by a SECOND ROUTE, not by the loop that classifies it. +# The first version incremented _reg in the same loop body as the buckets, so the +# sum equalled it for ANY reader -- OffgridwithJD proved it passes with an +# always-yes reader and with an always-no reader alike. A total derived from the +# loop that produces the buckets is an identity, which is the shape this file +# spends its length refusing. +# +# WHAT THIS ARM IS, said plainly so the next reader does not overrate it: a +# COVERAGE check. It fails when the classification loop does not see every +# registered suite -- a future `continue`, a read that drops a line, a list that +# changes between the two reads. It is NOT a check on the reader's correctness; +# the two arms below it, which require both buckets to be occupied, are what +# catch a reader answering the same way for everything. +_reg="$(listed_suites | grep -c . || true)" +_decl_n=0; _exempt_n=0; _absent_n=0 +while IFS= read -r _s; do + case "$(pgc_suite_declares_accounting "$PGC_TESTDIR/${_s}.sh")" in + yes) _decl_n=$((_decl_n + 1)) ;; + absent) _absent_n=$((_absent_n + 1)); echo " registered but has no file: $_s.sh" ;; + *) _exempt_n=$((_exempt_n + 1)) ;; + esac +done < <(listed_suites) + +echo " registered=$_reg | declares accounting=$_decl_n, does not=$_exempt_n, absent=$_absent_n | sum=$((_decl_n + _exempt_n + _absent_n))" + +check "premise: the registered list is not empty, so the partition means something" \ + "$([ "$_reg" -gt 0 ] && echo yes || echo no)" "yes" +check "the partition over the real suite list adds up" \ + "$((_decl_n + _exempt_n + _absent_n))" "$_reg" +check "every registered suite has a file" "$_absent_n" "0" + +# Both buckets must be occupied, or the reader is answering the same way for +# everything and the arms above would pass just as happily. +check "the reader does not answer yes for every registered suite" \ + "$([ "$_exempt_n" -gt 0 ] && echo yes || echo no)" "yes" +check "nor no for every one of them" \ + "$([ "$_decl_n" -gt 0 ] && echo yes || echo no)" "yes" + +# ---- the declaration reader must survive `set -o pipefail` ------------------ +# +# A REGRESSION ARM. The first version of pgc_suite_declares_accounting piped sed +# into `grep -q`, and this file runs under `set -o pipefail`. grep -q exits the +# moment it matches, closing the pipe while sed is still writing; sed takes EPIPE +# and exits non-zero, and pipefail reports the whole pipeline as failed even +# though grep matched. The function then answered "no" for a suite that plainly +# calls pgc_summary. +# +# It was caught here, and only here: on the real population two of the longest +# suites -- hilbert_curve, the longest at 1,899 lines, and analyze_function, the +# third at 806 -- read as not declaring +# accounting inside this run and as declaring it outside. Selftest 040 carries +# the same story from #473 and #476, where it named different innocent suites on +# every run. +# +# The arm is a file long enough to lose the race, with the call at the TOP so a +# matcher that exits early exits early. +_bigsuite="$_acc/big.sh" +{ + printf '. "$(dirname "$0")/lib.sh"\npgc_summary\n' + _i=0 + while [ "$_i" -lt 40000 ]; do printf 'echo padding line %s\n' "$_i"; _i=$((_i + 1)); done +} > "$_bigsuite" + +check "premise: pipefail is on, which is the condition the bug needs" \ + "$(set -o | grep -cE '^pipefail[[:space:]]+on$')" "1" +check "premise: the fixture is long enough to lose the race" \ + "$([ "$(wc -l < "$_bigsuite")" -gt 10000 ] && echo yes || echo no)" "yes" + +check "a long suite that calls pgc_summary still declares accounting" \ + "$(pgc_suite_declares_accounting "$_bigsuite")" "yes" + +# And prove the arm can fail. The twin is the SHAPE that was wrong, restated +# rather than extracted, because the wrong version is no longer in the tree. +# +# It lives in a QUOTED HEREDOC, like selftest 080's own control and for the same +# reason: 080 now sweeps every producer piped into an early-exit reader, so a +# deliberate demonstration of the forbidden shape has to be text being written to +# a file rather than a pipeline this suite runs. Exempted by property, not by a +# line number. +_grepq_twin_sh="$_acc/grepq_twin.sh" +cat > "$_grepq_twin_sh" <<'TWIN' +set -uo pipefail +sed 's/#.*$//' "$1" \ + | grep -qE '(^|[^_[:alnum:]])pgc_summary([^_[:alnum:]]|$)' && echo yes || echo no +TWIN +check "premise: the twin script was written and is runnable" \ + "$([ -s "$_grepq_twin_sh" ] && echo yes || echo no)" "yes" +check "the grep -q shape is the one that gets this wrong under pipefail" \ + "$(bash "$_grepq_twin_sh" "$_bigsuite")" "no" +check "and it agrees with the real reader on a SHORT file, which is why it survived review" \ + "$(bash "$_grepq_twin_sh" "$_acc/declares.sh")" \ + "$(pgc_suite_declares_accounting "$_acc/declares.sh")" + +# ---- the POPULATION, which the symmetry check above cannot see -------------- +# +# pgc_reconcile_accounting reconciles the DECLARED set against the OBSERVED one. +# Both are derived from the suites themselves, and the two directions catch +# opposite mistakes -- but the registered set is not one of its inputs, so a +# registered suite in NEITHER set is outside the universe being reconciled. +# Driven from the function: with all three files empty it prints +# `inputs=0 | both=0 ... sum=0` and returns 0, whatever SUITES holds. +# +# Reported by @linuxhikerpm, who put it structurally: treating absence of a +# declaration as absence from the population preserves the overcount. The title +# of this change claims to reconcile registered suites against accounted ones, +# and that claim needs the registered set as an input. +# +# So the population is its own check, over its own four buckets. A suite is +# ACCOUNTED when its log carries evidence it counted its checks -- either +# lib.sh's accounting line, or its own `checks run:` line, which is what +# bench_guards and docs_style print from private counters. Runtime-observable in +# both cases, and derived rather than declared, so a suite that adopts either +# mechanism leaves the debt bucket on its own. + +check "premise: the runner defines the population reconciliation" \ + "$(grep -c '^pgc_reconcile_population()' "$_rv")" "1" +check "premise: and the accounted reader that feeds it" \ + "$(grep -c '^pgc_log_shows_any_accounting()' "$_rv")" "1" + +eval "$(sed -n '/^pgc_log_shows_any_accounting()/,/^}/p' "$_rv")" +eval "$(sed -n '/^pgc_reconcile_population()/,/^}/p' "$_rv")" +check "premise: the population reconciliation is callable" \ + "$(type -t pgc_reconcile_population)" "function" + +# ---- the accounted reader takes EITHER mechanism ---------------------------- + +printf 'accounting: 1 passed + 0 failed + 0 unrunnable + 0 skipped = 1\nx.sh: PASSED\n' > "$_acc/lib.log" +check "a log carrying lib.sh's accounting line is accounted" \ + "$(pgc_log_shows_any_accounting "$_acc/lib.log")" "yes" + +printf 'checks run: 9\ndocs_style.sh: PASSED\n' > "$_acc/self.log" +check "and a log carrying only its OWN checks-run line is accounted too" \ + "$(pgc_log_shows_any_accounting "$_acc/self.log")" "yes" + +printf 'some output\nPASSED\n' > "$_acc/none.log" +check "a log carrying neither is not accounted" \ + "$(pgc_log_shows_any_accounting "$_acc/none.log")" "no" + +# ---- the red arm @linuxhikerpm asked for, exactly as asked ------------------ + +_reg_f="$_acc/registered"; _acct_f="$_acc/accounted"; _debt_f="$_acc/debt" +printf 'alpha\n' > "$_reg_f"; : > "$_acct_f"; : > "$_notdisp"; : > "$_debt_f" +check "a registered suite that is accounted by nothing FAILS" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" >/dev/null 2>&1 \ + && echo ok || echo unaccounted)" "unaccounted" +check "and it is named, which the symmetry check could never do" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" 2>&1 \ + | grep -c '^[[:space:]]*registered but accounted by nothing: alpha$')" "1" + +# Each of the three ways out must actually let it out, or the bucket is a name +# for "always fails" and the debt file is the only thing doing any work. +printf 'alpha\n' > "$_acct_f"; : > "$_notdisp"; : > "$_debt_f" +check "a suite that accounted passes" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" >/dev/null 2>&1 \ + && echo ok || echo unaccounted)" "ok" +: > "$_acct_f"; printf 'alpha\n' > "$_notdisp" +check "a suite the driver never dispatched passes" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" >/dev/null 2>&1 \ + && echo ok || echo unaccounted)" "ok" +: > "$_notdisp"; printf 'alpha\n' > "$_debt_f" +check "a suite recorded as known debt passes" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" >/dev/null 2>&1 \ + && echo ok || echo unaccounted)" "ok" + +# The debt file excuses ONLY what it names. A new unaccounted suite must fail +# even while the known ten are excused -- that is the whole point of recording +# them by name rather than as a count. +printf 'alpha\nbeta\n' > "$_reg_f"; : > "$_acct_f"; printf 'alpha\n' > "$_debt_f" +check "a NEW unaccounted suite fails even while the known debt is excused" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" 2>&1 \ + | grep -c '^[[:space:]]*registered but accounted by nothing: beta$')" "1" +check "and the excused one is not named as a failure" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" 2>&1 \ + | grep -c '^[[:space:]]*registered but accounted by nothing: alpha$')" "0" + +# Debt that no longer exists is debt that should have been removed. A name in the +# file that is not registered, or that now accounts, means the file is stale -- +# and a stale debt file is how a burn-down stops burning down. +printf 'alpha\n' > "$_reg_f"; printf 'alpha\n' > "$_acct_f"; printf 'alpha\n' > "$_debt_f" +check "a suite that now accounts but is still listed as debt is reported" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" 2>&1 \ + | grep -c '^[[:space:]]*listed as debt but now accounts: alpha$')" "1" + +printf 'alpha\n' > "$_reg_f"; printf 'alpha\n' > "$_acct_f"; printf 'gone\n' > "$_debt_f" +check "and debt naming a suite that is not registered is reported too" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" 2>&1 \ + | grep -c '^[[:space:]]*listed as debt but not registered: gone$')" "1" + +# inputs == sum(buckets) over the REGISTERED population, printed per the house +# rule. Like the symmetry check's, it cannot be false on the DATA -- the buckets +# are built by successive subtraction from the registered set, so their sum equals +# it identically. What it guards is comm reading unsorted input. The arms above, +# on the unaccounted bucket, are the ones that carry weight. +printf 'a\nb\nc\nd\n' > "$_reg_f" +printf 'a\n' > "$_acct_f"; printf 'b\n' > "$_notdisp"; printf 'c\n' > "$_debt_f" +check "the population partitions, and prints inputs == sum(buckets)" \ + "$(pgc_reconcile_population "$_reg_f" "$_acct_f" "$_notdisp" "$_debt_f" 2>&1 \ + | grep -c 'registered=4 .*accounted=1, not dispatched=1, known debt=1, unaccounted=1 | sum=4')" "1" + +# ---- and the RUNNER must call it, with the real registered set -------------- + +check "the runner calls the population reconciliation" \ + "$(grep -c '[^_[:alnum:]]pgc_reconcile_population "' "$_rv")" "1" +# The property, not a count of a substring: the registered file must be written +# from the SUITES array itself. The first version of this arm asserted the name +# appeared twice, which is a fact about how many times a variable is spelled -- +# it fails when the code is refactored and passes when the file is filled from +# the wrong source. +check "and the registered file is written from the SUITES array itself" \ + "$(grep -cF 'printf '"'"'%s\n'"'"' "${SUITES[@]}" >"$_acc_registered"' "$_rv")" "1" +check "and a failed population reconciliation fails the major" \ + "$(grep -A4 'pgc_reconcile_population "\$_acc_registered"' "$_rv" | grep -c 'verfail=1')" "1" + +# The debt file is tracked, so a change to it is a diff a reviewer sees -- which +# is the whole reason it is a file and not a number in the environment. +check "the debt file is in the tree" \ + "$([ -f "$PGC_TESTDIR/suites_without_accounting.txt" ] && echo yes || echo no)" "yes" diff --git a/test/selftest/400-a-check-result-must-be-machine.sh b/test/selftest/400-a-check-result-must-be-machine.sh new file mode 100644 index 00000000..806fa491 --- /dev/null +++ b/test/selftest/400-a-check-result-must-be-machine.sh @@ -0,0 +1,372 @@ +# ---- a check result must be machine-readable, from ONE counter -------------- +# +# Check results are prose. `check`, `check_num` and `check_text` print PASS or +# FAIL and nothing else, so proving that a mutation reddened one NAMED check +# means grepping text. Every mutation proof in this repository is currently a +# person reading `FAIL ` out of a log and retyping it into a comment. +# +# That is how a reverted guard once reported plain green while the check count +# fell from 190 to 186: the suite passed, and the only evidence anything had +# changed was a number nobody was comparing. +# +# The fix is NOT a second emitter beside the counters. A second source of truth +# for how many checks ran is the defect this issue family exists to close, and +# lib.sh had ELEVEN places that bumped PGC_CHECKS -- eleven chances to add the +# twelfth and forget the line beside it. +# +# So counting a check and recording it are ONE operation, pgc_record, and every +# helper routes through it. The arms below hold that shape rather than the +# behaviour of any one helper, because the shape is what stops the next +# expect_fail from being written. +# --------------------------------------------------------------------------- + +_libsh="$PGC_TESTDIR/lib.sh" + +check "premise: lib.sh is where the check helpers live" \ + "$(grep -c '^check() {' "$_libsh")" "1" + +# THE STRUCTURAL ARM. One counter site, not eleven. +check "lib.sh bumps PGC_CHECKS in exactly one place" \ + "$(grep -c 'PGC_CHECKS=\$((PGC_CHECKS' "$_libsh")" "1" +check "and that place is pgc_record" \ + "$(sed -n '/^pgc_record()/,/^}/p' "$_libsh" | grep -c 'PGC_CHECKS=\$((PGC_CHECKS')" "1" + +# ---- the record line itself ------------------------------------------------- +# +# Tab separated, so a name containing spaces survives. Five columns after the +# RESULT marker: suite, part, name, verdict, reason. The reason carries phase 1's +# REASON_CODE, which is what makes this more than a reformat: an unrunnable check +# is distinguishable from a passing one without parsing prose. The verdict is one +# of PASS, FAIL, UNRUN or SKIP. +# +# No mutation column: that is the LEDGER's (#918). A record is one observation, +# not a history. + +_rec() { # _rec HELPER ARGS... -> the RESULT lines that helper emitted + ( PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0 + "$@" 2>/dev/null | grep '^RESULT' ) +} +_human() { # _human HELPER ARGS... -> the human lines that helper emitted + ( PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0 + "$@" 2>/dev/null | grep -v '^RESULT' ) +} + +check "a passing check emits exactly one record" \ + "$(_rec check "a name" x x | wc -l)" "1" +check "and its verdict field says PASS" \ + "$(_rec check "a name" x x | cut -f5)" "PASS" +check "and its name field is the check's name, spaces intact" \ + "$(_rec check "a name" x x | cut -f4)" "a name" + +# ---- and WHICH PART asked it ------------------------------------------------ +# +# The suite is not enough. harness_selftest sources 40-odd parts into one shell, +# and its premises are phrased to be COPIED: "premise: the pytest layer is where +# THIS PART thinks it is" says "this part" so the same sentence works in any of +# them. main carries two copies of that one and two of another, and the count +# grows with every part anyone adds -- OffgridwithJD found all six of their own +# branches adding more. +# +# So (suite, name) is not a key of checks, it is a key of check NAMES. The part +# makes it a key of the thing it identifies, and it is derived from BASH_SOURCE +# rather than from a convention, so the next part written the same way is keyed +# correctly without anyone remembering. +check "the record names the part the check was asked from" \ + "$(_rec check "a name" x x | cut -f3)" "$(basename "${BASH_SOURCE[0]}" .sh)" +check "premise: and that is this fragment, not the suite" \ + "$([ "$(_rec check "n" x x | cut -f3)" != "$(_rec check "n" x x | cut -f2)" ] \ + && echo different || echo same)" "different" + +check "a failing check emits exactly one record" \ + "$(_rec check "a name" x y | wc -l)" "1" +check "and its verdict field says FAIL" \ + "$(_rec check "a name" x y | cut -f5)" "FAIL" + +check "an unrunnable check emits exactly one record" \ + "$(_rec check_unrunnable "a name" MISSING_DEPENDENCY "no jq" | wc -l)" "1" +check "and its verdict field says UNRUN, which is neither of the other two" \ + "$(_rec check_unrunnable "a name" MISSING_DEPENDENCY "no jq" | cut -f5)" "UNRUN" +check "and the REASON_CODE travels in the reason field, not in prose" \ + "$(_rec check_unrunnable "a name" MISSING_DEPENDENCY "no jq" | cut -f6)" "MISSING_DEPENDENCY" + +# A reason code the enum does not contain is already a FAIL. It must record that +# verdict, not the one it was asked for. +check "a bogus reason code records FAIL, not UNRUN" \ + "$(_rec check_unrunnable "a name" NOT_A_REASON "x" | cut -f5)" "FAIL" + +# ---- every helper, not just the two that were easy -------------------------- +# +# check_text, check_num, check_ratio and pgc_require_tools each had their own +# counter bump and their own outcome line. Each is one place the pair could come +# apart, which is why the arm is over ALL of them rather than a sample. + +check "check_text on an empty side emits one record" \ + "$(_rec check_text "n" "" "x" | wc -l)" "1" +check "and records FAIL, because nothing was compared" \ + "$(_rec check_text "n" "" "x" | cut -f5)" "FAIL" +check "check_num on a non-number emits one record" \ + "$(_rec check_num "n" "abc" "1" | wc -l)" "1" +check "and records FAIL" "$(_rec check_num "n" "abc" "1" | cut -f5)" "FAIL" +check "check_ratio on a non-number emits one record" \ + "$(_rec check_ratio "n" "abc" "1" "2" | wc -l)" "1" +check "check_ratio with a zero side emits one record" \ + "$(_rec check_ratio "n" "0" "1" "2" | wc -l)" "1" +check "check_ratio that forms a ratio emits one record" \ + "$(_rec check_ratio "n" "1" "1" "2" | wc -l)" "1" +check "and records PASS when the ratio is inside the bound" \ + "$(_rec check_ratio "n" "1" "1" "2" | cut -f5)" "PASS" +check "pgc_pass emits one record" "$(_rec pgc_pass "n" | wc -l)" "1" +check "pgc_fail emits one record" "$(_rec pgc_fail "n" "d" | wc -l)" "1" + +# ---- the human lines must not have changed ---------------------------------- +# +# 3,762 check sites, and suites, selftests and CI all grep `^FAIL` and `^PASS`. +# Adding a record beside them is only safe if the prose is byte-identical, so the +# arms pin the exact strings rather than trusting that a refactor was careful. + +check "a passing check still prints its old line" \ + "$(_human check "a name" x x)" "PASS a name" +check "a failing check still prints its old line" \ + "$(_human check "a name" x y)" "FAIL a name: got [x] want [y]" +check "an unrunnable check still prints its old line" \ + "$(_human check_unrunnable "a name" MISSING_DEPENDENCY "no jq")" "UNRUN a name: MISSING_DEPENDENCY: no jq" +check "check_text's empty-side line is unchanged" \ + "$(_human check_text "n" "" "x")" "FAIL n: a side is empty, so nothing was compared: got [] want [x]" +check "check_num's non-measurement line is unchanged" \ + "$(_human check_num "n" "abc" "1")" "FAIL n: not a measurement, so nothing was compared: got [abc] want [1]" + +# ---- the count and the records cannot come apart ---------------------------- +# +# They are one operation, so this cannot fail by drifting. It CAN fail if a +# helper is added that prints an outcome without recording it, which is exactly +# the expect_fail shape, so it is asserted rather than argued. + +_n_calls=7 +_recorded="$( ( PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0 + check a x x; check b x y; check_text c "" x; check_num d abc 1 + check_ratio e 1 1 2; pgc_pass f; check_unrunnable g MISSING_DEPENDENCY h + echo "COUNTED $PGC_CHECKS" ) )" +check "premise: the probe ran every helper shape once" \ + "$(printf '%s\n' "$_recorded" | grep -c '^RESULT')" "$_n_calls" +check "the record count equals the counter the summary reports" \ + "$(printf '%s\n' "$_recorded" | grep -c '^RESULT')" \ + "$(printf '%s\n' "$_recorded" | sed -n 's/^COUNTED //p')" + +# ---- and the RUNNER must reconcile them ------------------------------------- +# +# A suite's log states `checks run: N` and carries N record lines. Those are two +# artifacts of the same run and they can genuinely disagree: a suite killed +# mid-way, a truncated log, a helper that prints an outcome without recording it. + +_rv="$PGC_TESTDIR/run_all_versions.sh" +check "the runner defines the record reconciliation" \ + "$(grep -c '^pgc_reconcile_records()' "$_rv")" "1" + +eval "$(sed -n '/^pgc_reconcile_records()/,/^}/p' "$_rv")" +check "premise: it is callable" "$(type -t pgc_reconcile_records)" "function" + +_rl="$PGC_WORKDIR/rec.log" +printf 'RESULT\ts\ta\tPASS\t\nRESULT\ts\tb\tPASS\t\nchecks run: 2\n' > "$_rl" +check "a log whose records match its stated count reconciles" \ + "$(pgc_reconcile_records "$_rl" >/dev/null 2>&1 && echo ok || echo mismatch)" "ok" + +printf 'RESULT\ts\ta\tPASS\t\nchecks run: 2\n' > "$_rl" +check "a log with fewer records than it claims is caught" \ + "$(pgc_reconcile_records "$_rl" >/dev/null 2>&1 && echo ok || echo mismatch)" "mismatch" +check "and the two numbers are named, not just the verdict" \ + "$(pgc_reconcile_records "$_rl" 2>&1 | grep -c 'records=1 .*checks run: 2')" "1" + +printf 'RESULT\ts\ta\tPASS\t\nRESULT\ts\tb\tPASS\t\nRESULT\ts\tc\tPASS\t\nchecks run: 2\n' > "$_rl" +check "a log with more records than it claims is caught too" \ + "$(pgc_reconcile_records "$_rl" >/dev/null 2>&1 && echo ok || echo mismatch)" "mismatch" + +# A log with no `checks run:` line at all did not reach its summary. That is a +# different fault from a miscount and must not read as a clean reconciliation. +printf 'RESULT\ts\ta\tPASS\t\n' > "$_rl" +check "a log that never stated a count is not silently accepted" \ + "$(pgc_reconcile_records "$_rl" >/dev/null 2>&1 && echo ok || echo mismatch)" "mismatch" + +check "the runner calls the record reconciliation, not merely defines it" \ + "$(grep -c '[^_[:alnum:]]pgc_reconcile_records "' "$_rv")" "1" + +# ---- the timing helpers reported an outcome that nothing counted ------------- +# +# check_timing and check_ratio_needs_quiet_machine, under PGC_SKIP_TIMING=1, +# printed a human SKIP line and returned. Driven before the fix: +# +# SKIP a timing check (PGC_SKIP_TIMING: wall-clock measurement) +# SKIP a ratio check (PGC_SKIP_TIMING: wall-clock ratio) +# -> PGC_CHECKS=0 PGC_PASSED=0 PGC_FAILED=0 PGC_UNRUN=0 +# +# Two outcomes a reader sees, nothing counted, no record. That is the hole this +# part's whole argument cannot have, and NOTHING here reached those branches: +# removing both emitters left every arm above green. Found by @linuxhikerpm. +# +# SKIP IS A FOURTH OUTCOME, counted like the other three. `checks run: N` now +# reports the checks a suite ENCOUNTERED rather than the ones it managed to +# evaluate, and pgc_summary reconciles four counters against it instead of three +# -- the same shape, one term wider. +# +# It is NOT check_unrunnable. That third state exists for a check whose INPUT was +# absent and it exits the suite INCOMPLETE, which would turn every CI run red the +# moment PGC_SKIP_TIMING is set -- and CI sets it on every run. A wall-clock check +# on a shared runner is deliberately not asked, which is a different thing from a +# check that could not be answered. + +_tm() { # _tm SKIPFLAG HELPER ARGS... -> records emitted + ( PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0; PGC_SKIPPED=0 + PGC_SKIP_TIMING="$1"; shift; "$@" 2>/dev/null | grep '^RESULT' ) +} +_tmh() { # _tmh SKIPFLAG HELPER ARGS... -> the human lines + ( PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0; PGC_SKIPPED=0 + PGC_SKIP_TIMING="$1"; shift; "$@" 2>/dev/null | grep -v '^RESULT' ) +} +_tmc() { # _tmc SKIPFLAG HELPER ARGS... -> "CHECKS/PASSED/SKIPPED" + ( PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0; PGC_SKIPPED=0 + PGC_SKIP_TIMING="$1"; shift; "$@" >/dev/null 2>&1 + echo "$PGC_CHECKS/$PGC_PASSED/$PGC_SKIPPED" ) +} + +check "a skipped timing check emits exactly one record" \ + "$(_tm 1 check_timing "a timing check" 1 1 | wc -l)" "1" +check "and its verdict is SKIP" \ + "$(_tm 1 check_timing "a timing check" 1 1 | cut -f5)" "SKIP" +check "and it is counted, so checks run: reports it" \ + "$(_tmc 1 check_timing "a timing check" 1 1)" "1/0/1" +check "and its human line is unchanged" \ + "$(_tmh 1 check_timing "a timing check" 1 1)" \ + "SKIP a timing check (PGC_SKIP_TIMING: wall-clock measurement)" + +check "the same timing check, ENABLED, emits one record and passes" \ + "$(_tm 0 check_timing "a timing check" 1 1 | cut -f5)" "PASS" +check "and is counted as a pass, not a skip" \ + "$(_tmc 0 check_timing "a timing check" 1 1)" "1/1/0" + +check "a skipped ratio check emits exactly one record" \ + "$(_tm 1 check_ratio_needs_quiet_machine "a ratio check" 1 1 2 | wc -l)" "1" +check "and its verdict is SKIP" \ + "$(_tm 1 check_ratio_needs_quiet_machine "a ratio check" 1 1 2 | cut -f5)" "SKIP" +check "and it is counted" \ + "$(_tmc 1 check_ratio_needs_quiet_machine "a ratio check" 1 1 2)" "1/0/1" +check "and its human line is unchanged" \ + "$(_tmh 1 check_ratio_needs_quiet_machine "a ratio check" 1 1 2)" \ + "SKIP a ratio check (PGC_SKIP_TIMING: wall-clock ratio)" + +check "the same ratio check, ENABLED, emits one record and passes" \ + "$(_tm 0 check_ratio_needs_quiet_machine "a ratio check" 1 1 2 | cut -f5)" "PASS" +check "and is counted as a pass, not a skip" \ + "$(_tmc 0 check_ratio_needs_quiet_machine "a ratio check" 1 1 2)" "1/1/0" + +# ---- and the accounting line carries the fourth term ------------------------ + +_acct_line() { # _acct_line SKIPFLAG -> pgc_summary's accounting line + ( PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0; PGC_SKIPPED=0 + PGC_SKIP_TIMING="$1" + check "an ordinary check" x x + check_timing "a timing check" 1 1 + pgc_summary ) 2>/dev/null | sed -n 's/^\(accounting: .*\)$/\1/p' +} +check "the accounting line reconciles four outcomes against the count" \ + "$(_acct_line 1)" "accounting: 1 passed + 0 failed + 0 unrunnable + 1 skipped = 2" +check "and with timing enabled the skipped term is zero, not absent" \ + "$(_acct_line 0)" "accounting: 2 passed + 0 failed + 0 unrunnable + 0 skipped = 2" + +# A suite whose every check was skipped has evaluated nothing, so it must not +# report PASSED. Before the fourth counter it could not reach this state at all, +# because a skipped check left PGC_CHECKS at zero. +_allskip() { + ( PGC_FAIL=0; PGC_CHECKS=0; PGC_PASSED=0; PGC_FAILED=0; PGC_UNRUN=0; PGC_SKIPPED=0 + PGC_SKIP_TIMING=1 + check_timing "a timing check" 1 1 + pgc_summary ) 2>/dev/null | grep -oE ': (PASSED|FAILED|SKIPPED \(ran no checks\)|INCOMPLETE)$' +} +check "a suite that skipped every check did not pass" \ + "$(_allskip)" ": SKIPPED (ran no checks)" + +# ---- a check inside a pipeline loses its count, and the message must say so -- +# +# `printf ... | while read n; do check "$n" a a; done` runs the loop body in a +# SUBSHELL, so the counter bump dies with it while the outcome and the record are +# both printed to the parent's stdout. Driven: +# +# four checks print PASS, four RESULT lines appear, PGC_CHECKS=2 +# +# pgc_reconcile_records catches it -- that is what it is for -- but it reported +# `records=3 but the log states checks run: 1`, which is the BOOKKEEPING rather +# than the cause. A reader who has not met this before has no way from that line +# to the pipeline. Raised by OffgridwithJD. +# +# Latent today: four piped loops in the tree, none with a check inside. So the +# sweep below reports zero, and a fixture proves it can fire -- a rule whose only +# evidence is that the corpus is currently clean is not a rule. + +eval "$(sed -n '/^pgc_reconcile_records()/,/^}/p' "$_rv")" +_pl="$PGC_WORKDIR/piped.log" +printf 'RESULT\ts\tp\ta\tPASS\t\nRESULT\ts\tp\tb\tPASS\t\nRESULT\ts\tp\tc\tPASS\t\nchecks run: 1\n' > "$_pl" +check "more records than counted checks names the cause, not just the arithmetic" \ + "$(pgc_reconcile_records "$_pl" 2>&1 | grep -c 'a check ran in a subshell')" "1" +check "and still reports the two numbers" \ + "$(pgc_reconcile_records "$_pl" 2>&1 | grep -c 'records=3 .*checks run: 1')" "1" + +# Fewer records than checks is the OPPOSITE fault -- a counted check that emitted +# no record -- and must not be described as a subshell. +printf 'RESULT\ts\tp\ta\tPASS\t\nchecks run: 3\n' > "$_pl" +check "fewer records than counted checks is not described as a subshell" \ + "$(pgc_reconcile_records "$_pl" 2>&1 | grep -c 'a check ran in a subshell')" "0" +check "and names its own cause instead" \ + "$(pgc_reconcile_records "$_pl" 2>&1 | grep -c 'counted without emitting a record')" "1" + +# ---- and the shape is swept, the way selftest 080 sweeps its cousin ---------- + +_pipeloop_sites() { # _pipeloop_sites FILE... -> file:line of a check inside a piped loop + # Two refinements, both from measuring rather than reading. Requiring the + # closing `done` to be alone on its line left the scanner inside a loop for + # the rest of any file whose loop ended `done)"` -- 27 hits against a true + # zero. And a loop written entirely on ONE line, inside a command + # substitution, opened a block that never closed, flagging every later check. + # + # So: open only on a line that opens the loop and does NOT close it, and close + # on a `done` token wherever it sits. Comments are stripped first, because the + # rule's own explanation necessarily spells the shape out. + awk ' + FNR == 1 { inloop = 0 } + { line = $0; sub(/^[[:space:]]*#.*/, "", line) } + line ~ /\|[[:space:]]*(while|for)[[:space:]]/ && line ~ /(^|[[:space:];])do([[:space:]]|$)/ \ + && line !~ /(^|[[:space:]();])done([[:space:]();]|$)/ { inloop = 1; next } + inloop && line ~ /(^|[[:space:]();])done([[:space:]();]|$)/ { inloop = 0; next } + inloop && line ~ /(^|[^_[:alnum:]])(check|check_num|check_text|check_ratio|check_unrunnable|pgc_pass|pgc_fail)[[:space:]]/ { + print FILENAME ":" FNR + } + ' "$@" +} + +_pl_fx="$PGC_WORKDIR/plfx"; mkdir -p "$_pl_fx" +_pl_call="$(printf '%s "$n" a a' check)" +{ printf 'printf "a\\nb\\n" | while IFS= read -r n; do\n'; printf '\t%s\n' "$_pl_call"; printf 'done\n'; } \ + > "$_pl_fx/bad.sh" +{ printf 'while IFS= read -r n; do\n'; printf '\t%s\n' "$_pl_call"; printf 'done < <(printf "a\\nb\\n")\n'; } \ + > "$_pl_fx/good.sh" +{ printf 'printf "a\\nb\\n" | while IFS= read -r n; do\n'; printf '\techo "$n"\n'; printf 'done\n'; } \ + > "$_pl_fx/nocheck.sh" + +check "premise: the fixtures carry the shapes this sweep is about" \ + "$(grep -lc 'while IFS= read' "$_pl_fx"/*.sh | grep -c .)" "3" +check "the sweep finds a check inside a PIPED loop" \ + "$(_pipeloop_sites "$_pl_fx/bad.sh" | grep -c .)" "1" +check "and not one inside a process-substitution loop, which keeps its shell" \ + "$(_pipeloop_sites "$_pl_fx/good.sh" | grep -c .)" "0" +check "and not a piped loop with no check in it" \ + "$(_pipeloop_sites "$_pl_fx/nocheck.sh" | grep -c .)" "0" + +# A loop written entirely on one line inside a command substitution opens and +# closes in the same place. The first version of this sweep opened a block there +# and never closed it, flagging every check after it -- which is how twelve of +# its twenty-seven false hits were in this very file. +{ printf 'x="$(printf "a\\n" | while IFS= read -r n; do echo "$n"; done)"\n' + printf '%s\n' "$_pl_call"; } > "$_pl_fx/oneline.sh" +check "and not a check after a one-line piped loop that already closed" \ + "$(_pipeloop_sites "$_pl_fx/oneline.sh" | grep -c .)" "0" + +_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" diff --git a/test/sorted_mark_rename.sh b/test/sorted_mark_rename.sh index 8acff70e..bfc3c46c 100755 --- a/test/sorted_mark_rename.sh +++ b/test/sorted_mark_rename.sh @@ -175,9 +175,13 @@ check "so vacuum_sorted on the child does the work" \ # This is why the cascade is not a cosmetic gap. #751 reads the same mark to # claim a pathkey, so a stale mark makes the planner drop the Sort. sorts() { - env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At \ - -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null \ - | grep -qE '^ *(->)? *(Incremental )?Sort' && echo yes || echo no + # grep -c on a captured value, not a pipe into grep -q; see lib.sh's + # pgc_is_columnar_scan for the mechanism and the measurement. + local _plan + _plan="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At \ + -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null)" + [ "$(grep -cE '^ *(->)? *(Incremental )?Sort' <<<"$_plan" || true)" != 0 ] \ + && echo yes || echo no } psql_run "CREATE TABLE wp (k int, j int) PARTITION BY RANGE (k);" psql_run "CREATE TABLE wp1 PARTITION OF wp FOR VALUES FROM (0) TO (100000) USING pgcolumnar;" diff --git a/test/sorted_pathkeys.sh b/test/sorted_pathkeys.sh index 911a152d..aa4090cb 100755 --- a/test/sorted_pathkeys.sh +++ b/test/sorted_pathkeys.sh @@ -45,9 +45,13 @@ pgc_check_ordered_oracle # Does the plan for this query contain a Sort (or Incremental Sort) node? sorts() { # sorts QUERY -> yes|no - env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ - -d "$PGC_DB" -At -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null \ - | grep -qE '^ *(->)? *(Incremental )?Sort' && echo yes || echo no + # grep -c on a captured value, not a pipe into grep -q; see lib.sh's + # pgc_is_columnar_scan for the mechanism and the measurement. + local _plan + _plan="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -At -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null)" + [ "$(grep -cE '^ *(->)? *(Incremental )?Sort' <<<"$_plan" || true)" != 0 ] \ + && echo yes || echo no } inv() { # inversions on the lead column in the order the scan returns rows q "SELECT count(*) FROM (SELECT $2, lag($2) OVER () AS p FROM $1) s WHERE p > $2;" @@ -443,10 +447,14 @@ check "REFUSE: a recorded name that no longer resolves is not a claim" \ # sorts() runs one statement, so the SET has to travel with the connection. sorts_off() { - env PATH="$PGC_BINDIR:$PATH" PGOPTIONS="-c pgcolumnar.enable_sorted_pathkeys=off" \ + # grep -c on a captured value, not a pipe into grep -q; see lib.sh's + # pgc_is_columnar_scan for the mechanism and the measurement. + local _plan + _plan="$(env PATH="$PGC_BINDIR:$PATH" PGOPTIONS="-c pgcolumnar.enable_sorted_pathkeys=off" \ psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At \ - -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null \ - | grep -qE '^ *(->)? *(Incremental )?Sort' && echo yes || echo no + -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null)" + [ "$(grep -cE '^ *(->)? *(Incremental )?Sort' <<<"$_plan" || true)" != 0 ] \ + && echo yes || echo no } check "premise: the claim is live with the GUC on" "$(sorts 'SELECT k FROM c ORDER BY k')" "no" check "control: pgcolumnar.enable_sorted_pathkeys = off restores the Sort" \ diff --git a/test/suites_without_accounting.txt b/test/suites_without_accounting.txt new file mode 100644 index 00000000..674a67fd --- /dev/null +++ b/test/suites_without_accounting.txt @@ -0,0 +1,27 @@ +# Registered suites that count no checks at runtime -- DEBT, not an exemption. +# +# A suite here exits 0 without any observable accounting: it neither calls +# lib.sh's pgc_summary (which prints the "accounting:" line) nor prints its own +# "checks run:" line the way bench_guards and docs_style do from private +# counters. The matrix therefore reports it as a suite that "ran" while having no +# idea whether it asserted anything. +# +# THIS FILE MAY ONLY SHRINK. A name added here is a suite whose checks the harness +# cannot see, and adding one is a diff a reviewer sees -- which is the whole +# reason it is a tracked file and not a number in the environment. A name that +# starts accounting, or stops being registered, is reported by the matrix so the +# burn-down cannot stall silently. +# +# Generated from a measurement over test/run_all_versions.sh --list-suites, not +# typed. To regenerate, run the matrix and take the "registered but accounted by +# nothing" lines. +audit +concurrency +phase2 +phase3 +phase4 +phase5 +phase6 +smoke +unique_conc +update_conc diff --git a/test/unique_conc.sh b/test/unique_conc.sh index ead78fb4..a09c03cb 100755 --- a/test/unique_conc.sh +++ b/test/unique_conc.sh @@ -69,7 +69,11 @@ LOGFILE="$WORKDIR/server.log" port_is_free() { if command -v ss >/dev/null 2>&1; then - ! ss -Htln "sport = :$1" 2>/dev/null | grep -q ":$1" + # grep -c on a captured value, not a pipe into grep -q. A spurious + # EPIPE here answers "nothing is listening" for a port that IS taken, + # and the suite then starts a cluster on an occupied port. + _pif="$(ss -Htln "sport = :$1" 2>/dev/null || true)" + [ "$(grep -c ":$1" <<<"$_pif" || true)" = 0 ] else ! (exec 3<>"/dev/tcp/127.0.0.1/$1") 2>/dev/null fi diff --git a/test/update_conc.sh b/test/update_conc.sh index bd677399..a85e8b3d 100755 --- a/test/update_conc.sh +++ b/test/update_conc.sh @@ -77,7 +77,11 @@ LOGFILE="$WORKDIR/server.log" port_is_free() { if command -v ss >/dev/null 2>&1; then - ! ss -Htln "sport = :$1" 2>/dev/null | grep -q ":$1" + # grep -c on a captured value, not a pipe into grep -q. A spurious + # EPIPE here answers "nothing is listening" for a port that IS taken, + # and the suite then starts a cluster on an occupied port. + _pif="$(ss -Htln "sport = :$1" 2>/dev/null || true)" + [ "$(grep -c ":$1" <<<"$_pif" || true)" = 0 ] else ! (exec 3<>"/dev/tcp/127.0.0.1/$1") 2>/dev/null fi