From 2aa12843bda9de155262c31c02c5372f00a23c4d Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" <136637981+ChronicallyJD@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:40:20 -0600 Subject: [PATCH 1/2] test: a suite that ran no checks must stop reporting PASSED (#447) pgc_summary decided the verdict from PGC_FAIL alone. PGC_CHECKS was printed and never read. So a suite that asserted nothing printed PASSED and exited 0, which is indistinguishable from one that ran four hundred checks, and run_all_versions reads only the exit code. Fifteen suites do exactly that whenever pyarrow is absent: arrow_export arrow_nested parquet_export parquet_import parquet_nested fuzz_arrow fuzz_parquet native_parquet_codecs native_parquet_hardening native_parquet_multifile native_parquet_partition native_parquet_projection native_parquet_pushdown native_parquet_streaming native_parquet_units That is the whole Arrow and Parquet surface including both fuzzers, and the run says PASSED beside every one of them. It is the same defect #422 fixed one level up, where a matrix reported ALL VERSIONS PASSED having run none of them. Skipped is now its own state rather than a kind of pass. 0 passed, 1 failed, 2 ran no checks. Making it a failure instead would have been wrong and would have reddened the gate on the first run: PostgreSQL 15 has no WITHOUT OVERLAPS for temporal.sh to test, and a box without pg_isolation_regress is a supported configuration. A red everyone knows to ignore is a red nobody reads. Measured, on one box, same tree, same build: pyarrow present rc=0 PASSED 19 checks pyarrow absent rc=2 SKIPPED 0 checks pyarrow absent, PGC_REQUIRE_PYARROW=1 rc=1 FAILED native_roundtrip (control) rc=0 PASSED 8 checks harness_selftest rc=0 PASSED 33 checks The first two rows have to disagree. Before this they were both PASSED. PGC_REQUIRE_PYARROW follows PGC_REQUIRE_ISOLATION, which isolation.sh has used for this exact purpose since it was written, rather than inventing a second convention. Whether CI sets it is a policy call and is not made here. All four consumers had to learn the third state, because every one of them treated non-zero as failure: run_all_versions.sh, run_san.sh, run_coverage.sh and devloop.sh. run_all_versions now prints "suites that ran: n of m" per major and fails at zero. run_san fails if every suite it ran was a skip, because five of its default subset are pyarrow-gated and it would otherwise report a sanitizer gate passing having sanitized nothing. The PGC_SKIP_TIMING synthetic result now writes 2 rather than 0. Those suites do not run, and recording them as passes was the same claim, written by the driver. native_repack.sh and pg19_vacuum_options.sh printed their own SKIPPED line and exited 0 without calling pgc_summary, so no change inside pgc_summary could reach them. They now route through it and report the same state as everything else. Not fixed here, and worth naming: arrow_import, native_parquet_flba, native_parquet_schema and parquet_nested_import skip only a SECTION when pyarrow is missing and keep their other checks. They will still report a genuine pass having lost real coverage. This closes the zero-check gap, not the pyarrow gap. Running the suites this way immediately found a second instance: temporal.sh reports zero checks on PG18 on my bench, where the feature does exist, because btree_gist is not installed there. It has been reporting PASSED. --- test/arrow_export.sh | 4 +-- test/arrow_nested.sh | 4 +-- test/devloop.sh | 7 +++-- test/fuzz_arrow.sh | 3 +- test/fuzz_parquet.sh | 3 +- test/lib.sh | 52 ++++++++++++++++++++++++++++--- test/native_parquet_codecs.sh | 4 +-- test/native_parquet_hardening.sh | 4 +-- test/native_parquet_multifile.sh | 4 +-- test/native_parquet_partition.sh | 4 +-- test/native_parquet_projection.sh | 4 +-- test/native_parquet_pushdown.sh | 4 +-- test/native_parquet_streaming.sh | 4 +-- test/native_parquet_units.sh | 4 +-- test/native_repack.sh | 3 +- test/parquet_export.sh | 4 +-- test/parquet_import.sh | 4 +-- test/parquet_nested.sh | 4 +-- test/pg19_vacuum_options.sh | 3 +- test/run_all_versions.sh | 41 +++++++++++++++++++++--- test/run_coverage.sh | 14 ++++++--- test/run_san.sh | 24 +++++++++++--- 22 files changed, 136 insertions(+), 66 deletions(-) diff --git a/test/arrow_export.sh b/test/arrow_export.sh index 9d391e8..e62527e 100644 --- a/test/arrow_export.sh +++ b/test/arrow_export.sh @@ -22,9 +22,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow' 2>/dev/null; then - echo "-- pyarrow not available; skipping Arrow export verification" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; skipping Arrow export verification" fi expect_error() { diff --git a/test/arrow_nested.sh b/test/arrow_nested.sh index 21b8811..a5fa8c6 100755 --- a/test/arrow_nested.sh +++ b/test/arrow_nested.sh @@ -20,9 +20,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow' 2>/dev/null; then - echo "-- pyarrow not available; skipping Arrow nested export verification" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; skipping Arrow nested export verification" fi expect_error() { diff --git a/test/devloop.sh b/test/devloop.sh index 7d7151f..a4054ad 100755 --- a/test/devloop.sh +++ b/test/devloop.sh @@ -66,8 +66,11 @@ for s in "$@"; do # clusters did. Sourced rather than duplicated as literals -- an earlier # version of this comment claimed the literals were checked against the band # elsewhere, and no such check existed. - if ! PGC_SKIP_BUILD=1 PGC_PORT=$((PGC_PORT_LO + RANDOM % (PGC_PORT_HI - PGC_PORT_LO))) \ - bash "test/${s}.sh" "$PGC"; then + PGC_SKIP_BUILD=1 PGC_PORT=$((PGC_PORT_LO + RANDOM % (PGC_PORT_HI - PGC_PORT_LO))) \ + bash "test/${s}.sh" "$PGC" + _rc=$? + # 2 is pgc_summary's skipped state (#447), not a failure. + if [ "$_rc" != 0 ] && [ "$_rc" != 2 ]; then rc=1 fi done diff --git a/test/fuzz_arrow.sh b/test/fuzz_arrow.sh index 14cd8ab..946eb6d 100644 --- a/test/fuzz_arrow.sh +++ b/test/fuzz_arrow.sh @@ -39,8 +39,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.ipc' 2>/dev/null; then - echo "SKIP pyarrow not available; the Arrow fuzzer needs it to build seeds" - pgc_summary + pgc_skip pyarrow "pyarrow not available; the Arrow fuzzer needs it to build seeds" fi SEED="${PGC_SEED:-20260728}" diff --git a/test/fuzz_parquet.sh b/test/fuzz_parquet.sh index 277b46c..dd5762e 100755 --- a/test/fuzz_parquet.sh +++ b/test/fuzz_parquet.sh @@ -41,8 +41,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; the Parquet fuzzer needs it to build seeds" - pgc_summary + pgc_skip pyarrow "pyarrow not available; the Parquet fuzzer needs it to build seeds" fi SEED="${PGC_SEED:-20260728}" diff --git a/test/lib.sh b/test/lib.sh index 7e67cfb..39acf38 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -571,12 +571,50 @@ chunk_group_count() { # ---- summary --------------------------------------------------------------- +# A suite that could not run, because an optional capability is absent. +# +# Announces the skip, and honours PGC_REQUIRE_=1 so a gate box can demand a +# capability rather than silently losing its coverage. isolation.sh has had this +# exact shape for one capability since it was written (PGC_REQUIRE_ISOLATION, +# which CI sets); this generalises that convention instead of inventing a second. +# +# Ends the suite, because there is nothing after the guard to run. +pgc_skip() { # pgc_skip + local cap req val + cap="$(printf '%s' "$1" | tr '[:lower:]-' '[:upper:]_')" + req="PGC_REQUIRE_$cap" + val="${!req:-0}" + if [ "$val" = 1 ]; then + PGC_CHECKS=$((PGC_CHECKS + 1)) + PGC_FAIL=1 + echo "FAIL $2 (and $req=1 demands it)" + else + echo "SKIP $2" + fi + pgc_summary +} + +# Three states, not two (#447). +# +# The verdict used to be a function of PGC_FAIL alone, and PGC_CHECKS was printed +# and never read. So a suite that asserted NOTHING printed PASSED and exited 0, +# indistinguishable from one that ran four hundred checks. Fifteen suites do that +# whenever pyarrow is absent, which is how an entire Parquet and Arrow surface, +# including both fuzzers, can leave a run with every line still saying PASSED. +# +# A suite that ran no checks did not pass. It is also not a failure: PostgreSQL 15 +# genuinely has no WITHOUT OVERLAPS to test, and a developer box without an +# optional dependency is a supported configuration rather than a defect. Making +# those red is the "a red everyone knows to ignore is a red nobody reads" failure +# this tree keeps arguing against. +# +# So skipped is its own exit code. 0 passed, 1 failed, 2 ran nothing. The drivers +# count 2 separately and report how many suites actually ran, which is what #422 +# did one level up for how many VERSIONS actually ran. pgc_summary() { echo echo "checks run: $PGC_CHECKS" - if [ "$PGC_FAIL" = "0" ]; then - echo "$(basename "$0"): PASSED" - else + if [ "$PGC_FAIL" != "0" ]; then echo "$(basename "$0"): FAILED" # A source-shape suite (wal_envelope, decode_interrupts) never calls # pgc_setup, so there is no cluster and no log. Without this guard the @@ -586,6 +624,12 @@ pgc_summary() { echo "---- server log tail ----" pgc_pg "tail -40 '$PGC_LOGFILE'" 2>/dev/null || true fi + exit 1 + fi + if [ "$PGC_CHECKS" = "0" ]; then + echo "$(basename "$0"): SKIPPED (ran no checks)" + exit 2 fi - exit $PGC_FAIL + echo "$(basename "$0"): PASSED" + exit 0 } diff --git a/test/native_parquet_codecs.sh b/test/native_parquet_codecs.sh index 677cba1..d200687 100755 --- a/test/native_parquet_codecs.sh +++ b/test/native_parquet_codecs.sh @@ -16,9 +16,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; codec suite needs it to write compressed files" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; codec suite needs it to write compressed files" fi W="$PGC_WORKDIR" diff --git a/test/native_parquet_hardening.sh b/test/native_parquet_hardening.sh index 402c7d1..9a8a0b7 100755 --- a/test/native_parquet_hardening.sh +++ b/test/native_parquet_hardening.sh @@ -22,9 +22,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; Parquet hardening suite needs it" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; Parquet hardening suite needs it" fi W="$PGC_WORKDIR" diff --git a/test/native_parquet_multifile.sh b/test/native_parquet_multifile.sh index 76e2209..038918c 100755 --- a/test/native_parquet_multifile.sh +++ b/test/native_parquet_multifile.sh @@ -18,9 +18,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; multi-file suite needs it" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; multi-file suite needs it" fi W="$PGC_WORKDIR" diff --git a/test/native_parquet_partition.sh b/test/native_parquet_partition.sh index 2a02f52..340cb0a 100755 --- a/test/native_parquet_partition.sh +++ b/test/native_parquet_partition.sh @@ -21,9 +21,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; partition suite needs it" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; partition suite needs it" fi W="$PGC_WORKDIR" diff --git a/test/native_parquet_projection.sh b/test/native_parquet_projection.sh index f524b5e..8213fd7 100755 --- a/test/native_parquet_projection.sh +++ b/test/native_parquet_projection.sh @@ -17,9 +17,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; projection suite needs it" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; projection suite needs it" fi W="$PGC_WORKDIR" diff --git a/test/native_parquet_pushdown.sh b/test/native_parquet_pushdown.sh index b5feac4..40e7415 100755 --- a/test/native_parquet_pushdown.sh +++ b/test/native_parquet_pushdown.sh @@ -17,9 +17,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; predicate-pushdown suite needs it to write stats" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; predicate-pushdown suite needs it to write stats" fi PARQ="$PGC_WORKDIR/stats.parquet" diff --git a/test/native_parquet_streaming.sh b/test/native_parquet_streaming.sh index 27d1b90..f4206ab 100755 --- a/test/native_parquet_streaming.sh +++ b/test/native_parquet_streaming.sh @@ -22,9 +22,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; streaming suite needs it" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; streaming suite needs it" fi W="$PGC_WORKDIR" diff --git a/test/native_parquet_units.sh b/test/native_parquet_units.sh index cc66bff..b1b91b8 100755 --- a/test/native_parquet_units.sh +++ b/test/native_parquet_units.sh @@ -19,9 +19,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "SKIP pyarrow not available; Parquet unit suite needs it" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; Parquet unit suite needs it" fi W="$PGC_WORKDIR" diff --git a/test/native_repack.sh b/test/native_repack.sh index 3c2ad27..9ec7dab 100755 --- a/test/native_repack.sh +++ b/test/native_repack.sh @@ -48,8 +48,7 @@ pgc_setup "${1:-/usr/local/pg19/bin/pg_config}" srv="$(q 'SHOW server_version_num')" if [ "${srv:-0}" -lt 190000 ]; then echo "SKIP REPACK requires PostgreSQL 19 (server_version_num=$srv)" - echo "native_repack.sh: SKIPPED" - exit 0 + pgc_summary fi ROWS=${PGC_REPACK_ROWS:-20000} diff --git a/test/parquet_export.sh b/test/parquet_export.sh index d0ef231..5619714 100644 --- a/test/parquet_export.sh +++ b/test/parquet_export.sh @@ -24,9 +24,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow.parquet' 2>/dev/null; then - echo "-- pyarrow not available; skipping Parquet export verification" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; skipping Parquet export verification" fi expect_error() { diff --git a/test/parquet_import.sh b/test/parquet_import.sh index 1872d29..aa99d56 100755 --- a/test/parquet_import.sh +++ b/test/parquet_import.sh @@ -20,9 +20,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow' 2>/dev/null; then - echo "-- pyarrow not available; skipping Parquet import verification" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; skipping Parquet import verification" fi expect_error() { diff --git a/test/parquet_nested.sh b/test/parquet_nested.sh index 35b7ec2..aa40f5e 100755 --- a/test/parquet_nested.sh +++ b/test/parquet_nested.sh @@ -20,9 +20,7 @@ set -uo pipefail pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" if ! python3 -c 'import pyarrow' 2>/dev/null; then - echo "-- pyarrow not available; skipping Parquet nested export verification" - pgc_summary - exit 0 + pgc_skip pyarrow "pyarrow not available; skipping Parquet nested export verification" fi expect_error() { diff --git a/test/pg19_vacuum_options.sh b/test/pg19_vacuum_options.sh index c008e44..ec485c7 100755 --- a/test/pg19_vacuum_options.sh +++ b/test/pg19_vacuum_options.sh @@ -34,8 +34,7 @@ pgc_setup "${1:-/usr/local/pg19/bin/pg_config}" srv="$(q 'SHOW server_version_num')" if [ "${srv:-0}" -lt 190000 ]; then echo "SKIP parallel autovacuum requires PostgreSQL 19 (server_version_num=$srv)" - echo "pg19_vacuum_options.sh: SKIPPED" - exit 0 + pgc_summary fi ROWS=${PGC_AV_ROWS:-100000} diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index ca40ec7..ed8284a 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -434,7 +434,10 @@ for pgc in "${CONFIGS[@]}"; do # its assertions untrustworthy, only slower. if [ "${PGC_SKIP_TIMING:-0}" = 1 ] && is_timing_suite "$s"; then echo " SKIP $s (PGC_SKIP_TIMING)" - echo 0 >"$builddir/${s}.rc" + # 2, not 0. This suite did not run, and since #447 the collector has + # a state that says so. Recording it as a pass was the same lie the + # zero-check suites were telling, just written by the driver. + echo 2 >"$builddir/${s}.rc" continue fi port=$((BASE_PORT++)) @@ -444,10 +447,24 @@ for pgc in "${CONFIGS[@]}"; do done # collect results in suite order for a stable, readable summary + suites_ran=0 + suites_skipped=0 + skipped_names="" for s in "${SUITES[@]}"; do - if [ "$(cat "$builddir/${s}.rc" 2>/dev/null)" = 0 ]; then + _rc="$(cat "$builddir/${s}.rc" 2>/dev/null)" + if [ "$_rc" = 0 ]; then echo " PASS $s" results+="$s=PASS " + suites_ran=$((suites_ran + 1)) + elif [ "$_rc" = 2 ]; then + # Exit 2 is pgc_summary's third state: the suite ran no checks (#447). + # Not a pass, because it asserted nothing. Not a failure, because a + # major without the feature and a box without an optional dependency + # are both supported. Counted, so the total below can say so. + echo " SKIP $s (ran no checks)" + results+="$s=SKIP " + suites_skipped=$((suites_skipped + 1)) + skipped_names="$skipped_names $s" else echo " FAIL $s" # The failing check first, then the tail. A suite that prints a @@ -467,10 +484,26 @@ for pgc in "${CONFIGS[@]}"; do fi done + # How many suites actually asserted something, said out loud (#447). + # + # #422 added this one level up, after a matrix reported ALL VERSIONS PASSED + # having run none of them. The same hole existed per-suite: fifteen suites + # 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. + echo " suites that ran: $suites_ran of ${#SUITES[@]} (skipped: $suites_skipped)" + if [ "$suites_skipped" != 0 ]; then + echo " skipped:${skipped_names}" + fi + if [ "$suites_ran" = 0 ]; then + echo " NO SUITES RAN on PG$major, which is not a pass" + verfail=1 + fi + if [ "$verfail" = 0 ]; then - SUMMARY+=("PASS PG$major ${results}") + SUMMARY+=("PASS PG$major ($suites_ran ran, $suites_skipped skipped) ${results}") else - SUMMARY+=("FAIL PG$major ${results}") + SUMMARY+=("FAIL PG$major ($suites_ran ran, $suites_skipped skipped) ${results}") overall=1 fi rm -rf "$builddir" diff --git a/test/run_coverage.sh b/test/run_coverage.sh index bd7225a..a3dbaec 100755 --- a/test/run_coverage.sh +++ b/test/run_coverage.sh @@ -78,17 +78,23 @@ lcov --directory "$SRCDIR/src" --zerocounters >/dev/null 2>&1 . "$SRCDIR/test/portlib.sh" port="$(pgc_pick_port)" -pass=0; fail=0; failed="" +pass=0; fail=0; failed=""; skip=0; skipped="" for s in $SUITES; do port=$((port + 1)) - if PGC_SKIP_BUILD=1 PGC_PORT="$port" \ - bash "$SRCDIR/test/${s}.sh" "$PGC" >"$OUT/${s}.log" 2>&1; then + PGC_SKIP_BUILD=1 PGC_PORT="$port" \ + bash "$SRCDIR/test/${s}.sh" "$PGC" >"$OUT/${s}.log" 2>&1 + rc=$? + if [ "$rc" = 0 ]; then pass=$((pass + 1)) + elif [ "$rc" = 2 ]; then + # Ran no checks (#447). It contributed no coverage either, so counting it + # as a pass overstates what this report measured. + skip=$((skip + 1)); skipped="$skipped $s" else fail=$((fail + 1)); failed="$failed $s" fi done -echo "-- suites: $pass passed, $fail failed${failed:+ ($failed)}" +echo "-- suites: $pass passed, $fail failed${failed:+ ($failed)}, $skip skipped${skipped:+ ($skipped)}" echo "-- collect" lcov --directory "$SRCDIR/src" --capture --output-file "$OUT/coverage.info" \ diff --git a/test/run_san.sh b/test/run_san.sh index 0cfcaa6..bf3e35a 100644 --- a/test/run_san.sh +++ b/test/run_san.sh @@ -83,6 +83,7 @@ SUITES="${PGC_SAN_SUITES:-smoke native_writer native_roundtrip native_encoding \ echo "-- running the subset under ASAN+UBSAN (fatal)" fail=0 +skipped=0 ran=0 for s in $SUITES; do if [ ! -f "test/$s.sh" ]; then @@ -96,7 +97,14 @@ for s in $SUITES; do # A sanitizer report reaches here two ways: the backend aborts (SIGABRT, so the # suite's own crash checks or a nonzero rc), or the text appears in the output. san="$(printf '%s' "$out" | grep -icE 'runtime error:|AddressSanitizer|UndefinedBehaviorSanitizer|SUMMARY: .*Sanitizer|terminated by signal 6')" - if [ "$rc" != 0 ] || [ "$san" != 0 ]; then + if [ "$rc" = 2 ] && [ "$san" = 0 ]; then + # pgc_summary's skipped state (#447): the suite ran no checks. Five of + # this runner's default subset are pyarrow-gated, so on a sanitizer image + # without pyarrow this is the difference between "no sanitizer findings" + # and "nothing was sanitized". + skipped=$((skipped + 1)) + echo " SKIP $s (ran no checks)" + elif [ "$rc" != 0 ] || [ "$san" != 0 ]; then fail=$((fail + 1)) echo " FAIL $s (rc=$rc, sanitizer_lines=$san)" printf '%s\n' "$out" | grep -iE 'runtime error:|AddressSanitizer|SUMMARY: .*Sanitizer|FAIL' | head -4 | sed 's/^/ /' @@ -105,10 +113,16 @@ for s in $SUITES; do fi done -echo "-- $ran suites under sanitizers, $fail failed" -if [ "$fail" = 0 ]; then - echo "SANITIZER GATE PASSED" -else +echo "-- $ran suites under sanitizers, $fail failed, $skipped ran no checks" +if [ "$fail" != 0 ]; then echo "SANITIZER GATE FAILED" +elif [ "$skipped" = "$ran" ]; then + # Every suite skipped means nothing was sanitized. Five of the default subset + # are pyarrow-gated, so an image without pyarrow could otherwise report the + # gate passed having exercised no decode path at all (#447). + echo "SANITIZER GATE RAN NOTHING, which is not a pass" + fail=1 +else + echo "SANITIZER GATE PASSED" fi exit "$fail" From c6740c29e26b09c16735c9196cd32d6baf273b07 Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" <136637981+ChronicallyJD@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:49:04 -0600 Subject: [PATCH 2/2] test: a missing dependency fails, it does not skip (#447) Owner call, and the right one. The previous commit made a zero-check run visible but still let it pass the gate, which is the same instinct that produced the bug: protect the run from red. A skip is a red nobody has to look at. A box that cannot run a suite has not passed it. So a missing dependency now FAILS, and the waiver is explicit and per-capability rather than implied by silence: PGC_ALLOW_MISSING_PYARROW=1 test/native_parquet_units.sh PGC_ALLOW_MISSING=1 test/run_all_versions.sh Missing DEPENDENCY and not-applicable-to-this-MAJOR stay different code paths, because collapsing them would mean PG15 can never be green. PostgreSQL 15 has no WITHOUT OVERLAPS and no amount of installing will give it one. Those gates call pgc_summary directly and still report SKIPPED. pgc_skip is only for the case where something is genuinely absent that ought to be there. Ten arms, one box, same tree and build: pyarrow present rc=0 PASSED pyarrow absent rc=1 FAILED <- the change absent, PGC_ALLOW_MISSING_PYARROW=1 rc=2 SKIPPED absent, PGC_ALLOW_MISSING=1 rc=2 SKIPPED temporal on PG17, feature absent rc=2 SKIPPED <- must not be red native_repack on PG18 (PG19 only) rc=2 SKIPPED <- must not be red pg19_vacuum_options on PG18 rc=2 SKIPPED <- must not be red temporal on PG18, no btree_gist rc=1 FAILED <- real defect, found native_roundtrip (control) rc=0 PASSED harness_selftest (control) rc=0 PASSED Rows 5 through 7 are the ones that decide whether this is shippable rather than just satisfying. If any of them had gone red the matrix could never be green. temporal.sh's btree_gist gate moves to pgc_skip because that IS a missing dependency: the assert builds do not carry contrib, so temporal has been reporting PASSED on PG18 and PG19 on my bench while asserting nothing. That is now a failure telling someone to install btree_gist, which is what it always should have been. CI already installs pyarrow (ci.yml:396, deliberately under sudo, with a comment recording that installing it as the runner is how these suites came to report PASS in CI while skipping themselves). So the pyarrow half of this is proven safe there. Whether CI carries btree_gist I have NOT verified, and this PR's own checks are the measurement. If temporal reds on this branch, that is the answer and the fix is a package in ci.yml, not a softer verdict here. --- test/lib.sh | 41 ++++++++++++++++++++++++++--------------- test/temporal.sh | 4 +--- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/test/lib.sh b/test/lib.sh index 39acf38..c6f5001 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -571,26 +571,37 @@ chunk_group_count() { # ---- summary --------------------------------------------------------------- -# A suite that could not run, because an optional capability is absent. +# A dependency this suite needs is not installed. # -# Announces the skip, and honours PGC_REQUIRE_=1 so a gate box can demand a -# capability rather than silently losing its coverage. isolation.sh has had this -# exact shape for one capability since it was written (PGC_REQUIRE_ISOLATION, -# which CI sets); this generalises that convention instead of inventing a second. +# This FAILS, and that is the point. A skip is a red that nobody has to look at, +# which is how fifteen suites came to report PASSED while asserting nothing, and +# how temporal.sh has been green on PG18 without btree_gist. A box that cannot run +# a suite is not a box that passed it. # -# Ends the suite, because there is nothing after the guard to run. +# The opt-out is explicit and per-capability, so a developer without pyarrow can +# still work, and so the waiver is visible in the command rather than implied by +# silence: +# +# PGC_ALLOW_MISSING_PYARROW=1 test/native_parquet_units.sh +# PGC_ALLOW_MISSING=1 test/run_all_versions.sh +# +# Missing DEPENDENCY and not-applicable-to-this-MAJOR are different things and are +# deliberately not the same code path. PostgreSQL 15 has no WITHOUT OVERLAPS to +# test and no amount of installing will give it one, so those gates call +# pgc_summary directly and report SKIPPED. Nothing is broken there. Here it is. pgc_skip() { # pgc_skip - local cap req val + local cap allow_one cap="$(printf '%s' "$1" | tr '[:lower:]-' '[:upper:]_')" - req="PGC_REQUIRE_$cap" - val="${!req:-0}" - if [ "$val" = 1 ]; then - PGC_CHECKS=$((PGC_CHECKS + 1)) - PGC_FAIL=1 - echo "FAIL $2 (and $req=1 demands it)" - else - echo "SKIP $2" + allow_one="PGC_ALLOW_MISSING_$cap" + if [ "${PGC_ALLOW_MISSING:-0}" = 1 ] || [ "${!allow_one:-0}" = 1 ]; then + echo "SKIP $2 (waived by $allow_one or PGC_ALLOW_MISSING)" + pgc_summary fi + PGC_CHECKS=$((PGC_CHECKS + 1)) + PGC_FAIL=1 + echo "FAIL $2" + echo " A missing dependency is an environment defect, not a pass. Install" + echo " it, or set $allow_one=1 to run knowingly without this coverage." pgc_summary } diff --git a/test/temporal.sh b/test/temporal.sh index f9e2331..9fed2cd 100644 --- a/test/temporal.sh +++ b/test/temporal.sh @@ -30,9 +30,7 @@ if [ "$major" -lt 18 ]; then fi if ! psql_run "CREATE EXTENSION IF NOT EXISTS btree_gist;" >/dev/null 2>&1; then - echo "-- btree_gist not available; skipping temporal-constraint verification" - pgc_summary - exit 0 + pgc_skip btree_gist "btree_gist not available; temporal constraints need it" fi # expect_ok / expect_err run the same statement against heap and columnar and