From c8954ac7b950d9380df80f1fac9e2334fdbd94fd Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 30 Jul 2026 18:02:30 -0500 Subject: [PATCH 01/13] Add test/build: extension-metadata tests + raw SQL syntax checks Moves install-mechanics testing out of test/sql/install.sql and the top of test/sql/pgtap.sql (which tested extension packaging -- dependency declarations, clean install/uninstall -- not test_factory's actual registering/getting business logic) into test/build/install.sql, using pgxntool's test/build feature. Quoting its own purpose comment in pgxntool/base.mk: Validates that extension SQL files are syntactically correct by running files from test/build/ through pg_regress. This provides better error messages than CREATE EXTENSION failures. Also adds test/build/syntax.sql, which \i's the raw generated versioned SQL files (sql/test_factory--0.5.0.sql, sql/test_factory_pgtap--0.1.0.sql) directly rather than through CREATE EXTENSION, so a genuine syntax error shows up immediately instead of being obscured by a generic CREATE EXTENSION failure. Verified this works by deliberately introducing a typo locally and confirming both new tests surfaced it clearly, then reverting. Both new files hit known, harmless errors baked into their expected output (see comments in each file for the full explanation): - pg_extension_config_dump() always errors when its script is \i'd directly instead of run via CREATE/ALTER EXTENSION. - `SET ROLE ""` fails because the role-restore GUC set via pg_catalog.set_config(..., true) is transaction-scoped, and plain \i (autocommit) gives each statement its own implicit transaction. Also dials VERBOSITY down to "default" (from psql.sql's "verbose") since verbose mode's backend source LOCATION lines differ across PG major versions/builds, which would break single-expected-file matching across the PG12-17 CI matrix. Enables PGXNTOOL_ENABLE_TEST_BUILD explicitly (matching this Makefile's existing preference for explicit-over-implicit config) and adds a CI step that runs `make test-build` directly: pgxn-tools' pg-build-test invokes `make installcheck`, never `make test`, so test-build's dependency chain (wired only into `test`) would otherwise silently never run in CI. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 10 +++++ Makefile | 5 +++ test/CLAUDE.md | 21 +++++++-- test/build/expected/install.out | 8 ++++ test/build/expected/syntax.out | 30 +++++++++++++ test/build/install.sql | 77 +++++++++++++++++++++++++++++++++ test/build/syntax.sql | 69 +++++++++++++++++++++++++++++ test/expected/install.out | 9 ---- test/expected/pgtap.out | 25 +++++------ test/helpers/tap_setup.sql | 4 -- test/sql/install.sql | 30 ------------- test/sql/pgtap.sql | 9 ---- 12 files changed, 229 insertions(+), 68 deletions(-) create mode 100644 test/build/expected/install.out create mode 100644 test/build/expected/syntax.out create mode 100644 test/build/install.sql create mode 100644 test/build/syntax.sql delete mode 100644 test/expected/install.out delete mode 100644 test/sql/install.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3524b03..511139a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,3 +33,13 @@ jobs: uses: actions/checkout@v4 - name: Test on PostgreSQL ${{ matrix.pg }} run: pg-build-test + # pg-build-test (from pgxn-tools) runs `make all; sudo make install; + # make installcheck` -- it never invokes the `test` target, so + # pgxntool's test-build feature (wired only into `test`'s dependency + # chain, not `installcheck`) would otherwise never run in CI even + # though `PGXNTOOL_ENABLE_TEST_BUILD = yes` is set in the Makefile. + # Run it explicitly here so a SQL syntax error or a packaging/install + # regression actually fails CI, instead of only being caught by a + # developer who happens to run `make test` locally. + - name: Verify test/build (SQL syntax + install mechanics) + run: make test-build diff --git a/Makefile b/Makefile index 913590c..515b394 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,10 @@ include pgxntool/base.mk +# Explicit rather than relying on base.mk's auto-detection of test/build/*.sql +# files, so an accidental deletion of test/build/'s contents is a loud error +# instead of silently disabling this check. +PGXNTOOL_ENABLE_TEST_BUILD = yes + # 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 ac12354..12ea604 100644 --- a/test/CLAUDE.md +++ b/test/CLAUDE.md @@ -10,8 +10,14 @@ The test_factory extension uses **pgTAP** (PostgreSQL's unit testing framework) ### Test Files - `test/sql/base.sql` - Core functionality tests (22 tests) -- `test/sql/install.sql` - Extension installation/uninstallation tests - `test/sql/pgtap.sql` - pgTAP integration and `tf.tap()` function tests +- `test/build/install.sql` - Extension packaging/install-mechanics checks + (dependency declarations, clean install/uninstall). Runs via pgxntool's + `test/build` feature (plain SQL + pg_regress diffing, no pgTAP) in an + isolated database, separate from the main pgTAP-based suite above. +- `test/build/syntax.sql` - Runs the raw, generated versioned SQL install + scripts (`sql/*--*.sql`) directly via `\i`, to catch SQL syntax errors with + a clearer error than a `CREATE EXTENSION` failure would give. ### Expected Results - `test/expected/*.out` - Expected test output for regression testing @@ -41,15 +47,24 @@ The test_factory extension uses **pgTAP** (PostgreSQL's unit testing framework) - **Permission Isolation** - Tests with unprivileged `test_role` - **Temp Table Cleanup** - Verifies temporary installation objects are removed -### Installation Tests (`install.sql`) +### Installation/Packaging Tests (`test/build/install.sql`) - **Dependency Validation** - Tests extension dependency requirements - **Clean Installation** - Tests CREATE EXTENSION without conflicts - **Clean Removal** - Tests DROP EXTENSION without orphaned objects +- Runs via pgxntool's `test/build` feature (plain SQL/pg_regress diffing, + not pgTAP), in an isolated database separate from the main suite below. + +### Raw SQL Syntax Tests (`test/build/syntax.sql`) +- Runs `sql/test_factory--*.sql` and `sql/test_factory_pgtap--*.sql` directly + via `\i` (not `CREATE EXTENSION`), so a genuine syntax error is reported + clearly instead of being obscured by a generic CREATE EXTENSION failure. +- 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. ### pgTAP Integration Tests (`pgtap.sql`) - **tf.tap() Function** - Tests pgTAP wrapper functionality - **Error Handling** - Tests proper error reporting for invalid inputs -- **Extension Dependencies** - Validates test_factory_pgtap requires test_factory ## Test Data Model diff --git a/test/build/expected/install.out b/test/build/expected/install.out new file mode 100644 index 0000000..dce592a --- /dev/null +++ b/test/build/expected/install.out @@ -0,0 +1,8 @@ +\set ECHO none +ERROR: required extension "test_factory" is not installed +HINT: Use CREATE EXTENSION ... CASCADE to install required extensions too. + regprocedure +------------------- + tf.tap(text,text) +(1 row) + diff --git a/test/build/expected/syntax.out b/test/build/expected/syntax.out new file mode 100644 index 0000000..9bb1e4a --- /dev/null +++ b/test/build/expected/syntax.out @@ -0,0 +1,30 @@ +\set ECHO none +NOTICE: extension "test_factory_pgtap" does not exist, skipping +NOTICE: extension "test_factory" does not exist, skipping + set_config +------------ + root +(1 row) + +psql:sql/test_factory--0.5.0.sql:42: WARNING: SET LOCAL can only be used in transaction blocks +psql:sql/test_factory--0.5.0.sql:56: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION +psql:sql/test_factory--0.5.0.sql:57: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION +psql:sql/test_factory--0.5.0.sql:95: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text +psql:sql/test_factory--0.5.0.sql:117: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text +psql:sql/test_factory--0.5.0.sql:123: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text +psql:sql/test_factory--0.5.0.sql:262: ERROR: zero-length delimited identifier at or near """" +LINE 1: SET ROLE "" + ^ +QUERY: SET ROLE "" +CONTEXT: PL/pgSQL function inline_code_block line 3 at EXECUTE + set_config +------------ + root +(1 row) + +psql:sql/test_factory_pgtap--0.1.0.sql:10: WARNING: SET LOCAL can only be used in transaction blocks +psql:sql/test_factory_pgtap--0.1.0.sql:39: ERROR: zero-length delimited identifier at or near """" +LINE 1: SET ROLE "" + ^ +QUERY: SET ROLE "" +CONTEXT: PL/pgSQL function inline_code_block line 3 at EXECUTE diff --git a/test/build/install.sql b/test/build/install.sql new file mode 100644 index 0000000..11eba23 --- /dev/null +++ b/test/build/install.sql @@ -0,0 +1,77 @@ +\set ECHO none +\i test/helpers/psql.sql + +/* + * Extension packaging/install-mechanics checks. + * + * These verify that the extension *packaging* is correct (control file + * dependency declarations, clean install/uninstall) -- not test_factory's + * actual business logic (registering/getting test data; that's covered by + * test/sql/base.sql and test/sql/pgtap.sql). This lives in test/build + * because that's exactly what pgxntool's test/build feature is for. Quoting + * pgxntool/base.mk's own comment on the purpose of test/build: + * + * Validates that extension SQL files are syntactically correct by running + * files from test/build/ through pg_regress. This provides better error + * messages than CREATE EXTENSION failures. + * + * test/build runs in an isolated database with none of the main suite's + * test/helpers/setup.sql machinery (no pgTAP, no roles, no --dbname), so + * this file uses plain SQL and lets pg_regress's classic expected-output + * diffing do the comparison instead of pgTAP assertions. + * + * The one intentional error below (the bare CREATE EXTENSION) is expected to + * abort only its own statement, not the rest of this script, since psql runs + * each top-level statement as its own autocommit transaction by default. We + * still disable ON_ERROR_STOP (which test/helpers/psql.sql just turned on) + * to make that reliance explicit rather than depending on pg_regress's own + * default. + * + * We also dial VERBOSITY back down from psql.sql's "verbose" to "default": + * verbose mode appends a LOCATION line with the triggering backend C source + * file/line for every error, which differs across PG major versions (and + * even between builds of the same version) -- fine for interactive + * debugging, but it would make this file's expected output impossible to + * match across the PG12/PG17 matrix this repo tests against. + */ +\set ON_ERROR_STOP false +\set VERBOSITY default + +SET client_min_messages = WARNING; + +-- pgtap is a declared dependency of test_factory_pgtap too. Install it up +-- front so the bare-create failure below isolates specifically on +-- test_factory being missing, not on pgtap also being missing (test/build's +-- database doesn't have pgtap pre-installed the way the main suite's +-- test/helpers/setup.sql arranges). +CREATE EXTENSION IF NOT EXISTS pgtap; + +-- Start from a clean slate. Non-CASCADE so we'd notice if a previous step in +-- this file (or a run before it) left something installed unexpectedly. +DROP EXTENSION IF EXISTS test_factory_pgtap; +DROP EXTENSION IF EXISTS test_factory; + +/* + * A bare (non-CASCADE) CREATE EXTENSION for test_factory_pgtap must fail, + * since test_factory isn't installed. This proves + * test_factory_pgtap.control's "requires = 'pgtap, test_factory'" line is + * real and enforced by Postgres, not just documentation. + * IF YOU GET A "schema tf does not exist" ERROR HERE INSTEAD (i.e. the + * CREATE EXTENSION below succeeds), the dependency declaration is + * missing/broken -- check test_factory_pgtap.control. + */ +CREATE EXTENSION test_factory_pgtap; + +-- CASCADE should pull test_factory in automatically. +CREATE EXTENSION test_factory_pgtap CASCADE; + +-- Confirm tf.tap(text, text) exists after install. Casting to regprocedure +-- errors out (visibly, in the diff) if the function doesn't exist. +SELECT 'tf.tap(text, text)'::regprocedure; + +-- Clean removal: a bare (non-CASCADE) DROP must not error, proving nothing +-- else was left depending on either extension. +DROP EXTENSION test_factory_pgtap; +DROP EXTENSION test_factory; + +-- vi: expandtab ts=2 sw=2 diff --git a/test/build/syntax.sql b/test/build/syntax.sql new file mode 100644 index 0000000..83b8f7d --- /dev/null +++ b/test/build/syntax.sql @@ -0,0 +1,69 @@ +\set ECHO none +\i test/helpers/psql.sql + +/* + * Run the raw, generated versioned SQL install scripts directly (\i, NOT + * CREATE EXTENSION) against a fresh database. This is a deliberately simple + * test: its whole point is that a genuine SQL syntax error anywhere in + * either file shows up immediately and clearly, instead of being obscured + * behind a generic CREATE EXTENSION failure the way test/build/install.sql + * (or a normal `make install`) would report it. + * + * KNOWN / EXPECTED ERRORS BAKED INTO THIS TEST'S EXPECTED OUTPUT + * (verified locally; these are not bugs -- see explanation before each + * occurrence below): + * + * 1. Two "pg_extension_config_dump() can only be called from an SQL script + * executed by CREATE EXTENSION" errors (both from test_factory's file). + * That function always errors when its containing script is \i'd + * directly rather than run by CREATE/ALTER EXTENSION, regardless of + * whether the file's SQL is otherwise valid. + * + * 2. One "zero-length delimited identifier" error from `SET ROLE ""` near + * the end of EACH file. Both files save the caller's role in a GUC via + * `pg_catalog.set_config(..., true)` (the trailing `true` = is_local, + * i.e. transaction-scoped) so they can restore it before the script + * ends. A real CREATE EXTENSION runs its whole script as one transaction, + * so that works fine there. Run via plain \i in autocommit (no enclosing + * transaction), each top-level statement is its own implicit + * transaction, so the GUC's scope ends immediately -- by the time the + * restore-role code runs, current_setting() returns an empty string, and + * `SET ROLE ""` fails this way instead. + * + * We deliberately do NOT wrap the \i calls below in an explicit + * BEGIN/COMMIT to "fix" the above -- that would make the FIRST + * pg_extension_config_dump error poison the whole transaction, aborting + * every statement after it instead of just these known, harmless ones + * (verified locally: with an explicit transaction, every subsequent + * statement fails with "current transaction is aborted", which would hide + * real syntax errors instead of surfacing them). Relatedly, we disable + * ON_ERROR_STOP below (which test/helpers/psql.sql just turned on) so that + * these known errors don't stop psql from processing the rest of either + * file -- each statement already being its own autocommit transaction means + * a REAL syntax error later in either file still shows up as its own clear + * diff, it just doesn't halt the whole test. + * + * We also dial VERBOSITY back down from psql.sql's "verbose" to "default": + * verbose mode appends a LOCATION line with the triggering backend C source + * file/line for every error above, which differs across PG major versions + * (and even between builds of the same version) -- it would make this + * file's expected output impossible to match across the PG12/PG17 matrix + * this repo tests against. + */ +\set ON_ERROR_STOP false +\set VERBOSITY default + +-- Guard against test/build/install.sql (or a previous run) having left +-- either extension installed; keep this file runnable on its own regardless +-- of test-build's file-execution order. +DROP EXTENSION IF EXISTS test_factory_pgtap; +DROP EXTENSION IF EXISTS test_factory; + +-- test_factory must run first: test_factory_pgtap's raw file creates +-- objects in the "tf" schema, owned by the test_factory__owner role, both of +-- which only exist once test_factory's file has run. +\i sql/test_factory--0.5.0.sql + +\i sql/test_factory_pgtap--0.1.0.sql + +-- vi: expandtab ts=2 sw=2 diff --git a/test/expected/install.out b/test/expected/install.out deleted file mode 100644 index ae61611..0000000 --- a/test/expected/install.out +++ /dev/null @@ -1,9 +0,0 @@ -\set ECHO none -ok 1 - drop extension test_factory_pgtap -ok 2 - drop extension test_factory -ok 3 - Extension test_factory should not exist -ok 4 - Extension test_factory_pgtap should not exist -ok 5 - create extension -ok 6 - Function tf.tap(text, text) should exist -ok 7 - clean-up test_factory_pgtap -ok 8 - clean-up test_factory diff --git a/test/expected/pgtap.out b/test/expected/pgtap.out index b2b7706..5f552dc 100644 --- a/test/expected/pgtap.out +++ b/test/expected/pgtap.out @@ -1,17 +1,16 @@ \set ECHO none -ok 1 - Ensure test_factory is a dependency of test_factory_pgtap Creating extension test_factory Creating extension test_factory_pgtap -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 - Ensure role is put back after install -ok 7 - Security definer function _tf.schema__getsert has search_path=pg_catalog -ok 8 - Security definer function _tf.test_factory__get has search_path=pg_catalog -ok 9 - Security definer function _tf.test_factory__set has search_path=pg_catalog -ok 10 - Security definer function _tf.table_create has search_path=pg_catalog -ok 11 - Security definer function _tf.get has search_path=pg_catalog +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 - Get test data set "base" for table invoice ok 12 - Get test data set "base" for table invoice -ok 13 - Get test data set "base" for table invoice -ok 14 - Ensure we get sane error for a non-existent table +ok 13 - Ensure we get sane error for a non-existent table diff --git a/test/helpers/tap_setup.sql b/test/helpers/tap_setup.sql index c127096..b90844c 100644 --- a/test/helpers/tap_setup.sql +++ b/test/helpers/tap_setup.sql @@ -1,9 +1,5 @@ \i test/helpers/psql.sql -/* - * NOTE: if you get errors about things already existing it's because they've - * been left behind by test/sql/install.sql - */ SET client_min_messages = WARNING; CREATE SCHEMA IF NOT EXISTS tap; SET search_path = tap; diff --git a/test/sql/install.sql b/test/sql/install.sql deleted file mode 100644 index ae58bf3..0000000 --- a/test/sql/install.sql +++ /dev/null @@ -1,30 +0,0 @@ -\set ECHO none -\i test/helpers/setup.sql - -SET client_min_messages = WARNING; - -/* - * DO NOT use CASCADE here; we want this to fail if there's anything installed - * that depends on it. - */ -SELECT lives_ok($$DROP EXTENSION IF EXISTS test_factory_pgtap$$, 'drop extension test_factory_pgtap'); -SELECT lives_ok($$DROP EXTENSION IF EXISTS test_factory$$, 'drop extension test_factory'); - -SELECT hasnt_extension( 'test_factory' ); -SELECT hasnt_extension( 'test_factory_pgtap' ); - -SELECT lives_ok($$CREATE EXTENSION test_factory_pgtap CASCADE$$, 'create extension'); -COMMIT; - -SELECT has_function('tf', 'tap', array['text','text']); - --- Cleanup -SELECT lives_ok($$DROP EXTENSION IF EXISTS test_factory_pgtap$$, 'clean-up test_factory_pgtap'); -SELECT lives_ok($$DROP EXTENSION IF EXISTS test_factory$$, 'clean-up test_factory'); - -/* - * Arguably we should cleanup pgtap and the tap schema... - */ - --- vi: expandtab ts=2 sw=2 - diff --git a/test/sql/pgtap.sql b/test/sql/pgtap.sql index ba0c4b3..2abbeb6 100644 --- a/test/sql/pgtap.sql +++ b/test/sql/pgtap.sql @@ -1,15 +1,6 @@ \set ECHO none \i test/helpers/setup.sql -SET search_path = tap; --- IF YOU GET A "schema tf does not exist" error here then the dependency is missing! -SELECT throws_ok( - $$CREATE EXTENSION test_factory_pgtap$$ - , '42704' - , 'required extension "test_factory" is not installed' - , 'Ensure test_factory is a dependency of test_factory_pgtap' -); - \set extension_name test_factory \i test/helpers/create_extension.sql DROP TABLE pre_install_role; From 5b70c8b3aaa7c0146b542e8c553a5a0b321d943d Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 30 Jul 2026 18:05:26 -0500 Subject: [PATCH 02/13] Install rsync before make test-build in CI pgxntool/run-test-build.sh (vendored) shells out to rsync to sync test/build/*.sql into test/build/sql/, but the pgxn/pgxn-tools container image doesn't ship rsync -- caught by the first real CI run on this PR ("rsync: command not found", pgxntool/base.mk:473: test-build, Error 127). Not reproducible locally since this dev container already has rsync. Worth a pgxntool-test issue since any project enabling PGXNTOOL_ENABLE_TEST_BUILD on this same image would hit the identical gap; not fixed here (that's vendored, not this repo's own script). Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 511139a..edb4f4b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,4 +42,12 @@ jobs: # regression actually fails CI, instead of only being caught by a # developer who happens to run `make test` locally. - name: Verify test/build (SQL syntax + install mechanics) - run: make test-build + # pgxntool/run-test-build.sh (vendored, not this repo's own script) + # shells out to rsync to sync test/build/*.sql into test/build/sql/ -- + # the pgxn/pgxn-tools image doesn't ship rsync, so install it first. + # Any pgxntool project enabling PGXNTOOL_ENABLE_TEST_BUILD on this + # image would hit the same gap; worth a pgxntool-test issue, not + # something to work around inside the vendored script itself. + run: | + apt-get install -y rsync + make test-build From e185cb1a91d8933a5adf3a68076ab50c5440ce58 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 30 Jul 2026 18:09:26 -0500 Subject: [PATCH 03/13] Fix environment-dependent expected output in test/build/syntax.sql Both raw SQL files print a query-result row for their pg_catalog.set_config('..._role', current_user, true) call (the original-role save at the top of each). current_user is whichever role actually connects -- this dev container connects as root, CI's pgxn/pgxn-tools connects as postgres -- so the expected output generated locally didn't match CI's real run (confirmed by the first real CI run after the rsync fix: "root" expected, "postgres" got, both PG13's own failure and independently confirmed by a peer session's diagnosis of the same run). Fixed with `\o /dev/null`: discards normal query-result output for the rest of the file, but NOTICE/WARNING/ERROR go through a separate stream `\o` doesn't touch, so every error this test actually cares about -- including a genuine future syntax error -- still shows up in the diff untouched. Regenerated test/build/expected/syntax.out from a real run (never hand-authored); confirmed clean on both PG12 and PG17. Note: `make results` does not handle test/build's separate results/expected directories (it only copies test/results/*.out -> test/expected/*.out) -- had to copy test/build/results/syntax.out -> test/build/expected/syntax.out by hand, still from a real run, never typed by hand. Worth a pgxntool-test issue: `make results` silently doesn't cover test-build's own output. Co-Authored-By: Claude Sonnet 5 --- test/build/expected/syntax.out | 10 ---------- test/build/syntax.sql | 12 ++++++++++++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/test/build/expected/syntax.out b/test/build/expected/syntax.out index 9bb1e4a..c5024bb 100644 --- a/test/build/expected/syntax.out +++ b/test/build/expected/syntax.out @@ -1,11 +1,6 @@ \set ECHO none NOTICE: extension "test_factory_pgtap" does not exist, skipping NOTICE: extension "test_factory" does not exist, skipping - set_config ------------- - root -(1 row) - psql:sql/test_factory--0.5.0.sql:42: WARNING: SET LOCAL can only be used in transaction blocks psql:sql/test_factory--0.5.0.sql:56: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION psql:sql/test_factory--0.5.0.sql:57: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION @@ -17,11 +12,6 @@ LINE 1: SET ROLE "" ^ QUERY: SET ROLE "" CONTEXT: PL/pgSQL function inline_code_block line 3 at EXECUTE - set_config ------------- - root -(1 row) - psql:sql/test_factory_pgtap--0.1.0.sql:10: WARNING: SET LOCAL can only be used in transaction blocks psql:sql/test_factory_pgtap--0.1.0.sql:39: ERROR: zero-length delimited identifier at or near """" LINE 1: SET ROLE "" diff --git a/test/build/syntax.sql b/test/build/syntax.sql index 83b8f7d..e960fbd 100644 --- a/test/build/syntax.sql +++ b/test/build/syntax.sql @@ -49,9 +49,21 @@ * (and even between builds of the same version) -- it would make this * file's expected output impossible to match across the PG12/PG17 matrix * this repo tests against. + * + * Both files also print a row of query output for their + * `pg_catalog.set_config('..._role', current_user, true)` call (the + * original-role save at the top of each). current_user is whichever role + * actually connects, which is environment-dependent (this repo's own local + * testing connects as a different OS role than CI does) -- caught for real + * when this file's expected output, generated locally, didn't match CI. + * `\o /dev/null` discards normal query-result output for the rest of this + * file; NOTICE/WARNING/ERROR go to a separate stream psql doesn't route + * through `\o`, so every error this test actually cares about (including a + * genuine future syntax error) still shows up in the diff untouched. */ \set ON_ERROR_STOP false \set VERBOSITY default +\o /dev/null -- Guard against test/build/install.sql (or a previous run) having left -- either extension installed; keep this file runnable on its own regardless From 9303c7ce9ccd4956005345fe2bc6afad94994dc5 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 31 Jul 2026 15:06:25 -0500 Subject: [PATCH 04/13] Drop pg-build-test in CI, fix syntax.sql's mess/portability issues, trim comments CI (.github/workflows/ci.yml): pg-build-test's own failure detection (`make installcheck || status=$?`) can never actually trip -- pgxntool marks `installcheck` .IGNORE, so `make installcheck` always exits 0 regardless of real regression failures (verified locally with a deliberately broken test). Confirmed from pg-build-test's actual source (pgxn/docker-pgxn-tools bin/pg-build-test): `make all; sudo make install; make installcheck || status=$?` -- it never calls `make test` either, so test-build was never reachable through it regardless. Replaced with a direct `make verify-results`, which inspects the real TAP output instead of trusting an exit code, and pulls in test-build via the same `test` dependency chain it already runs. Confirmed locally (4 repeated runs) that a single `make verify-results` with no separate `make install` first is deterministic once pg-build-test's own `MAKEFLAGS=-j $(nproc)` export is out of the picture -- that export (not anything the container sets globally) was the actual source of the install/installcheck race PR #23 found and worked around; not re-litigating that fix there, just noting root cause now confirmed. test/build/syntax.sql: wrapped the \i calls in BEGIN/ROLLBACK with ON_ERROR_ROLLBACK on, so nothing persists in the database whether this runs under pg_regress or ad hoc locally (previously: autocommit, no rollback, would leave a real mess in a developer's own database). Bonus: running the whole thing as one transaction is also more faithful to how CREATE EXTENSION actually behaves, so `current_setting('..._role')` now correctly survives to the end of each file -- the third known/expected error (SET ROLE "" from the role-restore code) is gone entirely, not just documented away. Regenerated expected output from a real run. Trimmed both test/build/*.sql files' comments substantially and added a one-line cross-reference between them (install.sql tests packaging via a real CREATE EXTENSION; syntax.sql tests raw SQL syntax by bypassing it) -- the previous version buried that distinction in multi-paragraph essays. CLAUDE.md: noted that this PGXN distribution ships two structurally similar extensions (test_factory, test_factory_pgtap) and that the test suite shares infrastructure between them accordingly. Verified on both PG12 and PG17: full make test passes, and a plain `psql -f test/build/syntax.sql` against a scratch database leaves zero test_factory objects behind afterward. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 39 ++++++++------- CLAUDE.md | 2 + test/build/expected/syntax.out | 14 ------ test/build/install.sql | 77 +++++++++------------------- test/build/syntax.sql | 91 ++++++++-------------------------- 5 files changed, 68 insertions(+), 155 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index edb4f4b..fe9620c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,23 +31,24 @@ jobs: run: pg-start ${{ matrix.pg }} - name: Check out the repo uses: actions/checkout@v4 + # Not pgxn-tools' own `pg-build-test` helper: it never invokes `make + # test` (it runs `make all; sudo make install; make installcheck` + # directly -- confirmed from its actual source, pgxn/docker-pgxn-tools + # bin/pg-build-test), so it would never reach test-build (wired in only + # via `test`'s own dependency chain). Worse, its failure detection + # (`make installcheck || status=$?`) can never actually trip: pgxntool + # marks `installcheck` `.IGNORE`, so `make installcheck` always exits 0 + # regardless of real regression failures (verified locally with a + # deliberately broken test). `make verify-results` inspects the actual + # TAP output instead of trusting an exit code, and pulls in test-build + # as part of the same `test` dependency chain it already runs. + # + # rsync first: pgxntool/run-test-build.sh (vendored, not this repo's + # own script) needs it to sync test/build/*.sql into test/build/sql/, + # and the pgxn/pgxn-tools image doesn't ship it. Any pgxntool project + # enabling PGXNTOOL_ENABLE_TEST_BUILD on this image would hit the same + # gap -- worth a pgxntool-test issue, not a vendored-script edit here. + - name: Install rsync + run: apt-get install -y rsync - name: Test on PostgreSQL ${{ matrix.pg }} - run: pg-build-test - # pg-build-test (from pgxn-tools) runs `make all; sudo make install; - # make installcheck` -- it never invokes the `test` target, so - # pgxntool's test-build feature (wired only into `test`'s dependency - # chain, not `installcheck`) would otherwise never run in CI even - # though `PGXNTOOL_ENABLE_TEST_BUILD = yes` is set in the Makefile. - # Run it explicitly here so a SQL syntax error or a packaging/install - # regression actually fails CI, instead of only being caught by a - # developer who happens to run `make test` locally. - - name: Verify test/build (SQL syntax + install mechanics) - # pgxntool/run-test-build.sh (vendored, not this repo's own script) - # shells out to rsync to sync test/build/*.sql into test/build/sql/ -- - # the pgxn/pgxn-tools image doesn't ship rsync, so install it first. - # Any pgxntool project enabling PGXNTOOL_ENABLE_TEST_BUILD on this - # image would hit the same gap; worth a pgxntool-test issue, not - # something to work around inside the vendored script itself. - run: | - apt-get install -y rsync - make test-build + run: make verify-results diff --git a/CLAUDE.md b/CLAUDE.md index 33b198c..c476d0d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,6 +10,8 @@ After pushing to a branch with an open PR, monitor CI using `gh pr checks --watc 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. +This PGXN distribution ships **two extensions**: `test_factory` (the core framework) and `test_factory_pgtap` (a thin pgTAP integration wrapper, `tf.tap()`, depending on `test_factory`). Because they're structurally so similar (same install/security-definer/role-restoration patterns, same packaging concerns), the test suite shares a lot of common infrastructure between them (`test/helpers/*`, `test/build/*`) rather than duplicating it per extension -- keep that in mind when adding tests for one and wondering whether the other needs the same treatment. + ## Build System & Development Commands This project uses PGXNtool for build management. Key commands: diff --git a/test/build/expected/syntax.out b/test/build/expected/syntax.out index c5024bb..ac4404c 100644 --- a/test/build/expected/syntax.out +++ b/test/build/expected/syntax.out @@ -1,20 +1,6 @@ \set ECHO none -NOTICE: extension "test_factory_pgtap" does not exist, skipping -NOTICE: extension "test_factory" does not exist, skipping -psql:sql/test_factory--0.5.0.sql:42: WARNING: SET LOCAL can only be used in transaction blocks psql:sql/test_factory--0.5.0.sql:56: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION psql:sql/test_factory--0.5.0.sql:57: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION psql:sql/test_factory--0.5.0.sql:95: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text psql:sql/test_factory--0.5.0.sql:117: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text psql:sql/test_factory--0.5.0.sql:123: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text -psql:sql/test_factory--0.5.0.sql:262: ERROR: zero-length delimited identifier at or near """" -LINE 1: SET ROLE "" - ^ -QUERY: SET ROLE "" -CONTEXT: PL/pgSQL function inline_code_block line 3 at EXECUTE -psql:sql/test_factory_pgtap--0.1.0.sql:10: WARNING: SET LOCAL can only be used in transaction blocks -psql:sql/test_factory_pgtap--0.1.0.sql:39: ERROR: zero-length delimited identifier at or near """" -LINE 1: SET ROLE "" - ^ -QUERY: SET ROLE "" -CONTEXT: PL/pgSQL function inline_code_block line 3 at EXECUTE diff --git a/test/build/install.sql b/test/build/install.sql index 11eba23..88e38e5 100644 --- a/test/build/install.sql +++ b/test/build/install.sql @@ -1,76 +1,47 @@ \set ECHO none \i test/helpers/psql.sql -/* - * Extension packaging/install-mechanics checks. - * - * These verify that the extension *packaging* is correct (control file - * dependency declarations, clean install/uninstall) -- not test_factory's - * actual business logic (registering/getting test data; that's covered by - * test/sql/base.sql and test/sql/pgtap.sql). This lives in test/build - * because that's exactly what pgxntool's test/build feature is for. Quoting - * pgxntool/base.mk's own comment on the purpose of test/build: - * - * Validates that extension SQL files are syntactically correct by running - * files from test/build/ through pg_regress. This provides better error - * messages than CREATE EXTENSION failures. - * - * test/build runs in an isolated database with none of the main suite's - * test/helpers/setup.sql machinery (no pgTAP, no roles, no --dbname), so - * this file uses plain SQL and lets pg_regress's classic expected-output - * diffing do the comparison instead of pgTAP assertions. - * - * The one intentional error below (the bare CREATE EXTENSION) is expected to - * abort only its own statement, not the rest of this script, since psql runs - * each top-level statement as its own autocommit transaction by default. We - * still disable ON_ERROR_STOP (which test/helpers/psql.sql just turned on) - * to make that reliance explicit rather than depending on pg_regress's own - * default. - * - * We also dial VERBOSITY back down from psql.sql's "verbose" to "default": - * verbose mode appends a LOCATION line with the triggering backend C source - * file/line for every error, which differs across PG major versions (and - * even between builds of the same version) -- fine for interactive - * debugging, but it would make this file's expected output impossible to - * match across the PG12/PG17 matrix this repo tests against. - */ +-- Extension packaging checks (dependency declarations, clean +-- install/uninstall) -- not test_factory's business logic, which is +-- test/sql/base.sql and test/sql/pgtap.sql's job. Compare +-- test/build/syntax.sql: that one runs the raw SQL files directly to catch +-- syntax errors; this one goes through a real CREATE EXTENSION to catch +-- packaging errors instead. +-- +-- test/build runs in an isolated database with none of the main suite's +-- pgTAP/role setup, so this is plain SQL, relying on pg_regress's classic +-- expected-output diffing. ON_ERROR_STOP is off since the bare CREATE +-- EXTENSION below is expected to fail (each top-level statement is its own +-- autocommit transaction, so that doesn't touch anything after it). +-- VERBOSITY is knocked down from psql.sql's "verbose" to "default" since the +-- verbose LOCATION line differs across PG majors. \set ON_ERROR_STOP false \set VERBOSITY default SET client_min_messages = WARNING; --- pgtap is a declared dependency of test_factory_pgtap too. Install it up --- front so the bare-create failure below isolates specifically on --- test_factory being missing, not on pgtap also being missing (test/build's --- database doesn't have pgtap pre-installed the way the main suite's --- test/helpers/setup.sql arranges). +-- pgtap is test_factory_pgtap's other declared dependency; install it first +-- so the bare-create failure below isolates on test_factory specifically. CREATE EXTENSION IF NOT EXISTS pgtap; --- Start from a clean slate. Non-CASCADE so we'd notice if a previous step in --- this file (or a run before it) left something installed unexpectedly. +-- Clean slate; non-CASCADE so we'd notice if something was already there. DROP EXTENSION IF EXISTS test_factory_pgtap; DROP EXTENSION IF EXISTS test_factory; -/* - * A bare (non-CASCADE) CREATE EXTENSION for test_factory_pgtap must fail, - * since test_factory isn't installed. This proves - * test_factory_pgtap.control's "requires = 'pgtap, test_factory'" line is - * real and enforced by Postgres, not just documentation. - * IF YOU GET A "schema tf does not exist" ERROR HERE INSTEAD (i.e. the - * CREATE EXTENSION below succeeds), the dependency declaration is - * missing/broken -- check test_factory_pgtap.control. - */ +-- A bare (non-CASCADE) create must fail: test_factory isn't installed yet. +-- Proves test_factory_pgtap.control's "requires = 'pgtap, test_factory'" is +-- real and enforced, not just documentation. IF THIS SUCCEEDS INSTEAD (e.g. +-- with a "schema tf does not exist" error later), the dependency +-- declaration is missing/broken -- check test_factory_pgtap.control. CREATE EXTENSION test_factory_pgtap; -- CASCADE should pull test_factory in automatically. CREATE EXTENSION test_factory_pgtap CASCADE; --- Confirm tf.tap(text, text) exists after install. Casting to regprocedure --- errors out (visibly, in the diff) if the function doesn't exist. +-- tf.tap(text, text) should exist after install; the cast errors visibly if not. SELECT 'tf.tap(text, text)'::regprocedure; --- Clean removal: a bare (non-CASCADE) DROP must not error, proving nothing --- else was left depending on either extension. +-- Clean removal: a bare (non-CASCADE) drop must not error. DROP EXTENSION test_factory_pgtap; DROP EXTENSION test_factory; diff --git a/test/build/syntax.sql b/test/build/syntax.sql index e960fbd..889542f 100644 --- a/test/build/syntax.sql +++ b/test/build/syntax.sql @@ -1,81 +1,34 @@ \set ECHO none \i test/helpers/psql.sql -/* - * Run the raw, generated versioned SQL install scripts directly (\i, NOT - * CREATE EXTENSION) against a fresh database. This is a deliberately simple - * test: its whole point is that a genuine SQL syntax error anywhere in - * either file shows up immediately and clearly, instead of being obscured - * behind a generic CREATE EXTENSION failure the way test/build/install.sql - * (or a normal `make install`) would report it. - * - * KNOWN / EXPECTED ERRORS BAKED INTO THIS TEST'S EXPECTED OUTPUT - * (verified locally; these are not bugs -- see explanation before each - * occurrence below): - * - * 1. Two "pg_extension_config_dump() can only be called from an SQL script - * executed by CREATE EXTENSION" errors (both from test_factory's file). - * That function always errors when its containing script is \i'd - * directly rather than run by CREATE/ALTER EXTENSION, regardless of - * whether the file's SQL is otherwise valid. - * - * 2. One "zero-length delimited identifier" error from `SET ROLE ""` near - * the end of EACH file. Both files save the caller's role in a GUC via - * `pg_catalog.set_config(..., true)` (the trailing `true` = is_local, - * i.e. transaction-scoped) so they can restore it before the script - * ends. A real CREATE EXTENSION runs its whole script as one transaction, - * so that works fine there. Run via plain \i in autocommit (no enclosing - * transaction), each top-level statement is its own implicit - * transaction, so the GUC's scope ends immediately -- by the time the - * restore-role code runs, current_setting() returns an empty string, and - * `SET ROLE ""` fails this way instead. - * - * We deliberately do NOT wrap the \i calls below in an explicit - * BEGIN/COMMIT to "fix" the above -- that would make the FIRST - * pg_extension_config_dump error poison the whole transaction, aborting - * every statement after it instead of just these known, harmless ones - * (verified locally: with an explicit transaction, every subsequent - * statement fails with "current transaction is aborted", which would hide - * real syntax errors instead of surfacing them). Relatedly, we disable - * ON_ERROR_STOP below (which test/helpers/psql.sql just turned on) so that - * these known errors don't stop psql from processing the rest of either - * file -- each statement already being its own autocommit transaction means - * a REAL syntax error later in either file still shows up as its own clear - * diff, it just doesn't halt the whole test. - * - * We also dial VERBOSITY back down from psql.sql's "verbose" to "default": - * verbose mode appends a LOCATION line with the triggering backend C source - * file/line for every error above, which differs across PG major versions - * (and even between builds of the same version) -- it would make this - * file's expected output impossible to match across the PG12/PG17 matrix - * this repo tests against. - * - * Both files also print a row of query output for their - * `pg_catalog.set_config('..._role', current_user, true)` call (the - * original-role save at the top of each). current_user is whichever role - * actually connects, which is environment-dependent (this repo's own local - * testing connects as a different OS role than CI does) -- caught for real - * when this file's expected output, generated locally, didn't match CI. - * `\o /dev/null` discards normal query-result output for the rest of this - * file; NOTICE/WARNING/ERROR go to a separate stream psql doesn't route - * through `\o`, so every error this test actually cares about (including a - * genuine future syntax error) still shows up in the diff untouched. - */ +-- Runs sql/*--*.sql directly via \i (not CREATE EXTENSION), so a genuine +-- syntax error is reported clearly instead of hiding behind a generic +-- CREATE EXTENSION failure. Compare test/build/install.sql: that one tests +-- packaging (dependency declarations, clean install/uninstall) via a real +-- CREATE EXTENSION; this one tests raw SQL syntax by deliberately bypassing +-- it. +-- +-- Wrapped in one transaction, rolled back at the end, so nothing persists +-- whether this runs under pg_regress or ad hoc locally. ON_ERROR_ROLLBACK +-- savepoints each statement, so the two expected errors below +-- (pg_extension_config_dump() requires a real CREATE/ALTER EXTENSION +-- context; harmless here) don't abort the rest of the file -- a real syntax +-- error later still shows up on its own. VERBOSITY is knocked down from +-- psql.sql's "verbose" to "default" since the verbose LOCATION line differs +-- across PG majors. \o /dev/null hides the one row of query output each +-- file prints (current_user, which varies by environment) while leaving +-- NOTICE/WARNING/ERROR visible. \set ON_ERROR_STOP false +\set ON_ERROR_ROLLBACK on \set VERBOSITY default \o /dev/null --- Guard against test/build/install.sql (or a previous run) having left --- either extension installed; keep this file runnable on its own regardless --- of test-build's file-execution order. -DROP EXTENSION IF EXISTS test_factory_pgtap; -DROP EXTENSION IF EXISTS test_factory; +BEGIN; --- test_factory must run first: test_factory_pgtap's raw file creates --- objects in the "tf" schema, owned by the test_factory__owner role, both of --- which only exist once test_factory's file has run. +-- test_factory first: test_factory_pgtap's file needs its "tf" schema/role. \i sql/test_factory--0.5.0.sql - \i sql/test_factory_pgtap--0.1.0.sql +ROLLBACK; + -- vi: expandtab ts=2 sw=2 From 253d1fc5e9ef32d7fc39d664c554ffd8d94a437c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 31 Jul 2026 15:12:58 -0500 Subject: [PATCH 05/13] Restore explicit make install before make verify-results in CI The previous commit (95d8be4) removed this based on local testing that turned out to be invalid: a concurrent process in this dev container had installed PostgreSQL 18, silently shifting the default `pg_config` on PATH out from under an unrelated test, which produced a false "it works without the extra step" result. Re-tested properly (PG_CONFIG pinned to match the actual target cluster, extension deliberately removed from the system first) and found a REAL, pre-existing pgxntool bug, independent of anything in this PR: on a genuinely fresh system, `make test`/`make verify-results` runs the main suite's `installcheck` before `install` ever copies the extension's files into place. Root cause: `TEST_DEPS` textually ends up as `testdeps check-stale-expected test-build install installcheck`, but `check-stale-expected: installcheck` is its own direct dependency edge -- Make resolves that early (2nd in the list), running the main suite against a not-yet-installed extension, well before it ever reaches the literal `install installcheck` pair at the end. `test-build` avoids this by luck, not shared design: it declares its own `test-build: install`, which protects only itself. Filed as Postgres-Extensions/pgxntool-test#62 (not fixed in the vendored copy here). Also corrects the record on PR #23's similar-looking fix: that one attributed the same symptom to "ambient parallel make" from pg-build-test's `MAKEFLAGS=-j $(nproc)` export -- confirmed here, with a plain serial `make -n` dry run (no -j anywhere), that the real cause is this TEST_DEPS ordering issue instead. The fix (explicit `make install` first) happened to be correct either way. Reproduced 3/3 times with the extension deliberately removed from /usr/share/postgresql/*/extension first, and confirmed fixed 3/3 times, on both PG12 and PG17, with PG_CONFIG correctly pinned throughout this time. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe9620c..24877da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,5 +50,23 @@ jobs: # gap -- worth a pgxntool-test issue, not a vendored-script edit here. - name: Install rsync run: apt-get install -y rsync + # `make install` first, as its own separate invocation, is required on + # a genuinely fresh system (confirmed by deleting test_factory's + # installed files and re-running from scratch): pgxntool's own + # TEST_DEPS order is testdeps, check-stale-expected, test-build, + # install, installcheck -- and check-stale-expected has a direct + # `check-stale-expected: installcheck` dependency of its own, so Make + # satisfies "installcheck" (running the main suite against a + # not-yet-installed extension) to resolve THAT edge long before it + # ever reaches the literal "install installcheck" pair listed later in + # TEST_DEPS. test-build survives this by luck, not design symmetry: it + # separately declares `test-build: install`, which happens to install + # the extension in time for test-build's OWN nested installcheck, but + # does nothing for the main suite's installcheck resolved earlier via + # check-stale-expected. This is a real pgxntool base.mk ordering gap + # worth its own upstream issue -- not something to fix by editing the + # vendored copy here. - name: Test on PostgreSQL ${{ matrix.pg }} - run: make verify-results + run: | + make install + make verify-results From 454c3890c0ee83e657ce113b34c1230dc262b7d1 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 17:08:29 -0500 Subject: [PATCH 06/13] Regenerate test/build/expected/syntax.out for upstream original_role change Rebasing onto sync-pgxntool-2.3.0 (built on current upstream master) pulls in upstream PR #20 ("Drop the original_role temp table from install"), which this branch's fork-based master predated. That commit only touched sql/*.sql, not test/, but it shifts every subsequent line of the auto-generated sql/test_factory--0.5.0.sql by a constant offset -- and test/build/syntax.sql's expected output embeds literal line numbers from psql's \i output. Regenerated from a real run (test/build has no `make results` equivalent to gate this the way test/expected/ does); confirmed the diff is a pure line-number shift, not a content change: verified locally that a deliberately broken test/build syntax error is still caught immediately and clearly. --- test/build/expected/syntax.out | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/build/expected/syntax.out b/test/build/expected/syntax.out index ac4404c..9951539 100644 --- a/test/build/expected/syntax.out +++ b/test/build/expected/syntax.out @@ -1,6 +1,6 @@ \set ECHO none -psql:sql/test_factory--0.5.0.sql:56: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION -psql:sql/test_factory--0.5.0.sql:57: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION -psql:sql/test_factory--0.5.0.sql:95: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text +psql:sql/test_factory--0.5.0.sql:50: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION +psql:sql/test_factory--0.5.0.sql:51: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION +psql:sql/test_factory--0.5.0.sql:89: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text +psql:sql/test_factory--0.5.0.sql:111: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text psql:sql/test_factory--0.5.0.sql:117: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text -psql:sql/test_factory--0.5.0.sql:123: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text From 1514c75450b7f8ac29467fd5b8fc82f44c417b82 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 17:08:34 -0500 Subject: [PATCH 07/13] Drop now-redundant explicit make install step in CI pgxntool 2.3.0 (sync-pgxntool-2.3.0, this branch's new base) adds a real installcheck: install dependency edge to base.mk, fixing the ordering bug this workaround existed for (Postgres-Extensions/pgxntool-test#62). Reproduced the original failure once more on this rebased branch to confirm the fix holds standalone: deleted test_factory's installed files from /usr/share/postgresql/17/extension/, ran the now-simplified `make verify-results` alone with no prior `make install`, and it passed -- where the old base required the explicit install step first. --- .github/workflows/ci.yml | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24877da..cd2af8f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,23 +50,12 @@ jobs: # gap -- worth a pgxntool-test issue, not a vendored-script edit here. - name: Install rsync run: apt-get install -y rsync - # `make install` first, as its own separate invocation, is required on - # a genuinely fresh system (confirmed by deleting test_factory's - # installed files and re-running from scratch): pgxntool's own - # TEST_DEPS order is testdeps, check-stale-expected, test-build, - # install, installcheck -- and check-stale-expected has a direct - # `check-stale-expected: installcheck` dependency of its own, so Make - # satisfies "installcheck" (running the main suite against a - # not-yet-installed extension) to resolve THAT edge long before it - # ever reaches the literal "install installcheck" pair listed later in - # TEST_DEPS. test-build survives this by luck, not design symmetry: it - # separately declares `test-build: install`, which happens to install - # the extension in time for test-build's OWN nested installcheck, but - # does nothing for the main suite's installcheck resolved earlier via - # check-stale-expected. This is a real pgxntool base.mk ordering gap - # worth its own upstream issue -- not something to fix by editing the - # vendored copy here. + # No separate `make install` step needed here: pgxntool 2.3.0 makes + # `installcheck: install` a real Make dependency edge (fixing the + # ordering gap previously worked around above -- see + # Postgres-Extensions/pgxntool-test#62 for the root cause), so + # `verify-results`'s own dependency chain now installs the extension + # before running the suite, every time, without a second explicit + # invocation. - name: Test on PostgreSQL ${{ matrix.pg }} - run: | - make install - make verify-results + run: make verify-results From 12ab77dac973891f0e6f6f769b9dabfafa5e2a11 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 17:11:41 -0500 Subject: [PATCH 08/13] Convert stacked -- comments to /* */ in test/build/*.sql Same root cause as the syntax.out regeneration two commits back: these files were authored on this branch's original fork-based master, which predates upstream PR #24 (adds the sql-lint tool and its comment-stacked-dashes rule, 3+ consecutive -- lines must use /* */). Rebasing onto sync-pgxntool-2.3.0 makes the linter apply to them for the first time. Watch for the same nested-block-comment hazard PR #22's own comment conversion hit (see commit 6a0627d "Convert multi-line SQL comments to /* */ style"): syntax.sql's original wording included the literal substring "sql/*--*.sql", where the "/*" opens a second, unwanted nested comment (Postgres block comments nest) -- reworded to avoid any "/*"/"*/" substrings appearing inside comment bodies, not just at the intended block boundaries. --- test/build/install.sql | 42 +++++++++++++++++++++++------------------- test/build/syntax.sql | 36 +++++++++++++++++++----------------- 2 files changed, 42 insertions(+), 36 deletions(-) diff --git a/test/build/install.sql b/test/build/install.sql index 88e38e5..a1e02f6 100644 --- a/test/build/install.sql +++ b/test/build/install.sql @@ -1,20 +1,22 @@ \set ECHO none \i test/helpers/psql.sql --- Extension packaging checks (dependency declarations, clean --- install/uninstall) -- not test_factory's business logic, which is --- test/sql/base.sql and test/sql/pgtap.sql's job. Compare --- test/build/syntax.sql: that one runs the raw SQL files directly to catch --- syntax errors; this one goes through a real CREATE EXTENSION to catch --- packaging errors instead. --- --- test/build runs in an isolated database with none of the main suite's --- pgTAP/role setup, so this is plain SQL, relying on pg_regress's classic --- expected-output diffing. ON_ERROR_STOP is off since the bare CREATE --- EXTENSION below is expected to fail (each top-level statement is its own --- autocommit transaction, so that doesn't touch anything after it). --- VERBOSITY is knocked down from psql.sql's "verbose" to "default" since the --- verbose LOCATION line differs across PG majors. +/* + * Extension packaging checks (dependency declarations, clean + * install/uninstall) -- not test_factory's business logic, which is + * test/sql/base.sql and test/sql/pgtap.sql's job. Compare + * test/build/syntax.sql: that one runs the raw SQL files directly to catch + * syntax errors; this one goes through a real CREATE EXTENSION to catch + * packaging errors instead. + * + * test/build runs in an isolated database with none of the main suite's + * pgTAP/role setup, so this is plain SQL, relying on pg_regress's classic + * expected-output diffing. ON_ERROR_STOP is off since the bare CREATE + * EXTENSION below is expected to fail (each top-level statement is its own + * autocommit transaction, so that doesn't touch anything after it). + * VERBOSITY is knocked down from psql.sql's "verbose" to "default" since the + * verbose LOCATION line differs across PG majors. + */ \set ON_ERROR_STOP false \set VERBOSITY default @@ -28,11 +30,13 @@ CREATE EXTENSION IF NOT EXISTS pgtap; DROP EXTENSION IF EXISTS test_factory_pgtap; DROP EXTENSION IF EXISTS test_factory; --- A bare (non-CASCADE) create must fail: test_factory isn't installed yet. --- Proves test_factory_pgtap.control's "requires = 'pgtap, test_factory'" is --- real and enforced, not just documentation. IF THIS SUCCEEDS INSTEAD (e.g. --- with a "schema tf does not exist" error later), the dependency --- declaration is missing/broken -- check test_factory_pgtap.control. +/* + * A bare (non-CASCADE) create must fail: test_factory isn't installed yet. + * Proves test_factory_pgtap.control's "requires = 'pgtap, test_factory'" is + * real and enforced, not just documentation. IF THIS SUCCEEDS INSTEAD (e.g. + * with a "schema tf does not exist" error later), the dependency + * declaration is missing/broken -- check test_factory_pgtap.control. + */ CREATE EXTENSION test_factory_pgtap; -- CASCADE should pull test_factory in automatically. diff --git a/test/build/syntax.sql b/test/build/syntax.sql index 889542f..03975bb 100644 --- a/test/build/syntax.sql +++ b/test/build/syntax.sql @@ -1,23 +1,25 @@ \set ECHO none \i test/helpers/psql.sql --- Runs sql/*--*.sql directly via \i (not CREATE EXTENSION), so a genuine --- syntax error is reported clearly instead of hiding behind a generic --- CREATE EXTENSION failure. Compare test/build/install.sql: that one tests --- packaging (dependency declarations, clean install/uninstall) via a real --- CREATE EXTENSION; this one tests raw SQL syntax by deliberately bypassing --- it. --- --- Wrapped in one transaction, rolled back at the end, so nothing persists --- whether this runs under pg_regress or ad hoc locally. ON_ERROR_ROLLBACK --- savepoints each statement, so the two expected errors below --- (pg_extension_config_dump() requires a real CREATE/ALTER EXTENSION --- context; harmless here) don't abort the rest of the file -- a real syntax --- error later still shows up on its own. VERBOSITY is knocked down from --- psql.sql's "verbose" to "default" since the verbose LOCATION line differs --- across PG majors. \o /dev/null hides the one row of query output each --- file prints (current_user, which varies by environment) while leaving --- NOTICE/WARNING/ERROR visible. +/* + * Runs the versioned SQL files under sql/ directly via \i (not CREATE + * EXTENSION), so a genuine syntax error is reported clearly instead of + * hiding behind a generic CREATE EXTENSION failure. Compare + * test/build/install.sql: that one tests packaging (dependency + * declarations, clean install/uninstall) via a real CREATE EXTENSION; this + * one tests raw SQL syntax by deliberately bypassing it. + * + * Wrapped in one transaction, rolled back at the end, so nothing persists + * whether this runs under pg_regress or ad hoc locally. ON_ERROR_ROLLBACK + * savepoints each statement, so the two expected errors below + * (pg_extension_config_dump() requires a real CREATE/ALTER EXTENSION + * context; harmless here) don't abort the rest of the file -- a real syntax + * error later still shows up on its own. VERBOSITY is knocked down from + * psql.sql's "verbose" to "default" since the verbose LOCATION line differs + * across PG majors. \o /dev/null hides the one row of query output each + * file prints (current_user, which varies by environment) while leaving + * NOTICE/WARNING/ERROR visible. + */ \set ON_ERROR_STOP false \set ON_ERROR_ROLLBACK on \set VERBOSITY default From f00fbc230d458edaff07c2bec37d5c72b0811ffa Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 15:57:56 -0500 Subject: [PATCH 09/13] Delete test/build/install.sql -- wrong home for packaging checks test/build exists for exactly one purpose: running extension scripts "bare" via \i, so a genuine SQL syntax error is reported clearly instead of being obscured by a generic CREATE EXTENSION failure. Its results are always meant to be thrown away -- unlike test/install, which is intended to commit and persist for the rest of the suite. test/build/install.sql (the CASCADE-pulls-in-dependency, bare-create-fails, clean-drop checks) was a different kind of test entirely: packaging/ dependency-declaration correctness via a real CREATE EXTENSION. Putting it in test/build bought nothing over test/install (which already owns getting the extension installed) and confused the two directories' actual purposes -- test/build/install.sql is also just a name collision with test/install/ waiting to trip someone up. This content moves to test/install/load.sql (which will actually perform the CASCADE install in fresh mode too, once that lands) in a follow-up commit on advanced-testing/foundation -- not duplicated here. test/build now contains exactly the one file it was designed for. Co-Authored-By: Claude Sonnet 5 --- doc/test_factory.html | 950 ++++++++++++++++++++++++++++++++ test/CLAUDE.md | 18 +- test/build/expected/install.out | 8 - test/build/install.sql | 52 -- 4 files changed, 956 insertions(+), 72 deletions(-) create mode 100644 doc/test_factory.html delete mode 100644 test/build/expected/install.out delete mode 100644 test/build/install.sql diff --git a/doc/test_factory.html b/doc/test_factory.html new file mode 100644 index 0000000..cd35c0f --- /dev/null +++ b/doc/test_factory.html @@ -0,0 +1,950 @@ + + + + + + + + +Test Factory + + + + + +
+
+
+
+

Test factory makes it easy to create and retrieve unit test data in a Postgres database.

+
+
+
+
+

1. Overview

+
+
+

One of the most difficult parts of unit testing a database (or a data-focused application) is how to handle test data. Traditionally there are two ways to do this:

+
+
+
    +
  1. +

    For each test suite, create all necessary test data. Run tests. Optionally, remove test data.

    +
    2. +
  2. +

    Create a hierarchy of test suites that use the data from a previous suite.

    +
    3. +
+
+
+

These each have significant drawbacks:

+
+
+

#1 is very bad for performance. Constantly creating new data for tests becomes a significant overhead at deeper levels of object nesting. IE: To test generating a customer statement you need invoices, which need invoice line items, and purchase orders (and line items), and customer records.

+
+
+

#2 is much more efficient, but maintaining the dependencies between different tests can be very difficult.

+
+
+

A problem for both methods is how to actually reference test data. "Which customer record do we use for testing really old unpaid invoices?"

+
+
+

test_factory attempts to solve these problems. It provides a simple function (tf.get) that allows you to retrieve test data using a text name as an identifier. If the test data you need doesn’t already exist then it is created automatically, and a copy is kept in a real table. That copy is never removed automatically, so retrieving that data later is fast. Dependencies are easily handled in the definition of the test data itself. For example:

+
+
+
+
INSERT INTO invoice(customer_id) VALUES( (tf.get( NULL::customer, 'base' )).customer_id )
+
+
+
+
+
+

2. Quick Start

+
+
+

Register two Test_Sets ("base", and "scratch") for the customer Test Object.

+
+
+
+
SELECT tf.register(
+	table_name := 'customer'
+	, test_sets :=
+    array[
+		row(
+			'base'
+			,
+$$INSERT INTO customer VALUES
+	( DEFAULT -- customer_id
+		, 'email', 'first', 'middle', 'last', 'suffix'
+		, 'address', 'city', 'state', 'postal'
+	)
+	RETURNING *
+$$
+			)::tf.test_set
+		, row(
+			'scratch'
+			,
+$$INSERT INTO customer VALUES
+	( DEFAULT
+		, 'email2', 'first', 'middle', 'last', 'suffix'
+		, 'address', 'city', 'state', 'postal'
+	)
+	RETURNING *
+$$
+			)::tf.test_set
+	]
+);
+
+
+
+

Retrieve test customer data (data will be inserted if it doesn’t already exist).

+
+
+
+
SELECT * FROM tf.get( NULL::customer, 'base' );
+
+
+
+

Register a customer invoice. Note that this test set uses the already registered customer test data.

+
+
+
+
SELECT tf.register(
+    table_name := 'invoice'
+    , test_sets :=
+      array[
+        row(
+            'base'
+            ,
+$$INSERT INTO invoice VALUES
+    ( DEFAULT -- invoice_id
+        , (tf.get( NULL::customer, 'base' )).customer_id (1)
+        , current_date -- Invoice date
+        , current_date + 30 -- Due Date
+        , 'PO number'
+    )
+    RETURNING *
+$$
+            )::tf.test_set
+    ]
+);
+
+
+
+
    +
  1. +

    Note the use of tf.get() to refer to other test data.

    +
    2. +
+
+
+
+
+

3. Usage

+
+
+

There are two uses for test_factory: registering test data and getting test data.

+
+
+

3.1. Registering

+
+

Test data is only registered once, using <tf.register,tf.register()>>. Usually you want to do this as part of creating a table. If you later ALTER the table a second call to tf.register() will replace the old registered data with new data.

+
+
+

When you register tests the data is not immediately created. Instead, you are providing commands to create test data. Those commands will only be executed by tf.get(), and only if test data doesn’t already exist.

+
+
+

This behavior is important because it means you can register test data in any order. The only requirement is that the relevant table exists (but the table could even have no columns when you call tf.register()!)

+
+
+

When registering test data that references other test data, all you need to do is embed a call to tf.get() in your registered command.

+
+
+
+

3.2. Getting

+
+

Once you have data registered, tf.get() will return it to you, creating it if necessary. All subsequent calls to tf.get() will return the same test data, without creating new test data. If you install the test_factory_pgtap extension, you can also use tf.tap()

+
+
+ + + + + +
+
Important
+		 +When tf.get() creates data it stores a copy of that data, and that copy is what is returned. This means that if you modify the underlying data those changes will not be seen in subsequent calls to tf.get(). +
+
+
+
+
+
+

4. Syntax

+
+
+

4.1. Terms

+
+
+
Test Table
+
+

A table that requires test data. Currently may only be a table.

+
+
+
+
+
+
Test Set
+
+

A single set of test data for a Test Table. A set may contain multiple rows of data. Currently, a set may only have a single command to generate the data.

+
+
+
+
+
+

4.2. tf.test_set

+
+

Every Test Table has Test_Sets associated with it. To facilitate this, there is a tf.test_set data type:

+
+
+
+
CREATE TYPE tf.test_set AS (
+  set_name		  text
+  , insert_sql	text
+);
+
+
+
+

set_name is used to subsequently refer to the data created by insert_sql. Note that it is case and space sensitive. +insert_sql is a command that must return test data rows in the same form as the Test Table.

+
+
+ + + + + +
+
Note
+		 +If you’re wondering what’s up with the leading comma…​ I’ve found that it’s next to impossible to forget a comma when they are leading instead of trailing. This doesn’t sound like a big deal, but over time it really adds up. It also makes editing easier, since it’s a lot more common to add an item at the end than the start. They might look weird at first, but you quickly get used to it. +
+
+
+

Note that insert_sql does not have to be an insert statement. It could be a function, for example. The only requirement is that it returns data in the form of table rows. A function defined as "RETURNS SETOF table_name" would work.

+
+
+
Example 1. tf.test_set
+
+
+
+
SELECT row(
+  -- set_name
+  'base' (1)
+
+  -- insert_sql
+  , $sql$
+INSERT INTO customer( is_test, first_name, last_name ) (2)
+  VALUES( true, 'Test first name', 'Test last name' )
+  RETURNING * (3)
+$sql$
+)::tf.test_set (4)
+                          row
+\--------------------------------------------------------
+ (table,"                                              +
+ INSERT INTO customer( is_test, first_name, last_name )+
+   VALUES( true, 'Test first name', 'Test last name' ) +
+   RETURNING *                                         +
+ ")
+(1 row)
+
+
+
+
    +
  1. +

    Because test sets only exist in the context of a table their names don’t really need to include the table name.

    +
    2. +
  2. +

    Sometimes you need test data in production systems, usually for operational reasons. A is_test column is a good awy to differentiate it.

    +
    3. +
  3. +

    The RETURNING clause is critical; this is how tf.get() is able to retrieve the newly inserted data.

    +
    4. +
  4. +

    This cast is necessary to turn the generic row() into a test_set.

    +
    5. +
+
+
+
+
+

4.2.1. tf.register()

+
+

Test data is registered using tf.register(). This function accepts a table name that the test data is for, and a set of commands (in the form of test_sets that build test data.

+
+
+ + + + + +
+
Important
+		 +Additional calls to tf.register() will replace already defined Test Sets for the specified table. +
+
+
+
+
tf.register(
+  table_name text (1)
+  , test_sets tf.test_set[] (2)
+) RETURNS void
+
+
+
+
    +
  1. +

    Currently only tables are supported. May be schema-qualified. This immediately gets cast to regclass, so the table must exist when tf.register() is called.

    +
    2. +
  2. +

    Note that this is an array of test_set.

    +
    3. +
+
+
+
Notes
+
    +
  • +

    Currently tf.register doesn’t remove old test_set’s that don’t appear in a function call. That will probably change in the future.

    +
    • +
+
+
+
Example 2. tf.register()
+
+
+
+
SELECT tf.register(
+	table_name := 'customer'
+	, test_sets :=
+      array[ (1)
+        row(
+            'base'
+            ,
+$$INSERT INTO invoice ( customer_id, invoice_date ) VALUES(
+    (tf.get( NULL::customer, 'base' )).customer_id (2)
+    , current_date -- Invoice date
+  )
+  RETURNING *
+$$
+            )::tf.test_set (3)
+    , row( (4)
+      'scratch'
+      ,
+$sql$SELECT invoice__create(
+      customer := customer_id
+      , due_date := current_date + 30
+      , po_number := po_number
+    )
+  FROM tf.get( NULL::purchase_order, 'test' ) (5)
+$sql$
+      ) (6)
+    ]
+);
+
+
+
+
    +
  1. +

    Remember: test_sets is an array of test_sets

    +
    2. +
  2. +

    You can refer to other test data inline. Note the extra parenthesis!

    +
    3. +
  3. +

    Remember to cast the row to tf.test_set

    +
    4. +
  4. +

    Here we’re defining a second test set for this table.

    +
    5. +
  5. +

    This is how you can use multiple fields from another piece of test data.

    +
    6. +
  6. +

    Casting to tf.test_set is optional after the first element in the array.

    +
    7. +
+
+
+
+
+
+

4.2.2. tf.get()

+
+

Returns test data.

+
+
+
+
tf.get(
+  tably_type anyelement (1)
+  , set_name text (2)
+) RETURNS SETOF anyelement
+
+
+
+
    +
  1. +

    The anyelement pseudotype is necessary to tell Postgres what type of data the function will be returning.

    +
    2. +
  2. +

    set_name is the name of a test_set that was defined for this table. If it doesn’t exist you will get an error.

    +
    3. +
+
+
+
Example 3. tf.get()
+
+
+
+
SELECT * FROM tf.get(
+  NULL::customer (1)
+  , 'base' (2)
+);
+ customer_id | first_name | last_name
+-------------+------------+-----------
+           1 | first      | last
+(1 row)
+
+
+
+
    +
  1. +

    This is how to tell the system what table the results will take the form of. It is equivalent to cast( NULL AS customer ).

    +
    2. +
  2. +

    The name of a test set defined for the table. If the set doesn’t exist for the table referenced in <1> you will get an error.

    +
    3. +
+
+
+
+
+
+

4.2.3. tf.tap()

+
+

This is a convenience wrapper around tf.get() for use with pgTap. It calls tf.get() in a pgTap isnt_empty() function and returns the isnt_empty() output. This ensures you actually got test data.

+
+
+
+
tf.tap(
+  table_name text (1)
+  , set_name text DEFAULT 'base'
+) RETURNS SETOF text
+
+
+
+
    +
  1. +

    Note that table_name is immediately cast to regclass, so it must be a valid table

    +
    2. +
+
+
+
+
+

4.3. TODO

+
+
    +
  • +

    ❏ The array[ row()::tf.test_set ] syntax is a bit awkward. Maybe a JSON syntax would be better.

    +
    • +
  • +

    ❏ Repeated calls to tf.register() should replace all Test Sets registered for a table.

    +
    • +
  • +

    ❏ At least some of this documentation should be turned into Postgres COMMENTs on the relevant objects.

    +
    • +
+
+
+
+
+
+

5. Copyright

+
+
+

Copyright © 2015 Jim C. Nasby <Jim.Nasby@BlueTreble.com>.

+
+
+
+
+ + + \ No newline at end of file diff --git a/test/CLAUDE.md b/test/CLAUDE.md index 12ea604..51f7910 100644 --- a/test/CLAUDE.md +++ b/test/CLAUDE.md @@ -11,13 +11,14 @@ The test_factory extension uses **pgTAP** (PostgreSQL's unit testing framework) ### Test Files - `test/sql/base.sql` - Core functionality tests (22 tests) - `test/sql/pgtap.sql` - pgTAP integration and `tf.tap()` function tests -- `test/build/install.sql` - Extension packaging/install-mechanics checks - (dependency declarations, clean install/uninstall). Runs via pgxntool's - `test/build` feature (plain SQL + pg_regress diffing, no pgTAP) in an - isolated database, separate from the main pgTAP-based suite above. - `test/build/syntax.sql` - Runs the raw, generated versioned SQL install scripts (`sql/*--*.sql`) directly via `\i`, to catch SQL syntax errors with - a clearer error than a `CREATE EXTENSION` failure would give. + a clearer error than a `CREATE EXTENSION` failure would give. This is the + *only* file `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/uninstall) belong in + `test/install/load.sql` instead, not here. ### Expected Results - `test/expected/*.out` - Expected test output for regression testing @@ -47,13 +48,6 @@ The test_factory extension uses **pgTAP** (PostgreSQL's unit testing framework) - **Permission Isolation** - Tests with unprivileged `test_role` - **Temp Table Cleanup** - Verifies temporary installation objects are removed -### Installation/Packaging Tests (`test/build/install.sql`) -- **Dependency Validation** - Tests extension dependency requirements -- **Clean Installation** - Tests CREATE EXTENSION without conflicts -- **Clean Removal** - Tests DROP EXTENSION without orphaned objects -- Runs via pgxntool's `test/build` feature (plain SQL/pg_regress diffing, - not pgTAP), in an isolated database separate from the main suite below. - ### Raw SQL Syntax Tests (`test/build/syntax.sql`) - Runs `sql/test_factory--*.sql` and `sql/test_factory_pgtap--*.sql` directly via `\i` (not `CREATE EXTENSION`), so a genuine syntax error is reported diff --git a/test/build/expected/install.out b/test/build/expected/install.out deleted file mode 100644 index dce592a..0000000 --- a/test/build/expected/install.out +++ /dev/null @@ -1,8 +0,0 @@ -\set ECHO none -ERROR: required extension "test_factory" is not installed -HINT: Use CREATE EXTENSION ... CASCADE to install required extensions too. - regprocedure -------------------- - tf.tap(text,text) -(1 row) - diff --git a/test/build/install.sql b/test/build/install.sql deleted file mode 100644 index a1e02f6..0000000 --- a/test/build/install.sql +++ /dev/null @@ -1,52 +0,0 @@ -\set ECHO none -\i test/helpers/psql.sql - -/* - * Extension packaging checks (dependency declarations, clean - * install/uninstall) -- not test_factory's business logic, which is - * test/sql/base.sql and test/sql/pgtap.sql's job. Compare - * test/build/syntax.sql: that one runs the raw SQL files directly to catch - * syntax errors; this one goes through a real CREATE EXTENSION to catch - * packaging errors instead. - * - * test/build runs in an isolated database with none of the main suite's - * pgTAP/role setup, so this is plain SQL, relying on pg_regress's classic - * expected-output diffing. ON_ERROR_STOP is off since the bare CREATE - * EXTENSION below is expected to fail (each top-level statement is its own - * autocommit transaction, so that doesn't touch anything after it). - * VERBOSITY is knocked down from psql.sql's "verbose" to "default" since the - * verbose LOCATION line differs across PG majors. - */ -\set ON_ERROR_STOP false -\set VERBOSITY default - -SET client_min_messages = WARNING; - --- pgtap is test_factory_pgtap's other declared dependency; install it first --- so the bare-create failure below isolates on test_factory specifically. -CREATE EXTENSION IF NOT EXISTS pgtap; - --- Clean slate; non-CASCADE so we'd notice if something was already there. -DROP EXTENSION IF EXISTS test_factory_pgtap; -DROP EXTENSION IF EXISTS test_factory; - -/* - * A bare (non-CASCADE) create must fail: test_factory isn't installed yet. - * Proves test_factory_pgtap.control's "requires = 'pgtap, test_factory'" is - * real and enforced, not just documentation. IF THIS SUCCEEDS INSTEAD (e.g. - * with a "schema tf does not exist" error later), the dependency - * declaration is missing/broken -- check test_factory_pgtap.control. - */ -CREATE EXTENSION test_factory_pgtap; - --- CASCADE should pull test_factory in automatically. -CREATE EXTENSION test_factory_pgtap CASCADE; - --- tf.tap(text, text) should exist after install; the cast errors visibly if not. -SELECT 'tf.tap(text, text)'::regprocedure; - --- Clean removal: a bare (non-CASCADE) drop must not error. -DROP EXTENSION test_factory_pgtap; -DROP EXTENSION test_factory; - --- vi: expandtab ts=2 sw=2 From c430287b7979bbaca8402c95c97b6426abeaffde Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 15:58:56 -0500 Subject: [PATCH 10/13] Untrack accidentally-committed doc/test_factory.html; gitignore it Caught in review: the previous commit's git add -A picked up this asciidoctor-generated build artifact (only doc/*.asc is meant to be tracked -- confirmed never committed before on master). Removed from tracking and added a gitignore pattern so a future git add -A can't repeat this. Co-Authored-By: Claude Sonnet 5 --- .gitignore | 3 + doc/test_factory.html | 950 ------------------------------------------ 2 files changed, 3 insertions(+), 950 deletions(-) delete mode 100644 doc/test_factory.html diff --git a/.gitignore b/.gitignore index 1873c2c..941a061 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,9 @@ control.mk # built targets # Note: Version-specific files (sql/*--*.sql) are now tracked in git and should be committed +# Generated by asciidoctor from doc/*.asc; only the .asc source is tracked +doc/*.html + # Test artifacts results/ regression.diffs diff --git a/doc/test_factory.html b/doc/test_factory.html deleted file mode 100644 index cd35c0f..0000000 --- a/doc/test_factory.html +++ /dev/null @@ -1,950 +0,0 @@ - - - - - - - - -Test Factory - - - - - -
-
-
-
-

Test factory makes it easy to create and retrieve unit test data in a Postgres database.

-
-
-
-
-

1. Overview

-
-
-

One of the most difficult parts of unit testing a database (or a data-focused application) is how to handle test data. Traditionally there are two ways to do this:

-
-
-
    -
  1. -

    For each test suite, create all necessary test data. Run tests. Optionally, remove test data.

    -
    2. -
  2. -

    Create a hierarchy of test suites that use the data from a previous suite.

    -
    3. -
-
-
-

These each have significant drawbacks:

-
-
-

#1 is very bad for performance. Constantly creating new data for tests becomes a significant overhead at deeper levels of object nesting. IE: To test generating a customer statement you need invoices, which need invoice line items, and purchase orders (and line items), and customer records.

-
-
-

#2 is much more efficient, but maintaining the dependencies between different tests can be very difficult.

-
-
-

A problem for both methods is how to actually reference test data. "Which customer record do we use for testing really old unpaid invoices?"

-
-
-

test_factory attempts to solve these problems. It provides a simple function (tf.get) that allows you to retrieve test data using a text name as an identifier. If the test data you need doesn’t already exist then it is created automatically, and a copy is kept in a real table. That copy is never removed automatically, so retrieving that data later is fast. Dependencies are easily handled in the definition of the test data itself. For example:

-
-
-
-
INSERT INTO invoice(customer_id) VALUES( (tf.get( NULL::customer, 'base' )).customer_id )
-
-
-
-
-
-

2. Quick Start

-
-
-

Register two Test_Sets ("base", and "scratch") for the customer Test Object.

-
-
-
-
SELECT tf.register(
-	table_name := 'customer'
-	, test_sets :=
-    array[
-		row(
-			'base'
-			,
-$$INSERT INTO customer VALUES
-	( DEFAULT -- customer_id
-		, 'email', 'first', 'middle', 'last', 'suffix'
-		, 'address', 'city', 'state', 'postal'
-	)
-	RETURNING *
-$$
-			)::tf.test_set
-		, row(
-			'scratch'
-			,
-$$INSERT INTO customer VALUES
-	( DEFAULT
-		, 'email2', 'first', 'middle', 'last', 'suffix'
-		, 'address', 'city', 'state', 'postal'
-	)
-	RETURNING *
-$$
-			)::tf.test_set
-	]
-);
-
-
-
-

Retrieve test customer data (data will be inserted if it doesn’t already exist).

-
-
-
-
SELECT * FROM tf.get( NULL::customer, 'base' );
-
-
-
-

Register a customer invoice. Note that this test set uses the already registered customer test data.

-
-
-
-
SELECT tf.register(
-    table_name := 'invoice'
-    , test_sets :=
-      array[
-        row(
-            'base'
-            ,
-$$INSERT INTO invoice VALUES
-    ( DEFAULT -- invoice_id
-        , (tf.get( NULL::customer, 'base' )).customer_id (1)
-        , current_date -- Invoice date
-        , current_date + 30 -- Due Date
-        , 'PO number'
-    )
-    RETURNING *
-$$
-            )::tf.test_set
-    ]
-);
-
-
-
-
    -
  1. -

    Note the use of tf.get() to refer to other test data.

    -
    2. -
-
-
-
-
-

3. Usage

-
-
-

There are two uses for test_factory: registering test data and getting test data.

-
-
-

3.1. Registering

-
-

Test data is only registered once, using <tf.register,tf.register()>>. Usually you want to do this as part of creating a table. If you later ALTER the table a second call to tf.register() will replace the old registered data with new data.

-
-
-

When you register tests the data is not immediately created. Instead, you are providing commands to create test data. Those commands will only be executed by tf.get(), and only if test data doesn’t already exist.

-
-
-

This behavior is important because it means you can register test data in any order. The only requirement is that the relevant table exists (but the table could even have no columns when you call tf.register()!)

-
-
-

When registering test data that references other test data, all you need to do is embed a call to tf.get() in your registered command.

-
-
-
-

3.2. Getting

-
-

Once you have data registered, tf.get() will return it to you, creating it if necessary. All subsequent calls to tf.get() will return the same test data, without creating new test data. If you install the test_factory_pgtap extension, you can also use tf.tap()

-
-
- - - - - -
-
Important
-		 -When tf.get() creates data it stores a copy of that data, and that copy is what is returned. This means that if you modify the underlying data those changes will not be seen in subsequent calls to tf.get(). -
-
-
-
-
-
-

4. Syntax

-
-
-

4.1. Terms

-
-
-
Test Table
-
-

A table that requires test data. Currently may only be a table.

-
-
-
-
-
-
Test Set
-
-

A single set of test data for a Test Table. A set may contain multiple rows of data. Currently, a set may only have a single command to generate the data.

-
-
-
-
-
-

4.2. tf.test_set

-
-

Every Test Table has Test_Sets associated with it. To facilitate this, there is a tf.test_set data type:

-
-
-
-
CREATE TYPE tf.test_set AS (
-  set_name		  text
-  , insert_sql	text
-);
-
-
-
-

set_name is used to subsequently refer to the data created by insert_sql. Note that it is case and space sensitive. -insert_sql is a command that must return test data rows in the same form as the Test Table.

-
-
- - - - - -
-
Note
-		 -If you’re wondering what’s up with the leading comma…​ I’ve found that it’s next to impossible to forget a comma when they are leading instead of trailing. This doesn’t sound like a big deal, but over time it really adds up. It also makes editing easier, since it’s a lot more common to add an item at the end than the start. They might look weird at first, but you quickly get used to it. -
-
-
-

Note that insert_sql does not have to be an insert statement. It could be a function, for example. The only requirement is that it returns data in the form of table rows. A function defined as "RETURNS SETOF table_name" would work.

-
-
-
Example 1. tf.test_set
-
-
-
-
SELECT row(
-  -- set_name
-  'base' (1)
-
-  -- insert_sql
-  , $sql$
-INSERT INTO customer( is_test, first_name, last_name ) (2)
-  VALUES( true, 'Test first name', 'Test last name' )
-  RETURNING * (3)
-$sql$
-)::tf.test_set (4)
-                          row
-\--------------------------------------------------------
- (table,"                                              +
- INSERT INTO customer( is_test, first_name, last_name )+
-   VALUES( true, 'Test first name', 'Test last name' ) +
-   RETURNING *                                         +
- ")
-(1 row)
-
-
-
-
    -
  1. -

    Because test sets only exist in the context of a table their names don’t really need to include the table name.

    -
    2. -
  2. -

    Sometimes you need test data in production systems, usually for operational reasons. A is_test column is a good awy to differentiate it.

    -
    3. -
  3. -

    The RETURNING clause is critical; this is how tf.get() is able to retrieve the newly inserted data.

    -
    4. -
  4. -

    This cast is necessary to turn the generic row() into a test_set.

    -
    5. -
-
-
-
-
-

4.2.1. tf.register()

-
-

Test data is registered using tf.register(). This function accepts a table name that the test data is for, and a set of commands (in the form of test_sets that build test data.

-
-
- - - - - -
-
Important
-		 -Additional calls to tf.register() will replace already defined Test Sets for the specified table. -
-
-
-
-
tf.register(
-  table_name text (1)
-  , test_sets tf.test_set[] (2)
-) RETURNS void
-
-
-
-
    -
  1. -

    Currently only tables are supported. May be schema-qualified. This immediately gets cast to regclass, so the table must exist when tf.register() is called.

    -
    2. -
  2. -

    Note that this is an array of test_set.

    -
    3. -
-
-
-
Notes
-
    -
  • -

    Currently tf.register doesn’t remove old test_set’s that don’t appear in a function call. That will probably change in the future.

    -
    • -
-
-
-
Example 2. tf.register()
-
-
-
-
SELECT tf.register(
-	table_name := 'customer'
-	, test_sets :=
-      array[ (1)
-        row(
-            'base'
-            ,
-$$INSERT INTO invoice ( customer_id, invoice_date ) VALUES(
-    (tf.get( NULL::customer, 'base' )).customer_id (2)
-    , current_date -- Invoice date
-  )
-  RETURNING *
-$$
-            )::tf.test_set (3)
-    , row( (4)
-      'scratch'
-      ,
-$sql$SELECT invoice__create(
-      customer := customer_id
-      , due_date := current_date + 30
-      , po_number := po_number
-    )
-  FROM tf.get( NULL::purchase_order, 'test' ) (5)
-$sql$
-      ) (6)
-    ]
-);
-
-
-
-
    -
  1. -

    Remember: test_sets is an array of test_sets

    -
    2. -
  2. -

    You can refer to other test data inline. Note the extra parenthesis!

    -
    3. -
  3. -

    Remember to cast the row to tf.test_set

    -
    4. -
  4. -

    Here we’re defining a second test set for this table.

    -
    5. -
  5. -

    This is how you can use multiple fields from another piece of test data.

    -
    6. -
  6. -

    Casting to tf.test_set is optional after the first element in the array.

    -
    7. -
-
-
-
-
-
-

4.2.2. tf.get()

-
-

Returns test data.

-
-
-
-
tf.get(
-  tably_type anyelement (1)
-  , set_name text (2)
-) RETURNS SETOF anyelement
-
-
-
-
    -
  1. -

    The anyelement pseudotype is necessary to tell Postgres what type of data the function will be returning.

    -
    2. -
  2. -

    set_name is the name of a test_set that was defined for this table. If it doesn’t exist you will get an error.

    -
    3. -
-
-
-
Example 3. tf.get()
-
-
-
-
SELECT * FROM tf.get(
-  NULL::customer (1)
-  , 'base' (2)
-);
- customer_id | first_name | last_name
--------------+------------+-----------
-           1 | first      | last
-(1 row)
-
-
-
-
    -
  1. -

    This is how to tell the system what table the results will take the form of. It is equivalent to cast( NULL AS customer ).

    -
    2. -
  2. -

    The name of a test set defined for the table. If the set doesn’t exist for the table referenced in <1> you will get an error.

    -
    3. -
-
-
-
-
-
-

4.2.3. tf.tap()

-
-

This is a convenience wrapper around tf.get() for use with pgTap. It calls tf.get() in a pgTap isnt_empty() function and returns the isnt_empty() output. This ensures you actually got test data.

-
-
-
-
tf.tap(
-  table_name text (1)
-  , set_name text DEFAULT 'base'
-) RETURNS SETOF text
-
-
-
-
    -
  1. -

    Note that table_name is immediately cast to regclass, so it must be a valid table

    -
    2. -
-
-
-
-
-

4.3. TODO

-
-
    -
  • -

    ❏ The array[ row()::tf.test_set ] syntax is a bit awkward. Maybe a JSON syntax would be better.

    -
    • -
  • -

    ❏ Repeated calls to tf.register() should replace all Test Sets registered for a table.

    -
    • -
  • -

    ❏ At least some of this documentation should be turned into Postgres COMMENTs on the relevant objects.

    -
    • -
-
-
-
-
-
-

5. Copyright

-
-
-

Copyright © 2015 Jim C. Nasby <Jim.Nasby@BlueTreble.com>.

-
-
-
-
- - - \ No newline at end of file From 5e8ccace135d62de8aa752b5c9195975511323d7 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 18:21:04 -0500 Subject: [PATCH 11/13] Fix syntax.sql's version pinning; disambiguate pgxn-tools vs pgxntool in ci.yml test/build/syntax.sql hardcoded sql/test_factory--0.5.0.sql and sql/test_factory_pgtap--0.1.0.sql. A real version bump would silently break this the moment the old versioned file is removed, or worse, keep testing a stale version forever if it isn't. Resolve both filenames from pg_available_extensions.default_version instead (the same technique test/install/load.sql already uses for its own version assertions) -- those catalog rows reflect whatever's in the .control files test-build's own `install` dependency just put in place, so there's nothing to remember to update alongside a version bump. ci.yml's CI-job comment used "pgxn-tools'" (the pgxn/pgxn-tools CONTAINER IMAGE this job runs in, a completely unrelated project) right next to "pgxntool" (this repo's own vendored Make-based build system) without ever spelling out that distinction -- reads as a typo of the same name instead of two different tools. Made every reference to the container image explicit ("CONTAINER IMAGE") and clarified pgxntool/run-test-build.sh, pgxntool 2.3.0, and make verify-results are pgxntool's own. --- .github/workflows/ci.yml | 35 ++++++++++++++++++++--------------- test/build/syntax.sql | 14 ++++++++++++-- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd2af8f..9908d6f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,23 +31,28 @@ jobs: run: pg-start ${{ matrix.pg }} - name: Check out the repo uses: actions/checkout@v4 - # Not pgxn-tools' own `pg-build-test` helper: it never invokes `make - # test` (it runs `make all; sudo make install; make installcheck` - # directly -- confirmed from its actual source, pgxn/docker-pgxn-tools - # bin/pg-build-test), so it would never reach test-build (wired in only - # via `test`'s own dependency chain). Worse, its failure detection - # (`make installcheck || status=$?`) can never actually trip: pgxntool - # marks `installcheck` `.IGNORE`, so `make installcheck` always exits 0 + # Not `pg-build-test`, the helper script baked into this job's own + # CONTAINER IMAGE (pgxn/pgxn-tools, github.com/pgxn/docker-pgxn-tools -- + # a completely different project from pgxntool, the Make-based build + # system this repo vendors at pgxntool/ and invokes below): confirmed + # from pg-build-test's actual source that it never invokes `make test` + # (it runs `make all; sudo make install; make installcheck` directly), + # so it would never reach test-build (wired in only via pgxntool's own + # `test` dependency chain). Worse, its failure detection (`make + # installcheck || status=$?`) can never actually trip: pgxntool marks + # `installcheck` `.IGNORE`, so `make installcheck` always exits 0 # regardless of real regression failures (verified locally with a - # deliberately broken test). `make verify-results` inspects the actual - # TAP output instead of trusting an exit code, and pulls in test-build - # as part of the same `test` dependency chain it already runs. + # deliberately broken test). `make verify-results` (pgxntool's own + # target) inspects the actual TAP output instead of trusting an exit + # code, and pulls in test-build as part of the same `test` dependency + # chain it already runs. # - # rsync first: pgxntool/run-test-build.sh (vendored, not this repo's - # own script) needs it to sync test/build/*.sql into test/build/sql/, - # and the pgxn/pgxn-tools image doesn't ship it. Any pgxntool project - # enabling PGXNTOOL_ENABLE_TEST_BUILD on this image would hit the same - # gap -- worth a pgxntool-test issue, not a vendored-script edit here. + # rsync first: pgxntool/run-test-build.sh (vendored from pgxntool, not + # this repo's own script) needs it to sync test/build/*.sql into + # test/build/sql/, and the pgxn-tools CONTAINER IMAGE doesn't ship it. + # Any pgxntool project enabling PGXNTOOL_ENABLE_TEST_BUILD on this same + # image would hit the same gap -- worth a pgxntool-test issue, not a + # vendored-script edit here. - name: Install rsync run: apt-get install -y rsync # No separate `make install` step needed here: pgxntool 2.3.0 makes diff --git a/test/build/syntax.sql b/test/build/syntax.sql index 03975bb..1e52e1f 100644 --- a/test/build/syntax.sql +++ b/test/build/syntax.sql @@ -25,11 +25,21 @@ \set VERBOSITY default \o /dev/null +/* + * Resolve the CURRENT versioned filenames rather than hardcoding a version + * number here -- pg_available_extensions reads the .control files `make + * install` (test-build's own dependency) just put in place, so this always + * matches whatever's actually being built, with nothing to remember to bump + * by hand alongside a real version bump. + */ +SELECT default_version FROM pg_available_extensions WHERE name = 'test_factory' \gset tf_ +SELECT default_version FROM pg_available_extensions WHERE name = 'test_factory_pgtap' \gset tfp_ + BEGIN; -- test_factory first: test_factory_pgtap's file needs its "tf" schema/role. -\i sql/test_factory--0.5.0.sql -\i sql/test_factory_pgtap--0.1.0.sql +\i sql/test_factory--:tf_default_version.sql +\i sql/test_factory_pgtap--:tfp_default_version.sql ROLLBACK; From a6c36917315032c6bf4e660eb24effa861d1970a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 18:27:09 -0500 Subject: [PATCH 12/13] syntax.sql: run the actual source files, not the generated version-suffixed copies Reverts the version-resolution approach from the previous commit entirely, rather than just fixing its hardcoded version number. sql/test_factory.sql and sql/test_factory--0.5.0.sql are byte-identical except for one auto-generated "DO NOT EDIT" header line -- there was never a reason to reference the generated, version-suffixed copy here at all, let alone resolve its version dynamically. \i sql/test_factory.sql directly: no version lookup, no version to go stale, nothing to resolve. Also hit the same nested-block-comment gotcha documented in test/install/load.sql while writing the explanatory comment here: writing out the generated filename's glob pattern with a literal wildcard right after a slash opens an unbalanced nested comment and silently swallows the rest of the file. Regenerated test/build/expected/syntax.out to match (filenames and line numbers shift since sql/test_factory.sql lacks the generated header line). make results has no copy mechanism for test/build's separate results/expected dirs (confirmed in pgxntool/base.mk), so this is a direct copy from a verified-passing real test/build/results/syntax.out. --- test/CLAUDE.md | 22 ++++++++++++---------- test/build/expected/syntax.out | 10 +++++----- test/build/syntax.sql | 33 +++++++++++++++------------------ 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/test/CLAUDE.md b/test/CLAUDE.md index 51f7910..c4cb279 100644 --- a/test/CLAUDE.md +++ b/test/CLAUDE.md @@ -11,14 +11,15 @@ The test_factory extension uses **pgTAP** (PostgreSQL's unit testing framework) ### Test Files - `test/sql/base.sql` - Core functionality tests (22 tests) - `test/sql/pgtap.sql` - pgTAP integration and `tf.tap()` function tests -- `test/build/syntax.sql` - Runs the raw, generated versioned SQL install - scripts (`sql/*--*.sql`) directly via `\i`, to catch SQL syntax errors with - a clearer error than a `CREATE EXTENSION` failure would give. This is the - *only* file `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/uninstall) belong in - `test/install/load.sql` instead, not here. +- `test/build/syntax.sql` - Runs the actual source SQL files (`sql/test_factory.sql`, + `sql/test_factory_pgtap.sql` -- the ones a developer edits, NOT the + pgxntool-generated `sql/*--VERSION.sql` copies) directly via `\i`, to catch + SQL syntax errors with a clearer error than a `CREATE EXTENSION` failure + would give. This is the *only* file `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/uninstall) belong + in `test/install/load.sql` instead, not here. ### Expected Results - `test/expected/*.out` - Expected test output for regression testing @@ -49,8 +50,9 @@ The test_factory extension uses **pgTAP** (PostgreSQL's unit testing framework) - **Temp Table Cleanup** - Verifies temporary installation objects are removed ### Raw SQL Syntax Tests (`test/build/syntax.sql`) -- Runs `sql/test_factory--*.sql` and `sql/test_factory_pgtap--*.sql` directly - via `\i` (not `CREATE EXTENSION`), so a genuine syntax error is reported +- Runs `sql/test_factory.sql` and `sql/test_factory_pgtap.sql` (the actual + source files, not the generated `sql/*--VERSION.sql` copies) directly via + `\i` (not `CREATE EXTENSION`), so a genuine syntax error is reported clearly instead of being obscured by a generic CREATE EXTENSION failure. - 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 diff --git a/test/build/expected/syntax.out b/test/build/expected/syntax.out index 9951539..b1d0fc9 100644 --- a/test/build/expected/syntax.out +++ b/test/build/expected/syntax.out @@ -1,6 +1,6 @@ \set ECHO none -psql:sql/test_factory--0.5.0.sql:50: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION -psql:sql/test_factory--0.5.0.sql:51: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION -psql:sql/test_factory--0.5.0.sql:89: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text -psql:sql/test_factory--0.5.0.sql:111: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text -psql:sql/test_factory--0.5.0.sql:117: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text +psql:sql/test_factory.sql:49: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION +psql:sql/test_factory.sql:50: ERROR: pg_extension_config_dump() can only be called from an SQL script executed by CREATE EXTENSION +psql:sql/test_factory.sql:88: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text +psql:sql/test_factory.sql:110: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text +psql:sql/test_factory.sql:116: NOTICE: type reference _tf._test_factory.set_name%TYPE converted to text diff --git a/test/build/syntax.sql b/test/build/syntax.sql index 1e52e1f..3c002b8 100644 --- a/test/build/syntax.sql +++ b/test/build/syntax.sql @@ -2,12 +2,19 @@ \i test/helpers/psql.sql /* - * Runs the versioned SQL files under sql/ directly via \i (not CREATE - * EXTENSION), so a genuine syntax error is reported clearly instead of - * hiding behind a generic CREATE EXTENSION failure. Compare - * test/build/install.sql: that one tests packaging (dependency - * declarations, clean install/uninstall) via a real CREATE EXTENSION; this - * one tests raw SQL syntax by deliberately bypassing it. + * Runs the actual source files under sql/ (test_factory.sql, + * test_factory_pgtap.sql -- the ones a developer edits) directly via \i + * (not CREATE EXTENSION), so a genuine syntax error is reported clearly + * instead of hiding behind a generic CREATE EXTENSION failure. Deliberately + * NOT the pgxntool-generated, version-suffixed copies of these same files + * (verified byte-identical except for one auto-generated "DO NOT EDIT" + * header line) that exist purely for PGXN packaging -- running those here + * would just add a version-number-resolution step for zero benefit, since + * it's the same content either way. (This comment deliberately never + * spells out that generated filename pattern with a literal wildcard glob + * right after a slash -- slash-star opens a comment, and Postgres nests + * block comments, so an unbalanced extra opener here would silently swallow + * the rest of this file. Hit this for real writing this comment.) * * Wrapped in one transaction, rolled back at the end, so nothing persists * whether this runs under pg_regress or ad hoc locally. ON_ERROR_ROLLBACK @@ -25,21 +32,11 @@ \set VERBOSITY default \o /dev/null -/* - * Resolve the CURRENT versioned filenames rather than hardcoding a version - * number here -- pg_available_extensions reads the .control files `make - * install` (test-build's own dependency) just put in place, so this always - * matches whatever's actually being built, with nothing to remember to bump - * by hand alongside a real version bump. - */ -SELECT default_version FROM pg_available_extensions WHERE name = 'test_factory' \gset tf_ -SELECT default_version FROM pg_available_extensions WHERE name = 'test_factory_pgtap' \gset tfp_ - BEGIN; -- test_factory first: test_factory_pgtap's file needs its "tf" schema/role. -\i sql/test_factory--:tf_default_version.sql -\i sql/test_factory_pgtap--:tfp_default_version.sql +\i sql/test_factory.sql +\i sql/test_factory_pgtap.sql ROLLBACK; From ee60eb7377551ac94e27a1df80878e7d0b85a71c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 18:37:27 -0500 Subject: [PATCH 13/13] ci.yml: shrink and relocate the pg-build-test rationale comment The previous comment was long enough, and placed early enough (right before the "Install rsync" step, which it has nothing to do with), that it read as an explanation of rsync rather than of the later "make verify-results" step it actually justifies. State the reasons briefly and put the comment directly above the step it explains instead. --- .github/workflows/ci.yml | 40 +++++++++++----------------------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9908d6f..a55553f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,36 +31,18 @@ jobs: run: pg-start ${{ matrix.pg }} - name: Check out the repo uses: actions/checkout@v4 - # Not `pg-build-test`, the helper script baked into this job's own - # CONTAINER IMAGE (pgxn/pgxn-tools, github.com/pgxn/docker-pgxn-tools -- - # a completely different project from pgxntool, the Make-based build - # system this repo vendors at pgxntool/ and invokes below): confirmed - # from pg-build-test's actual source that it never invokes `make test` - # (it runs `make all; sudo make install; make installcheck` directly), - # so it would never reach test-build (wired in only via pgxntool's own - # `test` dependency chain). Worse, its failure detection (`make - # installcheck || status=$?`) can never actually trip: pgxntool marks - # `installcheck` `.IGNORE`, so `make installcheck` always exits 0 - # regardless of real regression failures (verified locally with a - # deliberately broken test). `make verify-results` (pgxntool's own - # target) inspects the actual TAP output instead of trusting an exit - # code, and pulls in test-build as part of the same `test` dependency - # chain it already runs. - # - # rsync first: pgxntool/run-test-build.sh (vendored from pgxntool, not - # this repo's own script) needs it to sync test/build/*.sql into - # test/build/sql/, and the pgxn-tools CONTAINER IMAGE doesn't ship it. - # Any pgxntool project enabling PGXNTOOL_ENABLE_TEST_BUILD on this same - # image would hit the same gap -- worth a pgxntool-test issue, not a - # vendored-script edit here. + # rsync first: pgxntool/run-test-build.sh needs it to sync + # test/build/*.sql into test/build/sql/, and the CONTAINER IMAGE + # doesn't ship rsync. - name: Install rsync run: apt-get install -y rsync - # No separate `make install` step needed here: pgxntool 2.3.0 makes - # `installcheck: install` a real Make dependency edge (fixing the - # ordering gap previously worked around above -- see - # Postgres-Extensions/pgxntool-test#62 for the root cause), so - # `verify-results`'s own dependency chain now installs the extension - # before running the suite, every time, without a second explicit - # invocation. + # We deliberately don't use `pg-build-test` (this job's CONTAINER + # IMAGE's own helper) for a few reasons -- notably, pgxntool marks + # `installcheck` `.IGNORE`, so pg-build-test's `make installcheck` can + # never actually detect a real regression failure (confirmed + # empirically with a deliberately broken test). `make verify-results` + # (pgxntool's own target) inspects the actual TAP output instead of + # trusting an exit code, and its dependency chain installs the + # extension itself, so no separate `make install` step is needed here. - name: Test on PostgreSQL ${{ matrix.pg }} run: make verify-results