diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f08de6..f5ac3f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,10 @@ jobs: update_pg: ${{ steps.pg.outputs.update_pg }} climb_pg: ${{ steps.pg.outputs.climb_pg }} legacy_pg: ${{ steps.pg.outputs.legacy_pg }} + # The `test` job's full {pg, test_schema} matrix (see the "Derive ..." + # step for why this is a pure include list rather than a second bare + # `test_schema` list crossed against `pg`). + test_matrix: ${{ steps.pg.outputs.test_matrix }} steps: - name: Check out the repo uses: actions/checkout@v6 @@ -232,6 +236,41 @@ jobs: # Space-separated for direct iteration in the stepwise bash loop. echo "climb_pg=$(echo $climb)" >> "$GITHUB_OUTPUT" + # test_matrix: the `test` job's ACTUAL matrix, one {pg, test_schema} + # object per combination, consumed via a pure `strategy.matrix.include` + # (no separate bare `pg:` list) so this is the ONLY source of truth for + # what that job runs -- see the `test` job for why a pure include list, + # rather than crossing a second `test_schema` list against `pg`, is + # required here: GitHub's matrix `include` MERGES an object into an + # existing combination that already matches all of the object's keys + # (rather than adding a new one), so pairing a `test_schema` value with + # a `pg` value already in a bare list would silently REPLACE that + # version's default (TEST_SCHEMA empty) run instead of adding a second, + # separate one. A pure include list sidesteps that entirely: every + # object here is its own job, unconditionally. + # + # Every supported major runs once with TEST_SCHEMA empty (the default -- + # CREATE EXTENSION cat_tools with no schema targeting). Exactly one + # extra leg, on the newest major, additionally sets TEST_SCHEMA to a + # mixed-case name that REQUIRES SQL identifier quoting -- proving the + # make -> PGOPTIONS -> GUC -> psql quoted-identifier pipeline handles + # such a name correctly end to end (see test/install/load.sql for what + # TEST_SCHEMA does and does NOT do). Only one leg needs it: the quoting + # pipeline doesn't depend on the PostgreSQL major at all, so crossing it + # against every supported version would add CI cost with no added + # confidence. + QUOTING_SCHEMA='CatToolsSchema' + test_matrix=$( + { + for pg in $supported; do + jq -n --arg pg "$pg" '{pg: ($pg | tonumber), test_schema: ""}' + done + jq -n --arg pg "$NEWEST" --arg test_schema "$QUOTING_SCHEMA" \ + '{pg: ($pg | tonumber), test_schema: $test_schema}' + } | jq -s -c . + ) + echo "test_matrix=$test_matrix" >> "$GITHUB_OUTPUT" + # =========================================================================== # Test strategy # @@ -240,8 +279,11 @@ 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, with + # TEST_SCHEMA empty (the baseline a brand-new user + # gets), PLUS one extra leg, on the newest major, + # with TEST_SCHEMA set to a quoting-requiring + # schema name. # extension-update-test -- IN-PLACE update: CREATE EXTENSION at an OLD version # then ALTER EXTENSION UPDATE (same PostgreSQL, no # pg_upgrade). @@ -281,9 +323,13 @@ jobs: if: needs.changes.outputs.docs_only != 'true' strategy: matrix: - # Current-supported majors, from the single source in the changes job. - pg: ${{ fromJSON(needs.changes.outputs.supported_pg) }} - name: 🐘 PostgreSQL ${{ matrix.pg }} + # {pg, test_schema} pairs from the single source in the changes job. + # A pure include list (no separate bare `pg:` list) -- see the + # "Derive ..." step's test_matrix comment for why. + include: ${{ fromJSON(needs.changes.outputs.test_matrix) }} + # Append the TEST_SCHEMA value to the check name only on the one leg that + # sets it, so the other legs' names are unchanged. + name: 🐘 PostgreSQL ${{ matrix.pg }}${{ matrix.test_schema != '' && format(' (TEST_SCHEMA={0})', matrix.test_schema) || '' }} runs-on: ubuntu-latest container: pgxn/pgxn-tools steps: @@ -306,7 +352,9 @@ jobs: # 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_SCHEMA is empty for every leg except the one dedicated + # quoting-requiring-schema leg (see the changes job's test_matrix). + make verify-results TEST_SCHEMA="${{ matrix.test_schema }}" # 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..21e2de5 100644 --- a/Makefile +++ b/Makefile @@ -56,7 +56,26 @@ 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) # 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 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