test: count a check and record it in one operation (#917)

.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.

CHANGELOG.md

@@ -18,6 +18,41 @@ true until the next version shipped.
 
 ### Added
 
+- Every check result is machine-readable, and counting a check is the same
+  operation as recording it (#917).
+
+  `check`, `check_num` and `check_text` printed `PASS` or `FAIL` and nothing
+  else, so proving that a mutation reddened one NAMED check meant grepping prose
+  and retyping the result. That is also how a reverted guard once reported plain
+  green while the check count fell from 190 to 186: the suite passed, and the only
+  evidence anything had changed was a number nobody compared.
+
+  `lib.sh` had eleven places that bumped `PGC_CHECKS`, each with its own outcome
+  line beside it, which is eleven chances to add a twelfth and forget the line.
+  `projections.sh` did exactly that with an `expect_fail` at ten call sites, for
+  as long as it existed. There is now one, `pgc_record`, so a helper cannot report
+  an outcome without being counted and cannot be counted without reporting one.
+  Every human line is byte-identical; 3,762 call sites is past what a careful
+  refactor can be trusted on, so both harnesses pin the exact strings.
+
+  Each record names the suite, the part, the check, the verdict and the reason.
+  The part matters because `harness_selftest` sources 40-odd parts into one shell
+  and phrases its premises to be copied, so a key of suite and name is a key of
+  check NAMES rather than of checks: 583 records give 579 distinct pairs against
+  582 distinct triples. It is derived from `BASH_SOURCE`, not from a convention.
+
+  A skipped wall-clock check is a fourth counted outcome. Under
+  `PGC_SKIP_TIMING`, `check_timing` and `check_ratio_needs_quiet_machine` printed
+  a `SKIP` line a reader sees while leaving the count at zero and emitting no
+  record, in branches no arm reached. `checks run:` now reports the checks a suite
+  encountered rather than the ones it evaluated, and the summary reconciles four
+  counters against it. A suite that skipped every check reports `SKIPPED` rather
+  than `PASSED`, which the old zero-check condition caught only by accident.
+
+  The matrix reconciles each suite's records against the count its log states, and
+  names the cause rather than the arithmetic: more records than counted is a check
+  that ran in a subshell, fewer is a counter bumped outside `pgc_record`.
+
 - Exact zone-map boundary coverage now lives in matching shell and pytest tests
   (#831).
 

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

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

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() {

test/analyze_differential.sh

@@ -54,9 +54,8 @@ ROWS=${PGC_ANALYZE_DIFF_ROWS:-50000}
 # The major is asserted first so an unreadable version is not mistaken for an old
 # one and reported as "supported, skipped".
 if ! pgc_is_number "${PGC_MAJOR:-}"; then
-	echo "FAIL  could not read the server major, so the gate below cannot be trusted: got [${PGC_MAJOR:-<none>}]"
-	PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1))
-	PGC_FAIL=1
+	pgc_fail "could not read the server major, so the gate below cannot be trusted" \
+		"got [${PGC_MAJOR:-<none>}]"
 	pgc_summary
 fi
 if [ "$PGC_MAJOR" -lt 18 ]; then

test/analyze_function.sh

@@ -63,9 +63,8 @@ ROWS=${PGC_ANALYZE_ROWS:-500000}
 # The major is asserted first. An unreadable version must not be mistaken for an
 # old one, or a broken environment would report SKIP and look supported.
 if ! pgc_is_number "${PGC_MAJOR:-}"; then
-	echo "FAIL  could not read the server major, so the gate below cannot be trusted: got [${PGC_MAJOR:-<none>}]"
-	PGC_CHECKS=$((PGC_CHECKS + 1)); PGC_FAILED=$((PGC_FAILED + 1))
-	PGC_FAIL=1
+	pgc_fail "could not read the server major, so the gate below cannot be trusted" \
+		"got [${PGC_MAJOR:-<none>}]"
 	pgc_summary
 fi
 if [ "$PGC_MAJOR" -lt 18 ]; then

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;

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

test/entry_point_privilege.sh

@@ -307,9 +307,12 @@ as_ep() {  # as_ep <sql> -> 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