From d62a875a4af13ecce481936dd2c275424ea72e2e Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 12 Oct 2026 18:28:04 -0500 Subject: [PATCH 1/5] Fail `make test` on pgTap failures and plan problems in actual output `make test` failed only on `regression.diffs`, so a `not ok` line blessed into an expected file passed, as did a test that died before printing its plan, and `test/install` output was never checked at all. Add `test/bin/check-tap-output.sh`, which fails a file with TAP output on a `not ok` without `# TODO`, pgTap's "Looks like you planned" line, a missing or duplicate `1..N` plan line, or a test count that differs from the plan. Files without TAP output pass untouched. - The `test` recipe runs it over `$(TESTOUT)/results/*.out` and the `test/install` actual output, then still prints `regression.diffs`; either failing fails the run. New `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT` (default `yes`) disables it. It lives in the recipe, not `TEST_DEPS`, so `make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results` still blesses. - `verify-results-pgtap.sh` uses the same script for its TAP phase, so `make results` gains the plan checks and the `test/install` scan. - The `test/install` actual output is `$(TESTOUT)/install/.out`; the base.mk comment now says so. Part of #106. Related changes in pgxntool-test: - Unit tests for `check-tap-output.sh` and the new `verify-results-pgtap.sh` behavior - `make test` wiring tests, plus an end-to-end blessed `not ok` failure Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 4 +-- HISTORY.asc | 22 ++++++++++++ README.asc | 14 +++++--- README.html | 28 ++++++++++----- base.mk | 63 ++++++++++++++++++++++++++++----- test/bin/check-tap-output.sh | 68 ++++++++++++++++++++++++++++++++++++ verify-results-pgtap.sh | 35 ++++++------------- 7 files changed, 187 insertions(+), 47 deletions(-) create mode 100755 test/bin/check-tap-output.sh diff --git a/CLAUDE.md b/CLAUDE.md index cca492c..696ef46 100644 --- a/CLAUDE.md +++ b/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; `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=no` disables it), 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, 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 diff --git a/HISTORY.asc b/HISTORY.asc index 3d588c0..1e0b13c 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -1,3 +1,25 @@ +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. See `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT` to +disable the `make test` side. Part of #106. + +== New features + +=== Add `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT` +Set to `no` to stop `make test` from checking pgTap output (see above). +`make results` still runs the check in `pgtap` mode; use +`PGXNTOOL_ENABLE_VERIFY_RESULTS=no` to bypass it there. + 2.4.0 ----- == Breaking changes diff --git a/README.asc b/README.asc index 92f132d..910003f 100644 --- a/README.asc +++ b/README.asc @@ -52,7 +52,9 @@ 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. See <<_pgxntool_enable_check_tap_output>>. - 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 +70,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 +208,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, 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_enable_check_tap_output>>), so a `not ok` or plan problem there does fail `make test`. ==== Update & Upgrade (U&U) Testing @@ -274,7 +276,7 @@ Never run `make results` without first verifying the test changes are correct. T ==== 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): -`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. +`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; it applies here regardless of `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT`), 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. 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`. @@ -826,6 +828,10 @@ Default: `yes`. Enables or disables the `_check-stale-expected` safeguard, which This is also the supported pattern for disabling one specific optional pgxntool feature: a documented `PGXNTOOL_ENABLE_*` variable, gating both the target's registration into `TEST_DEPS`/etc. and (where applicable) the target's own definition. Cleanly replacing or overriding an arbitrary pgxntool-generated recipe with your own entirely is a separate, larger, and not-yet-decided design question -- see https://github.com/Postgres-Extensions/pgxntool/issues/30[issue #30] (two proposed approaches) -- and is *not* what this mechanism solves. +=== PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT * + +Default: `yes`. Enables or disables the pgTap output check that `make test` runs after `pg_regress`: it fails on a `not ok` line without `# TODO`, a missing or duplicate `1..N` plan line, or a plan/test count mismatch in `test/results/*.out` or the <<_testinstall,test/install>> actual output. Files with no TAP output are not checked. This governs only `make test`; the <<_verify_results_safeguard,verify-results safeguard>>'s `pgtap` mode always runs the same check before `make results` (use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` to bypass it there). + === PGXNTOOL_CHECK_EXPECTED_FILE_TYPES * Default: `yes`. Sub-check of `_check-stale-expected`, independent of `PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED`: fails (with a distinct error message and exit code from the orphaned-`.out` check) if `test/expected/` (or `test/build/expected/`) contains any file that isn't `*.out`. Set to `no` to disable just this sub-check while leaving the orphaned-`.out` check active. diff --git a/README.html b/README.html index 10a92d7..8cea436 100644 --- a/README.html +++ b/README.html @@ -518,8 +518,9 @@

PGXNtool

  • 8.14. PGXNTOOL_ENABLE_PGXN_INSTALL *
  • 8.15. PGXNTOOL_ENABLE_VERIFY_RESULTS *
  • 8.16. PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED *
    • -
  • 8.17. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *
    • -
  • 8.18. PGXNTOOL_NO_PGXS_INCLUDE
    • +
  • 8.17. PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT *
    • +
  • 8.18. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *
    • +
  • 8.19. PGXNTOOL_NO_PGXS_INCLUDE
  • 9. General PostgreSQL Notes
    • @@ -633,11 +634,14 @@

    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 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. See PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT *.

      +
      • +

    • 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 *.

    • @@ -683,7 +687,7 @@

      Note

    -test exits non-zero (after printing regression.diffs) if any test fails, so CI and other automation can rely on its exit code. +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. @@ -926,7 +930,7 @@

    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). +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. 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_ENABLE_CHECK_TAP_OUTPUT *), so a not ok or plan problem there does fail make test. @@ -1088,7 +1092,7 @@

    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.

    +

    Scans test/results/*.out and the 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; it applies here regardless of PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT), 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
    @@ -2179,13 +2183,19 @@

    -

    8.17. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *

    +

    8.17. PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT *

    +
    +

    Default: yes. Enables or disables the pgTap output check that make test runs after pg_regress: it fails on a not ok line without # TODO, a missing or duplicate 1..N plan line, or a plan/test count mismatch in test/results/*.out or the test/install actual output. Files with no TAP output are not checked. This governs only make test; the verify-results safeguard's pgtap mode always runs the same check before make results (use PGXNTOOL_ENABLE_VERIFY_RESULTS=no to bypass it there).

    +
    + +
    +

    8.18. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *

    Default: yes. Sub-check of _check-stale-expected, independent of PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED: fails (with a distinct error message and exit code from the orphaned-.out check) if test/expected/ (or test/build/expected/) contains any file that isn’t *.out. Set to no to disable just this sub-check while leaving the orphaned-.out check active.

    -

    8.18. PGXNTOOL_NO_PGXS_INCLUDE

    +

    8.19. PGXNTOOL_NO_PGXS_INCLUDE

    Default: unset (PGXS is included normally). Skips including PGXS ($(PGXS)) entirely. This is only for advanced scenarios where you need to manage the PGXS include yourself; most projects should never set this.

    @@ -2217,7 +2227,7 @@

    diff --git a/base.mk b/base.mk index 36c7e79..e92f461 100644 --- a/base.mk +++ b/base.mk @@ -200,11 +200,13 @@ endif # IMPORTANT: test/install does NOT get a real pg_regress diff, unlike every # other test type pgxntool supports. Its schedule entries reference the # original file via a relative "../install/" path, so pg_regress -# resolves BOTH the expected output and the actual output to the exact same -# file (test/install/.out) -- the actual run silently overwrites the -# expected file in place, rather than ever comparing it against anything. A -# content difference (wrong output, a changed column, whatever) will NEVER -# fail this way, no matter how it changes -- see issue #97. +# resolves the expected output to $(TESTDIR)/install/.out and the actual +# output to $(TESTOUT)/install/.out -- the exact same file under the +# default TESTOUT = TESTDIR. The actual run silently overwrites the expected +# file in place, rather than ever comparing it against anything. A content +# difference (wrong output, a changed column, whatever) will NEVER fail this +# way, no matter how it changes -- see issue #97. The TAP check (see +# PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT) does still scan that actual output. # # The only thing that forces the build to fail is psql's own exit code: a # statement that raises a hard error only aborts psql (non-zero exit, which @@ -302,8 +304,9 @@ _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check # - Controls how verify-results detects test failures # - Allowed values: "pgtap" or "diffs" (case-insensitive); anything else, # including an empty value, is a parse-time error -# - "pgtap" (default): scans test/results/*.out for "not ok" lines and plan -# mismatches (TAP failures). Also checks regression.diffs as a fallback. +# - "pgtap" (default): runs check-tap-output.sh over test/results/*.out and +# the test/install actual output ("not ok" lines, missing/duplicate plan +# lines, plan mismatches). Also checks regression.diffs as a fallback. # Use this mode when your test suite uses pgTap. # - "diffs": checks only for regression.diffs existence (classic pg_regress behavior) # Use this mode when your tests use plain SQL expected-output comparison only. @@ -363,6 +366,34 @@ endif _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check-stale-expected.sh +# ------------------------------------------------------------------------------ +# TAP output check: fail `make test` on pgTap failures in actual output +# ------------------------------------------------------------------------------ +# Variable: PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT +# - Can be set manually in Makefile or command line +# - Allowed values: "yes" or "no" (case-insensitive) +# - Default: "yes" (enabled by default for all pgxntool projects) +# - When "yes", the `test` recipe runs check-tap-output.sh over +# $(TESTOUT)/results/*.out (plus the test/install actual output when +# test/install is enabled) and fails on a `not ok` without `# TODO`, a +# missing or duplicate plan line, or a plan/test count mismatch -- even +# when the output matches its expected file +# - Governs only `make test`: verify-results' pgtap mode always runs the +# same check before `make results` +# +# Variable: _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT (internal shim, not user-facing) +# - Path to the script the `test` recipe invokes +# - Default: $(PGXNTOOL_DIR)/test/bin/check-tap-output.sh +# +# Implementation: See the `test:` recipe (search for "test: $(TEST_DEPS)"). +ifdef PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT + override PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT := $(call pgxntool_validate_yesno,$(PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT),PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT) +else + PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT = yes +endif + +_PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check-tap-output.sh + # Generate unique database name for tests to prevent conflicts across projects # Uses project name + first 5 chars of md5 hash of current directory # This prevents multiple test runs in different directories from clobbering each other @@ -415,6 +446,10 @@ ifeq ($(PGXNTOOL_ENABLE_TEST_INSTALL),yes) _PGXNTOOL_INSTALL_SCHEDULE = $(TESTDIR)/install/schedule EXTRA_CLEAN += $(_PGXNTOOL_INSTALL_SCHEDULE) +# Actual output of each install file (see the test/install section above), +# for the TAP output check. +_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES = $(patsubst $(TESTDIR)/install/%.sql,$(TESTOUT)/install/%.out,$(TEST_INSTALL_SQL_FILES)) + # Add install schedule; REGRESS stays as-is (regular tests run after schedule) REGRESS_OPTS += --schedule=$(_PGXNTOOL_INSTALL_SCHEDULE) @@ -529,8 +564,20 @@ ifeq ($(PGXNTOOL_ENABLE_FS_INSTALL),yes) TEST_DEPS += install endif TEST_DEPS += installcheck +# +# The TAP check runs in this recipe rather than as a TEST_DEPS target so that +# `make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results` (which depends on TEST_DEPS, +# not on `test`) still blesses output unconditionally. Both checks always run, +# so a failure reports TAP problems and regression.diffs together. test: $(TEST_DEPS) +ifeq ($(PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT),yes) + @status=0; \ + $(_PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT) $(TESTOUT)/results/*.out $(_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES) || status=1; \ + if [ -r $(TESTOUT)/regression.diffs ]; then cat $(TESTOUT)/regression.diffs; status=1; fi; \ + exit $$status +else @if [ -r $(TESTOUT)/regression.diffs ]; then cat $(TESTOUT)/regression.diffs; exit 1; fi +endif # # verify-results: Safeguard for make results @@ -540,7 +587,7 @@ ifeq ($(PGXNTOOL_ENABLE_VERIFY_RESULTS),yes) .PHONY: verify-results ifeq ($(PGXNTOOL_VERIFY_RESULTS_MODE),pgtap) verify-results: - @$(PGXNTOOL_DIR)/verify-results-pgtap.sh $(TESTOUT) + @$(PGXNTOOL_DIR)/verify-results-pgtap.sh $(TESTOUT) $(_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES) else verify-results: @if [ -r $(TESTOUT)/regression.diffs ]; then \ diff --git a/test/bin/check-tap-output.sh b/test/bin/check-tap-output.sh new file mode 100755 index 0000000..1e4d10b --- /dev/null +++ b/test/bin/check-tap-output.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# +# check-tap-output.sh - Fail on TAP (pgTap) failures in pg_regress output +# +# A file that matches its expected output byte for byte still fails here if +# its TAP says the test failed, so a `not ok` blessed into an expected file +# can't pass `make test` (issue #106). +# +# A file counts as TAP output if it has a top-level test line +# (`ok N`/`not ok N`) or plan line (`1..N`); every other file passes +# untouched. A TAP file fails if: +# - it has a `not ok` line without `# TODO` +# - it has pgTap's "Looks like you planned" line +# - it has no plan line: a no_plan() test that died before finish() +# - it has more than one plan line: nothing sane to compare against +# - its count of top-level test lines differs from the plan: a plan(N) +# test that died partway through (`1..0` with no tests passes) +# Only unindented lines count, so subtest output is ignored. +# +# Arguments that don't exist are ignored, so an unmatched shell glob is +# harmless. +# +# Usage: check-tap-output.sh FILE... +# Exits 1 if any file fails, else 0. + +set -o errexit -o errtrace -o pipefail + +failed=0 +for f in "$@"; do + [ -f "$f" ] || continue + + # " ", where is the last plan's count. + read -r tests plans planned < <(awk ' + /^(not )?ok [0-9]/ { tests++ } + /^1\.\.[0-9]/ { plans++; planned = substr($1, 4) + 0 } + END { print tests + 0, plans + 0, planned + 0 } + ' "$f") + + [ "$tests" -gt 0 ] || [ "$plans" -gt 0 ] || continue + + notok=$(grep '^not ok' "$f" | grep -v '# TODO' || true) + if [ -n "$notok" ]; then + echo "ERROR: pgtap failure detected in $f" + echo "$notok" + failed=1 + fi + + # One plan problem per file: pgTap's own mismatch line already says what a + # count check would. + if grep -q 'Looks like you planned' "$f"; then + echo "ERROR: pgtap plan mismatch in $f" + grep 'Looks like you planned' "$f" + failed=1 + elif [ "$plans" -eq 0 ]; then + echo "ERROR: pgtap output in $f has no plan line (1..N); the test likely died before finishing" + failed=1 + elif [ "$plans" -gt 1 ]; then + echo "ERROR: pgtap output in $f has $plans plan lines (1..N); expected exactly one" + failed=1 + elif [ "$tests" -ne "$planned" ]; then + echo "ERROR: pgtap plan mismatch in $f: planned $planned tests but found $tests" + failed=1 + fi +done + +exit "$failed" + +# vi: expandtab ts=2 sw=2 diff --git a/verify-results-pgtap.sh b/verify-results-pgtap.sh index afd24a0..41a10f6 100755 --- a/verify-results-pgtap.sh +++ b/verify-results-pgtap.sh @@ -1,37 +1,24 @@ #!/usr/bin/env bash # pgxntool/verify-results-pgtap.sh - Check pgtap results before 'make results' # -# Scans pgtap output files for failures and plan mismatches, then checks -# regression.diffs as a fallback, ignoring entries for tests that have no -# expected output yet. Exits non-zero if any problems are found. +# Scans pgtap output files for failures and plan problems (see +# test/bin/check-tap-output.sh), then checks regression.diffs as a fallback, +# ignoring entries for tests that have no expected output yet. Exits non-zero +# if any problems are found. # -# Usage: verify-results-pgtap.sh TESTOUT +# Usage: verify-results-pgtap.sh TESTOUT [FILE...] +# FILE: additional pgtap output files to scan besides TESTOUT/results/*.out # # Called by the verify-results target in base.mk (pgtap mode). set -e -TESTOUT="${1:?Usage: verify-results-pgtap.sh TESTOUT}" +TESTOUT="${1:?Usage: verify-results-pgtap.sh TESTOUT [FILE...]}" +shift -# Check for pgtap failures in result files (excluding TODO items) -failed=0 -for f in "$TESTOUT"/results/*.out; do - [ -f "$f" ] || continue - if grep -q '^not ok' "$f"; then - notok=$(grep '^not ok' "$f" | grep -v '# TODO' || true) - if [ -n "$notok" ]; then - echo "ERROR: pgtap failure detected in $f" - echo "$notok" - failed=1 - fi - fi - if grep -q 'Looks like you planned' "$f"; then - echo "ERROR: pgtap plan mismatch in $f" - grep 'Looks like you planned' "$f" - failed=1 - fi -done -if [ $failed -ne 0 ]; then +# Check for pgtap failures in result files and any extra files (base.mk passes +# the test/install actual outputs). +if ! "$(dirname "$0")/test/bin/check-tap-output.sh" "$TESTOUT"/results/*.out "$@"; then echo echo "pgtap failures detected. Cannot run 'make results'." exit 1 From 3e8be3fae3af88e9e9bede87395fd57ae103cdd7 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 14:04:26 -0500 Subject: [PATCH 2/5] Fix `test/install` when `TESTOUT` differs from `TESTDIR` pg_regress writes `test/install` actual output to `$(TESTOUT)/results/../install/.out` but diffs it against `$(TESTDIR)/expected/../install/.out`. With a separate `TESTOUT`, `make test` aborted with "Directory nonexistent" and "Bail out!", and creating the directory only turned that into a missing-expected-file diff. When `test/install` is enabled and `TESTOUT` differs from `TESTDIR`, the new `_test-install-output-link` (an order-only prerequisite of `installcheck`) makes `$(TESTOUT)/install` a symlink to `$(TESTDIR)/install`, so both paths resolve to the same file again on every PostgreSQL version. Anything else already at that path fails the build rather than being replaced. `_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES` now names the real file in `$(TESTDIR)/install/`. Related changes in pgxntool-test: - Test that the rule creates the symlink for a separate `TESTOUT`, and fails on a pre-existing directory --- CLAUDE.md | 2 +- HISTORY.asc | 10 ++++++++++ README.asc | 4 ++-- base.mk | 29 +++++++++++++++++++++++++---- 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 696ef46..cd1820c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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. 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. +- **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 diff --git a/HISTORY.asc b/HISTORY.asc index 1e0b13c..afbea39 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -20,6 +20,16 @@ Set to `no` to stop `make test` from checking pgTap output (see above). `make results` still runs the check in `pgtap` mode; use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` to bypass it there. +== 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`. + 2.4.0 ----- == Breaking changes diff --git a/README.asc b/README.asc index 910003f..b620043 100644 --- a/README.asc +++ b/README.asc @@ -208,7 +208,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. 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_enable_check_tap_output>>), so a `not ok` or plan problem there does fail `make test`. +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_enable_check_tap_output>>), so a `not ok` or plan problem there does fail `make test`. ==== Update & Upgrade (U&U) Testing @@ -784,7 +784,7 @@ 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 diff --git a/base.mk b/base.mk index e92f461..a420a18 100644 --- a/base.mk +++ b/base.mk @@ -201,9 +201,12 @@ endif # other test type pgxntool supports. Its schedule entries reference the # original file via a relative "../install/" path, so pg_regress # resolves the expected output to $(TESTDIR)/install/.out and the actual -# output to $(TESTOUT)/install/.out -- the exact same file under the -# default TESTOUT = TESTDIR. The actual run silently overwrites the expected -# file in place, rather than ever comparing it against anything. A content +# output to $(TESTOUT)/install/.out. When TESTOUT differs from TESTDIR, +# $(TESTOUT)/install is a symlink to $(TESTDIR)/install (see +# _test-install-output-link), so the output always lands in +# $(TESTDIR)/install/ and both paths are the same file. The actual run +# silently overwrites the expected file in place, rather than ever comparing +# it against anything. A content # difference (wrong output, a changed column, whatever) will NEVER fail this # way, no matter how it changes -- see issue #97. The TAP check (see # PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT) does still scan that actual output. @@ -448,7 +451,7 @@ EXTRA_CLEAN += $(_PGXNTOOL_INSTALL_SCHEDULE) # Actual output of each install file (see the test/install section above), # for the TAP output check. -_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES = $(patsubst $(TESTDIR)/install/%.sql,$(TESTOUT)/install/%.out,$(TEST_INSTALL_SQL_FILES)) +_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES = $(TEST_INSTALL_SQL_FILES:.sql=.out) # Add install schedule; REGRESS stays as-is (regular tests run after schedule) REGRESS_OPTS += --schedule=$(_PGXNTOOL_INSTALL_SCHEDULE) @@ -462,6 +465,24 @@ $(_PGXNTOOL_INSTALL_SCHEDULE): done installcheck: $(_PGXNTOOL_INSTALL_SCHEDULE) + +# pg_regress writes the actual output to $(TESTOUT)/results/../install/ but +# diffs it against $(TESTDIR)/expected/../install/; those only agree if +# $(TESTOUT)/install and $(TESTDIR)/install are the same directory. Never +# replace something else already at that path: it isn't ours to delete. +ifneq ($(abspath $(TESTOUT)),$(abspath $(TESTDIR))) +.PHONY: _test-install-output-link +_test-install-output-link: + @link='$(TESTOUT)/install'; target='$(abspath $(TESTDIR)/install)'; \ + if [ -L "$$link" ] && [ "$$(readlink "$$link")" = "$$target" ]; then exit 0; fi; \ + if [ -e "$$link" ] || [ -L "$$link" ]; then \ + echo "ERROR: $$link exists and is not a symlink to $$target" >&2; \ + echo "test/install output must land in $$target; remove $$link and rerun" >&2; \ + exit 1; \ + fi; \ + mkdir -p '$(TESTOUT)' && ln -s "$$target" "$$link" +installcheck: | _test-install-output-link +endif endif PGXS := $(shell $(PG_CONFIG) --pgxs) From bde28bf2bb980599f91d907e684bcc511f50433d Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 14:05:03 -0500 Subject: [PATCH 3/5] Regenerate README.html for the `TESTOUT` symlink docs No related changes in pgxntool-test. --- README.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.html b/README.html index 8cea436..58df0f3 100644 --- a/README.html +++ b/README.html @@ -930,7 +930,7 @@

    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. 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_ENABLE_CHECK_TAP_OUTPUT *), so a not ok or plan problem there does fail make test. +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_ENABLE_CHECK_TAP_OUTPUT *), so a not ok or plan problem there does fail make test. @@ -2116,7 +2116,7 @@

    8.8. 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 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.

    @@ -2227,7 +2227,7 @@

    From 15e558bbf8fc546eb319c6e56d8da21b7a808c4c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 14:13:58 -0500 Subject: [PATCH 4/5] Add `PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only` pgTap suites whose assertion descriptions embed per-run names (a suffixed role, say) can never byte-match committed expected output, even when every assertion passes. In the new `pgtap-only` mode, a test whose result file is TAP output passes on the pgTap output check and on having no SQL error line; its `regression.diffs` block is ignored. This applies to `make test` as well as `make results`. Tests without TAP output are still judged by their diffs, and `test/install` output gets the TAP check only. - `verify-results-pgtap.sh` gains `--pgtap-only`, plus `--for-test` for `make test`'s rule that every remaining block fails. The same regression.diffs parser serves both. `make test` prints one NOTE line naming the TAP tests whose differences it ignored. - `check-tap-output.sh --list` prints which files count as TAP output, so both scripts share one definition. - `pgtap-only` with `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=no` is a parse-time error, since TAP tests would then pass on nothing. Fixes #106 Related changes in pgxntool-test: - Direct-script tests for `--pgtap-only`, `--for-test` and `--list` - Mode validation and `make test` dry-run wiring for `pgtap-only` --- HISTORY.asc | 10 ++++ README.asc | 20 +++++-- README.html | 31 +++++++++-- base.mk | 31 ++++++++--- test/bin/check-tap-output.sh | 16 +++++- verify-results-pgtap.sh | 104 ++++++++++++++++++++++++++++------- 6 files changed, 171 insertions(+), 41 deletions(-) diff --git a/HISTORY.asc b/HISTORY.asc index afbea39..4ff01d0 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -20,6 +20,14 @@ Set to `no` to stop `make test` from checking pgTap output (see above). `make results` still runs the check in `pgtap` mode; use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` to bypass it there. +=== 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. Requires +`PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes`. + == Bug fixes === Fix `test/install` when `TESTOUT` differs from `TESTDIR` @@ -30,6 +38,8 @@ 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 diff --git a/README.asc b/README.asc index b620043..f083d14 100644 --- a/README.asc +++ b/README.asc @@ -56,6 +56,8 @@ Runs your extension's test suite: filesystem-installs the extension (unless `PGX - 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. See <<_pgxntool_enable_check_tap_output>>. +- `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>>. @@ -274,9 +276,10 @@ 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` 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; it applies here regardless of `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT`), 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, 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. 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`. @@ -286,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): @@ -788,12 +791,21 @@ Default: `$(TESTDIR)`. Directory `pg_regress` writes actual test output to -- `$ === 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. 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. + +Requires `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes`; setting it to `no` in this mode is a hard `make` error, since TAP tests would then pass on nothing. + ==== diffs Checks only for `regression.diffs`, matching classic pg_regress behavior. Use this mode when your tests use plain SQL expected-output comparison only. @@ -830,7 +842,7 @@ This is also the supported pattern for disabling one specific optional pgxntool === PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT * -Default: `yes`. Enables or disables the pgTap output check that `make test` runs after `pg_regress`: it fails on a `not ok` line without `# TODO`, a missing or duplicate `1..N` plan line, or a plan/test count mismatch in `test/results/*.out` or the <<_testinstall,test/install>> actual output. Files with no TAP output are not checked. This governs only `make test`; the <<_verify_results_safeguard,verify-results safeguard>>'s `pgtap` mode always runs the same check before `make results` (use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` to bypass it there). +Default: `yes`. Enables or disables the pgTap output check that `make test` runs after `pg_regress`: it fails on a `not ok` line without `# TODO`, a missing or duplicate `1..N` plan line, or a plan/test count mismatch in `test/results/*.out` or the <<_testinstall,test/install>> actual output. Files with no TAP output are not checked. This governs only `make test`; the <<_verify_results_safeguard,verify-results safeguard>>'s `pgtap` and `pgtap-only` modes always run the same check before `make results` (use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` to bypass it there). === PGXNTOOL_CHECK_EXPECTED_FILE_TYPES * diff --git a/README.html b/README.html index 58df0f3..0993404 100644 --- a/README.html +++ b/README.html @@ -642,6 +642,9 @@

    The pgTap output check, run after pg_regress finishes: fails if test/results/*.out or the 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. See PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT *.

  • +

    regression.diffs: any difference from expected output fails, except, under 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 *.

  • @@ -1086,7 +1089,7 @@

    4.7.1. 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):

    @@ -1094,6 +1097,10 @@

    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; it applies here regardless of PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT), 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, 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.

    @@ -1112,7 +1119,7 @@ 

    # In your Makefile
-PGXNTOOL_VERIFY_RESULTS_MODE = diffs  # or pgtap (the default)
    +PGXNTOOL_VERIFY_RESULTS_MODE = diffs # or pgtap-only, or pgtap (the default)
    @@ -2122,7 +2129,7 @@

    8.9. PGXNTOOL_VERIFY_RESULTS_MODE

    -

    Default: pgtap. Controls how the 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 decides whether tests are failing. Must be pgtap, pgtap-only or diffs (case-insensitive) when given explicitly; any other value, including an empty one, is a hard make error.

    8.9.1. pgtap

    @@ -2131,7 +2138,19 @@

    -

    8.9.2. diffs

    +

    8.9.2. 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. test/install output gets the pgTap output check only.

    +
    +
    +

    Requires PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes; setting it to no in this mode is a hard make error, since TAP tests would then pass on nothing.

    +
    +
    +
    +

    8.9.3. diffs

    Checks only for regression.diffs, matching classic pg_regress behavior. Use this mode when your tests use plain SQL expected-output comparison only.

    @@ -2185,7 +2204,7 @@

    8.17. PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT *

    -

    Default: yes. Enables or disables the pgTap output check that make test runs after pg_regress: it fails on a not ok line without # TODO, a missing or duplicate 1..N plan line, or a plan/test count mismatch in test/results/*.out or the test/install actual output. Files with no TAP output are not checked. This governs only make test; the verify-results safeguard's pgtap mode always runs the same check before make results (use PGXNTOOL_ENABLE_VERIFY_RESULTS=no to bypass it there).

    +

    Default: yes. Enables or disables the pgTap output check that make test runs after pg_regress: it fails on a not ok line without # TODO, a missing or duplicate 1..N plan line, or a plan/test count mismatch in test/results/*.out or the test/install actual output. Files with no TAP output are not checked. This governs only make test; the verify-results safeguard's pgtap and pgtap-only modes always run the same check before make results (use PGXNTOOL_ENABLE_VERIFY_RESULTS=no to bypass it there).

    @@ -2227,7 +2246,7 @@

    diff --git a/base.mk b/base.mk index a420a18..a4a65d8 100644 --- a/base.mk +++ b/base.mk @@ -304,13 +304,19 @@ _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check # - Usage: Controls whether verify-results target exists and blocks make results # # Variable: PGXNTOOL_VERIFY_RESULTS_MODE -# - Controls how verify-results detects test failures -# - Allowed values: "pgtap" or "diffs" (case-insensitive); anything else, -# including an empty value, is a parse-time error +# - Controls how verify-results detects test failures, and in "pgtap-only" +# mode how `make test` does too +# - Allowed values: "pgtap", "pgtap-only" or "diffs" (case-insensitive); +# anything else, including an empty value, is a parse-time error # - "pgtap" (default): runs check-tap-output.sh over test/results/*.out and # the test/install actual output ("not ok" lines, missing/duplicate plan # lines, plan mismatches). Also checks regression.diffs as a fallback. # Use this mode when your test suite uses pgTap. +# - "pgtap-only": like "pgtap", but a test whose result file is TAP output +# passes on its TAP and on having no SQL error line; its regression.diffs +# block is ignored, in `make test` as well as verify-results. For pgTap +# suites whose assertion descriptions vary per run. Requires +# PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes. # - "diffs": checks only for regression.diffs existence (classic pg_regress behavior) # Use this mode when your tests use plain SQL expected-output comparison only. # @@ -326,7 +332,7 @@ endif # Default mode: pgtap (scans results/*.out for TAP failures) PGXNTOOL_VERIFY_RESULTS_MODE ?= pgtap -override PGXNTOOL_VERIFY_RESULTS_MODE := $(call _pgxntool_validate_choice,$(PGXNTOOL_VERIFY_RESULTS_MODE),PGXNTOOL_VERIFY_RESULTS_MODE,pgtap diffs) +override PGXNTOOL_VERIFY_RESULTS_MODE := $(call _pgxntool_validate_choice,$(PGXNTOOL_VERIFY_RESULTS_MODE),PGXNTOOL_VERIFY_RESULTS_MODE,pgtap pgtap-only diffs) # ------------------------------------------------------------------------------ # _check-stale-expected: catch orphaned/unexpected test/expected/ files @@ -381,8 +387,10 @@ _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check-stale-ex # test/install is enabled) and fails on a `not ok` without `# TODO`, a # missing or duplicate plan line, or a plan/test count mismatch -- even # when the output matches its expected file -# - Governs only `make test`: verify-results' pgtap mode always runs the +# - Governs only `make test`: verify-results' pgtap modes always run the # same check before `make results` +# - Must be "yes" when PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only, since that +# mode's TAP tests would otherwise pass on nothing # # Variable: _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT (internal shim, not user-facing) # - Path to the script the `test` recipe invokes @@ -394,6 +402,9 @@ ifdef PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT else PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT = yes endif +ifeq ($(PGXNTOOL_VERIFY_RESULTS_MODE)$(PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT),pgtap-onlyno) + $(error PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only requires PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes; otherwise TAP tests pass on nothing) +endif _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check-tap-output.sh @@ -589,12 +600,14 @@ TEST_DEPS += installcheck # The TAP check runs in this recipe rather than as a TEST_DEPS target so that # `make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results` (which depends on TEST_DEPS, # not on `test`) still blesses output unconditionally. Both checks always run, -# so a failure reports TAP problems and regression.diffs together. +# so a failure reports TAP problems and regression.diffs together. In +# pgtap-only mode, verify-results-pgtap.sh decides which regression.diffs +# blocks still count. test: $(TEST_DEPS) ifeq ($(PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT),yes) @status=0; \ $(_PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT) $(TESTOUT)/results/*.out $(_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES) || status=1; \ - if [ -r $(TESTOUT)/regression.diffs ]; then cat $(TESTOUT)/regression.diffs; status=1; fi; \ + $(if $(filter pgtap-only,$(PGXNTOOL_VERIFY_RESULTS_MODE)),$(PGXNTOOL_DIR)/verify-results-pgtap.sh --pgtap-only --for-test $(TESTOUT) || status=1;,if [ -r $(TESTOUT)/regression.diffs ]; then cat $(TESTOUT)/regression.diffs; status=1; fi;) \ exit $$status else @if [ -r $(TESTOUT)/regression.diffs ]; then cat $(TESTOUT)/regression.diffs; exit 1; fi @@ -606,9 +619,9 @@ endif # Checks if tests are passing before allowing make results to proceed ifeq ($(PGXNTOOL_ENABLE_VERIFY_RESULTS),yes) .PHONY: verify-results -ifeq ($(PGXNTOOL_VERIFY_RESULTS_MODE),pgtap) +ifneq ($(PGXNTOOL_VERIFY_RESULTS_MODE),diffs) verify-results: - @$(PGXNTOOL_DIR)/verify-results-pgtap.sh $(TESTOUT) $(_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES) + @$(PGXNTOOL_DIR)/verify-results-pgtap.sh $(if $(filter pgtap-only,$(PGXNTOOL_VERIFY_RESULTS_MODE)),--pgtap-only )$(TESTOUT) $(_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES) else verify-results: @if [ -r $(TESTOUT)/regression.diffs ]; then \ diff --git a/test/bin/check-tap-output.sh b/test/bin/check-tap-output.sh index 1e4d10b..e813f44 100755 --- a/test/bin/check-tap-output.sh +++ b/test/bin/check-tap-output.sh @@ -20,11 +20,18 @@ # Arguments that don't exist are ignored, so an unmatched shell glob is # harmless. # -# Usage: check-tap-output.sh FILE... -# Exits 1 if any file fails, else 0. +# Usage: check-tap-output.sh [--list] FILE... +# Exits 1 if any file fails, else 0. With --list, checks nothing: prints +# each FILE that counts as TAP output, one per line, and exits 0. set -o errexit -o errtrace -o pipefail +list= +if [ "${1:-}" = --list ]; then + list=1 + shift +fi + failed=0 for f in "$@"; do [ -f "$f" ] || continue @@ -38,6 +45,11 @@ for f in "$@"; do [ "$tests" -gt 0 ] || [ "$plans" -gt 0 ] || continue + if [ -n "$list" ]; then + printf '%s\n' "$f" + continue + fi + notok=$(grep '^not ok' "$f" | grep -v '# TODO' || true) if [ -n "$notok" ]; then echo "ERROR: pgtap failure detected in $f" diff --git a/verify-results-pgtap.sh b/verify-results-pgtap.sh index 41a10f6..0920ee5 100755 --- a/verify-results-pgtap.sh +++ b/verify-results-pgtap.sh @@ -6,24 +6,76 @@ # ignoring entries for tests that have no expected output yet. Exits non-zero # if any problems are found. # -# Usage: verify-results-pgtap.sh TESTOUT [FILE...] +# Usage: verify-results-pgtap.sh [--pgtap-only] [--for-test] TESTOUT [FILE...] # FILE: additional pgtap output files to scan besides TESTOUT/results/*.out +# --pgtap-only: a test whose result file is TAP output is judged by its TAP +# and by having no SQL error line, not by its regression.diffs block +# --for-test: judge regression.diffs the way `make test` does: every block +# left fails, and the TAP scan is left to the caller # -# Called by the verify-results target in base.mk (pgtap mode). +# Called by the verify-results target in base.mk, and by the `test` recipe +# when PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only. set -e -TESTOUT="${1:?Usage: verify-results-pgtap.sh TESTOUT [FILE...]}" +pgtap_only= +for_test= +while :; do + case ${1:-} in + --pgtap-only) pgtap_only=1; shift ;; + --for-test) for_test=1; shift ;; + *) break ;; + esac +done + +TESTOUT="${1:?Usage: verify-results-pgtap.sh [--pgtap-only] [--for-test] TESTOUT [FILE...]}" shift +check_tap="$(dirname "$0")/test/bin/check-tap-output.sh" + +if [ -n "$for_test" ]; then + cannot= +else + cannot=" Cannot run 'make results'." +fi +status=0 + +# pg_regress feeds psql on stdin, so a failing statement in the test file +# itself reports as "ERROR: ..." at column 0, while one inside an \i'd file +# (setup.sql, finish.sql) reports as "psql:test/pgxntool/setup.sql:3: ERROR: +# ...". Matching either position, and psql's own two spaces after the +# severity, keeps a passing test whose description merely mentions "ERROR:" +# out of it. +sql_error='(^|: )ERROR: ' + # Check for pgtap failures in result files and any extra files (base.mk passes # the test/install actual outputs). -if ! "$(dirname "$0")/test/bin/check-tap-output.sh" "$TESTOUT"/results/*.out "$@"; then +if [ -z "$for_test" ] && ! "$check_tap" "$TESTOUT"/results/*.out "$@"; then echo - echo "pgtap failures detected. Cannot run 'make results'." + echo "pgtap failures detected.$cannot" exit 1 fi +# With its diff ignored, a SQL error is the one thing besides its TAP that can +# still fail a TAP test. +if [ -n "$pgtap_only" ]; then + errored= + while IFS= read -r f; do + if [ -n "$f" ] && grep -qE "$sql_error" "$f"; then + errored="$errored $f +" + fi + done < Date: Thu, 15 Oct 2026 16:45:36 -0500 Subject: [PATCH 5/5] Remove `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT` `PGXNTOOL_VERIFY_RESULTS_MODE` already says whether a suite uses pgTap, so it now also decides whether `make test` runs the TAP output check: it runs in the `pgtap` (default) and `pgtap-only` modes and is skipped in `diffs` mode. That removes the separate toggle and the parse-time error for `pgtap-only` combined with it. Part of #106. Related changes in pgxntool-test: - The wiring tests use `PGXNTOOL_VERIFY_RESULTS_MODE=diffs` to prove the TAP check is skipped; tests for the removed variable are dropped --- CLAUDE.md | 2 +- HISTORY.asc | 13 +++------- README.asc | 18 +++++-------- README.html | 32 ++++++++--------------- base.mk | 74 ++++++++++++++++++----------------------------------- 5 files changed, 47 insertions(+), 92 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cd1820c..5cb06a5 100644 --- a/CLAUDE.md +++ b/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 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; `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=no` disables it), 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. diff --git a/HISTORY.asc b/HISTORY.asc index 4ff01d0..8b7977f 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -10,23 +10,18 @@ blessed into an expected file passed. It now also scans 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. See `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT` to -disable the `make test` side. Part of #106. +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_ENABLE_CHECK_TAP_OUTPUT` -Set to `no` to stop `make test` from checking pgTap output (see above). -`make results` still runs the check in `pgtap` mode; use -`PGXNTOOL_ENABLE_VERIFY_RESULTS=no` to bypass it there. - === 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. Requires -`PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes`. +`make results`. Other tests are still judged by their diffs. == Bug fixes diff --git a/README.asc b/README.asc index f083d14..c874765 100644 --- a/README.asc +++ b/README.asc @@ -54,7 +54,7 @@ Runs your extension's test suite: filesystem-installs the extension (unless `PGX `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. See <<_pgxntool_enable_check_tap_output>>. +- 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. @@ -210,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 (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_enable_check_tap_output>>), so a `not ok` or plan problem there does fail `make test`. +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 @@ -278,9 +278,9 @@ Never run `make results` without first verifying the test changes are correct. T ==== 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 three values (case-insensitive; anything else is a hard `make` error): -`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; it applies here regardless of `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT`), 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` (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, 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. +`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`. @@ -791,7 +791,7 @@ Default: `$(TESTDIR)`. Directory `pg_regress` writes actual test output to -- `$ === PGXNTOOL_VERIFY_RESULTS_MODE -Default: `pgtap`. Controls how the <<_verify_results_safeguard,verify-results safeguard>> decides whether tests are failing. Must be `pgtap`, `pgtap-only` 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 @@ -804,11 +804,9 @@ For pgTap suites whose output can't byte-match a committed expected file, such a 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. -Requires `PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes`; setting it to `no` in this mode is a hard `make` error, since TAP tests would then pass on nothing. - ==== 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 * @@ -840,10 +838,6 @@ Default: `yes`. Enables or disables the `_check-stale-expected` safeguard, which This is also the supported pattern for disabling one specific optional pgxntool feature: a documented `PGXNTOOL_ENABLE_*` variable, gating both the target's registration into `TEST_DEPS`/etc. and (where applicable) the target's own definition. Cleanly replacing or overriding an arbitrary pgxntool-generated recipe with your own entirely is a separate, larger, and not-yet-decided design question -- see https://github.com/Postgres-Extensions/pgxntool/issues/30[issue #30] (two proposed approaches) -- and is *not* what this mechanism solves. -=== PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT * - -Default: `yes`. Enables or disables the pgTap output check that `make test` runs after `pg_regress`: it fails on a `not ok` line without `# TODO`, a missing or duplicate `1..N` plan line, or a plan/test count mismatch in `test/results/*.out` or the <<_testinstall,test/install>> actual output. Files with no TAP output are not checked. This governs only `make test`; the <<_verify_results_safeguard,verify-results safeguard>>'s `pgtap` and `pgtap-only` modes always run the same check before `make results` (use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` to bypass it there). - === PGXNTOOL_CHECK_EXPECTED_FILE_TYPES * Default: `yes`. Sub-check of `_check-stale-expected`, independent of `PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED`: fails (with a distinct error message and exit code from the orphaned-`.out` check) if `test/expected/` (or `test/build/expected/`) contains any file that isn't `*.out`. Set to `no` to disable just this sub-check while leaving the orphaned-`.out` check active. diff --git a/README.html b/README.html index 0993404..f513946 100644 --- a/README.html +++ b/README.html @@ -518,9 +518,8 @@

    PGXNtool

  • 8.14. PGXNTOOL_ENABLE_PGXN_INSTALL *
  • 8.15. PGXNTOOL_ENABLE_VERIFY_RESULTS *
  • 8.16. PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED *
    • -
  • 8.17. PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT *
    • -
  • 8.18. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *
    • -
  • 8.19. PGXNTOOL_NO_PGXS_INCLUDE
    • +
  • 8.17. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *
    • +
  • 8.18. PGXNTOOL_NO_PGXS_INCLUDE
  • 9. General PostgreSQL Notes
    • @@ -639,7 +638,7 @@

    • -

      The pgTap output check, run after pg_regress finishes: fails if test/results/*.out or the 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. See PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT *.

      +

      The pgTap output check, run after pg_regress finishes: fails if test/results/*.out or the 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 is diffs.

    • regression.diffs: any difference from expected output fails, except, under 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.

      @@ -933,7 +932,7 @@

      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_ENABLE_CHECK_TAP_OUTPUT *), so a not ok or plan problem there does fail make test. +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. @@ -1095,7 +1094,7 @@

    pgtap (the default)
    -

    Scans test/results/*.out and the 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; it applies here regardless of PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT), 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.

    +

    Scans test/results/*.out and the 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
    @@ -1103,7 +1102,7 @@

    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.

    +

    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.

    @@ -2129,7 +2128,7 @@

    8.9. PGXNTOOL_VERIFY_RESULTS_MODE

    -

    Default: pgtap. Controls how the verify-results safeguard decides whether tests are failing. Must be pgtap, pgtap-only 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 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.

    8.9.1. pgtap

    @@ -2145,14 +2144,11 @@

    A test without TAP output is still judged by its diff: any difference fails make test, and make results applies the pgtap rules above. test/install output gets the pgTap output check only.

    -
    -

    Requires PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes; setting it to no in this mode is a hard make error, since TAP tests would then pass on nothing.

    -

    8.9.3. 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.

    @@ -2202,19 +2198,13 @@

    -

    8.17. PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT *

    -
    -

    Default: yes. Enables or disables the pgTap output check that make test runs after pg_regress: it fails on a not ok line without # TODO, a missing or duplicate 1..N plan line, or a plan/test count mismatch in test/results/*.out or the test/install actual output. Files with no TAP output are not checked. This governs only make test; the verify-results safeguard's pgtap and pgtap-only modes always run the same check before make results (use PGXNTOOL_ENABLE_VERIFY_RESULTS=no to bypass it there).

    -
    - -
    -

    8.18. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *

    +

    8.17. PGXNTOOL_CHECK_EXPECTED_FILE_TYPES *

    Default: yes. Sub-check of _check-stale-expected, independent of PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED: fails (with a distinct error message and exit code from the orphaned-.out check) if test/expected/ (or test/build/expected/) contains any file that isn’t *.out. Set to no to disable just this sub-check while leaving the orphaned-.out check active.

    -

    8.19. PGXNTOOL_NO_PGXS_INCLUDE

    +

    8.18. PGXNTOOL_NO_PGXS_INCLUDE

    Default: unset (PGXS is included normally). Skips including PGXS ($(PGXS)) entirely. This is only for advanced scenarios where you need to manage the PGXS include yourself; most projects should never set this.

    @@ -2246,7 +2236,7 @@

    diff --git a/base.mk b/base.mk index a4a65d8..61778a4 100644 --- a/base.mk +++ b/base.mk @@ -209,7 +209,7 @@ endif # it against anything. A content # difference (wrong output, a changed column, whatever) will NEVER fail this # way, no matter how it changes -- see issue #97. The TAP check (see -# PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT) does still scan that actual output. +# PGXNTOOL_VERIFY_RESULTS_MODE) does still scan that actual output. # # The only thing that forces the build to fail is psql's own exit code: a # statement that raises a hard error only aborts psql (non-zero exit, which @@ -304,24 +304,31 @@ _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check # - Usage: Controls whether verify-results target exists and blocks make results # # Variable: PGXNTOOL_VERIFY_RESULTS_MODE -# - Controls how verify-results detects test failures, and in "pgtap-only" -# mode how `make test` does too +# - Controls how verify-results and `make test` detect test failures, +# including whether `make test` runs the TAP check # - Allowed values: "pgtap", "pgtap-only" or "diffs" (case-insensitive); # anything else, including an empty value, is a parse-time error -# - "pgtap" (default): runs check-tap-output.sh over test/results/*.out and -# the test/install actual output ("not ok" lines, missing/duplicate plan -# lines, plan mismatches). Also checks regression.diffs as a fallback. -# Use this mode when your test suite uses pgTap. +# - "pgtap" (default): runs check-tap-output.sh over $(TESTOUT)/results/*.out +# and the test/install actual output, failing on a `not ok` without +# `# TODO`, a missing or duplicate plan line, or a plan/test count +# mismatch -- even when the output matches its expected file. Also +# checks regression.diffs. Applies to both `make test` and +# verify-results. Use this mode when your test suite uses pgTap. # - "pgtap-only": like "pgtap", but a test whose result file is TAP output # passes on its TAP and on having no SQL error line; its regression.diffs # block is ignored, in `make test` as well as verify-results. For pgTap -# suites whose assertion descriptions vary per run. Requires -# PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes. -# - "diffs": checks only for regression.diffs existence (classic pg_regress behavior) -# Use this mode when your tests use plain SQL expected-output comparison only. +# suites whose assertion descriptions vary per run. +# - "diffs": checks only for regression.diffs existence (classic pg_regress +# behavior); `make test` skips the TAP check. Use this mode when your +# tests use plain SQL expected-output comparison only. # -# Implementation: See verify-results target definition and results target modification -# (search for "verify-results" and "results:" in this file) +# Variable: _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT (internal shim, not user-facing) +# - Path to the script the `test` recipe invokes +# - Default: $(PGXNTOOL_DIR)/test/bin/check-tap-output.sh +# +# Implementation: See verify-results target definition, results target modification +# and the `test:` recipe (search for "verify-results", "results:" +# and "test: $(TEST_DEPS)" in this file) # ifdef PGXNTOOL_ENABLE_VERIFY_RESULTS override PGXNTOOL_ENABLE_VERIFY_RESULTS := $(call pgxntool_validate_yesno,$(PGXNTOOL_ENABLE_VERIFY_RESULTS),PGXNTOOL_ENABLE_VERIFY_RESULTS) @@ -334,6 +341,8 @@ endif PGXNTOOL_VERIFY_RESULTS_MODE ?= pgtap override PGXNTOOL_VERIFY_RESULTS_MODE := $(call _pgxntool_validate_choice,$(PGXNTOOL_VERIFY_RESULTS_MODE),PGXNTOOL_VERIFY_RESULTS_MODE,pgtap pgtap-only diffs) +_PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check-tap-output.sh + # ------------------------------------------------------------------------------ # _check-stale-expected: catch orphaned/unexpected test/expected/ files # ------------------------------------------------------------------------------ @@ -375,39 +384,6 @@ endif _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check-stale-expected.sh -# ------------------------------------------------------------------------------ -# TAP output check: fail `make test` on pgTap failures in actual output -# ------------------------------------------------------------------------------ -# Variable: PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT -# - Can be set manually in Makefile or command line -# - Allowed values: "yes" or "no" (case-insensitive) -# - Default: "yes" (enabled by default for all pgxntool projects) -# - When "yes", the `test` recipe runs check-tap-output.sh over -# $(TESTOUT)/results/*.out (plus the test/install actual output when -# test/install is enabled) and fails on a `not ok` without `# TODO`, a -# missing or duplicate plan line, or a plan/test count mismatch -- even -# when the output matches its expected file -# - Governs only `make test`: verify-results' pgtap modes always run the -# same check before `make results` -# - Must be "yes" when PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only, since that -# mode's TAP tests would otherwise pass on nothing -# -# Variable: _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT (internal shim, not user-facing) -# - Path to the script the `test` recipe invokes -# - Default: $(PGXNTOOL_DIR)/test/bin/check-tap-output.sh -# -# Implementation: See the `test:` recipe (search for "test: $(TEST_DEPS)"). -ifdef PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT - override PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT := $(call pgxntool_validate_yesno,$(PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT),PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT) -else - PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT = yes -endif -ifeq ($(PGXNTOOL_VERIFY_RESULTS_MODE)$(PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT),pgtap-onlyno) - $(error PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only requires PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT=yes; otherwise TAP tests pass on nothing) -endif - -_PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check-tap-output.sh - # Generate unique database name for tests to prevent conflicts across projects # Uses project name + first 5 chars of md5 hash of current directory # This prevents multiple test runs in different directories from clobbering each other @@ -599,12 +575,12 @@ TEST_DEPS += installcheck # # The TAP check runs in this recipe rather than as a TEST_DEPS target so that # `make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results` (which depends on TEST_DEPS, -# not on `test`) still blesses output unconditionally. Both checks always run, -# so a failure reports TAP problems and regression.diffs together. In +# not on `test`) still blesses output unconditionally. Outside diffs mode both +# checks always run, so a failure reports TAP problems and regression.diffs together. In # pgtap-only mode, verify-results-pgtap.sh decides which regression.diffs # blocks still count. test: $(TEST_DEPS) -ifeq ($(PGXNTOOL_ENABLE_CHECK_TAP_OUTPUT),yes) +ifneq ($(PGXNTOOL_VERIFY_RESULTS_MODE),diffs) @status=0; \ $(_PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT) $(TESTOUT)/results/*.out $(_PGXNTOOL_TEST_INSTALL_OUTPUT_FILES) || status=1; \ $(if $(filter pgtap-only,$(PGXNTOOL_VERIFY_RESULTS_MODE)),$(PGXNTOOL_DIR)/verify-results-pgtap.sh --pgtap-only --for-test $(TESTOUT) || status=1;,if [ -r $(TESTOUT)/regression.diffs ]; then cat $(TESTOUT)/regression.diffs; status=1; fi;) \