diff --git a/CLAUDE.md b/CLAUDE.md index cca492c..04ec09f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -206,8 +206,8 @@ 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-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/`, or `$(TESTOUT)/build/results/` when `TESTOUT` is set, 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 (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. 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..ecda3a4 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -1,3 +1,23 @@ +STABLE +------ +== Behavior changes + +=== `test-build` output follows `TESTOUT` +`test-build` and `results-build` now read and write test-build's actual +output (`results/`, `regression.diffs`) under `$(TESTOUT)/build/` +instead of always `$(TESTDIR)/build/`. Nothing changes with the default +`TESTOUT`. Input files and expected output stay in `$(TESTDIR)/build/`. + +== Bug fixes + +=== `test/install` works 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 92f132d..891052b 100644 --- a/README.asc +++ b/README.asc @@ -162,7 +162,7 @@ make results-build If your project intentionally exercises a build-time error (e.g. asserting a migration fails the way it should), bless that file by hand instead — this is a deliberate, supported case, not a workaround, and you'll need to repeat it by hand every time that file's output legitimately changes, since `results-build` will always skip it: ---- -cp test/build/results/.out test/build/expected/.out +cp test/build/results/.out test/build/expected/.out # $(TESTOUT)/build/results/ if TESTOUT is set ---- === test/install @@ -206,7 +206,7 @@ Without `test/install`, each test file typically needs to run `CREATE EXTENSION` **Key detail:** Install files and regular tests run in a single `pg_regress` invocation. This means the database is NOT dropped between install and test phases — state created by install files persists into the main test suite. -WARNING: **`+test/install/*.out+` is never actually compared against anything.** Unlike every other test type pgxntool supports, the `test/install` directory's actual output is written to the exact same file as its expected output, so there is no diff — a wrong or changed result will never fail the build, no matter how much it changes. The *only* thing that still fails the build is a hard SQL error, and only if the file sets `ON_ERROR_STOP` (directly, or via `\i test/pgxntool/psql.sql`) — without it, psql prints the error, keeps going, and the file "passes" regardless. pgxntool enforces this by default (`_check-test-install-error-stop`, see `PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK` to disable), but that only guarantees errors are caught — it does not give you real output validation. If you need actual output comparison, put that logic in `test/sql` instead (or use pgTap assertions from within the install file itself). +WARNING: **`+test/install/*.out+` is never actually compared against anything.** Unlike every other test type pgxntool supports, the `test/install` directory's actual output is written to the exact same file as its expected output (in `$(TESTDIR)/install/`, even when `TESTOUT` is set; see <<_testout>>), so there is no diff — a wrong or changed result will never fail the build, no matter how much it changes. The *only* thing that still fails the build is a hard SQL error, and only if the file sets `ON_ERROR_STOP` (directly, or via `\i test/pgxntool/psql.sql`) — without it, psql prints the error, keeps going, and the file "passes" regardless. pgxntool enforces this by default (`_check-test-install-error-stop`, see `PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK` to disable), but that only guarantees errors are caught — it does not give you real output validation. If you need actual output comparison, put that logic in `test/sql` instead (or use pgTap assertions from within the install file itself). ==== Update & Upgrade (U&U) Testing @@ -782,7 +782,9 @@ 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 actual test output goes to: the main suite's `$(TESTOUT)/results/`, `$(TESTOUT)/regression.diffs`, etc. (via `--outputdir` in `REGRESS_OPTS`), and <<_test_build,test-build>>'s `$(TESTOUT)/build/results/` and `$(TESTOUT)/build/regression.diffs`. 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. + +Test input files, including test-build's `.sql` and expected files, stay in `$(TESTDIR)`. <<_testinstall,test/install>> output does not move either: it always lands in `$(TESTDIR)/install/` (gitignored), because `pg_regress` writes each install file's actual output over its own expected file. When `TESTOUT` differs from `TESTDIR`, `make test` makes `$(TESTOUT)/install` a symlink to `$(TESTDIR)/install`, and fails rather than replace anything else already at that path. === PGXNTOOL_VERIFY_RESULTS_MODE diff --git a/README.html b/README.html index 10a92d7..1696bb8 100644 --- a/README.html +++ b/README.html @@ -842,7 +842,7 @@

-
cp test/build/results/<name>.out test/build/expected/<name>.out
+
cp test/build/results/<name>.out test/build/expected/<name>.out   # $(TESTOUT)/build/results/ if TESTOUT is set
@@ -926,7 +926,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 (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. 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). @@ -2112,7 +2112,10 @@

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 actual test output goes to: the main suite’s $(TESTOUT)/results/, $(TESTOUT)/regression.diffs, etc. (via --outputdir in REGRESS_OPTS), and test-build's $(TESTOUT)/build/results/ and $(TESTOUT)/build/regression.diffs. 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.

+
+
+

Test input files, including test-build’s .sql and expected files, stay in $(TESTDIR). test/install output does not move either: it always lands in $(TESTDIR)/install/ (gitignored), because pg_regress writes each install file’s actual output over its own expected file. When TESTOUT differs from TESTDIR, make test makes $(TESTOUT)/install a symlink to $(TESTDIR)/install, and fails rather than replace anything else already at that path.

@@ -2217,7 +2220,7 @@

diff --git a/base.mk b/base.mk index 36c7e79..1d74a10 100644 --- a/base.mk +++ b/base.mk @@ -201,10 +201,12 @@ endif # 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. +# file ($(TESTDIR)/install/.out; when TESTOUT differs from TESTDIR, +# _test-install-output-link makes that hold) -- 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 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 @@ -427,6 +429,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) @@ -705,10 +725,11 @@ test-build: install endif test-build: @$(PGXNTOOL_DIR)/run-test-build.sh $(TESTDIR) - $(MAKE) -C . _PGXNTOOL_TEST_BUILD_ACTIVE=yes REGRESS="$(TEST_BUILD_REGRESS)" REGRESS_OPTS="--inputdir=$(TESTDIR)/build --outputdir=$(TESTDIR)/build" installcheck - @if [ -r $(TESTDIR)/build/regression.diffs ]; then \ - echo "test-build failed - see $(TESTDIR)/build/regression.diffs"; \ - cat $(TESTDIR)/build/regression.diffs; \ + @mkdir -p $(TESTOUT)/build + $(MAKE) -C . _PGXNTOOL_TEST_BUILD_ACTIVE=yes REGRESS="$(TEST_BUILD_REGRESS)" REGRESS_OPTS="--inputdir=$(TESTDIR)/build --outputdir=$(TESTOUT)/build" installcheck + @if [ -r $(TESTOUT)/build/regression.diffs ]; then \ + echo "test-build failed - see $(TESTOUT)/build/regression.diffs"; \ + cat $(TESTOUT)/build/regression.diffs; \ exit 1; \ fi @@ -735,12 +756,12 @@ endif # as the new baseline would defeat the point of test-build. If a project # intentionally exercises an error case in test/build (e.g. verifying a # migration fails as expected), bless that file by hand instead: -# cp $(TESTDIR)/build/results/.out $(TESTDIR)/build/expected/.out +# cp $(TESTOUT)/build/results/.out $(TESTDIR)/build/expected/.out # # This ERROR-scanning safeguard is only possible because test-build writes # actual output to a location genuinely separate from its expected output -# (--inputdir/--outputdir both point at test/build, but pg_regress keeps -# expected/ and results/ distinct there) -- test/install intentionally does +# ($(TESTDIR)/build/expected/ vs $(TESTOUT)/build/results/, distinct even +# when TESTOUT = TESTDIR) -- test/install intentionally does # NOT work this way (see test/install's own comments above): its actual # output lands on top of its expected file, so there's nothing to diff. We # don't generally care about test-build's own output content, but when a @@ -753,7 +774,7 @@ endif # fails, the actual output results-build needs is already on disk. But # test-build can also fail for reasons that leave nothing fresh to bless -- # install broke, run-test-build.sh errored, pg_regress couldn't even -# connect -- in which case test/build/results/*.out is stale leftovers from +# connect -- in which case $(TESTOUT)/build/results/*.out is stale leftovers from # whatever run last populated it, and blessing it would silently paper over # the real failure instead of surfacing it. We clear regression.diffs # beforehand and only treat a failure as "there's a diff to bless" if it @@ -764,14 +785,14 @@ endif # below. .PHONY: results-build results-build: - @rm -f $(TESTDIR)/build/regression.diffs - $(MAKE) -C . test-build || test -s $(TESTDIR)/build/regression.diffs || { \ + @rm -f $(TESTOUT)/build/regression.diffs + $(MAKE) -C . test-build || test -s $(TESTOUT)/build/regression.diffs || { \ echo "results-build: test-build failed for a reason other than a diff to bless; not blessing stale output" >&2; \ exit 1; \ } @mkdir -p $(TESTDIR)/build/expected @skipped=0; \ - for f in $(TESTDIR)/build/results/*.out; do \ + for f in $(TESTOUT)/build/results/*.out; do \ [ -f "$$f" ] || continue; \ if grep -q 'ERROR:' "$$f"; then \ echo "results-build: skipping $$f (actual output contains ERROR:)" >&2; \