From 7f1b5564a9984d25459da8339c31fce34278810a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 15:15:23 -0500 Subject: [PATCH 1/2] 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 | 32 ++---- test/expected/dependency_guard.out | 6 ++ test/expected/schema.out | 10 +- test/expected/zzz_build.out | 28 ++++- test/install/.gitignore | 14 +++ test/install/load.sql | 162 +++++++++++++++++++++++++++++ test/sql/dependency_guard.sql | 64 ++++++++++++ test/sql/schema.sql | 32 +++++- test/sql/simple.sql | 18 ++-- test/sql/zzz_build.sql | 11 ++ 12 files changed, 403 insertions(+), 43 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 aad0d23..4e758ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,15 @@ jobs: - name: Check out the repo uses: actions/checkout@v5 - 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 8b37be9..5e5e2ee 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 testdeps: test_extension diff --git a/test/deps.sql b/test/deps.sql index bc5e06d..f53733f 100644 --- a/test/deps.sql +++ b/test/deps.sql @@ -1,32 +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; +/* + * 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); -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; - -- 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/expected/schema.out b/test/expected/schema.out index 12db609..511254e 100644 --- a/test/expected/schema.out +++ b/test/expected/schema.out @@ -4,12 +4,12 @@ ok 1 - Create test extension ok 2 - Test extension exists ok 3 - Drop test extension ok 4 - Test extension does not exist -ok 5 - Table _test_ed.extension_drop__commands should exist +ok 5 - Table "_Test_Ed".extension_drop__commands should exist ok 6 - Drop extension -ok 7 - Create extension in schema _test_ed_2 -ok 8 - Table _test_ed_2.extension_drop__commands should exist -ok 9 - Create test extension in _test_ed_2 +ok 7 - Create extension in schema _Test_Ed_2 +ok 8 - Table "_Test_Ed_2".extension_drop__commands should exist +ok 9 - Create test extension in _Test_Ed_2 ok 10 - extension_drop__update() ok 11 - Verify extension_drop__get() -ok 12 - Drop schema _test_ed without cascade succeeds +ok 12 - Drop schema _Test_Ed without cascade succeeds # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/zzz_build.out b/test/expected/zzz_build.out index c66ee3c..ccefb84 100644 --- a/test/expected/zzz_build.out +++ b/test/expected/zzz_build.out @@ -1,4 +1,6 @@ \set ECHO none +NOTICE: 42710: extension "cat_tools" already exists, skipping +LOCATION: CreateExtension, extension.c:2011 INSTALL @@ -9,14 +11,36 @@ INSTALL +psql:sql/extension_drop.sql:198: NOTICE: 00000: type reference extension_drop__commands.extension_name%TYPE converted to name +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:198: NOTICE: 00000: type reference extension_drop__commands.extension_name%TYPE converted to name +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:218: NOTICE: 00000: type reference extension_drop__commands.extension_name%TYPE converted to name +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:218: NOTICE: 00000: type reference extension_drop__commands.sql%TYPE converted to text +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:218: NOTICE: 00000: type reference extension_drop__commands.extension_name%TYPE converted to name +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:218: NOTICE: 00000: type reference extension_drop__commands.sql%TYPE converted to text +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:236: NOTICE: 00000: type reference extension_drop__commands.extension_name%TYPE converted to name +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:236: NOTICE: 00000: type reference extension_drop__commands.extension_name%TYPE converted to name +LOCATION: LookupTypeNameExtended, parse_type.c:156 - - +psql:sql/extension_drop.sql:256: NOTICE: 00000: type reference extension_drop__commands.extension_name%TYPE converted to name +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:256: NOTICE: 00000: type reference extension_drop__commands.sql%TYPE converted to text +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:256: NOTICE: 00000: type reference extension_drop__commands.extension_name%TYPE converted to name +LOCATION: LookupTypeNameExtended, parse_type.c:156 +psql:sql/extension_drop.sql:256: NOTICE: 00000: type reference extension_drop__commands.sql%TYPE converted to text +LOCATION: LookupTypeNameExtended, parse_type.c:156 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..06f13d7 --- /dev/null +++ b/test/sql/dependency_guard.sql @@ -0,0 +1,64 @@ +\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 fa70ed4..20c278e 100644 --- a/test/sql/schema.sql +++ b/test/sql/schema.sql @@ -1,8 +1,30 @@ \set ECHO none -\set TEST_SCHEMA _test_ed +\set TEST_SCHEMA _Test_Ed \i test/pgxntool/setup.sql -CREATE SCHEMA _test_ed_2; +/* + * 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 is mixed-case, so every reference to it MUST be + * identifier-quoted (:"TEST_SCHEMA", or %I via format()) -- an unquoted + * reference would silently fold to lowercase and test a different, + * unquoted schema instead of this one, without erroring. 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( 0 @@ -28,7 +50,7 @@ SELECT lives_ok( , 'Drop extension' ); -\set TEST_SCHEMA_2 _test_ed_2 +\set TEST_SCHEMA_2 _Test_Ed_2 SELECT lives_ok( format( $$CREATE EXTENSION extension_drop SCHEMA %I$$, :'TEST_SCHEMA_2' ) , 'Create extension in schema ' || :'TEST_SCHEMA_2' @@ -44,11 +66,11 @@ SELECT lives_ok( SET search_path = "$user", public, tap; SELECT lives_ok( - $$SELECT _test_ed_2.extension_drop__update('extension_drop_test', 'moo')$$ + format( $$SELECT %I.extension_drop__update('extension_drop_test', 'moo')$$, :'TEST_SCHEMA_2' ) , 'extension_drop__update()' ); SELECT bag_eq( - $$SELECT * FROM _test_ed_2.extension_drop__get('extension_drop_test')$$ + format( $$SELECT * FROM %I.extension_drop__get('extension_drop_test')$$, :'TEST_SCHEMA_2' ) , $$SELECT 'extension_drop_test'::name , 'moo'::text$$ , 'Verify extension_drop__get()' ); 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( diff --git a/test/sql/zzz_build.sql b/test/sql/zzz_build.sql index 354c480..f17d6d4 100644 --- a/test/sql/zzz_build.sql +++ b/test/sql/zzz_build.sql @@ -4,6 +4,17 @@ BEGIN; CREATE EXTENSION IF NOT EXISTS cat_tools; +/* + * extension_drop is already installed globally (test/install/load.sql, + * committed) by the time this file runs. This test's whole point is running + * the raw install SCRIPT directly (not via CREATE EXTENSION) to verify it + * builds cleanly, so it needs the committed copy out of the way first -- + * otherwise the script's own CREATE TABLE extension_drop__commands collides + * with the one that's already there. Safe to drop here: like every other + * test/sql/ file, this one's changes never survive past its own session. + */ +DROP EXTENSION IF EXISTS extension_drop CASCADE; + \echo \echo INSTALL \t From b780d1731b3603c244a582af0f068466dbb8c8bf Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 15:19:52 -0500 Subject: [PATCH 2/2] 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 4e758ba..aad0d23 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,15 +22,7 @@ jobs: - name: Check out the repo uses: actions/checkout@v5 - 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