diff --git a/CLAUDE.md b/CLAUDE.md index c476d0d..9d060fc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,6 +6,23 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co After pushing to a branch with an open PR, monitor CI using `gh pr checks --watch` in a background subagent until all jobs pass or a failure is confirmed. Investigate and fix failures immediately rather than leaving them for the user to notice. +## psql Script Conventions + +Any `\if`/`\elsif`/`\else`/`\endif` block spanning more than ~a dozen lines needs a short comment naming the block on each of its control statements (including `\endif`), so a reader scrolling past `\else`/`\endif` on their own can tell at a glance which `\if` they belong to without scrolling back up. Put the comment on its own line immediately above the control statement, NOT trailing on the same line -- unlike SQL statements, psql's `\if`/`\else`/`\endif` don't treat a trailing `--` as a comment to strip: `\if` parses the entire rest of the line as its boolean expression (so a trailing comment breaks parsing outright), and `\else`/`\endif` parse it as an "extra argument" that gets ignored but still prints a warning into the actual output. Example: + +```sql +-- existing-mode install +\if :is_existing + ... +-- existing-mode install +\else + ... +-- existing-mode install +\endif +``` + +Short `\if` blocks don't need this -- the `\if` is still visible on screen alongside its `\else`/`\endif`. + ## Project Overview This is **test_factory**, a PostgreSQL extension that provides a framework for managing unit test data in databases. It solves the common problem of creating and maintaining test data by providing a system to register test data definitions once and retrieve them efficiently with automatic dependency resolution. diff --git a/Makefile b/Makefile index 515b394..c8787b5 100644 --- a/Makefile +++ b/Makefile @@ -5,6 +5,32 @@ include pgxntool/base.mk # instead of silently disabling this check. PGXNTOOL_ENABLE_TEST_BUILD = yes +PGXNTOOL_ENABLE_TEST_INSTALL = yes + +# ------------------------------------------------------------------------------ +# TEST_LOAD_SOURCE: how test/install/load.sql gets the extension to its +# target state (fresh/update/existing). See test/install/load.sql for what +# each mode actually does. +# ------------------------------------------------------------------------------ +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 + +# Only meaningful in 'update' mode; empty TO means "update to current". +TEST_UPDATE_FROM ?= 0.5.0 +TEST_UPDATE_TO ?= + +# Export unconditionally -- load.sql must never treat "absent" as "fresh". +export PGOPTIONS := $(PGOPTIONS) -c test_factory.test_load_mode=$(TEST_LOAD_SOURCE) -c test_factory.test_update_from=$(TEST_UPDATE_FROM) -c test_factory.test_update_to=$(TEST_UPDATE_TO) + +# make test-update: convenience wrapper. Must re-invoke $(MAKE) (not just +# depend on test) so the parse-time TEST_LOAD_SOURCE validation above +# re-evaluates for the child invocation. +.PHONY: test-update +test-update: + $(MAKE) test TEST_LOAD_SOURCE=update + # Hook for test to ensure dependencies in control file are set correctly testdeps: check_control diff --git a/test/CLAUDE.md b/test/CLAUDE.md index c4cb279..b536df9 100644 --- a/test/CLAUDE.md +++ b/test/CLAUDE.md @@ -28,27 +28,82 @@ The test_factory extension uses **pgTAP** (PostgreSQL's unit testing framework) ### Test Helpers - `test/helpers/setup.sql` - Test environment initialization and pgTAP setup - `test/helpers/create.sql` - Test data registration and security validation -- `test/helpers/create_extension.sql` - Extension creation wrapper -- `test/helpers/deps.sql` - Test dependency management +- `test/helpers/deps.sql` - Test dependency management (`\i`'s `test/roles.sql`) +- `test/roles.sql` - Single source of truth for test-only role names - Other helper files for role management and pgTAP integration +## Load Modes (`TEST_LOAD_SOURCE`) + +`test/install/load.sql` runs once, committed, before the regular test files +(pgxntool's `PGXNTOOL_ENABLE_TEST_INSTALL` feature), so its state survives +into every test file. It is the *only* place that knows how the extension +gets onto the system -- `test/sql/base.sql` and `test/sql/pgtap.sql` always +assume both extensions are already installed, in every mode, and are +otherwise identical regardless of which mode ran. `TEST_LOAD_SOURCE` +(default `fresh`) picks how the extension gets to its target state: + +- **fresh** (default) - drops both extensions first (so a re-run against a + non-fresh dev DB starts clean), lands `pgtap` in a dedicated `tap` schema, + then `CREATE EXTENSION test_factory_pgtap CASCADE` (proving + `test_factory_pgtap.control`'s `requires` line actually pulls + `test_factory` in). +- **update** - `CREATE EXTENSION test_factory VERSION :from` then + `ALTER EXTENSION UPDATE` (`TEST_UPDATE_FROM`/`TEST_UPDATE_TO` make vars), + then installs `test_factory_pgtap` at current (it has only ever shipped + one version, so there's no update path of its own to exercise yet -- the + mechanism exists for when a second version ships, but no CI job drives it + yet). `make test-update` is a shorthand for `make test + TEST_LOAD_SOURCE=update`. +- **existing** - the extension is already installed (a real `pg_upgrade` + target, or an out-of-band update) -- `load.sql` only asserts it's present + at the current version, plants a dependency guard (see below), and + proves it; never drops/creates/updates anything. + + Run against a real pre-existing install with: + ``` + make test TEST_LOAD_SOURCE=existing CONTRIB_TESTDB= \ + EXTRA_REGRESS_OPTS=--use-existing PGXNTOOL_ENABLE_TEST_BUILD=no + ``` + +All three modes leave the system in the same observable end state (both +extensions installed, `pgtap` in schema `tap`), so `base.sql`/`pgtap.sql` +and their expected output are shared across all of them -- no +per-mode alternate expected files. + +### Dependency Guard + +Planted only in `existing` mode (see `load.sql`): a view in schema +`test_factory_drop_guard` depending on `tf.tap(text,text)` blocks a +non-CASCADE `DROP EXTENSION test_factory_pgtap`. `test_factory` itself +doesn't need an artificial guard -- `test_factory_pgtap`'s own control file +(`requires = 'pgtap, test_factory'`) already blocks a non-CASCADE +`DROP EXTENSION test_factory` as long as `test_factory_pgtap` is installed; +`load.sql` proves that natural protection too. The point of the guard: in +`existing` mode, nothing else stops a stray drop (or a logic bug that falls +through to the fresh/update branch) from silently destroying the real +upgraded/updated objects this mode exists to test. + ## Test Coverage Analysis ### Core Functionality Tests (`base.sql`) -1. **Extension Setup** - Creates extension and test tables -2. **Data Registration** - Tests `tf.register()` with multiple test sets -3. **Basic Retrieval** - Tests `tf.get()` returns correct data -4. **Dependency Resolution** - Tests automatic creation of dependent data (customer → invoice) -5. **Caching Behavior** - Verifies data consistency across multiple `tf.get()` calls -6. **Table Independence** - Tests that cached data persists after source table changes -7. **Function-based Test Data** - Tests using functions as test data sources +Assumes both extensions are already installed (by `test/install/load.sql`, +in every mode) and test tables not yet created: +1. **Data Registration** - Tests `tf.register()` with multiple test sets +2. **Basic Retrieval** - Tests `tf.get()` returns correct data +3. **Dependency Resolution** - Tests automatic creation of dependent data (customer → invoice) +4. **Caching Behavior** - Verifies data consistency across multiple `tf.get()` calls +5. **Table Independence** - Tests that cached data persists after source table changes +6. **Function-based Test Data** - Tests using functions as test data sources ### Security Tests (`create.sql`) -- **Role Management** - Validates proper role restoration after installation - **Security Definer Functions** - Ensures all privileged functions use `search_path=pg_catalog` - **Permission Isolation** - Tests with unprivileged `test_role` - **Temp Table Cleanup** - Verifies temporary installation objects are removed +Role-restore verification (does `CREATE EXTENSION` correctly restore the +calling role?) lives in `test/install/load.sql` instead, where `CREATE +EXTENSION` actually runs. + ### Raw SQL Syntax Tests (`test/build/syntax.sql`) - Runs `sql/test_factory.sql` and `sql/test_factory_pgtap.sql` (the actual source files, not the generated `sql/*--VERSION.sql` copies) directly via @@ -57,8 +112,18 @@ The test_factory extension uses **pgTAP** (PostgreSQL's unit testing framework) - See the comments in that file for the known/expected errors baked into its expected output (`pg_extension_config_dump()` and `SET ROLE ""`), which are artifacts of running the file outside of CREATE EXTENSION, not bugs. +- This is the *only* thing `test/build` is for: running extension scripts + "bare" for better error context. Its results are always thrown away + (unlike `test/install`, which is intended to commit and persist). + Packaging checks (dependency declarations, clean install) belong in + `test/install/load.sql` instead. ### pgTAP Integration Tests (`pgtap.sql`) +- **Dependency Enforcement** - Confirms `test_factory_pgtap.control`'s + `requires = 'pgtap, test_factory'` line is real and enforced by Postgres, + via a `pg_depend` extension-requires-extension edge (`deptype = 'n'`), + not just documentation. Checking final catalog state this way works + uniformly in every `TEST_LOAD_SOURCE` mode. - **tf.tap() Function** - Tests pgTAP wrapper functionality - **Error Handling** - Tests proper error reporting for invalid inputs diff --git a/test/expected/base.out b/test/expected/base.out index 57f115a..75a9c3a 100644 --- a/test/expected/base.out +++ b/test/expected/base.out @@ -1,24 +1,22 @@ \set ECHO none -Creating extension test_factory ok 1 - Register test customers ok 2 - Create function customer__add ok 3 - Register test invoices ok 4 - Ensure original_role temp table was dropped -ok 5 - Ensure role is put back after install -ok 6 - Security definer function _tf.schema__getsert has search_path=pg_catalog -ok 7 - Security definer function _tf.test_factory__get has search_path=pg_catalog -ok 8 - Security definer function _tf.test_factory__set has search_path=pg_catalog -ok 9 - Security definer function _tf.table_create has search_path=pg_catalog -ok 10 - Security definer function _tf.get has search_path=pg_catalog -ok 11 - customer table is empty -ok 12 - invoice table is empty -ok 13 - invoice factory output -ok 14 - invoice table content -ok 15 - customer table content -ok 16 - invoice factory second call -ok 17 - invoice table content stayed constant -ok 18 - customer table content stayed constant -ok 19 - Test function factory -ok 20 - customer table has new row -ok 21 - truncate invoice -ok 22 - invoice factory get remains the same after truncate +ok 5 - Security definer function _tf.schema__getsert has search_path=pg_catalog +ok 6 - Security definer function _tf.test_factory__get has search_path=pg_catalog +ok 7 - Security definer function _tf.test_factory__set has search_path=pg_catalog +ok 8 - Security definer function _tf.table_create has search_path=pg_catalog +ok 9 - Security definer function _tf.get has search_path=pg_catalog +ok 10 - customer table is empty +ok 11 - invoice table is empty +ok 12 - invoice factory output +ok 13 - invoice table content +ok 14 - customer table content +ok 15 - invoice factory second call +ok 16 - invoice table content stayed constant +ok 17 - customer table content stayed constant +ok 18 - Test function factory +ok 19 - customer table has new row +ok 20 - truncate invoice +ok 21 - invoice factory get remains the same after truncate diff --git a/test/expected/pgtap.out b/test/expected/pgtap.out index 5f552dc..2e529f2 100644 --- a/test/expected/pgtap.out +++ b/test/expected/pgtap.out @@ -1,11 +1,9 @@ \set ECHO none -Creating extension test_factory -Creating extension test_factory_pgtap -ok 1 - Register test customers -ok 2 - Create function customer__add -ok 3 - Register test invoices -ok 4 - Ensure original_role temp table was dropped -ok 5 - Ensure role is put back after install +ok 1 - test_factory_pgtap depends on test_factory (control file requires is real and enforced) +ok 2 - Register test customers +ok 3 - Create function customer__add +ok 4 - Register test invoices +ok 5 - Ensure original_role temp table was dropped ok 6 - Security definer function _tf.schema__getsert has search_path=pg_catalog ok 7 - Security definer function _tf.test_factory__get has search_path=pg_catalog ok 8 - Security definer function _tf.test_factory__set has search_path=pg_catalog diff --git a/test/helpers/create.sql b/test/helpers/create.sql index 9897f75..8ff7b7f 100644 --- a/test/helpers/create.sql +++ b/test/helpers/create.sql @@ -1,13 +1,17 @@ SET ROLE = DEFAULT; -CREATE ROLE test_role; -GRANT USAGE ON SCHEMA tap TO test_role; +/* + * test_role itself is created once, idempotently, by test/install/load.sql + * (test/roles.sql is the single source of truth for the name; \i'd via + * test/helpers/deps.sql). + */ +GRANT USAGE ON SCHEMA tap TO :test_role; /* * DO NOT GRANT test_role TO test_factory__owner; the whole point test_role is * to check for security problems. */ -CREATE SCHEMA test AUTHORIZATION test_role; -SET ROLE = test_role; +CREATE SCHEMA test AUTHORIZATION :test_role; +SET ROLE = :test_role; SET search_path = test, tap; CREATE TABLE customer( @@ -79,11 +83,12 @@ SELECT hasnt_table( , 'Ensure original_role temp table was dropped' ); -SELECT is( - (SELECT * FROM post_install_role) - , (SELECT * FROM pre_install_role) - , 'Ensure role is put back after install' -); +/* + * Role-restore verification (does CREATE EXTENSION correctly restore the + * calling role?) now lives in test/install/load.sql, where CREATE + * EXTENSION actually runs in every mode -- there's nothing to check here + * anymore now that this file no longer runs it itself. + */ SELECT cmp_ok( proconfig diff --git a/test/helpers/create_extension.sql b/test/helpers/create_extension.sql deleted file mode 100644 index 1fd2308..0000000 --- a/test/helpers/create_extension.sql +++ /dev/null @@ -1,7 +0,0 @@ -\echo Creating extension :extension_name --- No IF NOT EXISTS because we'll be confused if we're not loading the new stuff -CREATE TEMP TABLE pre_install_role AS SELECT current_user; -GRANT SELECT ON pre_install_role TO public; -- In case role is different -CREATE EXTENSION :extension_name; -CREATE TEMP TABLE post_install_role AS SELECT current_user; -GRANT SELECT ON post_install_role TO public; -- In case role is different diff --git a/test/helpers/deps.sql b/test/helpers/deps.sql index e69de29..455686f 100644 --- a/test/helpers/deps.sql +++ b/test/helpers/deps.sql @@ -0,0 +1 @@ +\i test/roles.sql diff --git a/test/install/load.out b/test/install/load.out new file mode 100644 index 0000000..25fdbb1 --- /dev/null +++ b/test/install/load.out @@ -0,0 +1 @@ +\set ECHO none diff --git a/test/install/load.sql b/test/install/load.sql new file mode 100644 index 0000000..1ad130c --- /dev/null +++ b/test/install/load.sql @@ -0,0 +1,271 @@ +\set ECHO none +/* + * Foundation installer for the whole regression run. pgxntool's + * PGXNTOOL_ENABLE_TEST_INSTALL feature runs this file, committed, in its + * own pg_regress session before the regular test SQL files run -- so + * whatever it commits here survives into every test file, instead of each + * one creating its own install from scratch. This is the ONLY place that + * knows how the extension gets onto the system, in every mode -- the + * regular test files (test/sql/base.sql, test/sql/pgtap.sql) always assume + * both extensions are already installed. NOTE: this comment deliberately + * never spells out a bare wildcard glob right after a slash -- slash-star + * is a comment opener, and Postgres nests block comments, so an unbalanced + * extra opener earlier in a comment silently swallows the rest of the file + * instead of erroring where you'd notice (hit this for real while writing + * this file -- see the PR description). + * + * TEST_LOAD_SOURCE (make var -> test_factory.test_load_mode GUC, see + * Makefile) picks how the extension gets to its target state: + * fresh (default) - CREATE EXTENSION test_factory_pgtap CASCADE. + * update - CREATE EXTENSION VERSION :from, then ALTER EXTENSION UPDATE, + * then install test_factory_pgtap too. + * existing - extension is already installed (a real pg_upgrade target, or + * an out-of-band update) -- assert-only, never drop/create. + */ +\i test/helpers/psql.sql +\i test/roles.sql + +SET client_min_messages = WARNING; + +/* + * Read unconditionally, without missing_ok: the Makefile always exports + * test_factory.test_load_mode, so an unset GUC here means the harness + * itself is broken, not "assume fresh". + */ +SELECT + current_setting('test_factory.test_load_mode') AS load_mode + , current_setting('test_factory.test_update_from') AS update_from + , current_setting('test_factory.test_update_to') AS update_to + , current_setting('test_factory.test_update_to') <> '' AS has_update_to +\gset + +DO $$ +BEGIN + IF current_setting('test_factory.test_load_mode') NOT IN ('fresh', 'update', 'existing') THEN + RAISE EXCEPTION 'test_factory.test_load_mode must be fresh, update or existing, got %', current_setting('test_factory.test_load_mode'); + END IF; +END $$; + +/* + * test_role is test infrastructure, not part of what fresh/update/existing + * describe -- (re)create it idempotently in every mode. A real pg_upgrade + * target (existing mode) carries global objects like roles over, but don't + * assume that; a from-scratch "existing" target might not have it. + */ +SELECT NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = :'test_role') AS need_role +\gset +\if :need_role +CREATE ROLE :test_role; +\endif + +/* + * psql's \if only accepts a plain boolean token, not a comparison + * expression -- compute it via SQL first (\if :load_mode = 'existing' would + * silently misparse instead of erroring). + */ +SELECT :'load_mode' = 'existing' AS is_existing +\gset +-- existing-vs-fresh/update +\if :is_existing + + /* + * existing: never drop/create/update -- only assert the extensions are + * actually there, at the version this build considers current. Never + * hardcode the expected version (see doc's CI dynamic-version-assertion + * guidance); default_version comes from the same control file `make` + * itself builds from. + */ + DO $$ + DECLARE + v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'test_factory'); + v_default text := (SELECT default_version FROM pg_available_extensions WHERE name = 'test_factory'); + BEGIN + IF v_installed IS NULL THEN + RAISE EXCEPTION 'test_load_mode=existing but test_factory is not installed'; + END IF; + IF v_installed IS DISTINCT FROM v_default THEN + RAISE EXCEPTION 'test_factory installed at % but default_version is %', v_installed, v_default; + END IF; + END $$; + + DO $$ + DECLARE + v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'test_factory_pgtap'); + v_default text := (SELECT default_version FROM pg_available_extensions WHERE name = 'test_factory_pgtap'); + BEGIN + IF v_installed IS NULL THEN + RAISE EXCEPTION 'test_load_mode=existing but test_factory_pgtap is not installed'; + END IF; + IF v_installed IS DISTINCT FROM v_default THEN + RAISE EXCEPTION 'test_factory_pgtap installed at % but default_version is %', v_installed, v_default; + END IF; + END $$; + + /* + * Dependency guard (advanced-extension-testing checklist item 6): nothing + * else stops a stray CASCADE drop, or a logic bug that falls through to + * the fresh/update branch below, from silently destroying the real + * upgraded/updated objects this mode exists to test -- the suite would + * then quietly pass against a fresh reinstall instead. + * + * Only test_factory_pgtap needs an artificial guard here. test_factory + * already has a natural one for free: test_factory_pgtap's own control + * file (`requires = 'pgtap, test_factory'`) already makes a non-CASCADE + * DROP EXTENSION test_factory fail on its own, as long as + * test_factory_pgtap is still installed -- proved below too, so a future + * change that weakens that dependency doesn't go unnoticed. Nothing + * depends on test_factory_pgtap itself, so it gets an explicit guard. + * + * Only planted in existing mode, not fresh/update: the guard's whole + * point is protecting the real upgraded/updated objects this mode exists + * to test, and fresh/update modes have nothing irreplaceable to protect + * (a re-run recreates everything from scratch there anyway). Note the + * guard only blocks a non-CASCADE drop -- it can't stop a logic bug that + * misroutes into the fresh/update branch below, since that branch's own + * drop-first reset uses CASCADE. Real protection against that specific + * failure mode is the is_existing check itself being correct, not this + * guard; this guard's job is narrower (an accidental *non-cascade* drop + * elsewhere while existing mode's state is live). + */ + CREATE SCHEMA IF NOT EXISTS test_factory_drop_guard; + CREATE OR REPLACE VIEW test_factory_drop_guard.guard AS + SELECT 'tf.tap(text,text)'::regprocedure AS guarded_member; + + DO $$ + BEGIN + BEGIN + DROP EXTENSION test_factory_pgtap; + RAISE EXCEPTION 'dependency guard is not working: non-CASCADE DROP EXTENSION test_factory_pgtap succeeded'; + EXCEPTION + WHEN dependent_objects_still_exist THEN + NULL; -- expected: the guard view blocked it + END; + END $$; + + DO $$ + BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'test_factory_pgtap') THEN + RAISE EXCEPTION 'dependency guard proof left test_factory_pgtap dropped'; + END IF; + IF NOT EXISTS ( + SELECT 1 FROM pg_class + WHERE relname = 'guard' AND relnamespace = 'test_factory_drop_guard'::regnamespace + ) THEN + RAISE EXCEPTION 'dependency guard proof left the guard view itself missing'; + END IF; + END $$; + + DO $$ + BEGIN + BEGIN + DROP EXTENSION test_factory; + RAISE EXCEPTION 'test_factory''s natural drop-guard (test_factory_pgtap''s requires clause) is not working: non-CASCADE DROP EXTENSION test_factory succeeded'; + EXCEPTION + WHEN dependent_objects_still_exist THEN + NULL; -- expected + END; + END $$; + +-- existing-vs-fresh/update +\else + + /* + * fresh/update: drop-first reset, so a re-run against a non-fresh DB + * (e.g. local dev) starts from a known state. test_factory's own role + * bootstrapping (CREATE ROLE test_factory__owner, guarded by WHEN + * duplicate_object in sql/test_factory.sql) already tolerates being + * re-run, so unlike pgxntool's own drop-first example there's no + * separate role-drop step needed here. + */ + DROP EXTENSION IF EXISTS test_factory_pgtap CASCADE; + DROP EXTENSION IF EXISTS test_factory CASCADE; + + /* + * pgtap must land in a dedicated "tap" schema BEFORE test_factory_pgtap + * installs, not after. test_factory_pgtap.control declares pgtap as a + * requirement too, so the CREATE EXTENSION ... CASCADE (or plain CREATE + * EXTENSION, in update mode) below would otherwise cascade-install pgtap + * itself into whatever schema this session's ambient search_path + * resolves to (public, by default) -- and since that satisfies "pgtap is + * already installed", the main suite's own tap_setup.sql (which does + * CREATE EXTENSION IF NOT EXISTS pgtap SCHEMA tap) then skips creating it + * in "tap" at all, leaving every pgTAP-based test file failing with + * "function no_plan() does not exist" (hit this for real writing this + * file). IF NOT EXISTS on both statements: harmless no-op on a rerun + * where a previous pass already did this and the drop-first reset above + * didn't touch it (cascading test_factory_pgtap's drop doesn't remove + * pgtap -- pgtap is its dependency, not the other way around). + */ + CREATE SCHEMA IF NOT EXISTS tap; + CREATE EXTENSION IF NOT EXISTS pgtap SCHEMA tap; + + -- Captured before either branch below runs CREATE EXTENSION, so the + -- role-restore proof after \endif covers whichever one actually ran. + SELECT current_user AS role_before_install + \gset + + SELECT :'load_mode' = 'update' AS is_update + \gset + -- update-vs-fresh install + \if :is_update + + CREATE EXTENSION test_factory VERSION :'update_from'; + SET client_min_messages = ERROR; -- suppress update-script deprecation NOTICEs + \if :has_update_to + ALTER EXTENSION test_factory UPDATE TO :'update_to'; + \else + ALTER EXTENSION test_factory UPDATE; + \endif + SET client_min_messages = WARNING; + /* + * test_factory_pgtap has only ever shipped one version, so there's no + * update path of its own to exercise -- just install it at current so + * the rest of the suite can assume it's present, same as fresh mode. + */ + CREATE EXTENSION test_factory_pgtap; + + -- update-vs-fresh install + \else + + /* + * fresh: install for real, right here, uniformly with update/existing -- + * so test/sql/base.sql and test/sql/pgtap.sql can assume both + * extensions are already present in every mode, instead of each mode + * needing its own install-or-skip dance (what test/helpers/ + * create_extension.sql used to do; deleted along with this change). + * CASCADE proves test_factory_pgtap.control's "requires = 'pgtap, + * test_factory'" line actually pulls test_factory in -- + * test/sql/pgtap.sql separately proves the dependency is *enforced* + * (via pg_depend), not just that cascade happens to work. + */ + CREATE EXTENSION test_factory_pgtap CASCADE; + + -- update-vs-fresh install + \endif + + /* + * Prove CREATE EXTENSION restored the calling role (whichever branch + * above actually ran it) -- a real security property, not a formality: + * test_factory.sql's install script does its own work as + * test_factory__owner via SET LOCAL ROLE, saving/restoring the original + * role around it. Compared from OUTSIDE (current_user before vs after), + * not by inspecting the internal GUC test_factory.sql saves it to -- + * that GUC is transaction-scoped (set_config(..., true)) and would + * already be gone by the time a later statement in this autocommit + * session could read it. + */ + SELECT current_user AS role_after_install + \gset + SELECT :'role_before_install' = :'role_after_install' AS role_was_restored + \gset + \if :role_was_restored + \else + DO $$ BEGIN RAISE EXCEPTION 'CREATE EXTENSION did not restore the calling role'; END $$; + \endif + +-- existing-vs-fresh/update +\endif + +SET client_min_messages = NOTICE; + +-- vi: expandtab ts=2 sw=2 diff --git a/test/roles.sql b/test/roles.sql new file mode 100644 index 0000000..a48f56f --- /dev/null +++ b/test/roles.sql @@ -0,0 +1,10 @@ +/* + * Single source of truth for test-only role names. Roles are global (not + * schema- or session-scoped), so this can't be a table constant -- it's a + * psql variable, \i'd from any session that needs to reference the name: + * test/install/load.sql, and test/helpers/create.sql (via + * test/helpers/deps.sql) for the *.sql files under test/sql/. + */ +\set test_role test_role + +-- vi: expandtab ts=2 sw=2 diff --git a/test/sql/base.sql b/test/sql/base.sql index f52bf4a..f41978c 100644 --- a/test/sql/base.sql +++ b/test/sql/base.sql @@ -1,8 +1,7 @@ \set ECHO none \i test/helpers/setup.sql -\set extension_name test_factory -\i test/helpers/create_extension.sql +-- test/install/load.sql already installed the extension, in every mode. -- NOTE: This runs some tests itself \i test/helpers/create.sql diff --git a/test/sql/pgtap.sql b/test/sql/pgtap.sql index 2abbeb6..f0ecbcb 100644 --- a/test/sql/pgtap.sql +++ b/test/sql/pgtap.sql @@ -1,12 +1,26 @@ \set ECHO none \i test/helpers/setup.sql -\set extension_name test_factory -\i test/helpers/create_extension.sql -DROP TABLE pre_install_role; -DROP TABLE post_install_role; -\set extension_name test_factory_pgtap -\i test/helpers/create_extension.sql +SET search_path = tap; + +/* + * Confirms test_factory_pgtap.control's "requires = 'pgtap, test_factory'" + * is actually enforced by Postgres, via a pg_depend extension-requires- + * extension edge -- deptype 'n' (normal), NOT 'e' (DEPENDENCY_EXTENSION, + * which instead means "this object belongs to this extension"). + */ +SELECT ok( + EXISTS ( + SELECT 1 + FROM pg_depend d + JOIN pg_extension req ON d.objid = req.oid AND d.classid = 'pg_extension'::regclass + JOIN pg_extension dep ON d.refobjid = dep.oid AND d.refclassid = 'pg_extension'::regclass + WHERE req.extname = 'test_factory_pgtap' + AND dep.extname = 'test_factory' + AND d.deptype = 'n' + ) + , 'test_factory_pgtap depends on test_factory (control file requires is real and enforced)' +); -- NOTE: This runs some tests itself. It also changes search_path \i test/helpers/create.sql