Gate the main test suite on test-build; add build-results; require ON_ERROR_STOP in test/install

CLAUDE.md

@@ -181,9 +181,9 @@ Note: `make test` intentionally does *not* depend on `clean` — depending on `c
 
 **Database Connection Requirement**: PostgreSQL must be running before executing `make test`. If you get connection errors (e.g., "could not connect to server"), stop and ask the user to start PostgreSQL.
 
-**Claude Code MUST NEVER run `make results`**. This target updates test expected output files and requires manual human verification of test changes before execution.
+**Claude Code MUST NEVER run `make results` or `make build-results`**. Both update test expected output files and require manual human verification of test changes before execution.
 
-**Claude Code MUST NEVER modify files in `test/expected/`**. These are expected test outputs that define correct behavior and must only be updated through the `make results` workflow.
+**Claude Code MUST NEVER modify files in `test/expected/` or `test/build/expected/`**. These are expected test outputs that define correct behavior and must only be updated through the `make results`/`make build-results` workflows.
 
 The workflow is:
 1. Human runs `make test` and examines diffs
@@ -200,6 +200,11 @@ pgxntool uses PostgreSQL's pg_regress test framework:
 
 When tests fail, examine the diff output carefully. The actual test output in `test/results/` shows what your code produced, while `test/expected/` shows what was expected.
 
+**Exceptions to the above** -- `test-build` and `test/install` (both optional, see `README.asc`) don't follow the `test/results` vs `test/expected` model:
+
+- **test-build** runs first, in its own separate `pg_regress` pass over `test/build/*.sql`, and gates the main suite: if it fails, `test/install`/`test/sql` never run at all. It does compare actual vs expected normally (`test/build/results/` vs `test/build/expected/`) -- use `make build-results` to refresh its expected output, not `make results`.
+- **test/install** does NOT get a real diff at all: its actual output is written to the exact same file as its expected output, so a content difference can never fail the build, no matter what changed. The only thing that still fails the build is a hard SQL error, and only if the file has `ON_ERROR_STOP` set (directly or via `\i test/pgxntool/psql.sql`) -- pgxntool checks for this by default. If a `test/install/*.sql` file is misbehaving, don't go looking for a diff; check whether it errored, and don't assume a stale-looking `.out` for it means anything.
+
 ## Key Implementation Details
 
 ### PostgreSQL Version Handling

HISTORY.asc

@@ -1,3 +1,31 @@
+STABLE
+------
+== `test-build` now gates the main test suite
+`test-build` now has a real dependency edge on `installcheck`, so it runs
+(and must pass) before `test/install`/`test/sql` do, including under
+parallel `make` -- there's no point validating install/query behavior
+against a build that doesn't even come up cleanly.
+
+== Add `make build-results`
+Refreshes `test/build/expected/*.out` from the last `test-build` run's
+actual output, mirroring `make results` for the main suite. Refuses to
+bless any file whose actual output contains an `ERROR:` line, since that
+would defeat the point of test-build; skip such a file and bless it by hand
+if the error is intentional.
+
+== `test/install` now requires `ON_ERROR_STOP`
+`test/install/*.sql` files never get a real diff -- their actual output is
+written to the same file as their expected output, so a content difference
+can never fail the build. Without `ON_ERROR_STOP`, a hard SQL error was
+silently swallowed too, making the file "pass" regardless of what happened.
+`make test` now fails if a `test/install/*.sql` file doesn't set
+`ON_ERROR_STOP` (directly, or via `\i test/pgxntool/psql.sql`); see
+`PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK` to disable. Since
+`test/install/*.out` was never really compared against anything, and is
+rewritten by every run, it's now gitignored -- stop committing it.
+
+Issues fixed in this release: #97, #108
+
 2.3.0
 -----
 == Rename `PGTLE_VERSION` to `PGXNTOOL_PGTLE_VERSION`

README.asc

@@ -47,6 +47,13 @@ This will build any .html files that can be created. See <<_Document_Handling>>.
 === test
 Runs your extension's test suite: installs the extension and runs it through PGXS's `installcheck`, first pulling in anything you've hooked into <<_testdeps>> and, if enabled, sanity-checking your test SQL via <<_test_build>>.
 
+`make test` runs `pg_regress` in up to two separate passes:
+
+1. If <<_test_build,test-build>> is enabled, it runs first, in its own isolated pass over `test/build/*.sql`. If it fails, the second pass never happens — there's no point checking install/query behavior against a build that doesn't even come up cleanly.
+2. The main pass follows: any <<_testinstall,test/install>> files, then your regular `test/sql/*.sql` files, together in a single `pg_regress` invocation (so state the install files create persists into the regular tests).
+
+Within each pass, files run in alphabetical order by filename.
+
 Whether `test-build` runs is controlled by the `PGXNTOOL_ENABLE_TEST_BUILD` variable — see <<_test_build>> for what it does and how to turn it on/off.
 
 NOTE: `test` intentionally does *not* depend on `clean` — that caused problems with incremental/watch-based builds. If your tests need a clean build to pass, that's a sign of a missing dependency elsewhere rather than something to fix by adding `clean` back.
@@ -63,7 +70,9 @@ Validates that extension SQL files are syntactically correct before running the
 1. Place SQL files in `test/build/*.sql`
 2. Place expected output in `test/build/expected/*.out`
 3. These files run through `pg_regress` before `make test` runs the main test suite
-4. If any build test fails, the test run stops immediately with clear error messages
+4. If any build test fails, `make test` stops immediately — the main suite (test/install + test/sql) never runs, since its results would be meaningless against a broken build
+
+NOTE: This also means a stale or simply wrong `test/build/expected/*.out` -- not just a genuinely broken build -- blocks the whole suite. See <<_build_results,build-results>> below for the supported way to refresh it, including how to handle a file that's *supposed* to show an error.
 
 **Directory structure:**
 
@@ -128,27 +137,42 @@ This approach catches SQL syntax errors *before* running `CREATE EXTENSION`, giv
 
 When `CREATE EXTENSION` fails, PostgreSQL shows only "syntax error" with limited context. Running the SQL directly via `\i` shows the exact line and position of errors, making debugging much faster.
 
+==== build-results
+
+Refreshes `test/build/expected/*.out` from the actual output of the last `test-build` run, mirroring <<_results,results>> for the main suite:
+
+----
+make build-results
+----
+
+`build-results` will *not* bless a file whose actual output contains an `ERROR:` line — accepting an errored build as the new expected baseline would defeat the point of test-build. It skips that file (leaving its existing expected output in place), tells you exactly which file and the command to bless it with, and exits non-zero so the skip can't go unnoticed.
+
+If your project intentionally exercises a build-time error (e.g. asserting a migration fails the way it should), bless that file by hand instead — this is a deliberate, supported case, not a workaround, and you'll need to repeat it by hand every time that file's output legitimately changes, since `build-results` will always skip it:
+
+----
+cp test/build/results/<name>.out test/build/expected/<name>.out
+----
+
 === test/install
 Runs setup files before the main test suite within the same `pg_regress` invocation. This allows expensive one-time operations (like extension installation) to set up state that persists into the regular test files.
 
 **How it works:**
 
 1. Place SQL files in `test/install/*.sql`
-2. Place expected output alongside as `test/install/*.out`
-3. A schedule file is auto-generated that lists install files with `../install/` relative paths
-4. `pg_regress` processes the install schedule first, then runs regular test files — all in one invocation, so database state persists
+2. A schedule file is auto-generated that lists install files with `../install/` relative paths
+3. `pg_regress` processes the install schedule first, then runs regular test files — all in one invocation, so database state persists
 
 **Directory structure:**
 
 ----
 test/install/
 ├── *.sql              # SQL setup files (checked in)
-├── *.out              # Expected output (checked in, alongside .sql)
+├── *.out              # GENERATED, gitignored -- see WARNING below
 ├── .gitignore         # Ignores pg_regress artifacts (*.out.diff)
 └── schedule           # GENERATED - auto-created by make
 ----
 
-The `schedule` file is generated automatically and listed in `.gitignore`. Do not edit it.
+The `schedule` file is generated automatically and listed in `.gitignore`. Do not edit it. `*.out` is gitignored too, for the reason explained below — don't commit it.
 
 **Configuration:**
 
@@ -169,6 +193,8 @@ Without `test/install`, each test file typically needs to run `CREATE EXTENSION`
 
 **Key detail:** Install files and regular tests run in a single `pg_regress` invocation. This means the database is NOT dropped between install and test phases — state created by install files persists into the main test suite.
 
+WARNING: **`test/install/*.out` is never actually compared against anything.** Unlike every other test type pgxntool supports, `test/install`'s actual output is written to the exact same file as its expected output, so there is no diff — a wrong or changed result will never fail the build, no matter how much it changes. The *only* thing that still fails the build is a hard SQL error, and only if the file sets `ON_ERROR_STOP` (directly, or via `\i test/pgxntool/psql.sql`) — without it, psql prints the error, keeps going, and the file "passes" regardless. pgxntool enforces this by default (`check-test-install-error-stop`, see `PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK` to disable), but that only guarantees errors are caught — it does not give you real output validation. If you need actual output comparison, put that logic in `test/sql` instead (or use pgTap assertions from within the install file itself).
+
 ==== Update & Upgrade (U&U) Testing
 
 Beyond validating a plain install, it's worth testing that your extension behaves correctly across the two transitions every extension with more than one release eventually goes through:
@@ -718,6 +744,10 @@ Default: auto-detected -- `yes` if `test/build/*.sql` files exist, `no` otherwis
 
 Default: auto-detected -- `yes` if `test/install/*.sql` files exist, `no` otherwise. Enables or disables the <<_testinstall,test/install>> schedule-based setup feature. Same explicit-override semantics as `PGXNTOOL_ENABLE_TEST_BUILD`.
 
+=== PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK *
+
+Default: `yes`. Enables or disables a build-time check that every `test/install/*.sql` file sets `ON_ERROR_STOP` (directly, or via `\i test/pgxntool/psql.sql`) -- see <<_testinstall,test/install>> for why this is the only thing that makes a hard error in one of those files actually fail the build. Set to `no` to disable the check.
+
 === PGXNTOOL_ENABLE_VERIFY_RESULTS *
 
 Default: `yes`. Enables or disables the <<_verify_results_safeguard,verify-results safeguard>> that blocks `make results` when tests are failing. Setting it to empty on the command line (`make PGXNTOOL_ENABLE_VERIFY_RESULTS= results`) also disables it.