diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d4aaec9..556ed93 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,5 +13,42 @@ 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 + # `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: pg-build-test + run: | + make install + 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/Makefile b/Makefile index 9bc4fd6..bc29d40 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..ac4404c --- /dev/null +++ b/test/build/expected/syntax.out @@ -0,0 +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: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 diff --git a/test/build/install.sql b/test/build/install.sql new file mode 100644 index 0000000..88e38e5 --- /dev/null +++ b/test/build/install.sql @@ -0,0 +1,48 @@ +\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 diff --git a/test/build/syntax.sql b/test/build/syntax.sql new file mode 100644 index 0000000..889542f --- /dev/null +++ b/test/build/syntax.sql @@ -0,0 +1,34 @@ +\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. +\set ON_ERROR_STOP false +\set ON_ERROR_ROLLBACK on +\set VERBOSITY default +\o /dev/null + +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 + +ROLLBACK; + +-- 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;