diff --git a/CLAUDE.md b/CLAUDE.md index 30bb6d9..3b5e227 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -181,9 +181,9 @@ Note: `make test` intentionally does *not* depend on `clean` — depending on `c **Database Connection Requirement**: PostgreSQL must be running before executing `make test`. If you get connection errors (e.g., "could not connect to server"), stop and ask the user to start PostgreSQL. -**Claude Code MUST NEVER run `make results` or `make build-results`**. Both update test expected output files and require manual human verification of test changes before execution. +**Claude Code MUST NEVER run `make results` or `make results-build`**. Both update test expected output files and require manual human verification of test changes before execution. -**Claude Code MUST NEVER modify files in `test/expected/` or `test/build/expected/`**. These are expected test outputs that define correct behavior and must only be updated through the `make results`/`make build-results` workflows. +**Claude Code MUST NEVER modify files in `test/expected/` or `test/build/expected/`**. These are expected test outputs that define correct behavior and must only be updated through the `make results`/`make results-build` workflows. The workflow is: 1. Human runs `make test` and examines diffs @@ -202,7 +202,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 build-results` to refresh its expected output, not `make results`. +- **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. 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 b3e1fbb..0b02dcd 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -43,7 +43,7 @@ None of these are documented anywhere as override points, but if you happened to reference one directly (unsupported, but possible), update to the new name. -== Add `make build-results` +== Add `make results-build` Refreshes `test/build/expected/*.out` from the last `test-build` run's actual output, mirroring `make results` for the main suite. Refuses to bless any file whose actual output contains an `ERROR:` line, since that diff --git a/README.asc b/README.asc index f1580d1..3c56ac7 100644 --- a/README.asc +++ b/README.asc @@ -74,7 +74,7 @@ Validates that extension SQL files are syntactically correct before running the 3. These files run through `pg_regress` before `make test` runs the main test suite 4. If any build test fails, `make test` stops immediately — the main suite (test/install + test/sql) never runs, since its results would be meaningless against a broken build -NOTE: This also means a stale or simply wrong `+test/build/expected/*.out+` -- not just a genuinely broken build -- blocks the whole suite. See <<_build_results,build-results>> below for the supported way to refresh it, including how to handle a file that's *supposed* to show an error. +NOTE: This also means a stale or simply wrong `+test/build/expected/*.out+` -- not just a genuinely broken build -- blocks the whole suite. See <<_results_build,results-build>> below for the supported way to refresh it, including how to handle a file that's *supposed* to show an error. **Directory structure:** @@ -139,17 +139,17 @@ This approach catches SQL syntax errors *before* running `CREATE EXTENSION`, giv When `CREATE EXTENSION` fails, PostgreSQL shows only "syntax error" with limited context. Running the SQL directly via `\i` shows the exact line and position of errors, making debugging much faster. -==== build-results +==== results-build Refreshes `test/build/expected/*.out` from the actual output of the last `test-build` run, mirroring <<_results,results>> for the main suite: ---- -make build-results +make results-build ---- -`build-results` will *not* bless a file whose actual output contains an `ERROR:` line — accepting an errored build as the new expected baseline would defeat the point of test-build. It skips that file (leaving its existing expected output in place), tells you exactly which file and the command to bless it with, and exits non-zero so the skip can't go unnoticed. +`results-build` will *not* bless a file whose actual output contains an `ERROR:` line — accepting an errored build as the new expected baseline would defeat the point of test-build. It skips that file (leaving its existing expected output in place), tells you exactly which file and the command to bless it with, and exits non-zero so the skip can't go unnoticed. -If your project intentionally exercises a build-time error (e.g. asserting a migration fails the way it should), bless that file by hand instead — this is a deliberate, supported case, not a workaround, and you'll need to repeat it by hand every time that file's output legitimately changes, since `build-results` will always skip it: +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 diff --git a/README.html b/README.html index 630b006..191cd0a 100644 --- a/README.html +++ b/README.html @@ -709,7 +709,7 @@

Note -This also means a stale or simply wrong test/build/expected/*.out — not just a genuinely broken build — blocks the whole suite. See build-results below for the supported way to refresh it, including how to handle a file that’s supposed to show an error. +This also means a stale or simply wrong test/build/expected/*.out — not just a genuinely broken build — blocks the whole suite. See results-build below for the supported way to refresh it, including how to handle a file that’s supposed to show an error. @@ -799,20 +799,20 @@

When CREATE EXTENSION fails, PostgreSQL shows only "syntax error" with limited context. Running the SQL directly via \i shows the exact line and position of errors, making debugging much faster.

-

4.3.1. build-results

+

4.3.1. results-build

Refreshes test/build/expected/*.out from the actual output of the last test-build run, mirroring results for the main suite:

-
make build-results
+
make results-build
-

build-results will not bless a file whose actual output contains an ERROR: line — accepting an errored build as the new expected baseline would defeat the point of test-build. It skips that file (leaving its existing expected output in place), tells you exactly which file and the command to bless it with, and exits non-zero so the skip can’t go unnoticed.

+

results-build will not bless a file whose actual output contains an ERROR: line — accepting an errored build as the new expected baseline would defeat the point of test-build. It skips that file (leaving its existing expected output in place), tells you exactly which file and the command to bless it with, and exits non-zero so the skip can’t go unnoticed.

-

If your project intentionally exercises a build-time error (e.g. asserting a migration fails the way it should), bless that file by hand instead — this is a deliberate, supported case, not a workaround, and you’ll need to repeat it by hand every time that file’s output legitimately changes, since build-results will always skip it:

+

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:

@@ -2134,7 +2134,7 @@

diff --git a/base.mk b/base.mk index e6b4a53..fd796d3 100644 --- a/base.mk +++ b/base.mk @@ -698,7 +698,7 @@ test-build: # Tradeoff: this also blocks the main suite on a stale/wrong # test/build/expected/*.out, not just a genuinely broken build -- there's # no `make results`-equivalent for test/build, so today that means either -# hand-editing the expected file or using `make build-results` below. +# hand-editing the expected file or using `make results-build` below. # # Guarded by _PGXNTOOL_TEST_BUILD_ACTIVE: test-build's own recipe above # recurses into `installcheck` to reuse PGXS's pg_regress plumbing for its @@ -708,7 +708,7 @@ ifneq ($(_PGXNTOOL_TEST_BUILD_ACTIVE),yes) installcheck: test-build endif -# build-results: bless test/build/'s actual output as the new expected +# results-build: bless test/build/'s actual output as the new expected # output, mirroring `make results` for the main suite. Refuses to bless any # file whose actual output contains "ERROR:" -- accepting an errored build # as the new baseline would defeat the point of test-build. If a project @@ -729,7 +729,7 @@ endif # # Runs test-build itself instead of duplicating its run-test-build.sh + # installcheck steps: by the time test-build's own regression.diffs check -# fails, the actual output build-results needs is already on disk. But +# 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 @@ -741,11 +741,11 @@ endif # target Postgres instance was unreachable), so mere existence isn't # enough. Any other failure aborts here instead of reaching the copy loop # below. -.PHONY: build-results -build-results: +.PHONY: results-build +results-build: @rm -f $(TESTDIR)/build/regression.diffs $(MAKE) -C . test-build || test -s $(TESTDIR)/build/regression.diffs || { \ - echo "build-results: test-build failed for a reason other than a diff to bless; not blessing stale output" >&2; \ + 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 @@ -753,7 +753,7 @@ build-results: for f in $(TESTDIR)/build/results/*.out; do \ [ -f "$$f" ] || continue; \ if grep -q 'ERROR:' "$$f"; then \ - echo "build-results: skipping $$f (actual output contains ERROR:)" >&2; \ + echo "results-build: skipping $$f (actual output contains ERROR:)" >&2; \ echo " If this is intentional, bless it by hand:" >&2; \ echo " cp $$f $(TESTDIR)/build/expected/$$(basename "$$f")" >&2; \ skipped=1; continue; \