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/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/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..8af23700 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -1249,9 +1249,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 +1286,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 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..cdf3cfb8 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 } 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_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/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/pytest/TESTS.md b/test/pytest/TESTS.md index f759f5b3..2f591ddd 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -59,9 +59,10 @@ 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. Adding a test](#15-adding-a-test) +- [16. What this corpus does NOT yet refuse](#16-what-this-corpus-does-not-yet-refuse) +- [17. Traps this corpus records](#17-traps-this-corpus-records) ## 1. How to read a test in here @@ -1012,7 +1013,142 @@ 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. 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 +1175,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 +## 16. 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 +1187,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 +## 17. 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_suite_accounting.py b/test/pytest/test_suite_accounting.py new file mode 100644 index 00000000..764e5d63 --- /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 = 3\nx.sh: PASSED\n", + "fail": "accounting: 1 passed + 2 failed + 0 unrunnable = 3\nx.sh: FAILED\n", + "skip": "accounting: 0 passed + 0 failed + 0 unrunnable = 0\nx.sh: SKIPPED (ran no checks)\n", + "inc": "accounting: 2 passed + 0 failed + 1 unrunnable = 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 = 3\nx.sh: PASSED\n") + expect.num(pathlib.Path(indented).read_text() + .count("accounting: 3 passed + 0 failed + 0 unrunnable = 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 = 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..cb3a09ce 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,281 @@ 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]+$' "$_log"; then + echo yes + else + echo no + fi +} + +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. + local _log="$1" + [ -f "$_log" ] || { echo no; return 0; } + if [ "$(grep -cE '^accounting: [0-9]+ passed \+ [0-9]+ failed \+ [0-9]+ unrunnable = [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 @@ -925,7 +1205,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..e6b56e12 100644 --- a/test/selftest/320-a-check-that-could-not-run.sh +++ b/test/selftest/320-a-check-that-could-not-run.sh @@ -212,8 +212,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 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..42321d4a --- /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 = 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 = 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\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 = 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 = 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 = 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 = 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/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