diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f08de6..ec95fa3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -240,8 +240,18 @@ jobs: # carry the details; this is the big picture): # # test -- FRESH install: CREATE EXTENSION at the current - # version on every supported PostgreSQL. The baseline - # a brand-new user gets. + # version on every supported PostgreSQL. Its "Test + # on PostgreSQL" step runs `make test-long`, which + # loops the full suite once per TEST_SCHEMA value + # in TEST_SCHEMAS (empty, the baseline a brand-new + # user gets, and a quoting-requiring schema name) + # -- so every major covers both schema variants, + # with no separate matrix leg for it. See the + # TEST_SCHEMA/TEST_SCHEMAS comments in the + # Makefile. NOTE: extension-update-test and + # pg-upgrade-test below do NOT exercise + # TEST_SCHEMA at all yet -- a deliberately + # deferred follow-up, not an oversight. # extension-update-test -- IN-PLACE update: CREATE EXTENSION at an OLD version # then ALTER EXTENSION UPDATE (same PostgreSQL, no # pg_upgrade). @@ -301,12 +311,12 @@ jobs: # Fail if the relkind drift source is empty (headers missing): the # drift check must actually run on every version, not pass silently. make check-relkind-source - # verify-results is the real gate: base.mk declares `verify-results: - # test`, so this runs the suite via its `test` prerequisite and then - # checks the pgtap/regression.diffs. A bare `make test` is redundant here - # and would not gate anyway -- it never exits non-zero on regressions - # (pgxntool marks installcheck `.IGNORE`), so failures would pass silently. - make verify-results + # test-long loops verify-results (this repo's documented CI-safe + # gate -- see CLAUDE.md) once per TEST_SCHEMA value in TEST_SCHEMAS, + # so this one step covers both the TEST_SCHEMA-empty baseline and the + # quoting-requiring-schema variant on EVERY supported major. See the + # Makefile's TEST_SCHEMA/TEST_SCHEMAS/test-long comments. + make test-long # Style linter (https://github.com/Postgres-Extensions/linter, vendored at # .vendor/linter). Deliberately checked out WITHOUT submodules -- `make diff --git a/Makefile b/Makefile index b293fc1..5d68785 100644 --- a/Makefile +++ b/Makefile @@ -56,7 +56,53 @@ endif TEST_UPDATE_FROM ?= 0.2.2 TEST_UPDATE_TO ?= -export PGOPTIONS := $(PGOPTIONS) -c cat_tools.test_load_mode=$(TEST_LOAD_SOURCE) -c cat_tools.test_update_from=$(TEST_UPDATE_FROM) -c cat_tools.test_update_to=$(TEST_UPDATE_TO) +# TEST_SCHEMA is a second, independent GUC switch, same propagation mechanism +# as TEST_LOAD_SOURCE above: +# - empty (default): load.sql does not create or target any schema at all -- +# CREATE EXTENSION cat_tools runs exactly as a brand-new user would type +# it, landing wherever the session's ambient search_path already resolves. +# - non-empty: load.sql creates that schema (quoting it, so a name that +# requires quoting -- e.g. mixed case -- works) and SETs search_path to it +# before installing. cat_tools' control file pins schema = 'cat_tools' with +# relocatable = false, so this does NOT relocate the extension itself +# (CREATE EXTENSION ignores search_path for a non-relocatable extension); +# what it DOES prove is that the make -> PGOPTIONS -> GUC -> psql +# quoted-identifier pipeline handles a quoting-requiring name correctly, +# and that installing while some other, possibly hostile, schema is +# ambient doesn't break anything. See test/install/load.sql. +# +# Exported unconditionally, same reasoning as TEST_UPDATE_FROM/TO: an empty +# default is fine, and load.sql reads it without missing_ok. +TEST_SCHEMA ?= + +export PGOPTIONS := $(PGOPTIONS) -c cat_tools.test_load_mode=$(TEST_LOAD_SOURCE) -c cat_tools.test_update_from=$(TEST_UPDATE_FROM) -c cat_tools.test_update_to=$(TEST_UPDATE_TO) -c cat_tools.test_schema=$(TEST_SCHEMA) + +# Schema variants test-long exercises on every PG major: empty (the +# ambient-search_path default) and one mixed-case name that requires SQL +# identifier quoting (proving the make -> PGOPTIONS -> GUC -> psql pipeline +# handles a quoting-requiring name end to end -- see TEST_SCHEMA above). CI's +# `test` job runs test-long on every supported major, so both variants are +# covered everywhere -- no separate matrix leg for it. +# +# Scope boundary (deliberate, not an oversight): extension-update-test and +# pg-upgrade-test do NOT exercise TEST_SCHEMA/TEST_SCHEMAS at all yet. Wiring +# TEST_SCHEMA through the update/upgrade paths is left as a follow-up. +TEST_SCHEMAS ?= "" CatToolsSchema + +# Loops the full suite once per TEST_SCHEMA value via `verify-results`, NOT +# plain `test`: verify-results is this repo's documented CI-safe gate +# (make test alone doesn't reliably fail on regressions -- see CLAUDE.md), +# and CI relies on test-long to fail loudly on a schema-specific regression +# the same way a single `make verify-results TEST_SCHEMA=X` already does. +# Must recurse (a fresh $(MAKE) per iteration, not a plain shell variable) +# for the same reason test-update recurses: TEST_SCHEMA only takes effect +# if it's exported into PGOPTIONS before the sub-make's own parse phase. +.PHONY: test-long +test-long: + @for schema in $(TEST_SCHEMAS); do \ + echo "=== TEST_SCHEMA=$$schema ==="; \ + $(MAKE) verify-results TEST_SCHEMA="$$schema" || exit 1; \ + done # Convenience wrapper: `make test-update` == `make test TEST_LOAD_SOURCE=update`. # Must recurse (a fresh $(MAKE)) rather than depend on `test`, so the parse-time @@ -65,6 +111,18 @@ export PGOPTIONS := $(PGOPTIONS) -c cat_tools.test_load_mode=$(TEST_LOAD_SOURCE) test-update: $(MAKE) test TEST_LOAD_SOURCE=update +# Runs every test target in sequence: fresh, update, and the schema +# variants. Sequential $(MAKE) calls in the recipe body, NOT bare +# prerequisites -- listing them as prerequisites would let Make run them +# concurrently under -j, and they all share the same throwaway test +# database (same hazard already called out by verify-results's own +# dependency-ordering comment in pgxntool/base.mk). +.PHONY: test-all +test-all: + $(MAKE) test + $(MAKE) test-update + $(MAKE) test-long + # Versioned SQL is generated from .sql.in at build time. That generation, the # DATA list that installs it, and the relkind drift source all live in sql.mk, # which also owns `include pgxntool/base.mk` (base.mk has no include guard, so it diff --git a/test/install/load.sql b/test/install/load.sql index f0fd25a..7e32fff 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -48,6 +48,52 @@ SET client_min_messages = WARNING; */ \i test/roles.sql +/* + * TEST_SCHEMA targeting, independent of the mode selection below. The + * Makefile always exports cat_tools.test_schema via PGOPTIONS (empty by + * default); read it WITHOUT missing_ok, same reasoning as test_load_mode. + * + * Empty (the default): do nothing -- no CREATE SCHEMA, no SET search_path. + * CREATE EXTENSION cat_tools below then runs exactly as a brand-new user + * would type it, landing wherever the session's ambient search_path already + * resolves. + * + * Non-empty: create that schema and SET search_path to it before installing. + * cat_tools' control file pins schema = 'cat_tools' with relocatable = false, + * so CREATE EXTENSION ignores search_path for ITS OWN placement -- this does + * not relocate the extension. What it does prove: the make -> PGOPTIONS -> + * GUC -> psql quoted-identifier pipeline handles a quoting-requiring schema + * name (e.g. mixed case) correctly end to end, and that installing while some + * other, possibly hostile, schema is ambient on search_path doesn't break + * anything. The DO block below is a real check of that pipeline: a quoting + * bug (e.g. an unquoted reference silently case-folding) would leave + * search_path pointing at a schema that was never created -- CREATE SCHEMA + * would create one name, SET search_path would reference another, and + * PostgreSQL does not error on a search_path entry that doesn't exist, so + * nothing downstream would fail loudly without this explicit check. + */ +SELECT current_setting('cat_tools.test_schema') AS cat_tools_test_schema \gset +SELECT :'cat_tools_test_schema' <> '' AS cat_tools_has_schema \gset + +\if :cat_tools_has_schema +CREATE SCHEMA IF NOT EXISTS :"cat_tools_test_schema"; +SET search_path = :"cat_tools_test_schema"; + +DO $DO$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_namespace WHERE nspname = current_setting('cat_tools.test_schema') + ) THEN + RAISE EXCEPTION + 'TEST_SCHEMA target schema % was not created as named -- quoting bug?' + , current_setting('cat_tools.test_schema') + ; + END IF; +END +$DO$; +\endif +-- end \if :cat_tools_has_schema + /* * Mode selection. The Makefile always exports cat_tools.test_load_mode via * PGOPTIONS. Read it WITHOUT missing_ok: if the GUC did not propagate (a break