diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3524b03..a55553f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,5 +31,18 @@ jobs: run: pg-start ${{ matrix.pg }} - name: Check out the repo uses: actions/checkout@v4 + # 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 + # 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: pg-build-test + run: make verify-results 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/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 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..c4cb279 100644 --- a/test/CLAUDE.md +++ b/test/CLAUDE.md @@ -10,8 +10,16 @@ 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/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 @@ -41,15 +49,18 @@ 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`) -- **Dependency Validation** - Tests extension dependency requirements -- **Clean Installation** - Tests CREATE EXTENSION without conflicts -- **Clean Removal** - Tests DROP EXTENSION without orphaned objects +### Raw SQL Syntax Tests (`test/build/syntax.sql`) +- Runs `sql/test_factory.sql` and `sql/test_factory_pgtap.sql` (the actual + source files, not the generated `sql/*--VERSION.sql` copies) directly via + `\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/syntax.out b/test/build/expected/syntax.out new file mode 100644 index 0000000..b1d0fc9 --- /dev/null +++ b/test/build/expected/syntax.out @@ -0,0 +1,6 @@ +\set ECHO none +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 new file mode 100644 index 0000000..3c002b8 --- /dev/null +++ b/test/build/syntax.sql @@ -0,0 +1,43 @@ +\set ECHO none +\i test/helpers/psql.sql + +/* + * 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 + * 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.sql +\i sql/test_factory_pgtap.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;