diff --git a/test/audit.sh b/test/audit.sh index 04f8943..065e7bf 100755 --- a/test/audit.sh +++ b/test/audit.sh @@ -48,6 +48,25 @@ set -euo pipefail +# set -e aborts this suite at the first unguarded failure, and the verdict block +# is at the bottom. So an abort ends the run non-zero having printed neither a +# FAIL line nor AUDIT TEST FAILED, and every check below the abort silently did +# not run (#418). +# +# Observed: a CREATE TABLE carrying COLLATE "en_US" on a box where that locale +# is not generated killed this suite partway through. The runner recorded +# audit=FAIL, correctly, and the output showed five PASS lines, one ERROR, and +# no verdict at all, which reads like a suite that stopped for no reason. +# +# The trap says which it was. It does not change the exit status, and it does +# not turn an abort into a pass; it stops an abort from being mistaken for a +# short run that went fine. +AUDIT_REACHED_END=0 +trap 'rc=$?; if [ "$AUDIT_REACHED_END" = 0 ]; then + echo; echo "AUDIT TEST ABORTED (exit $rc) before reaching its verdict."; + echo "The checks after the failing command did not run."; +fi' EXIT + . "$(dirname "${BASH_SOURCE[0]}")/portlib.sh" PG_CONFIG="${1:-/usr/local/pg17/bin/pg_config}" @@ -285,6 +304,7 @@ check "same-statement duplicate inserted nothing" \ q "DROP TABLE ss_dup;" >/dev/null echo +AUDIT_REACHED_END=1 if [ "$fail" = "0" ]; then echo "AUDIT TEST PASSED" else diff --git a/test/column_projection.sh b/test/column_projection.sh index a04e4c7..ff45da2 100755 --- a/test/column_projection.sh +++ b/test/column_projection.sh @@ -33,7 +33,8 @@ bufs() { # bufs -d "$PGC_DB" -Atq -c "$NOPAR" -c "SET $GUC=$1" \ -c "EXPLAIN (ANALYZE, BUFFERS) $2" 2>&1 | grep -m1 -oE 'Buffers: shared[^)]*' | - grep -oE '(hit|read)=[0-9]+' | cut -d= -f2 | paste -sd+ | bc + grep -oE '(hit|read)=[0-9]+' | cut -d= -f2 | + awk '{ n += $1 } END { print n + 0 }' } # The core oracle: projection on must equal projection off, byte for byte. @@ -94,13 +95,16 @@ check "premise: columnar and heap hold the same rows" \ # thing. B_ON="$(bufs on "SELECT sum(v) FROM t")" B_OFF="$(bufs off "SELECT sum(v) FROM t")" +# This premise exists because the ratio below means nothing on a tiny read. It +# now also catches a missing measurement: bufs() sums with awk and yields 0 +# rather than the empty string bc left behind when it was not installed (#418). check "premise: the unprojected read is large enough to measure (off: $B_OFF)" \ - "$([ "$B_OFF" -gt 1000 ] && echo yes || echo no)" yes + "$([ "${B_OFF:-0}" -gt 1000 ] && echo yes || echo no)" yes # One float8 column out of fourteen. A third is far above the ideal ~1/14 and # far below the 1.0 a broken projection would give, so it fails on a regression # without tracking encoding-ratio drift. -check "one column costs less than a third of reading all of them (on: $B_ON, off: $B_OFF)" \ - "$([ "$B_ON" -lt $((B_OFF / 3)) ] && echo yes || echo no)" yes +check_ratio "one column costs less than a third of reading all of them" \ + "$B_ON" "$B_OFF" 0.333 # A qual-only column must be read (it is filtered on) even though it is never # emitted -- if it were skipped the filter would silently match nothing. @@ -113,8 +117,10 @@ check "qual-only column still produces the right count" \ # count(*) metadata path, which reads no columns at all and would make this # check pass for the wrong reason. B_ALL="$(bufs on "SELECT count(*) FROM (SELECT t.* FROM t OFFSET 0) s")" -check "SELECT * reads as much as projection-off does ($B_ALL vs $B_OFF)" \ - "$([ "$B_ALL" -gt $((B_OFF * 8 / 10)) ] && echo yes || echo no)" yes +# Inverted, so the same helper carries it: projection-off over SELECT * must not +# exceed 1.25, which is "SELECT * costs at least 80 percent of projection-off". +check_ratio "SELECT * reads as much as projection-off does" \ + "$B_OFF" "$B_ALL" 1.25 # ---- 2. it is faithful ------------------------------------------------------ ab "single column agg" "SELECT sum(v) FROM t" diff --git a/test/harness_selftest.sh b/test/harness_selftest.sh index ca73439..e232613 100755 --- a/test/harness_selftest.sh +++ b/test/harness_selftest.sh @@ -257,4 +257,51 @@ done check "no test picks a port from inside the ephemeral range" \ "$([ -z "$_offenders" ] && echo none || echo "$_offenders")" "none" +# ---- the assertions that refuse an empty measurement (#418) ----------------- +# +# check "" "" prints PASS. Every way a measurement goes missing produces exactly +# that, so check_num and check_ratio exist to refuse it. Those two are now +# load-bearing, and a guard nobody tests is a guard that quietly stops working. +# +# Each probe runs in a subshell, because a deliberate failure must not fail this +# suite: PGC_FAIL and PGC_CHECKS are the harness's own state. The probe reports +# PGC_FAIL, so 1 means the assertion rejected what it was given. +_probe() { # _probe -> 0 when the assertion passed, 1 when it failed + ( PGC_FAIL=0; PGC_CHECKS=0; "$@" >/dev/null 2>&1; echo "$PGC_FAIL" ) +} + +check "check compares two empty strings and passes, which is why the rest exist" \ + "$(_probe check "empty vs empty" "" "")" "0" +check "check_num refuses two empty strings" \ + "$(_probe check_num "empty vs empty" "" "")" "1" +check "check_num refuses a psql error message" \ + "$(_probe check_num "error text" "ERROR: relation does not exist" "42")" "1" +check "check_num refuses the word a yes/no check would produce" \ + "$(_probe check_num "yes" "yes" "yes")" "1" +check "check_num still compares two real numbers" \ + "$(_probe check_num "equal" "42" "42")" "0" +check "check_num still fails two unequal numbers" \ + "$(_probe check_num "unequal" "41" "42")" "1" +check "check_num accepts a decimal and a sign" \ + "$(_probe check_num "decimal" "-1.5" "-1.5")" "0" + +check "check_ratio refuses an empty measurement" \ + "$(_probe check_ratio "empty" "" "100" "0.5")" "1" +check "check_ratio refuses a zero denominator rather than dividing by it" \ + "$(_probe check_ratio "zero denom" "10" "0" "0.5")" "1" +# The numerator matters as much, and for a while this helper only checked the +# denominator while its comment claimed both. A measurement of zero is inside +# every bound, so it passed. +check "check_ratio refuses a zero numerator, which is inside every bound" \ + "$(_probe check_ratio "zero numerator" "0" "100" "0.5")" "1" +check "check_ratio passes a ratio inside its bound" \ + "$(_probe check_ratio "inside" "10" "100" "0.5")" "0" +check "check_ratio fails a ratio outside its bound" \ + "$(_probe check_ratio "outside" "90" "100" "0.5")" "1" + +check "pgc_require_tools passes on tools that exist" \ + "$(_probe pgc_require_tools awk sed)" "0" +check "pgc_require_tools fails on one that does not" \ + "$(_probe pgc_require_tools pgc_no_such_tool_exists)" "1" + pgc_summary diff --git a/test/lib.sh b/test/lib.sh index 6fe9f76..4e63340 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -309,6 +309,109 @@ check() { fi } +# ---- assertions that refuse to compare a measurement nobody took ------------ +# +# check "$label" "$a" "$b" with both sides empty compares "" with "" and prints +# PASS (#418). Every way a measurement goes missing produces exactly that: a +# tool that is not installed, a grep that matched no line, a psql against a +# cluster that is down, a substitution that expanded to nothing. The suite then +# reports success for a number nobody has. +# +# This is not hypothetical and it is not rare. column_projection.sh piped its +# buffer count through bc, bc was absent on one machine, and both sides came +# back empty. It failed there only because one side happened to be non-empty. +# Two green checks were produced the same day that measured nothing at all. +# +# So: a measurement must look like a number before it is compared, and the +# failure says which side was not one. "got [] want []" is the message that cost +# the time. + +# Integer or decimal, optional leading sign. Deliberately strict: an empty +# string, a psql error message, and "no" are all not numbers. +pgc_is_number() { # $1 -> 0 when $1 is a number + local v="${1#-}" + v="${v#+}" + case "$v" in + '' | . | *[!0-9.]* | *.*.*) return 1 ;; + esac + return 0 +} + +# check_num LABEL GOT WANT -- check, with both sides required to be numbers. +check_num() { + local name="$1" got="$2" want="$3" + if ! pgc_is_number "$got" || ! pgc_is_number "$want"; then + PGC_CHECKS=$((PGC_CHECKS + 1)) + PGC_FAIL=1 + echo "FAIL $name: not a measurement, so nothing was compared:" \ + "got [$got] want [$want]" + return 1 + fi + check "$name" "$got" "$want" +} + +# check_ratio LABEL A B MAX -- assert A divided by B is at most MAX. +# +# awk rather than bc, on purpose. bc is not part of a base install and its +# absence is what produced the empty measurement in the first place; awk is +# required by POSIX and is present wherever these suites can run at all. +# +# Zero on EITHER side is refused, not only the denominator. +# +# The first version of this checked only the denominator while this comment +# claimed both. The same commit changed column_projection.sh's bufs() to sum with +# awk, which returns 0 where it used to return the empty string, and that +# converted a failure mode this helper rejects into one it accepted: a projected +# read touching no buffers gives a ratio of 0.00, inside any bound, and passes. +# #418 moved rather than closed, inside the very check that started it. +# +# A zero numerator is "the thing we measured cost nothing", which is nearly +# always "the thing we measured did not happen". A call site that genuinely needs +# to permit zero should say so under its own name rather than get it by default. +check_ratio() { # $1 label, $2 a, $3 b, $4 max + local name="$1" a="$2" b="$3" max="$4" ratio + + if ! pgc_is_number "$a" || ! pgc_is_number "$b" || ! pgc_is_number "$max"; then + PGC_CHECKS=$((PGC_CHECKS + 1)) + PGC_FAIL=1 + echo "FAIL $name: not a measurement, so no ratio was formed:" \ + "a=[$a] b=[$b] max=[$max]" + return 1 + fi + if [ "$(awk -v x="$a" -v y="$b" 'BEGIN { print (x + 0 == 0 || y + 0 == 0) ? "yes" : "no" }')" = yes ]; then + PGC_CHECKS=$((PGC_CHECKS + 1)) + PGC_FAIL=1 + echo "FAIL $name: a side of the ratio is zero, so nothing was measured:" \ + "a=[$a] b=[$b]" + return 1 + fi + ratio="$(awk -v a="$a" -v b="$b" 'BEGIN { printf "%.2f", a / b }')" + PGC_CHECKS=$((PGC_CHECKS + 1)) + if [ "$(awk -v r="$ratio" -v m="$max" 'BEGIN { print (r <= m) ? "yes" : "no" }')" = yes ]; then + echo "PASS $name (${ratio}x, bound ${max}x, from a=$a b=$b)" + else + echo "FAIL $name: ${ratio}x exceeds the ${max}x bound (a=$a b=$b)" + PGC_FAIL=1 + fi +} + +# pgc_require_tools TOOL... -- one clear line at the top, rather than an empty +# string three checks later. A suite that needs a tool it does not have has not +# been skipped; it has been silently narrowed. +pgc_require_tools() { + local t missing="" + for t in "$@"; do + command -v "$t" >/dev/null 2>&1 || missing="$missing $t" + done + if [ -n "$missing" ]; then + echo "FAIL the tools this suite measures with are missing:$missing" + PGC_CHECKS=$((PGC_CHECKS + 1)) + PGC_FAIL=1 + return 1 + fi + return 0 +} + # A check whose subject is a wall-clock ratio. # # PGC_SKIP_TIMING exists because a shared runner cannot hold a ratio still, and diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 3bb2cce..811c6df 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -338,12 +338,18 @@ runs_alone() { esac } +# How many majors were actually built and run. A matrix that ran nothing is not +# a matrix that passed, and until #418 it said "ALL VERSIONS PASSED" and exited +# 0 when every configured pg_config was missing. See the summary block. +VERSIONS_RUN=0 + for pgc in "${CONFIGS[@]}"; do if [ ! -x "$pgc" ]; then echo "SKIP $pgc (not executable)" SUMMARY+=("SKIP $pgc") continue fi + VERSIONS_RUN=$((VERSIONS_RUN + 1)) ver="$("$pgc" --version)" major="$(echo "$ver" | sed -E 's/^[^0-9]*([0-9]+).*/\1/')" @@ -581,9 +587,31 @@ echo "===================== MATRIX SUMMARY ============================" for line in "${SUMMARY[@]}"; do echo " $line" done +echo " versions run: $VERSIONS_RUN of ${#CONFIGS[@]} configured" echo "================================================================" + +# A run that built nothing is not a pass (#418). +# +# The default list names /usr/local/pg15 through /usr/local/pg19. On a box whose +# assert builds are pg15a through pg19a, every entry misses, each prints one +# SKIP line, and this block used to print ALL VERSIONS PASSED and exit 0. That +# output then gets pasted into a pull request as the gate. It is the same defect +# as check "" "" one level up, and it is worse, because this is the line people +# read instead of the checks. +# +# Reported rather than merely counted, because the count is what nobody looks at. +if [ "$VERSIONS_RUN" = 0 ]; then + echo "NO VERSIONS RAN: every configured pg_config was missing or not executable." + echo " configured: ${CONFIGS[*]}" + echo " Pass the pg_configs this box has, e.g. test/run_all_versions.sh /usr/local/pg18a/bin/pg_config" + exit 1 +fi if [ "$overall" = 0 ]; then - echo "ALL VERSIONS PASSED" + if [ "$VERSIONS_RUN" -lt "${#CONFIGS[@]}" ]; then + echo "VERSIONS RUN PASSED ($VERSIONS_RUN of ${#CONFIGS[@]}; the rest were skipped)" + else + echo "ALL VERSIONS PASSED" + fi else echo "SOME VERSIONS FAILED" fi