Fail make test on pgTap problems in actual output; add pgtap-only mode

CLAUDE.md

@@ -179,7 +179,7 @@ make pgxntool-sync     # Update to latest pgxntool via git subtree pull
 - Correct test dependency installation (`testdeps`, and `test-build` when enabled); `testdeps`' default `pgtap` prerequisite auto-installs pgtap via `pgxn install`, a no-op when `PGXNTOOL_ENABLE_PGXN_INSTALL=no`
 - Extension is filesystem-installed before tests run (`install`), unless `PGXNTOOL_ENABLE_FS_INSTALL=no`
 - Test comparison via `installcheck`, with diffs shown on failure
-- Safeguard checks that fail the run: the stale-expected-file check (orphaned or non-`.out` files in `test/expected/`, run after `pg_regress`), and, when `test/install` is in use, the check that every `test/install/*.sql` file sets `ON_ERROR_STOP`
+- Safeguard checks that fail the run: the pgTap output check (a `not ok` without `# TODO`, a missing or duplicate `1..N` plan, or a plan/test count mismatch in `test/results/*.out` or the `test/install` actual output -- even when the output matches its expected file; skipped when `PGXNTOOL_VERIFY_RESULTS_MODE=diffs`), the stale-expected-file check (orphaned or non-`.out` files in `test/expected/`, run after `pg_regress`), and, when `test/install` is in use, the check that every `test/install/*.sql` file sets `ON_ERROR_STOP`
 
 Note: `make test` intentionally does *not* depend on `clean` — depending on `clean` caused problems with incremental/watch-based builds (see `base.mk`). If your tests need a clean build to pass, that's a sign of a missing dependency elsewhere, not something to fix by adding `clean` back.
 
@@ -207,7 +207,7 @@ When tests fail, examine the diff output carefully. The actual test output in `t
 **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 results-build` 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, passing any file that includes `psql.sql` or has its own `\set`/`\unset ON_ERROR_STOP`, whatever the value. 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.
+- **test/install** does NOT get a real diff at all: its actual output is written to the exact same file as its expected output (in `test/install/`, even when `TESTOUT` is set: `$(TESTOUT)/install` is then a symlink to it), so a content difference can never fail the build, no matter what changed. Apart from pgTap failures (caught by the pgTap output check above), 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, passing any file that includes `psql.sql` or has its own `\set`/`\unset ON_ERROR_STOP`, whatever the value. 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
 

HISTORY.asc

@@ -1,3 +1,40 @@
+STABLE
+------
+== Breaking changes
+
+=== `make test` now fails on pgTap failures in actual output
+`make test` used to fail only on `regression.diffs`, so a `not ok` line
+blessed into an expected file passed. It now also scans
+`test/results/*.out` and the `test/install` actual output, failing on a
+`not ok` without `# TODO`, a missing or duplicate `1..N` plan line, or a
+plan that doesn't match the number of tests run (a test that died partway
+through). Files with no TAP output are not checked. `make results` (via
+`verify-results` in its default `pgtap` mode) gains the same plan checks
+and the `test/install` scan. `make test` runs this check in the `pgtap`
+(default) and `pgtap-only` modes of `PGXNTOOL_VERIFY_RESULTS_MODE`, and
+skips it in `diffs` mode. Part of #106.
+
+== New features
+
+=== Add `PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only`
+For pgTap suites whose output can't byte-match expected files, such as
+assertion descriptions that embed a per-run role name. A test with TAP
+output passes on the pgTap output check and on having no SQL error line;
+its `regression.diffs` entry is ignored, in `make test` as well as
+`make results`. Other tests are still judged by their diffs.
+
+== Bug fixes
+
+=== Fix `test/install` when `TESTOUT` differs from `TESTDIR`
+`pg_regress` writes `test/install` actual output under `$(TESTOUT)` but
+reads the expected output from `$(TESTDIR)`, so `make test TESTOUT=dir`
+aborted with "Directory nonexistent" and "Bail out!". `make test` now
+makes `$(TESTOUT)/install` a symlink to `$(TESTDIR)/install`, so that
+output always lands in `$(TESTDIR)/install/`. It fails rather than replace
+anything else already at `$(TESTOUT)/install`.
+
+Issues fixed in this release: #106
+
 2.4.0
 -----
 == Breaking changes

README.asc

@@ -52,7 +52,11 @@ Warns if two `$(DOCS)` entries would install to the same filename, which would o
 === test
 Runs your extension's test suite: filesystem-installs the extension (unless `PGXNTOOL_ENABLE_FS_INSTALL=no`) 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` also fails on two safeguard checks:
+`make test` also fails on these safeguard checks:
+
+- The pgTap output check, run after `pg_regress` finishes: fails if `test/results/*.out` or the <<_testinstall,test/install>> actual output has a `not ok` line without `# TODO`, a missing or duplicate `1..N` plan line, or a different number of tests than planned -- even when the output matches its expected file, so a blessed failure still fails. Files with no TAP output are not checked. Runs unless <<_pgxntool_verify_results_mode,`PGXNTOOL_VERIFY_RESULTS_MODE`>> is `diffs`.
+
+- `regression.diffs`: any difference from expected output fails, except, under <<_pgtap_only,`PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only`>>, in a test whose output is TAP. Such a test fails instead on a SQL error line, and `make test` prints a `NOTE` naming the TAP tests whose differences it ignored.
 
 - The stale-expected-file check, run after `pg_regress` finishes: fails if `test/expected/` (or `test/build/expected/`) has a `.out` file with no matching `.sql`, or a file that isn't `.out`. See <<_pgxntool_enable_check_stale_expected>>.
 - When <<_testinstall,test/install>> is in use, the `ON_ERROR_STOP` check: fails if any `test/install/*.sql` file neither includes `test/pgxntool/psql.sql` nor sets `ON_ERROR_STOP` itself. See <<_pgxntool_enable_test_install_error_stop_check>>.
@@ -68,7 +72,7 @@ Whether `test-build` runs is controlled by the `PGXNTOOL_ENABLE_TEST_BUILD` vari
 
 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.
 
-NOTE: `test` exits non-zero (after printing `regression.diffs`) if any test fails, so CI and other automation can rely on its exit code.
+NOTE: `test` exits non-zero (after printing any pgTap problems and `regression.diffs`) if any test fails, so CI and other automation can rely on its exit code.
 
 NOTE: While you can still run `make installcheck` or any other valid PGXS make target directly, it's recommended to use `make test` when using pgxntool. The `test` target ensures proper test isolation and correct dependency installation.
 
@@ -206,7 +210,7 @@ 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, the `test/install` directory'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).
+WARNING: **`+test/install/*.out+` is never actually compared against anything.** Unlike every other test type pgxntool supports, the `test/install` directory's actual output is written to the exact same file as its expected output (in `$(TESTDIR)/install/`, even when `TESTOUT` is set; see <<_testout>>), so there is no diff — a wrong or changed result will never fail the build, no matter how much it changes. Apart from pgTap failures (below), 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: its actual output goes through the same pgTap output check as `test/sql` (see <<_pgxntool_verify_results_mode>>), so a `not ok` or plan problem there does fail `make test`.
 
 ==== Update & Upgrade (U&U) Testing
 
@@ -272,10 +276,11 @@ IMPORTANT: *`make results` requires manual verification first*. The correct work
 Never run `make results` without first verifying the test changes are correct. The `results` target copies files from `test/results/` to `test/expected/`, so running it blindly will make incorrect output become the new expected behavior.
 
 ==== verify-results safeguard
-By default, `make results` will refuse to run if your tests are failing, so you can't accidentally promote incorrect output into the new expected results. Which failures it looks for is controlled by `PGXNTOOL_VERIFY_RESULTS_MODE`, which must be one of these two values (case-insensitive; anything else is a hard `make` error):
+By default, `make results` will refuse to run if your tests are failing, so you can't accidentally promote incorrect output into the new expected results. Which failures it looks for is controlled by `PGXNTOOL_VERIFY_RESULTS_MODE`, which must be one of these three values (case-insensitive; anything else is a hard `make` error):
 
-`pgtap` (the default):: Scans `test/results/*.out` for pgTAP `not ok` lines (ignoring `# TODO` items) and plan-count mismatches, then also checks `test/regression.diffs` for any output mismatch pgTAP itself wouldn't catch. A test that has no expected output file yet is the one exception: it always shows up in `regression.diffs` (pgxntool creates an empty expected file for it, because `pg_regress` aborts on a missing one), so `make results` is allowed to create its first expected output — provided its own pgTAP output is clean and free of SQL errors. If a new test legitimately needs output containing a SQL error blessed, use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` for that one run.
-`diffs`:: Only checks whether `test/regression.diffs` exists, so it can't create a new test's first expected output; use `pgtap` mode, or `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` for that one run.
+`pgtap` (the default):: Scans `test/results/*.out` and the <<_testinstall,test/install>> actual output for pgTAP `not ok` lines (ignoring `# TODO` items), missing or duplicate plan lines, and plan-count mismatches (the same check `make test` runs in this mode), then also checks `test/regression.diffs` for any output mismatch pgTAP itself wouldn't catch. A test that has no expected output file yet is the one exception: it always shows up in `regression.diffs` (pgxntool creates an empty expected file for it, because `pg_regress` aborts on a missing one), so `make results` is allowed to create its first expected output — provided its own pgTAP output is clean and free of SQL errors. If a new test legitimately needs output containing a SQL error blessed, use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` for that one run.
+`pgtap-only`:: Like `pgtap`, except that a test whose output is TAP is judged only by its pgTAP output and by having no SQL error line; its `regression.diffs` entry is ignored. This mode governs `make test` too; see <<_pgtap_only>>.
+`diffs`:: Only checks whether `test/regression.diffs` exists, and `make test` skips the pgTap output check. It can't create a new test's first expected output; use `pgtap` mode, or `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` for that one run.
 
 If tests are failing, `make results` prints an explanation of what failed and exits without touching `test/expected/`; fix the failures, then re-run `make results`.
 
@@ -284,7 +289,7 @@ It also refuses when `pg_regress` didn't finish at all — an unreachable server
 To select a mode explicitly:
 ----
 # In your Makefile
-PGXNTOOL_VERIFY_RESULTS_MODE = diffs  # or pgtap (the default)
+PGXNTOOL_VERIFY_RESULTS_MODE = diffs  # or pgtap-only, or pgtap (the default)
 ----
 
 To disable this safeguard entirely (not recommended):
@@ -782,19 +787,26 @@ Default: `test`. Root directory for test input files: `$(TESTDIR)/sql/`, `$(TEST
 
 === TESTOUT
 
-Default: `$(TESTDIR)`. Directory `pg_regress` writes actual test output to -- `$(TESTOUT)/results/`, `$(TESTOUT)/regression.diffs`, etc. -- via `--outputdir` in `REGRESS_OPTS`. Kept separate from `TESTDIR` so generated output can be pointed somewhere other than the directory holding your checked-in `sql`/`expected` files, if you want that separation.
+Default: `$(TESTDIR)`. Directory `pg_regress` writes actual test output to -- `$(TESTOUT)/results/`, `$(TESTOUT)/regression.diffs`, etc. -- via `--outputdir` in `REGRESS_OPTS`. Kept separate from `TESTDIR` so generated output can be pointed somewhere other than the directory holding your checked-in `sql`/`expected` files, if you want that separation. The one exception is <<_testinstall,test/install>> output, which always lands in `$(TESTDIR)/install/` (gitignored): when `TESTOUT` differs from `TESTDIR`, `make test` creates `$(TESTOUT)/install` as a symlink to it, and fails rather than replace anything else already at that path.
 
 === PGXNTOOL_VERIFY_RESULTS_MODE
 
-Default: `pgtap`. Controls how the <<_verify_results_safeguard,verify-results safeguard>> decides whether tests are failing. Must be `pgtap` or `diffs` (case-insensitive) when given explicitly; any other value, including an empty one, is a hard `make` error.
+Default: `pgtap`. Controls how the <<_verify_results_safeguard,verify-results safeguard>> decides whether tests are failing, and whether `make test` runs the pgTap output check (it does in `pgtap` and `pgtap-only` modes, and skips it in `diffs` mode). Must be `pgtap`, `pgtap-only` or `diffs` (case-insensitive) when given explicitly; any other value, including an empty one, is a hard `make` error.
 
 ==== pgtap
 
 Scans `test/results/*.out` for pgTap `not ok` lines and plan mismatches, falling back to checking `regression.diffs` too — except for a test that has no expected output yet, whose first expected file `make results` is allowed to create. Use this mode when your test suite uses pgTap.
 
+[[_pgtap_only]]
+==== pgtap-only
+
+For pgTap suites whose output can't byte-match a committed expected file, such as assertion descriptions that embed a per-run role name. A test whose result file is TAP output (it has a top-level `ok N`, `not ok N` or `1..N` line) passes on two things: the pgTap output check, and no SQL error line (`ERROR:` as psql prints it). Its `regression.diffs` entry is ignored. This applies to `make test` as well as `make results`; `make test` prints one `NOTE` line naming the TAP tests whose differences it ignored.
+
+A test without TAP output is still judged by its diff: any difference fails `make test`, and `make results` applies the `pgtap` rules above. <<_testinstall,test/install>> output gets the pgTap output check only.
+
 ==== diffs
 
-Checks only for `regression.diffs`, matching classic pg_regress behavior. Use this mode when your tests use plain SQL expected-output comparison only.
+Checks only for `regression.diffs`, matching classic pg_regress behavior; `make test` skips the pgTap output check. Use this mode when your tests use plain SQL expected-output comparison only.
 
 === PGXNTOOL_ENABLE_TEST_BUILD *