diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c53a74975..607b1a622 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -259,6 +259,12 @@ jobs: test -n "$WANT" echo "running $(set -- $FILES; echo $#) cluster file(s): $FILES" echo "expecting $WANT collected test(s)" + # PYARROW IS REQUIRED HERE (#1215). A ported pyarrow test declines green on a + # developer box without it; in CI that would be green forever, so CI proves + # the module imports and sets PGC_REQUIRE, which turns a decline into a + # failure (OffgridwithJD, review of #1286). + /tmp/pgcvenv/bin/python -c 'import pyarrow, pyarrow.parquet; print("pyarrow", pyarrow.__version__)' + export PGC_REQUIRE=pyarrow PYTHONPATH=. /tmp/pgcvenv/bin/pytest -q \ --pg-config /usr/lib/postgresql/${{ matrix.pg }}/bin/pg_config \ --pgc-expect-tests "$WANT" \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 85a87d7a8..eee5185c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,6 +164,19 @@ true until the next version shipped. release notes' repair procedure, reading its query from the notes, and a docs check refuses any documented query that tests `pg_extension` while listing databases. +- Without pyarrow, the 25 Arrow and Parquet suites that call `pgc_skip pyarrow` + now report `SKIPPED` and exit 66, and the run does not fail (#1215). Before, + each reported one `FAIL` and reddened the whole run on a box without pyarrow. + The owner decided this on 2026-10-12. Every other missing dependency, such as + `btree_gist` for `temporal.sh`, still fails unless it is waived. The nightly + workflow installs pyarrow and stops if the suites cannot import it, so CI + does not lose these suites silently. `docs/testing.md` also said a skipped + suite exits 2; it exits 66, and the page now says so. + The nightly `capability_sweep.sh` counts these suites as `declined`, a fourth + outcome beside `preserved`, `refused` and `silent`. A suite is `declined` only + if it records a SKIP naming the capability, and `silent` must still be zero. + The sweep now also asserts that the suites that decline are exactly the ones + whose code calls `pgc_skip pyarrow`. - `test/pytest/README.md` now names all four registries a new pytest test has to be entered in, and which of them a local run can catch. Adding one test reddens diff --git a/docs/testing.md b/docs/testing.md index 32d9f1fea..ab9f5c0af 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -317,14 +317,14 @@ you got before deciding whether something is wrong. | outcome | exit | when | | --- | :-: | --- | | `PASSED` | 0 | the suite ran its checks and they held | -| `SKIPPED (ran no checks)` | 2 | the feature under test does not exist on this major | +| `SKIPPED (ran no checks)` | 66 | the feature under test does not exist on this major, or pyarrow is not installed | | `FAILED` | 1 | a check failed, **or** a tool the suite measures with is missing | The matrix prints `suites that ran: n of m (skipped: k)` for each major, and names the skipped suites. A skip counts in `k`, not in `n`. -**A missing dependency fails rather than skips, and that is deliberate.** The box -cannot gate that suite. Reporting a pass would claim coverage nobody has. Two +**A missing dependency fails rather than skips, except pyarrow.** The box cannot +gate that suite, and reporting a pass would claim coverage nobody has. Two dependencies come from outside this repository: | suite | needs | where it comes from | @@ -332,15 +332,22 @@ dependencies come from outside this repository: | the Arrow and Parquet suites | `pyarrow` | `pip install pyarrow`, as the user the suites run as | | `temporal.sh` | `btree_gist` | PostgreSQL **contrib**. A PGDG package ships it; a source build configured without contrib does not | +**Without pyarrow, the Arrow and Parquet suites report `SKIPPED` and the run does +not fail** (#1215). A box without pyarrow is a supported developer configuration. +The matrix still names each skipped suite. The nightly workflow installs pyarrow +and stops if the suites' interpreter cannot import it, so CI does not lose these +suites silently. + Build and install `contrib/btree_gist` against each source tree when the matrix -runs against source builds. To run knowingly without a dependency, waive it: +runs against source builds. To run knowingly without it, or without any other +dependency, waive it: ```sh PGC_ALLOW_MISSING_BTREE_GIST=1 test/temporal.sh /path/to/pg_config PGC_ALLOW_MISSING=1 test/run_all_versions.sh ``` -A waived suite reports `SKIPPED` and exits 2, so the coverage loss stays visible +A waived suite reports `SKIPPED` and exits 66, so the coverage loss stays visible in the skipped count rather than becoming a pass. Measured on PostgreSQL 18, one suite, four configurations: @@ -349,8 +356,8 @@ Measured on PostgreSQL 18, one suite, four configurations: | --- | :-: | --- | | `btree_gist` present | 0 | `PASSED`, 5 checks | | `btree_gist` absent | 1 | `FAILED` | -| `btree_gist` absent, waived | 2 | `SKIPPED` | -| PostgreSQL 17, feature absent | 2 | `SKIPPED (ran no checks)` | +| `btree_gist` absent, waived | 66 | `SKIPPED` | +| PostgreSQL 17, feature absent | 66 | `SKIPPED (ran no checks)` | ## Cross-major upgrade diff --git a/test/capability_sweep.sh b/test/capability_sweep.sh index 33e69f1d8..acbca66ec 100755 --- a/test/capability_sweep.sh +++ b/test/capability_sweep.sh @@ -160,22 +160,37 @@ check_num "premise: and pyarrow IS importable without the shim, so absence is th # ---- the verdict, a pure function so the selftest can drive it ------------- # -# Two inputs and three outcomes. It takes the numbers rather than the logs so it +# Three inputs and four outcomes. It takes the numbers rather than the logs so it # can be exercised without running a suite, the same reason pgc_freshness_verdict # and pgc_build_needs_clean take theirs. -pgc_capability_verdict() { # pgc_capability_verdict LOST_NAMES FAIL_RECORDS -> verdict - local lost="${1:-}" fails="${2:-}" +# +# DECLINED IS ITS OWN OUTCOME (#1215, the owner's rule of 2026-10-12): a missing +# capability may be bypassed as a SKIP, but no test may be silent. A suite that +# loses its names and records a SKIP naming the capability has said what it did +# not run; a suite that loses them and records nothing has not. Folding the first +# into `silent` reddened the sweep on the 25 suites the rule allows to skip, and +# folding it into `preserved` would hide them. So the two stay apart, and `silent` +# must still be zero. +pgc_capability_verdict() { # pgc_capability_verdict LOST FAILS NAMED_SKIPS -> verdict + local lost="${1:-}" fails="${2:-}" skips="${3:-}" case "$lost" in ''|*[!0-9]*) echo unknown; return ;; esac case "$fails" in ''|*[!0-9]*) echo unknown; return ;; esac + case "$skips" in ''|*[!0-9]*) echo unknown; return ;; esac if [ "$lost" -eq 0 ]; then echo preserved elif [ "$fails" -ge 1 ]; then echo refused + elif [ "$skips" -ge 1 ]; then + echo declined else echo silent fi } +# Its four cases are asserted by selftest part 610, which evals this text: they +# need no suite run, and a sweep built on a verdict that misfiled one case would +# count it wrongly thirty times over and still reconcile. + cap_names() { # cap_names LOGFILE -> the recorded check names, sorted grep -oP '^RESULT\t[^\t]*\t[^\t]*\t\K[^\t]*' "$1" 2>/dev/null | LC_ALL=C sort -u } @@ -195,8 +210,10 @@ check_num "premise: the priming run built and installed cleanly" "$prime_rc" "0" n_preserved=0 n_refused=0 +n_declined=0 n_silent=0 silent_names="" +declined_names="" mask_failures=0 for s in $NARROW; do @@ -211,22 +228,47 @@ for s in $NARROW; do [ "$(cap_rc "$SHIM" 'import pyarrow')" = 1 ] || mask_failures=$((mask_failures + 1)) lost="$(LC_ALL=C comm -23 <(cap_names "$w_log") <(cap_names "$o_log") | grep -c .)" fails="$(grep -cP '^RESULT\t.*\tFAIL\t' "$o_log")" - case "$(pgc_capability_verdict "$lost" "$fails")" in + # A SKIP COUNTS ONLY IF ITS REASON NAMES THE CAPABILITY, read from the record + # rather than from the exit code: exit 66 also means a major without the + # feature, which is not this. + skips="$(awk -F'\t' '$1 == "RESULT" && $5 == "SKIP" && $7 ~ /pyarrow/' "$o_log" | grep -c .)" + case "$(pgc_capability_verdict "$lost" "$fails" "$skips")" in preserved) n_preserved=$((n_preserved + 1)) ;; refused) n_refused=$((n_refused + 1)) ;; + declined) n_declined=$((n_declined + 1)); declined_names="$declined_names $s" ;; *) n_silent=$((n_silent + 1)); silent_names="$silent_names $stem" ;; esac rm -f "$w_log" "$o_log" done -echo "-- preserved=$n_preserved refused=$n_refused silent=$n_silent of $n_narrow" +echo "-- preserved=$n_preserved refused=$n_refused declined=$n_declined silent=$n_silent of $n_narrow" [ -n "$silent_names" ] && echo "-- silent:$silent_names" check_num "premise: the mask held for every run in the sweep" "$mask_failures" "0" check_num "premise: every suite in the population was classified" \ - "$((n_preserved + n_refused + n_silent))" "$n_narrow" + "$((n_preserved + n_refused + n_declined + n_silent))" "$n_narrow" check_num "no suite loses a check without recording a skip or a refusal" \ "$n_silent" "0" +# THE SUITES THAT DECLINE ARE THE SUITES THAT ASK TO, derived from the source and +# compared as SETS. A count would let a suite that stopped declining trade places +# with one that started; a literal 25 would go stale the day a suite is added. A +# suite that stops calling pgc_skip pyarrow leaves this side, and must then read +# preserved, or the silent arm above catches it. +DECLINERS="$(for _cap_b in $NARROW; do + # grep -c, NOT grep -q: -q exits at the first match, sed dies of SIGPIPE, and + # under pipefail the pipeline fails, so a suite that DOES call pgc_skip drops + # out at random. Measured here: three of 25 dropped on one run. The header of + # this file records the same race for the population itself. + [ "$(sed 's/#.*//' "$TESTDIR/$_cap_b" | grep -cE 'pgc_skip[[:space:]]+"?pyarrow')" -gt 0 ] && printf '%s\n' "$_cap_b" +done | LC_ALL=C sort)" +check_num "premise: some suites in the population call pgc_skip pyarrow" \ + "$([ -n "$DECLINERS" ] && echo 1 || echo 0)" "1" +_cap_diff="$(LC_ALL=C comm -3 <(printf '%s\n' "$DECLINERS") \ + <(printf '%s\n' $declined_names | LC_ALL=C sort) | tr -d '\t' | grep .)" +[ -n "$_cap_diff" ] && echo "-- decline mismatch: $(printf '%s ' $_cap_diff)" +check_num "the suites that decline are exactly the suites that call pgc_skip pyarrow" \ + "$(printf '%s\n' "$_cap_diff" | grep -c .)" "0" + rm -rf "$SHIM" "$prime_log" pgc_summary diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 0e449de9b..7e1c6e20b 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -48,12 +48,13 @@ base_scan_io base_scan_io premise: the later plan is still a base columnar scan base_scan_io base_scan_io premise: the later plan still does not name a covering projection 15;16;17;18;19 never - base_scan_io base_scan_io premise: the plan is a base columnar scan 15;16;17;18;19 never - base_scan_io base_scan_io premise: the table holds every inserted row 15;16;17;18;19 never - -capability_sweep capability_sweep no suite loses a check without recording a skip or a refusal 15;16;17;18;19 2026-09-23 a check_skip block removed from a suite so its arms vanish with no skip and no fail +capability_sweep capability_sweep no suite loses a check without recording a skip or a refusal 15;16;17;18;19 2026-10-12 a check_skip block removed from a suite so its arms vanish with no skip and no fail;the pyarrow decline reason stops naming the capability, so no skip counts as declined capability_sweep capability_sweep premise: and no member of the population is this very file 15;16;17;18;19 2026-09-23 the self-exclusion removed from pgc_cap_population, so the sweep enters its own population capability_sweep capability_sweep premise: and pyarrow IS importable without the shim, so absence is the shim's doing 15;16;17;18;19 never - capability_sweep capability_sweep premise: and pyarrow.parquet with it 15;16;17;18;19 2026-09-23 the shim's pyarrow.py emptied, so both runs are the present run capability_sweep capability_sweep premise: and the standard library still imports under it 15;16;17;18;19 never - capability_sweep capability_sweep premise: every suite in the population was classified 15;16;17;18;19 never - +capability_sweep capability_sweep premise: some suites in the population call pgc_skip pyarrow 15;16;17;18;19 never - capability_sweep capability_sweep premise: the mask held for every run in the sweep 15;16;17;18;19 2026-09-23 the shim's pyarrow.py emptied, so both runs are the present run capability_sweep capability_sweep premise: the population is the size this tree holds 15;16;17;18;19 never - capability_sweep capability_sweep premise: the priming run built and installed cleanly 15;16;17;18;19 never - @@ -61,6 +62,7 @@ capability_sweep capability_sweep premise: the shim makes pyarrow unimportable 1 capability_sweep capability_sweep premise: the sweep is not in the population it sweeps 15;16;17;18;19 2026-09-23 the self-exclusion removed from pgc_cap_population, so the sweep enters its own population capability_sweep capability_sweep premise: the two derivations of the population name the same suites 15;16;17;18;19 never - capability_sweep capability_sweep premise: while python itself still runs under the shim 15;16;17;18;19 never - +capability_sweep capability_sweep the suites that decline are exactly the suites that call pgc_skip pyarrow 15;16;17;18;19 2026-10-12 the pyarrow decline reason stops naming the capability, so no skip counts as declined catalog_delete_index catalog_delete_index A1 with more catalog pages the default does less work than reading every one whole 15;16;17;18;19 2026-10-12 pgcolumnar_index_min_blocks default 3 -> 2147483647, so no site probes;pgcolumnar_scan_index_oid stops asking the relation its size, so every site probes;the eight converted scan sites back to InvalidOid catalog_delete_index catalog_delete_index B1 with large catalogs the default does far less work than reading every one whole 15;16;17;18;19 2026-10-12 pgcolumnar_index_min_blocks default 3 -> 2147483647, so no site probes;pgcolumnar_scan_index_oid stops asking the relation its size, so every site probes;the eight converted scan sites back to InvalidOid catalog_delete_index catalog_delete_index P1 with a few catalog pages the default does less work than probing every one 15;16;17;18;19 2026-10-12 pgcolumnar_scan_index_oid stops asking the relation its size, so every site probes;the eight converted scan sites back to InvalidOid @@ -1599,6 +1601,16 @@ harness_selftest 590-a-declared-suite-must-account the EXIT trap runs the wrappe harness_selftest 590-a-declared-suite-must-account the line it emits is the line the reconciliation reads 15;16;17;18;19 2026-09-23 the EXIT trap reverted to the bare pgc_teardown, so the eight pgc_setup exits emit no accounting harness_selftest 590-a-declared-suite-must-account the suite's own counters are restored after every driving block 15;16;17;18;19 never - harness_selftest 590-a-declared-suite-must-account the wrapper captures the exit status before anything overwrites it 15;16;17;18;19 2026-09-23 the EXIT trap reverted to the bare pgc_teardown, so the eight pgc_setup exits emit no accounting +harness_selftest 600-a-missing-pyarrow-declines a missing pyarrow declines the suite instead of failing it 15;16;17;18;19 2026-10-12 pgc_skip treats pyarrow like any other capability again, so a missing pyarrow fails the suite +harness_selftest 600-a-missing-pyarrow-declines and it records a skip under the check's own name, not a failure 15;16;17;18;19 2026-10-12 pgc_skip treats pyarrow like any other capability again, so a missing pyarrow fails the suite +harness_selftest 600-a-missing-pyarrow-declines control: a missing btree_gist still fails the suite 15;16;17;18;19 2026-10-12 pgc_skip declines every missing capability, not only pyarrow +harness_selftest 600-a-missing-pyarrow-declines control: and an explicit waiver still turns that failure into a skip 15;16;17;18;19 2026-10-12 pgc_skip ignores PGC_ALLOW_MISSING_, so only the run-wide waiver works +harness_selftest 600-a-missing-pyarrow-declines premise: lib.sh defines pgc_skip 15;16;17;18;19 never - +harness_selftest 610-a-declined-capability-is-not-silent premise: the sweep defines its verdict where this part reads it 15;16;17;18;19 never - +harness_selftest 610-a-declined-capability-is-not-silent the verdict: names lost beside a failure are refused 15;16;17;18;19 2026-10-12 pgc_capability_verdict answers declined for every input +harness_selftest 610-a-declined-capability-is-not-silent the verdict: names lost beside a skip naming the capability are declined 15;16;17;18;19 2026-10-12 pgc_capability_verdict has no declined outcome, so a lost name beside a skip reads silent +harness_selftest 610-a-declined-capability-is-not-silent the verdict: names lost with neither a failure nor a skip are silent 15;16;17;18;19 2026-10-12 pgc_capability_verdict answers declined for every input +harness_selftest 610-a-declined-capability-is-not-silent the verdict: no name lost is preserved 15;16;17;18;19 2026-10-12 pgc_capability_verdict answers declined for every input index_am_support index_am_support btree answers its operator with the same rows the scan returns 15;16;17;18;19 2026-09-23 pgcolumnar_index_build_range_scan drops two rows in three, so the index misses entries index_am_support index_am_support btree builds an index over a columnar table 15;16;17;18;19 2026-09-23 pgcolumnar_index_build_range_scan errors, so CREATE INDEX fails index_am_support index_am_support gist answers its operator with the same rows the scan returns 15;16;17;18;19 2026-09-23 pgcolumnar_index_build_range_scan drops two rows in three, so the index misses entries diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index b28c42098..90d0a683b 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -1276,4 +1276,24 @@ suites_not_covered 247 # never 1664 + 7 = 1671 # dated 219 + 5 = 224 # 1671 + 224 == 1895 -checks_never_observed_red 1671 +# +# #1215 (PR #1286): 1671 -> 1674. Twelve new checks, nine dated, three premises: +# +# harness_selftest 600 5 checks: 4 dated, 1 premise +# harness_selftest 610 5 checks: 4 dated, 1 premise +# capability_sweep 2 checks: 1 dated, 1 premise; and the silent arm +# re-dated +# +# Every date comes from a mutation run on all five majors, merged one log at a +# time with --mutation and --target, replayed from the saved logs onto #1288's +# ledger. The part 600 mutation logs predate #1284's renames, so the three +# old-name rows they carried were removed again, as #1284 did. Carried onto +# #1302's ledger unchanged: #1291's rows are another suite, and the result is +# main's rows less the one re-dated row plus 13 (12 new, 1 re-dated), checked +# row by row. +# +# ledger rows 1895 + 12 = 1907 +# never 1671 + 3 = 1674 +# dated 224 + 9 = 233 +# 1674 + 233 == 1907 +checks_never_observed_red 1674 diff --git a/test/lib.sh b/test/lib.sh index 52c33324f..703a67c72 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -1971,17 +1971,29 @@ chunk_group_count() { # 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. +# +# EXCEPT PYARROW, which declines by default (#1215, owner's decision 2026-10-12). +# A box without pyarrow is a supported developer configuration, and the 25 suites +# that need it were the reddest thing such a box produced. The nightly installs +# pyarrow and stops when the suites' interpreter cannot import it, so CI cannot +# lose them silently; the run summary still names every suite that skipped. +# Every other capability keeps the rule above. pgc_skip() { # pgc_skip - local cap allow_one + local cap allow_one why cap="$(printf '%s' "$1" | tr '[:lower:]-' '[:upper:]_')" allow_one="PGC_ALLOW_MISSING_$cap" - if [ "${PGC_ALLOW_MISSING:-0}" = 1 ] || [ "${!allow_one:-0}" = 1 ]; then + why="" + if [ "$cap" = PYARROW ]; then + why="$1 is optional on a developer box, see #1215" + elif [ "${PGC_ALLOW_MISSING:-0}" = 1 ] || [ "${!allow_one:-0}" = 1 ]; then + why="waived by $allow_one or PGC_ALLOW_MISSING" + fi + if [ -n "$why" ]; then # The unwaived branch below records a FAIL. This one printed and left # PGC_CHECKS at zero, so waiving a dependency also erased the outcome -- # the same asymmetry check_timing had, in the function whose whole # subject is "a missing dependency is not a pass". - check_skip "$2" "SKIP $2 (waived by $allow_one or PGC_ALLOW_MISSING)" \ - "waived by $allow_one or PGC_ALLOW_MISSING" + check_skip "$2" "SKIP $2 ($why)" "$why" pgc_summary fi pgc_record FAIL "$2" "FAIL $2" diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 5d9ebc594..a4437f9c5 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -136,6 +136,7 @@ behaviour, the source of that number is named. - [88. test_storage_row_repair.py: the alpha5 release notes' repair procedure works as written](#88-test_storage_row_repairpy-the-alpha5-release-notes-repair-procedure-works-as-written) - [89. test_docs_cross_database_query.py: no documented query asks another database for its extensions](#89-test_docs_cross_database_querypy-no-documented-query-asks-another-database-for-its-extensions) - [90. test_partman_columnar.py: pg_partman makes columnar partitions with one setting](#90-test_partman_columnarpy-pg_partman-makes-columnar-partitions-with-one-setting) +- [91. test_capability_decline.py: a missing pyarrow declines by name, and nothing declines silently](#91-test_capability_declinepy-a-missing-pyarrow-declines-by-name-and-nothing-declines-silently) ## 1. How to read a test in here @@ -6455,3 +6456,30 @@ Removal proof, one mutation per arm, every major on the shell twin: the drop hoo | test | what it holds | | --- | --- | | `test_pg_partman_makes_columnar_partitions_with_one_setting` | pg_partman installed; without the setting every partition is heap; with it, a `DEFAULT` partition exists and every partition is columnar; the rows land columnar and read back as heap does; maintenance drops partitions past retention, creates the ones ahead columnar, and leaves no orphaned storage or row-group row; and it holds `AccessExclusiveLock` on the parent | + +## 91. test_capability_decline.py: a missing pyarrow declines by name, and nothing declines silently + +The pytest half of selftest parts 600 and 610, held by the Python harness's own machinery. It does not source, call or read `lib.sh`, `capability_sweep.sh` or either part: the twins are independent (the first version drove `lib.sh` from Python, and the harness-independence inventory refused it). + +The owner's rule of 2026-10-12 (#1215): a missing capability may be bypassed as a named skip, the run stays green, and no test may be silent. On this side that is `expect.declines(capability, *, name)`, a fourth record verdict `DECLINED`, and `expect.require_module(module, *, name)`. Five holes are closed, from OffgridwithJD's review of #1286: + +1. where `PGC_REQUIRE` names the capability, as CI does, a decline FAILS; +2. a decline must be the test's first outcome, and no check may follow it; +3. only an ABSENT module declines: a broken install (an `ImportError` from inside) fails; +4. a test file that imports pyarrow outside `require_module` is refused, in each of six import spellings, and each one is proven seen; +5. the run's accounting line carries `+ N declined` when there are any, and each decline is printed by name with its capability. + +Written red first: all seven failed before `pgc_vacuity` had the methods. The four failure arms pin their failure's MESSAGE, because a missing method also fails an inner test, so outcomes alone could not tell "refused for the right reason" from "crashed". + +### Every test + +| test | what it holds | +| --- | --- | +| `test_a_missing_pyarrow_declines_by_name_and_the_run_stays_green` | the inner run passes and exits 0; the accounting line carries `+ 1 declined`; the decline is printed with its node id and capability | +| `test_pgc_require_turns_a_decline_into_a_failure` | with `PGC_REQUIRE=pyarrow` the same test fails, and says pyarrow is required | +| `test_a_decline_after_a_counted_check_is_refused` | a decline after a recorded check is refused as not the first outcome | +| `test_a_check_after_a_decline_is_refused` | a check recorded after a decline is refused | +| `test_only_a_declinable_capability_may_decline` | `btree_gist` cannot decline; it is not in the closed list | +| `test_a_broken_install_fails_rather_than_declines` | a `pyarrow` that raises `ImportError` from inside fails with its own error | +| `test_a_missing_dependency_of_pyarrow_fails_rather_than_declines` | a `pyarrow` that cannot import its own dependency raises `ModuleNotFoundError` naming that dependency, and fails rather than declines; shown red with `require_module`'s `.name` check deleted (OffgridwithJD, review of #1286) | +| `test_every_unguarded_import_spelling_is_refused` | each of six spellings, `__import__` among them, is named and refused; a file using `require_module` is not | diff --git a/test/pytest/compare_to_bash.py b/test/pytest/compare_to_bash.py index fcbfd2102..329b5e694 100755 --- a/test/pytest/compare_to_bash.py +++ b/test/pytest/compare_to_bash.py @@ -148,6 +148,9 @@ def test_a_role_with_only_schema_usage_is_refused(..., func, name): "cannot_run": None, "plan_marker": None, "plan_node": None, + # #1215: both take the check name as a required KEYWORD, like cannot_run's. + "declines": None, + "require_module": None, } diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index fe2dabe79..019b63b3d 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -263,7 +263,11 @@ # answer for a change that touches only the layer and its own guard. # 404 -> 405 for #1288: test_docs_cross_database_query.py, cluster-free, in NO_CLUSTER. # Re-derived by collection on 038600e (after #1303): `405 tests collected`. -guard_tests 405 +# 405 -> 412 for #1215: test_capability_decline.py, seven tests, cluster-free and in +# NO_CLUSTER. Re-derived by collection on 73d9663: `412 tests collected`. +# guard_tests 412 -> 413 for test_a_missing_dependency_of_pyarrow_fails_rather_than_declines +# (#1286 review), collected over the NO_CLUSTER files on this tree. +guard_tests 413 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index 64152697f..cd038a55a 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -20,6 +20,7 @@ """ import ast +import os import numbers import pathlib import re @@ -232,7 +233,15 @@ def note_write(nodeid, cur): # no outcome recorded, which is the reconciliation failure itself. SKIP has no # counterpart here: a bare skip is refused at collection, and the honest form is # expect.cannot_run(), which records UNRUN. -RECORD_VERDICTS = ("PASS", "FAIL", "UNRUN") +RECORD_VERDICTS = ("PASS", "FAIL", "UNRUN", "DECLINED") + +# The capabilities a test may DECLINE on, rather than declare unrunnable (#1215, the +# owner's decision of 2026-10-12, mirrored from lib.sh's pgc_skip). A decline is a +# named outcome that leaves the run green; everything else that cannot run goes +# through cannot_run and ends the run INCOMPLETE. Closed, so "optional" cannot grow +# by one call site at a time. PGC_REQUIRE (comma-separated) names capabilities that +# must be present here, as CI does, and turns a decline of them into a failure. +DECLINABLE_CAPABILITIES = ("pyarrow",) class VacuityError(AssertionError): @@ -347,6 +356,7 @@ def __init__(self, nodeid): self.nodeid = nodeid self._records = [] self.unrunnable = None + self.declined = None @property def records(self): @@ -420,6 +430,11 @@ def _record(self, name, verdict="PASS", reason=""): operation; it is phase 2's work and is not claimed here. What IS claimed here is the count, which the probe above shows is 2 and correct. """ + if self.declined is not None and verdict != "DECLINED": + raise VacuityError( + f"{name}: a check recorded after this test declined on " + f"{self.declined!r}. A decline means the test measured nothing, so it " + f"must be the test's only outcome: return right after declines().") self._records.append(_Record(name, verdict, reason)) # -- numbers ----------------------------------------------------------- @@ -1023,6 +1038,64 @@ def plan_marker(self, plan, key, *, name=None, absent=False): ) # -- the third state --------------------------------------------------- + @_resolving + def declines(self, capability, *, name, detail=""): + """Decline this test because an OPTIONAL capability is absent (#1215). + + A named outcome, verdict DECLINED, that leaves the run green. The shell's + pgc_skip does the same for pyarrow; the two harnesses do not call each + other. Five things keep it from becoming a way to stop asserting quietly + (OffgridwithJD, review of #1286): + + - only a capability in DECLINABLE_CAPABILITIES may decline; + - where PGC_REQUIRE names the capability -- CI -- this FAILS instead; + - it must be the test's first outcome, and no check may follow it; + - require_module() declines only on the module being ABSENT; + - each decline is printed by name, and counted in the accounting line. + """ + if capability not in DECLINABLE_CAPABILITIES: + raise VacuityError( + f"{name}: {capability!r} is not a declinable capability " + f"{DECLINABLE_CAPABILITIES}. A missing dependency that is not optional " + f"is unrunnable: use cannot_run('MISSING_DEPENDENCY', ...).") + if self._records: + raise VacuityError( + f"{name}: a decline must be the test's first outcome, and this test " + f"already recorded {len(self._records)} check(s). A test that half ran " + f"and then declined would read as one that measured nothing.") + required = [c.strip() for c in os.environ.get("PGC_REQUIRE", "").split(",") + if c.strip()] + # ONE record, after every refusal: a VacuityError raised after the record + # would be recorded as a failed assertion (test_every_refusal_precedes_its_ + # record). Where the capability is required, the record is a FAIL and the + # AssertionError below is the ordinary failure of a comparison. + must_have = capability in required + self._record(name, verdict="FAIL" if must_have else "DECLINED", + reason=f"{capability}: {detail}".rstrip(": ")) + if must_have: + raise AssertionError( + f"{name}: PGC_REQUIRE names {capability}, so it must be present here, " + f"and it is not: {detail}") + self.declined = capability + + def require_module(self, module, *, name): + """-> the imported module, or None after declining because it is ABSENT. + + Declines only on ModuleNotFoundError naming the module or its top-level + package. A module that is installed but broken -- an ImportError raised from + inside it, or an OSError loading its library -- propagates and fails the test, + because a broken install is not an absent one. + """ + import importlib + top = module.split(".")[0] + try: + return importlib.import_module(module) + except ModuleNotFoundError as e: + if e.name not in (module, top): + raise + self.declines(top, name=name, detail=f"{module} is not installed") + return None + @_resolving def cannot_run(self, reason, detail="", *, name=None): """Declare this test unrunnable. Not a pass, and not a silent skip. @@ -1296,6 +1369,7 @@ class _RecordCollector: def __init__(self): self.records = [] # (nodeid, verdict, name) self.offences = [] + self.declined = [] # (nodeid, name, reason): the capability is in reason def pytest_runtest_logreport(self, report): if report.when != "call": @@ -1305,6 +1379,8 @@ def pytest_runtest_logreport(self, report): for key, value in getattr(report, "user_properties", ()): if key == "pgc_records_held": held = value + elif key == "pgc_declined": + self.declined.extend((report.nodeid, n, r) for n, r in value) elif key == "pgc_records": arrived = list(value) if held is None and arrived is None: @@ -1351,6 +1427,13 @@ def pytest_runtest_makereport(item, call): report.user_properties.append(("pgc_records_held", rec.count)) report.user_properties.append( ("pgc_records", [(r.verdict, r.name) for r in rec.records])) + # A decline's capability rides in its reason, which pgc_records does not + # carry. Its own property, like pgc_unrunnable, so the record pairs that + # the reconciliation compares keep their shape. + if rec.declined is not None: + report.user_properties.append( + ("pgc_declined", [(r.name, r.reason) for r in rec.records + if r.verdict == "DECLINED"])) return report @@ -1367,6 +1450,13 @@ def pytest_terminal_summary(terminalreporter): claims in one test and one in another, where a per-test count would say 2 and the records say 5. """ + # DIRECT IMPORTS OF A DECLINABLE CAPABILITY, named whatever collection did. A file + # whose top-level import fails yields no items, so no hook that walks items ever + # sees it -- measured: the scan found the file and nothing printed. + # pytest_sessionfinish turns the same list into a failure. + for site in getattr(terminalreporter.config, "pgc_capability_imports", None) or []: + terminalreporter.write_line(f"REFUSED {site}") + collector = getattr(terminalreporter.config, "pgc_unrunnable", None) if collector is not None and collector.items: terminalreporter.write_line("") @@ -1381,10 +1471,17 @@ def pytest_terminal_summary(terminalreporter): for _nodeid, verdict, _name in records.records: if verdict in tally: tally[verdict] += 1 + if records.declined: + terminalreporter.write_line("") + for nodeid, nm, why in records.declined: + terminalreporter.write_line(f"DECLINED {nodeid}: {why}: {nm}") terminalreporter.write_line(f"checks run: {len(records.records)}") + # DECLINED joins the line only when present, so a run with no declines prints the + # line every reader and guard already parses, byte for byte. + shown = [v for v in RECORD_VERDICTS if v != "DECLINED" or tally[v]] terminalreporter.write_line( "accounting: " - + " + ".join(f"{tally[v]} {v.lower()}" for v in RECORD_VERDICTS) + + " + ".join(f"{tally[v]} {v.lower()}" for v in shown) + f" = {sum(tally.values())}" ) @@ -1454,6 +1551,12 @@ def pytest_sessionfinish(session, exitstatus): an unrunnable test is a failure, because the failure is the more urgent fact. So this only ever moves a run OFF zero, and never off a non-zero status. """ + # A test file that imports a declinable capability outside require_module is + # refused (#1286 review): it would fail on a box without the module, or pass + # having caught its own ImportError, and either way the decline is not named. + if getattr(session.config, "pgc_capability_imports", None) and session.exitstatus == 0: + session.exitstatus = 1 + # THE RECONCILIATION, FIRST, because it is a statement about whether the run # can be believed at all rather than about one test (#937 phase 3). # @@ -1885,6 +1988,63 @@ def _sorted_ordered_sites(path): _order_killed_names, _sorted_ordered_sites = _order_guard() +def _capability_import_sites(path, capabilities=DECLINABLE_CAPABILITIES): + """-> one line per place a test file imports a declinable capability directly. + + A test that imports pyarrow itself, instead of through expect.require_module, + fails at import when pyarrow is absent, or worse, wraps the import in its own + try and passes having measured nothing. Either way the decline is not named. + Read from the AST, so a comment or a string that mentions the module cannot + trip it, and every spelling is covered: `import m`, `import m.sub as x`, + `from m import x`, `from m.sub import x`, `importlib.import_module('m...')` + and `__import__('m...')` (OffgridwithJD, review of #1286). + """ + try: + tree = ast.parse(pathlib.Path(path).read_text(), filename=str(path)) + except (OSError, SyntaxError, UnicodeDecodeError): + return [] + base = pathlib.Path(path).name + sites = [] + for node in ast.walk(tree): + names = [] + if isinstance(node, ast.Import): + names = [a.name for a in node.names] + elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module: + names = [node.module] + elif isinstance(node, ast.Call) and node.args: + f = node.func + fname = f.attr if isinstance(f, ast.Attribute) else ( + f.id if isinstance(f, ast.Name) else "") + arg = node.args[0] + if fname in ("import_module", "__import__") and isinstance(arg, ast.Constant) \ + and isinstance(arg.value, str): + names = [arg.value] + for n in names: + top = n.split(".")[0] + if top in capabilities: + sites.append(f"{base} imports {top} outside require_module " + f"(line {node.lineno})") + return sites + + +def pytest_collect_file(file_path, parent): + """Scan each test file for direct imports of a declinable capability. + + At FILE collection, before the module is imported: a file whose top-level + `import pyarrow` fails produces no items at all, so a scan over items would + never see the one file it exists for. + """ + if file_path.suffix == ".py" and file_path.name.startswith("test_"): + found = _capability_import_sites(file_path) + if found: + store = getattr(parent.config, "pgc_capability_imports", None) + if store is None: + store = [] + parent.config.pgc_capability_imports = store + store.extend(found) + return None + + def _broad_except_sites(path): try: tree = ast.parse(pathlib.Path(path).read_text()) diff --git a/test/pytest/requirements-test.txt b/test/pytest/requirements-test.txt index 6e3c6672f..147de622f 100644 --- a/test/pytest/requirements-test.txt +++ b/test/pytest/requirements-test.txt @@ -7,3 +7,4 @@ pytest==9.1.1 pytest-xdist==3.8.0 psycopg[binary]==3.3.5 +pyarrow==23.0.1 diff --git a/test/pytest/test_capability_decline.py b/test/pytest/test_capability_decline.py new file mode 100644 index 000000000..d7ba9aa1c --- /dev/null +++ b/test/pytest/test_capability_decline.py @@ -0,0 +1,176 @@ +"""A missing pyarrow declines a pytest test by name; nothing declines silently (#1215). + +The pytest half of selftest parts 600 and 610, held by the PYTHON harness's own +machinery. It does not source, call or read lib.sh, capability_sweep.sh or either +part: the twins are independent, and each harness answers for itself. + +The owner's rule of 2026-10-12: a missing capability may be bypassed as a named skip, +the run stays green, and no test may be silent or vacuous. On this side that is +`expect.declines` and `expect.require_module`, with five ways a decline could hide +something closed off (OffgridwithJD, review of #1286): + + 1 where pyarrow is supposed to be (CI, PGC_REQUIRE=pyarrow) a decline FAILS + 2 a decline is a test's only outcome: not after a check, and no check after it + 3 only an ABSENT module declines; a broken install fails + 4 a test file that imports pyarrow outside require_module is refused, in every + import spelling, each proven seen + 5 the run's accounting line carries the declines + +Runs pytest inside pytest; needs no cluster and no driver. +""" + +# Absent for ONE inner test, through monkeypatch so it is undone: pytester runs +# in-process, and a module-level sys.modules entry would leak into every later test. +ABSENT = " import sys\n" + + +def test_a_missing_pyarrow_declines_by_name_and_the_run_stays_green(pytester, monkeypatch, expect): + # The caller's PGC_REQUIRE must not decide this test (CI exports it). + monkeypatch.delenv("PGC_REQUIRE", raising=False) + pytester.makepyfile(ABSENT + """ + def test_inner_declines_green(expect, monkeypatch): + monkeypatch.setitem(sys.modules, "pyarrow", None) + pa = expect.require_module("pyarrow", name="the parquet fixture can be written") + assert pa is None +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a declined test leaves the run green", passed=1, failed=0) + expect.num(result.ret, 0, "and the run exits 0") + out = result.stdout.str() + expect.contains(out, "+ 1 declined", + "the accounting line carries the decline (hole 5)") + expect.contains(out, "DECLINED test_a_missing_pyarrow_declines_by_name_and_the_run_stays_green.py::test_inner_declines_green: pyarrow", + "and the decline is named, with its capability") + + +def test_pgc_require_turns_a_decline_into_a_failure(pytester, monkeypatch, expect): + monkeypatch.setenv("PGC_REQUIRE", "pyarrow") + pytester.makepyfile(ABSENT + """ + def test_inner_required_fails(expect, monkeypatch): + monkeypatch.setitem(sys.modules, "pyarrow", None) + expect.require_module("pyarrow", name="the parquet fixture can be written") +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "with PGC_REQUIRE=pyarrow a missing pyarrow fails (hole 1)", + passed=0, failed=1) + expect.contains(result.stdout.str(), "PGC_REQUIRE names pyarrow", + "and it fails BECAUSE pyarrow is required, not for another reason") + + +def test_a_decline_after_a_counted_check_is_refused(pytester, monkeypatch, expect): + # The caller's PGC_REQUIRE must not decide this test (CI exports it). + monkeypatch.delenv("PGC_REQUIRE", raising=False) + pytester.makepyfile(""" + def test_half_ran(expect): + expect.num(1, 1, "a check that ran") + expect.declines("pyarrow", name="too late to decline") +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a decline after a counted check is refused (hole 2)", + passed=0, failed=1) + expect.contains(result.stdout.str(), "a decline must be the test's first outcome", + "and it is refused for that reason") + + +def test_a_check_after_a_decline_is_refused(pytester, monkeypatch, expect): + # The caller's PGC_REQUIRE must not decide this test (CI exports it). + monkeypatch.delenv("PGC_REQUIRE", raising=False) + pytester.makepyfile(""" + def test_kept_going(expect): + expect.declines("pyarrow", name="declined") + expect.num(1, 1, "a check after the decline") +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a check after a decline is refused (hole 2)", + passed=0, failed=1) + expect.contains(result.stdout.str(), "after this test declined", + "and it is refused for that reason") + + +def test_only_a_declinable_capability_may_decline(pytester, monkeypatch, expect): + # The caller's PGC_REQUIRE must not decide this test (CI exports it). + monkeypatch.delenv("PGC_REQUIRE", raising=False) + pytester.makepyfile(""" + def test_wrong_capability(expect): + expect.declines("btree_gist", name="not optional") +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a capability outside the closed list cannot decline", + passed=0, failed=1) + expect.contains(result.stdout.str(), "is not a declinable capability", + "and it is refused for that reason") + + +def test_a_broken_install_fails_rather_than_declines(pytester, monkeypatch, expect): + # The caller's PGC_REQUIRE must not decide this test (CI exports it). + monkeypatch.delenv("PGC_REQUIRE", raising=False) + pkg = pytester.mkpydir("pyarrow") + (pkg / "__init__.py").write_text("raise ImportError('pyarrow built against another ABI')\n") + pytester.syspathinsert() + pytester.makepyfile(""" + import sys + + def test_inner_broken_install_fails(expect, monkeypatch): + # Drop any pyarrow already imported in this process, so the broken stub is + # what the import finds. + for k in [k for k in sys.modules if k == "pyarrow" or k.startswith("pyarrow.")]: + monkeypatch.delitem(sys.modules, k) + expect.require_module("pyarrow", name="the parquet fixture can be written") +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "an ImportError from inside an installed pyarrow fails (hole 3)", + passed=0, failed=1) + expect.contains(result.stdout.str(), "pyarrow built against another ABI", + "and the failure is the broken install's own error, not a decline") + + +def test_a_missing_dependency_of_pyarrow_fails_rather_than_declines(pytester, monkeypatch, expect): + # An installed pyarrow that cannot import one of ITS dependencies raises + # ModuleNotFoundError too, but naming the dependency. Declining on that would + # green a broken install (hole 3); only a missing pyarrow itself declines. + monkeypatch.delenv("PGC_REQUIRE", raising=False) + pkg = pytester.mkpydir("pyarrow") + (pkg / "__init__.py").write_text("import numpy_is_not_here\n") + pytester.syspathinsert() + pytester.makepyfile(""" + import sys + + def test_inner_missing_dependency_fails(expect, monkeypatch): + for k in [k for k in sys.modules if k == "pyarrow" or k.startswith("pyarrow.")]: + monkeypatch.delitem(sys.modules, k) + expect.require_module("pyarrow", name="the parquet fixture can be written") +""") + result = pytester.runpytest("-p", "pgc_vacuity") + expect.outcomes(result, "a pyarrow missing its own dependency fails (hole 3)", + passed=0, failed=1) + expect.contains(result.stdout.str(), "No module named 'numpy_is_not_here'", + "and the failure names the missing dependency, not a decline") + + +SPELLINGS = { + "test_spell_dunder.py": "m = __import__('pyarrow')\n", + "test_spell_import.py": "import pyarrow\n", + "test_spell_import_sub.py": "import pyarrow.parquet as pq\n", + "test_spell_from.py": "from pyarrow import table\n", + "test_spell_from_sub.py": "from pyarrow.parquet import write_table\n", + "test_spell_importlib.py": "import importlib\nm = importlib.import_module('pyarrow.parquet')\n", +} + + +def test_every_unguarded_import_spelling_is_refused(pytester, monkeypatch, expect): + # The caller's PGC_REQUIRE must not decide this test (CI exports it). + monkeypatch.delenv("PGC_REQUIRE", raising=False) + for fname, body in SPELLINGS.items(): + pytester.makepyfile(**{fname[:-3]: body + "\ndef test_x(expect):\n expect.num(1, 1, 'x')\n"}) + pytester.makepyfile(test_guarded=""" + def test_guarded(expect): + expect.require_module("pyarrow", name="guarded") +""") + result = pytester.runpytest("-p", "pgc_vacuity") + out = result.stdout.str() + seen = [f for f in SPELLINGS if f"{f[:-3]}.py imports pyarrow outside require_module" in out] + expect.num(len(seen), len(SPELLINGS), + "premise: the scan finds each of the six import spellings (hole 4)") + expect.num(1 if result.ret != 0 else 0, 1, "and a run carrying them is refused") + expect.num(out.count("test_guarded.py imports pyarrow"), 0, + "while a file that goes through require_module is not named") diff --git a/test/pytest/test_failed_query_sentinel.py b/test/pytest/test_failed_query_sentinel.py index d2bbfb241..f00da4f3c 100644 --- a/test/pytest/test_failed_query_sentinel.py +++ b/test/pytest/test_failed_query_sentinel.py @@ -67,6 +67,8 @@ def _is_comparison(fn): "plan": "EXPLAIN plan, not a query value", "exc": "exception object, not a query value", "reason": "unrunnable declaration, not a comparison", + "capability": "a decline on an optional capability (#1215), not a comparison", + "module": "an optional module imported or declined on (#1215), not a comparison", } diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index f9441a811..d5b16c956 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -73,6 +73,9 @@ "test_suite_accounting.py", # #1288: reads docs/*.md and README.md only. "test_docs_cross_database_query.py", + # #1215: inner pytest runs of expect.declines and require_module. Needs no + # cluster and no driver; it never reaches into the shell harness. + "test_capability_decline.py", # The classifier's own controls, split out of this file so the job whose file # list IS this list actually runs them. This file cannot be in the list: it # hands cluster-bound file names to pytest, so it needs what they need. diff --git a/test/selftest/600-a-missing-pyarrow-declines.sh b/test/selftest/600-a-missing-pyarrow-declines.sh new file mode 100644 index 000000000..07e09cba1 --- /dev/null +++ b/test/selftest/600-a-missing-pyarrow-declines.sh @@ -0,0 +1,61 @@ +# ---- a missing pyarrow declines a suite; other dependencies still fail ----- +# +# THE OWNER'S DECISION, 2026-10-12 (#1215). When pyarrow is absent, the 25 suites +# that call `pgc_skip pyarrow` report SKIPPED and exit 66, and the run does not +# fail. Every other missing dependency still FAILS unless it is waived, because +# the decision was about pyarrow only: `temporal.sh` without btree_gist is still +# a box that cannot gate that suite. +# +# Why a skip is safe for pyarrow and was not before: the nightly installs pyarrow +# and refuses to continue when the suites' interpreter cannot import it, so CI +# cannot lose these suites silently. A developer box without pyarrow is a +# supported configuration, and the run summary still names every skipped suite. +# +# The suites here source lib.sh and call pgc_skip without pgc_setup, so no +# cluster starts. Each differs from the others in one respect. + +_mpd_lib="$PGC_TESTDIR/lib.sh" +_mpd_dir="$PGC_WORKDIR/missing_pyarrow"; mkdir -p "$_mpd_dir" + +check "premise: lib.sh defines pgc_skip" \ + "$(grep -c '^pgc_skip()' "$_mpd_lib")" "1" + +_mpd_make() { # _mpd_make NAME BODY + { + printf '#!/usr/bin/env bash\n' + printf '. "%s"\n' "$_mpd_lib" + printf 'PGC_SUITE=%s\n' "$1" + printf '%s\n' "$2" + } > "$_mpd_dir/$1.sh" + chmod 755 "$_mpd_dir/$1.sh" +} + +# Run with every waiver cleared, so the caller's environment cannot decide the +# answer. `env -u` on a name that is not set is a no-op. +_mpd_run() { # _mpd_run NAME [VAR=VALUE ...] -> " " + local name="$1" out rc; shift + out="$(env -u PGC_ALLOW_MISSING -u PGC_ALLOW_MISSING_PYARROW \ + -u PGC_ALLOW_MISSING_BTREE_GIST "$@" bash "$_mpd_dir/$name.sh" 2>&1)"; rc=$? + printf '%s %s' "$rc" "$(printf '%s' "$out" | grep -oE '(PASSED|FAILED|INCOMPLETE|SKIPPED)' | tail -1)" +} + +_mpd_records() { # _mpd_records NAME VERDICT -> how many RESULT records carry VERDICT + env -u PGC_ALLOW_MISSING -u PGC_ALLOW_MISSING_PYARROW \ + -u PGC_ALLOW_MISSING_BTREE_GIST bash "$_mpd_dir/$1.sh" 2>&1 | + awk -F'\t' -v v="$2" '$1 == "RESULT" && $5 == v' | wc -l | tr -d ' ' +} + +_mpd_make nopyarrow 'pgc_skip pyarrow "pyarrow is required to write the fixture"' +_mpd_make nogist 'pgc_skip btree_gist "btree_gist is required for WITHOUT OVERLAPS"' + +check "a missing pyarrow declines the suite instead of failing it" \ + "$(_mpd_run nopyarrow)" "66 SKIPPED" +check "and it records a skip under the check's own name, not a failure" \ + "$(_mpd_records nopyarrow SKIP)/$(_mpd_records nopyarrow FAIL)" "1/0" + +check "control: a missing btree_gist still fails the suite" \ + "$(_mpd_run nogist)" "1 FAILED" +check "control: and an explicit waiver still turns that failure into a skip" \ + "$(_mpd_run nogist PGC_ALLOW_MISSING_BTREE_GIST=1)" "66 SKIPPED" + +rm -rf "$_mpd_dir" diff --git a/test/selftest/610-a-declined-capability-is-not-silent.sh b/test/selftest/610-a-declined-capability-is-not-silent.sh new file mode 100644 index 000000000..72ead5d4e --- /dev/null +++ b/test/selftest/610-a-declined-capability-is-not-silent.sh @@ -0,0 +1,27 @@ +# ---- a declined capability is not a silent one ------------------------------ +# +# capability_sweep.sh classifies each pyarrow suite by what it lost when pyarrow +# was masked. #1215 made 25 of them decline with a SKIP, and the owner's rule of +# 2026-10-12 is that a missing capability may be skipped but no test may be +# silent. So the verdict has four outcomes and the sweep must keep the last two +# apart. The sweep runs nightly and needs every pyarrow suite twice; the verdict +# is a pure function, so its cases are asserted here, from the sweep's own text. + +_dcs_src="$PGC_TESTDIR/capability_sweep.sh" +_dcs_fn="$(sed -n '/^pgc_capability_verdict() {/,/^}/p' "$_dcs_src")" + +check_num "premise: the sweep defines its verdict where this part reads it" \ + "$(printf '%s\n' "$_dcs_fn" | grep -c '^pgc_capability_verdict() {')" "1" + +eval "$_dcs_fn" + +check_text "the verdict: no name lost is preserved" \ + "$(pgc_capability_verdict 0 0 0)" "preserved" +check_text "the verdict: names lost beside a failure are refused" \ + "$(pgc_capability_verdict 3 1 0)" "refused" +check_text "the verdict: names lost beside a skip naming the capability are declined" \ + "$(pgc_capability_verdict 3 0 1)" "declined" +check_text "the verdict: names lost with neither a failure nor a skip are silent" \ + "$(pgc_capability_verdict 3 0 0)" "silent" + +unset -f pgc_capability_verdict diff --git a/test/selftest/parts.manifest b/test/selftest/parts.manifest index 5881cf592..e1964777a 100644 --- a/test/selftest/parts.manifest +++ b/test/selftest/parts.manifest @@ -56,3 +56,5 @@ 570-a-checkout-test-must-ask-git.sh 580-a-hand-rebuild-must-record.sh 590-a-declared-suite-must-account.sh +600-a-missing-pyarrow-declines.sh +610-a-declined-capability-is-not-silent.sh