From 2dab9c8307efb9ea65030174a88328ede4bf8bb1 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 15:15:23 -0500 Subject: [PATCH 1/6] Add test/install foundation: TEST_LOAD_SOURCE modes, dependency guard, quoting-requiring schema test Builds the U&U (update & upgrade) test infrastructure that doesn't require a real second extension_drop version or pg_upgrade CI to already exist: - PGXNTOOL_ENABLE_TEST_INSTALL = yes, with test/install/load.sql as the committed-once installer for the extension (no test roles exist for this extension, so unlike cat_tools there's nothing role-related to add). - TEST_LOAD_SOURCE (fresh/update/existing) GUC/make-var switch, parse-time validated, exported unconditionally, read in load.sql without missing_ok. `existing` mode is fully exercised locally (verified against a real, already-installed database, including the failure path when the extension is genuinely absent). `update` mode is wired up and structurally verified end-to-end, but extension_drop has no real prior released version to update FROM yet -- the Makefile refuses to run it without TEST_UPDATE_FROM set explicitly, and no CI leg exercises it in this repo today. - Dependency guard (test/sql/dependency_guard.sql): a view depending on extension_drop__commands' row type blocks a non-CASCADE DROP EXTENSION; proven by actually attempting the drop and asserting failure, not assumed. - test/sql/schema.sql's custom-schema test names renamed to mixed case (requires identifier quoting), reusing its existing coverage rather than adding a new schema-testing dimension. - ci.yml: run `make test && make verify-results` instead of pg-build-test, so a real regression actually fails the build (pgxntool's .IGNORE: installcheck otherwise reports green regardless of test results, per RELEASE.md's existing note about PRs #6/#7). Moving the extension's own installation into test/install/load.sql required adapting every test file that used to install it per-test in a rolled-back transaction (test/deps.sql, test/sql/simple.sql, test/sql/schema.sql, test/sql/zzz_build.sql) to work against the new committed-once install instead, since an extension name is a database-wide singleton. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 10 +- Makefile | 59 +++++++++++ test/deps.sql | 38 ++----- test/expected/dependency_guard.out | 6 ++ test/install/.gitignore | 14 +++ test/install/load.sql | 162 +++++++++++++++++++++++++++++ test/sql/dependency_guard.sql | 66 ++++++++++++ test/sql/schema.sql | 14 +++ test/sql/simple.sql | 18 ++-- 9 files changed, 350 insertions(+), 37 deletions(-) create mode 100644 test/expected/dependency_guard.out create mode 100644 test/install/.gitignore create mode 100644 test/install/load.sql create mode 100644 test/sql/dependency_guard.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc82d51..354b400 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,7 +60,15 @@ jobs: - name: Check out the repo uses: actions/checkout@v7 - name: Test on PostgreSQL ${{ matrix.pg }} - run: pg-build-test + # pgxntool's base.mk marks installcheck .IGNORE, so a plain `make + # test` (what pg-build-test itself invokes under the hood) exits 0 + # even when every pg_regress test fails -- confirmed happening for + # real on PRs #6/#7 (RELEASE.md). `make verify-results` is the + # actual pass/fail signal (it scans for raw pgTAP failures and plan + # mismatches, not just installcheck's own exit code), so run it + # explicitly after `make test` instead of relying on pg-build-test + # alone. + run: make test && make verify-results # A single stable check name for use as a required status check in branch # protection. Matrix jobs produce names like "🐘 PostgreSQL 14" that change diff --git a/Makefile b/Makefile index cce47d3..960c018 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,62 @@ +# Run test/install/load.sql (extension install) COMMITTED, once, before the +# main pgTAP suite, via pgxntool's test/install feature. Set explicitly +# (rather than left to auto-detect) so an accidentally emptied test/install/ +# is a hard build error instead of silently falling back to "disabled". +# Must be set before `include pgxntool/base.mk` below -- base.mk reads it +# while parsing. +PGXNTOOL_ENABLE_TEST_INSTALL = yes + +# TEST_LOAD_SOURCE selects how test/install/load.sql installs extension_drop: +# - fresh (default): CREATE EXTENSION extension_drop (current version). +# - update: CREATE EXTENSION at TEST_UPDATE_FROM, then ALTER EXTENSION +# UPDATE -- to TEST_UPDATE_TO if set, otherwise to the current version. +# Running the SAME suite/expected output against the result asserts +# update behaves identically to a fresh install. NOTE: extension_drop has +# never had a real second released version (PGXN's only listing is +# 0.1.x from 2017, predating the current SQL entirely -- see HISTORY.asc +# and RELEASE.md), so TEST_UPDATE_FROM has no safe default; this mode is +# wired up and structurally ready, but there is nothing real to update +# FROM yet, and so no CI leg exercises it in this repo today. +# - existing: the extension is ALREADY installed (a real pg_upgrade, or an +# ALTER EXTENSION UPDATE done outside the suite). load.sql does not +# touch it; it only asserts presence + current version. Pair with +# CONTRIB_TESTDB= and EXTRA_REGRESS_OPTS=--use-existing to point +# pg_regress at that database instead of a throwaway one. +# +# Propagated to load.sql as a GUC: pg_regress doesn't forward make variables, +# but the psql processes it spawns inherit the environment, so PGOPTIONS +# reaches load.sql. Exported UNCONDITIONALLY so load.sql can read it without +# missing_ok and fail loudly if it didn't propagate, rather than silently +# defaulting to the wrong mode. The mode is also validated here at +# make-parse-time, so a typo like `TEST_LOAD_SOURCE=fresh ` or +# `TEST_LOAD_SOURCE=typo` fails immediately instead of quietly running the +# default. +TEST_LOAD_SOURCE ?= fresh +ifeq ($(filter $(TEST_LOAD_SOURCE),fresh update existing),) +$(error TEST_LOAD_SOURCE must be 'fresh', 'update' or 'existing', got '$(TEST_LOAD_SOURCE)') +endif + +# update-mode version range (load.sql only reads these in update mode). +# Empty TEST_UPDATE_TO means "update to the current default_version". There +# is no safe default for TEST_UPDATE_FROM (see above) -- require it +# explicitly rather than pointing it at a version that doesn't exist. +TEST_UPDATE_FROM ?= +TEST_UPDATE_TO ?= +ifeq ($(TEST_LOAD_SOURCE),update) + ifeq ($(strip $(TEST_UPDATE_FROM)),) +$(error TEST_UPDATE_FROM must be set when TEST_LOAD_SOURCE=update -- extension_drop has no prior released version yet to default it to) + endif +endif + +export PGOPTIONS := $(PGOPTIONS) -c extension_drop.test_load_mode=$(TEST_LOAD_SOURCE) -c extension_drop.test_update_from=$(TEST_UPDATE_FROM) -c extension_drop.test_update_to=$(TEST_UPDATE_TO) + +# make test-update == make test TEST_LOAD_SOURCE=update. Must recurse (a +# fresh $(MAKE)) rather than depend on `test`, so the parse-time +# TEST_LOAD_SOURCE conditional above re-evaluates with update set. +.PHONY: test-update +test-update: + $(MAKE) test TEST_LOAD_SOURCE=update + include pgxntool/base.mk # Explicit rather than relying on auto-detect (which enables this whenever diff --git a/test/deps.sql b/test/deps.sql index e98348d..f53733f 100644 --- a/test/deps.sql +++ b/test/deps.sql @@ -1,38 +1,16 @@ --- IF NOT EXISTS will emit NOTICEs, which is annoying -SET client_min_messages = WARNING; - -- Add any test dependency statements here -- Note: pgTap is loaded by setup.sql --- Re-enable notices -SET client_min_messages = NOTICE; - -\set TT extension_drop_test_table -CREATE TEMP TABLE :TT (i int); - /* - * :TEST_SCHEMA can be mixed-case (see test/sql/schema.sql), so it MUST be - * identifier-quoted here -- an unquoted interpolation would silently fold - * to lowercase and every test would end up running against a different, - * unquoted schema than the one it thinks it's using. + * extension_drop itself used to be (re)installed here, per test file. It's + * now installed ONCE, COMMITTED, by test/install/load.sql (pgxntool's + * test/install feature) before this suite runs at all -- this file no + * longer touches it. test/sql/schema.sql is the one test that actually + * drops/recreates the extension itself (that's what it's testing); every + * other test file just uses the extension load.sql already installed. */ -CREATE SCHEMA :"TEST_SCHEMA"; -SET search_path = :"TEST_SCHEMA", tap, "$user"; -/* - * Now load our extension. We don't use IF NOT EXISTs here because we want an - * error if the extension is already loaded (because we want to ensure we're - * getting the very latest version). - */ -SET client_min_messages = WARNING; -- Squelch notice from CASCADE -DO $$ BEGIN - IF current_setting('server_version_num')::int < 100000 THEN - CREATE EXTENSION IF NOT EXISTS cat_tools; - CREATE EXTENSION extension_drop ; - ELSE - EXECUTE $exec$CREATE EXTENSION extension_drop CASCADE$exec$; - END IF; -END$$; -SET client_min_messages = NOTICE; +\set TT extension_drop_test_table +CREATE TEMP TABLE :TT (i int); -- vi: expandtab ts=2 sw=2 diff --git a/test/expected/dependency_guard.out b/test/expected/dependency_guard.out new file mode 100644 index 0000000..5d22292 --- /dev/null +++ b/test/expected/dependency_guard.out @@ -0,0 +1,6 @@ +\set ECHO none +1..3 +ok 1 - Non-CASCADE DROP EXTENSION extension_drop is blocked by the dependency guard +ok 2 - extension_drop is still installed after the blocked drop attempt +ok 3 - Dependency guard view is still present after the blocked drop attempt +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/install/.gitignore b/test/install/.gitignore new file mode 100644 index 0000000..5ec5e32 --- /dev/null +++ b/test/install/.gitignore @@ -0,0 +1,14 @@ +# pg_regress writes the install step's result here, because the install +# schedule references tests as ../install/ -- one directory up from +# both test/expected/ and test/results/, which cancels back out to this same +# directory for both. So load.out is simultaneously "expected" and "actual": +# confirmed by hand (deliberately breaking load.sql's existing-mode assertion +# and seeing pg_regress still report the step "ok" while the real error text +# showed up in this file) that pg_regress can never see a diff for it here, +# regardless of what load.sql actually does. Never track it -- it would just +# be reformatted/overwritten noise on every run, not a real expectation. +load.out +# Precautionary: haven't observed pg_regress emit a *.diff for this +# self-comparing path locally, but if it ever does, it'd be equally +# meaningless to track for the same reason as load.out above. +install.out.diff diff --git a/test/install/load.sql b/test/install/load.sql new file mode 100644 index 0000000..b29636e --- /dev/null +++ b/test/install/load.sql @@ -0,0 +1,162 @@ +\set ECHO none +/* + * Committed-once installer for the test suite's one real dependency: the + * extension_drop extension itself. (No test roles exist for this extension + * -- see test/deps.sql -- so unlike cat_tools' equivalent load.sql, there is + * nothing role-related to install here.) + * + * pgxntool's test/install feature runs this file COMMITTED, in its own + * pg_regress session, BEFORE the main pgTAP suite, so the extension persists + * into every (rolled-back) test/sql/ file instead of each one re-installing + * it from scratch. test/deps.sql (run per test) no longer creates the + * extension; it only sets the psql variables the suite references. + * test/sql/schema.sql is the one exception: proving the schema-targeting + * pipeline works is its actual job, so it explicitly drops this committed + * install and recreates its own copies in schemas it chooses -- safely, + * since that all happens inside its own rolled-back transaction and never + * escapes that one file. + * + * Three modes, selected by the extension_drop.test_load_mode placeholder + * GUC, which the Makefile's TEST_LOAD_SOURCE block sets via PGOPTIONS + * (fresh is the default): + * - fresh (default): plain CREATE EXTENSION extension_drop (current + * version). + * - update: CREATE EXTENSION at an older version + * (extension_drop.test_update_from) then ALTER EXTENSION UPDATE -- to + * extension_drop.test_update_to when that GUC is non-empty, otherwise to + * the current default_version. NOTE: extension_drop has never had a + * real second released version -- PGXN's only listing (0.1.x, 2017) + * predates the current SQL entirely (see HISTORY.asc/RELEASE.md), so + * there is no version that could legitimately fill + * extension_drop.test_update_from today. This branch is wired up and + * structurally correct (the Makefile refuses to select this mode + * without TEST_UPDATE_FROM set explicitly), but has nothing real to + * update FROM yet, so it exists ready for the day a second version + * ships rather than because it's exercised in CI now. + * - existing: the extension is ALREADY installed (by a real binary + * pg_upgrade, or an ALTER EXTENSION UPDATE performed outside the + * suite). This branch must NOT drop/create/update it -- that would + * destroy exactly what "existing" mode exists to test. It only asserts + * presence + current version. + * + * Unlike cat_tools (whose control file pins schema = 'cat_tools' -- + * CREATE EXTENSION always lands in the same place, no choice), extension_drop's + * control file has no schema= line, so CREATE EXTENSION here lands wherever + * the ambient search_path resolves when this file runs -- a fresh psql + * session's default "$user", public, i.e. public in practice. That's a + * deliberate, useful default: it proves nothing in extension_drop's install + * script is hardcoded to a specific schema, the same property + * test/sql/schema.sql proves again explicitly for non-default schemas. + */ +SET client_min_messages = WARNING; + +/* + * The Makefile always exports extension_drop.test_load_mode via PGOPTIONS. + * Read it WITHOUT missing_ok: if the GUC did not propagate (a break + * anywhere in make -> PGOPTIONS -> env -> psql), current_setting errors here + * and the whole install step fails loudly, instead of silently defaulting + * and running the wrong suite. + */ +SELECT current_setting('extension_drop.test_load_mode') AS extension_drop_test_load_mode +\gset + +DO $DO$ +BEGIN + IF current_setting('extension_drop.test_load_mode') NOT IN ('fresh', 'update', 'existing') THEN + RAISE EXCEPTION + 'extension_drop.test_load_mode must be ''fresh'', ''update'' or ''existing'', got ''%''' + , current_setting('extension_drop.test_load_mode') + ; + END IF; +END +$DO$; + +SELECT + :'extension_drop_test_load_mode' = 'update' AS extension_drop_mode_update + , :'extension_drop_test_load_mode' = 'existing' AS extension_drop_mode_existing +\gset + +\if :extension_drop_mode_existing +/* + * existing mode: do NOT touch the extension. Assert it is installed and at + * the current default_version -- the pg_upgrade / external update the + * database just went through is exactly what the suite is validating, so + * dropping or reinstalling it would defeat the test. Fail loudly on absence + * or mismatch. + */ +DO $DO$ +DECLARE + v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'extension_drop'); + v_default text := (SELECT default_version FROM pg_available_extensions WHERE name = 'extension_drop'); +BEGIN + IF v_installed IS NULL THEN + RAISE EXCEPTION 'test_load_mode=existing but the extension_drop extension is not installed'; + END IF; + IF v_installed IS DISTINCT FROM v_default THEN + RAISE EXCEPTION + 'extension_drop is installed at version % but the current default_version is %' + , v_installed, v_default + ; + END IF; +END +$DO$; +\else +/* + * fresh / update: (re)install from scratch. Drop-first (CASCADE, matching + * cat_tools' own load.sql) so a re-run on a persistent cluster installs the + * newest build instead of reusing stale objects. + * + * extension_drop requires cat_tools. CASCADE auto-installs it on PG10+; + * event triggers exist from 9.3 but CREATE EXTENSION ... CASCADE was only + * added in PG10, so pre-PG10 needs cat_tools created explicitly first. This + * mirrors the check test/deps.sql used to do per-test before this file took + * over installing the extension. server_version_num is read once into a + * psql variable rather than a runtime DO block, so it can drive \if + * (client-side) branching around the VERSION-qualified CREATE EXTENSION + * calls below without needing psql variables interpolated inside a + * dollar-quoted DO body. + */ +DROP EXTENSION IF EXISTS extension_drop CASCADE; + +SELECT current_setting('server_version_num')::int >= 100000 AS extension_drop_pg10_plus +\gset + +\if :extension_drop_mode_update +SELECT current_setting('extension_drop.test_update_from') AS extension_drop_test_update_from \gset +SELECT current_setting('extension_drop.test_update_to') AS extension_drop_test_update_to \gset +/* + * Build the optional target clause once so a SINGLE ALTER EXTENSION covers + * both cases: an empty test_update_to yields '' (update to the current + * default_version -- the widest path); a non-empty value yields + * "TO ''". format(%L) quotes the version literal safely. + */ +SELECT CASE WHEN :'extension_drop_test_update_to' = '' THEN '' + ELSE format('TO %L', :'extension_drop_test_update_to') END + AS extension_drop_update_to_clause \gset + +\if :extension_drop_pg10_plus +CREATE EXTENSION extension_drop VERSION :'extension_drop_test_update_from' CASCADE; +\else +CREATE EXTENSION IF NOT EXISTS cat_tools; +CREATE EXTENSION extension_drop VERSION :'extension_drop_test_update_from'; +\endif + +/* + * Suppress the deprecation NOTICEs an update script might emit. + */ +SET client_min_messages = ERROR; +ALTER EXTENSION extension_drop UPDATE :extension_drop_update_to_clause; +SET client_min_messages = WARNING; +\else +\if :extension_drop_pg10_plus +CREATE EXTENSION extension_drop CASCADE; +\else +CREATE EXTENSION IF NOT EXISTS cat_tools; +CREATE EXTENSION extension_drop; +\endif +\endif +-- end \if :extension_drop_mode_update (fresh vs. update install branch) +\endif +-- end \if :extension_drop_mode_existing (existing mode skips the whole (re)install block) + +-- vi: expandtab ts=2 sw=2 diff --git a/test/sql/dependency_guard.sql b/test/sql/dependency_guard.sql new file mode 100644 index 0000000..9cdec96 --- /dev/null +++ b/test/sql/dependency_guard.sql @@ -0,0 +1,66 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * Dependency-guard proof. This protects a future "existing" mode CI run + * (extension_drop already installed by a real pg_upgrade, or an ALTER + * EXTENSION UPDATE done outside the suite -- see test/install/load.sql and + * the Makefile's TEST_LOAD_SOURCE machinery): nothing today stops an + * accidental CASCADE drop, a stray CI step, or a logic bug from silently + * destroying the real updated/upgraded objects that mode exists to + * validate -- after which the suite would quietly pass again against a + * fresh reinstall instead of the thing it was supposed to check. + * + * The fix is a view with a HARD pg_depend dependency on a stable + * extension_drop member: something the extension only ever extends, never + * drops or redefines. extension_drop__commands is exactly that -- it's the + * one state table every other object in this extension revolves around + * (get/add/remove/update, the sanity checks, and the event trigger all key + * off it); getting rid of it or changing its identity would be a rewrite of + * the whole extension, not a routine update. Referencing its row type + * (rather than a specific column) means the guard doesn't need updating + * even if a future release adds a column to it. extension_drop has no + * enums (unlike cat_tools' own guard, which types on an enum grown via ADD + * VALUE) -- a stable table's row type serves the same purpose here. + * + * This test PROVES the guard works instead of assuming the SQL is correct: + * it attempts the actual non-CASCADE DROP EXTENSION and asserts it fails, + * then asserts both the extension and the guard view are still present + * afterward. Everything here runs inside pgTAP's own rolled-back + * transaction, so the guard schema/view never leaks into any other test + * file. + */ +CREATE SCHEMA extension_drop_drop_guard; +CREATE VIEW extension_drop_drop_guard.guard AS + SELECT NULL::extension_drop__commands AS guarded_member; + +SELECT plan( + 0 + + 1 -- non-CASCADE drop is blocked + + 1 -- extension_drop is still installed + + 1 -- guard view still present +); + +/* + * 2BP01 = dependent_objects_still_exist: the standard error DROP ... RESTRICT + * (the implicit default for DROP EXTENSION) raises when another object + * depends on something the extension owns. throws_ok's 3-arg overload is + * (sql, message, description), not (sql, sqlstate, description) -- passing + * just the sqlstate there matches message text literally instead of + * checking the code, so the sqlstate AND the real message both need to be + * given explicitly (4-arg form) to actually check the error class. + */ +SELECT throws_ok( + $$DROP EXTENSION extension_drop$$ + , '2BP01' + , 'cannot drop extension extension_drop because other objects depend on it' + , 'Non-CASCADE DROP EXTENSION extension_drop is blocked by the dependency guard' +); + +SELECT has_extension('extension_drop', 'extension_drop is still installed after the blocked drop attempt'); + +SELECT has_view('extension_drop_drop_guard', 'guard', 'Dependency guard view is still present after the blocked drop attempt'); + +\i test/pgxntool/finish.sql + +-- vi: expandtab ts=2 sw=2 diff --git a/test/sql/schema.sql b/test/sql/schema.sql index ccc1999..60acd6d 100644 --- a/test/sql/schema.sql +++ b/test/sql/schema.sql @@ -3,6 +3,15 @@ \i test/pgxntool/setup.sql /* + * extension_drop is already installed (test/install/load.sql, committed, + * landing wherever the ambient search_path resolves -- public in practice) + * before this suite runs. This file's actual job is proving the + * schema-targeting/quoting pipeline works, so it drops that committed + * install and recreates its own copies in schemas it chooses instead. Safe + * to drop here: this whole file runs inside pgTAP's own rolled-back + * transaction, so load.sql's committed install is back in place for the + * next test file regardless of what happens below. + * * :TEST_SCHEMA and :TEST_SCHEMA_2 are mixed-case, so every reference to * them MUST be identifier-quoted (:"TEST_SCHEMA", or %I via format()) -- * an unquoted reference would silently fold to lowercase and test a @@ -10,6 +19,11 @@ * That's deliberate: it turns a missing-quote bug in the code under test * into a hard failure instead of a silent pass. */ +DROP EXTENSION extension_drop; +CREATE SCHEMA :"TEST_SCHEMA"; +CREATE EXTENSION extension_drop SCHEMA :"TEST_SCHEMA"; +SET search_path = :"TEST_SCHEMA", tap, "$user"; + CREATE SCHEMA "_Test_Ed_2"; SELECT plan( diff --git a/test/sql/simple.sql b/test/sql/simple.sql index 721a825..b01269d 100644 --- a/test/sql/simple.sql +++ b/test/sql/simple.sql @@ -1,5 +1,4 @@ \set ECHO none -\set TEST_SCHEMA _test_ed \i test/pgxntool/setup.sql SELECT plan( @@ -48,17 +47,24 @@ SELECT lives_ok( ); /* - * Check search path for add command + * These calls used to be schema-qualified (_test_ed.extension_drop__remove + * etc.) back when this file's own per-test deps.sql install put + * extension_drop in a private schema and then this section intentionally + * moved search_path away from it, to prove a qualified call still worked. + * extension_drop is now installed once, ambiently (in public, see + * test/install/load.sql), by the time this file runs -- 'public' is always + * on search_path regardless of the change below, so there's no longer a + * schema this file controls to qualify against here. Proving + * schema-qualified access explicitly is test/sql/schema.sql's job now. */ --- Intentionally change our search path SET search_path = "$user", public, tap; SELECT lives_ok( - $$SELECT _test_ed.extension_drop__remove('extension_drop_test')$$ + $$SELECT extension_drop__remove('extension_drop_test')$$ , 'Drop extension command' ); SELECT lives_ok( - $$SELECT _test_ed.extension_drop__add('extension_drop_test', 'moo')$$ + $$SELECT extension_drop__add('extension_drop_test', 'moo')$$ , 'Add extension command' ); @@ -70,7 +76,7 @@ SELECT throws_ok( ); SELECT lives_ok( - $$SELECT _test_ed.extension_drop__remove('extension_drop_test')$$ + $$SELECT extension_drop__remove('extension_drop_test')$$ , 'Drop extension command' ); SELECT lives_ok( From 7bb239d8cd018440ceacd5472a1515083b649036 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 15:19:52 -0500 Subject: [PATCH 2/6] Revert ci.yml pg-build-test switch: pre-existing failures on old PG predate this branch CI on this branch showed the switch to `make test && make verify-results` surfacing real pgTAP failures on PostgreSQL 9.3/9.6 (cat_tools/extension_drop never actually install there). Checked PR #10's own baseline CI (https://github.com/Postgres-Extensions/extension_tools/pull/10, run 30665031257): PG 9.3 and 9.6 already report "3 of 3 tests failed" in the raw job log there too, just silently reported as a passing check because pg-build-test's underlying `make test` hits pgxntool's `.IGNORE: installcheck` the same way. So this isn't a regression from this PR's own changes -- it's the exact masking problem RELEASE.md already documents, just now applying to a different, older part of the PG matrix than the PRs (#6/#7) it originally cites. Reverting the ci.yml step back to pg-build-test here keeps this PR scoped to test/install infrastructure; fixing cat_tools's install path on pre-PG10 belongs to whoever owns that dependency setup (PR #10 or a follow-up), not this PR. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 354b400..dc82d51 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,15 +60,7 @@ jobs: - name: Check out the repo uses: actions/checkout@v7 - name: Test on PostgreSQL ${{ matrix.pg }} - # pgxntool's base.mk marks installcheck .IGNORE, so a plain `make - # test` (what pg-build-test itself invokes under the hood) exits 0 - # even when every pg_regress test fails -- confirmed happening for - # real on PRs #6/#7 (RELEASE.md). `make verify-results` is the - # actual pass/fail signal (it scans for raw pgTAP failures and plan - # mismatches, not just installcheck's own exit code), so run it - # explicitly after `make test` instead of relying on pg-build-test - # alone. - run: make test && make verify-results + run: pg-build-test # A single stable check name for use as a required status check in branch # protection. Matrix jobs produce names like "🐘 PostgreSQL 14" that change From c5b4a8537f2a169c2de79166d60ced2bff48f798 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 14:38:38 -0500 Subject: [PATCH 3/6] Trigger a clean, final CI run From a3dd8822d413b2528a100e9d9b4036639c21f200 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 14:24:08 -0500 Subject: [PATCH 4/6] test/install/load.sql: drop stale before/after narration from comments Two comments described what test/deps.sql or this file's CASCADE logic used to be responsible for, instead of just stating current behavior. That kind of history belongs in commit messages/PR descriptions, not in comments that will rot as the code moves on. --- test/install/load.sql | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/install/load.sql b/test/install/load.sql index b29636e..9637268 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -8,8 +8,8 @@ * pgxntool's test/install feature runs this file COMMITTED, in its own * pg_regress session, BEFORE the main pgTAP suite, so the extension persists * into every (rolled-back) test/sql/ file instead of each one re-installing - * it from scratch. test/deps.sql (run per test) no longer creates the - * extension; it only sets the psql variables the suite references. + * it from scratch. test/deps.sql (run per test) only sets the psql + * variables the suite references; it does not install anything itself. * test/sql/schema.sql is the one exception: proving the schema-targeting * pipeline works is its actual job, so it explicitly drops this committed * install and recreates its own copies in schemas it chooses -- safely, @@ -108,10 +108,9 @@ $DO$; * * extension_drop requires cat_tools. CASCADE auto-installs it on PG10+; * event triggers exist from 9.3 but CREATE EXTENSION ... CASCADE was only - * added in PG10, so pre-PG10 needs cat_tools created explicitly first. This - * mirrors the check test/deps.sql used to do per-test before this file took - * over installing the extension. server_version_num is read once into a - * psql variable rather than a runtime DO block, so it can drive \if + * added in PG10, so pre-PG10 needs cat_tools created explicitly first. + * server_version_num is read once into a psql variable rather than a + * runtime DO block, so it can drive \if * (client-side) branching around the VERSION-qualified CREATE EXTENSION * calls below without needing psql variables interpolated inside a * dollar-quoted DO body. From bff7b498642647460a375a2360090cdec24c76cc Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 15:51:58 -0500 Subject: [PATCH 5/6] Drop dead search_path manipulation and stale comments from simple.sql/deps.sql simple.sql exercises the public/ambient install path, where 'public' is always on search_path -- so its SET search_path line couldn't move the extension's schema off the path and never proved anything; schema.sql already covers that property for the custom-schema path, which is the only path that needs to. The deps.sql comment was fully redundant with test/install/load.sql's own comment about what schema.sql tests. --- test/deps.sql | 9 --------- test/sql/simple.sql | 14 -------------- 2 files changed, 23 deletions(-) diff --git a/test/deps.sql b/test/deps.sql index f53733f..64cea76 100644 --- a/test/deps.sql +++ b/test/deps.sql @@ -1,15 +1,6 @@ -- Add any test dependency statements here -- Note: pgTap is loaded by setup.sql -/* - * extension_drop itself used to be (re)installed here, per test file. It's - * now installed ONCE, COMMITTED, by test/install/load.sql (pgxntool's - * test/install feature) before this suite runs at all -- this file no - * longer touches it. test/sql/schema.sql is the one test that actually - * drops/recreates the extension itself (that's what it's testing); every - * other test file just uses the extension load.sql already installed. - */ - \set TT extension_drop_test_table CREATE TEMP TABLE :TT (i int); diff --git a/test/sql/simple.sql b/test/sql/simple.sql index b01269d..e2a41ee 100644 --- a/test/sql/simple.sql +++ b/test/sql/simple.sql @@ -12,7 +12,6 @@ SELECT plan( + 1 -- Verify test table is empty + 1 -- Create test extension again - -- Change search path + 2 -- Test __remove and add + 1 -- Drop fails + 2 -- __remove and drop succeeds @@ -46,19 +45,6 @@ SELECT lives_ok( , 'Create test extension again' ); -/* - * These calls used to be schema-qualified (_test_ed.extension_drop__remove - * etc.) back when this file's own per-test deps.sql install put - * extension_drop in a private schema and then this section intentionally - * moved search_path away from it, to prove a qualified call still worked. - * extension_drop is now installed once, ambiently (in public, see - * test/install/load.sql), by the time this file runs -- 'public' is always - * on search_path regardless of the change below, so there's no longer a - * schema this file controls to qualify against here. Proving - * schema-qualified access explicitly is test/sql/schema.sql's job now. - */ -SET search_path = "$user", public, tap; - SELECT lives_ok( $$SELECT extension_drop__remove('extension_drop_test')$$ , 'Drop extension command' From 03b03550d851c2b86c465391103854ae9c539cd4 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 17:14:44 -0500 Subject: [PATCH 6/6] test/install/load.sql: move \if/\else/\endif comments before their command Comments describing a branch were placed after the \if/\else/\endif that opens it, reading as if they described the code above instead of below. Move each comment to precede its command, and add the missing one-line rationale to the fresh/update \if pair, which had it on neither side. --- test/install/load.sql | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/install/load.sql b/test/install/load.sql index 9637268..6fccae1 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -76,7 +76,6 @@ SELECT , :'extension_drop_test_load_mode' = 'existing' AS extension_drop_mode_existing \gset -\if :extension_drop_mode_existing /* * existing mode: do NOT touch the extension. Assert it is installed and at * the current default_version -- the pg_upgrade / external update the @@ -84,6 +83,7 @@ SELECT * dropping or reinstalling it would defeat the test. Fail loudly on absence * or mismatch. */ +\if :extension_drop_mode_existing DO $DO$ DECLARE v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'extension_drop'); @@ -100,7 +100,6 @@ BEGIN END IF; END $DO$; -\else /* * fresh / update: (re)install from scratch. Drop-first (CASCADE, matching * cat_tools' own load.sql) so a re-run on a persistent cluster installs the @@ -115,11 +114,13 @@ $DO$; * calls below without needing psql variables interpolated inside a * dollar-quoted DO body. */ +\else DROP EXTENSION IF EXISTS extension_drop CASCADE; SELECT current_setting('server_version_num')::int >= 100000 AS extension_drop_pg10_plus \gset +-- update mode: install at the OLD version, then ALTER EXTENSION UPDATE below. \if :extension_drop_mode_update SELECT current_setting('extension_drop.test_update_from') AS extension_drop_test_update_from \gset SELECT current_setting('extension_drop.test_update_to') AS extension_drop_test_update_to \gset @@ -146,6 +147,7 @@ CREATE EXTENSION extension_drop VERSION :'extension_drop_test_update_from'; SET client_min_messages = ERROR; ALTER EXTENSION extension_drop UPDATE :extension_drop_update_to_clause; SET client_min_messages = WARNING; +-- fresh mode: plain CREATE EXTENSION at the current version. \else \if :extension_drop_pg10_plus CREATE EXTENSION extension_drop CASCADE; @@ -153,9 +155,9 @@ CREATE EXTENSION extension_drop CASCADE; CREATE EXTENSION IF NOT EXISTS cat_tools; CREATE EXTENSION extension_drop; \endif -\endif -- end \if :extension_drop_mode_update (fresh vs. update install branch) \endif -- end \if :extension_drop_mode_existing (existing mode skips the whole (re)install block) +\endif -- vi: expandtab ts=2 sw=2