Let make results seed a new test's first expected output

[jnasbyupgrade]

verify-results blocked make results whenever regression.diffs existed at all, which made seeding the first test/expected/<name>.out for a brand-new test structurally impossible. base.mk touches an empty expected file for any test/sql/*.sql that lacks one (pg_regress aborts outright on a missing expected file), so pg_regress always reports a difference against that placeholder — no matter how clean the new test's actual output is. Rerunning never helps; it is not a transient failure.

verify-results now classifies each regression.diffs block by its hunk headers: a block whose hunks all read @@ -0,0 +N,M @@ was diffed against an empty expected file and is let through, and anything else still blocks make results exactly as before.

Also documents the behavior in README.asc (and fixes a stale test/results/regression.diffs path in that same passage — the file is at test/regression.diffs), adds a HISTORY.asc entry, and appends #119 to the release's fixed-issues line.

Fixes #119

Companion pgxntool-test PR: Postgres-Extensions/pgxntool-test#85

Behavior change worth calling out: an empty regression.diffs is now its own refusal

pg_regress truncates regression.diffs at startup and deletes it again only on the clean exit path when it has nothing to report, so every bail leaves a zero-byte file behind — unreachable server, interrupt, could not open file. Combined with base.mk's .IGNORE: installcheck, make results reaches verify-results anyway, with test/results/ still holding whatever an earlier run left there.

Blocking on the file's mere existence used to cover that case as a side effect. Classifying its contents does not, so it is now checked explicitly and reported as what it is:

$ rm -f test/regression.diffs && PGPORT=5999 make verify-results
# command failed: ".../psql" -X -q -c "SET client_min_messages = warning" ...
Bail out!make: [.../pgxs.mk:436: installcheck] Error 2 (ignored)
ERROR: pg_regress did not complete. Cannot run 'make results'.
test/regression.diffs is empty, so nothing was actually compared;
anything in test/results/ is left over from an earlier run.
make: *** [pgxntool/base.mk:457: verify-results] Error 1

Net effect versus before this PR: unchanged (still refuses), with a message that names the real cause instead of reporting failing tests it never saw.

Why this can't false-positive on alternate expected files

results_differ() in pg_regress.c returns early the moment any expected file matches — the default one or an _N.out alternate — and only appends a block to regression.diffs when none matched. Verified empirically: with test/expected/newtest_1.out matching actual output and test/expected/newtest.out still the empty placeholder, pg_regress reports ok 3 - newtest and writes no regression.diffs at all. So a test that passed via an alternate never reaches this code, and nothing here re-derives a pass/fail pg_regress already decided.

Classification keys on ^diff and ^@@ lines rather than the ---/+++ pair, because every content line in a unified diff carries a ' '/'+'/'-' prefix but a deleted -- comment line renders as --- comment, which is indistinguishable from a from-file header. Anything matching neither a header nor a diff body line (' ', '+', '-', '\', or empty) blocks the whole file rather than being skipped — so junk after an otherwise-blessable block can't slip through. \ No newline at end of file is diff syntax, not junk, and is covered by a test.

Empirical evidence

Real regression.diffs from a template extension with a new test/sql/newtest.sql (all pgtap assertions passing) and no test/expected/newtest.out, trimmed:

diff -U3 /tmp/i119repro/test/expected/newtest.out /tmp/i119repro/test/results/newtest.out
--- /tmp/i119repro/test/expected/newtest.out	2026-09-15 17:12:28.242204151 -0500
+++ /tmp/i119repro/test/results/newtest.out	2026-09-15 17:16:14.087574613 -0500
@@ -0,0 +1,57 @@
+\i test/pgxntool/setup.sql
...
+SELECT plan(1);
+1..1
+SELECT is( 1+1, 2 );
+ok 1
+SELECT finish();

Before: make results refused with "Tests are failing." After: it succeeds, prints NOTE: these tests have no expected output yet; 'make results' will create it: and seeds test/expected/newtest.out; the following make test passes. Corrupting that now-real baseline makes make results block again.

One case that is still blocked, and how it's matched

An as-yet-unblessed file whose output contains a SQL error. ON_ERROR_STOP aborts the script at the error, so pgtap emits neither not ok nor its plan-mismatch line and the pgtap scan above sees nothing wrong — but blessing it would make the error the new baseline.

The match has to fit psql's two real renderings, which were captured rather than assumed. pg_regress feeds psql on stdin (psql -X -a -q -d "%s" %s < "%s" > "%s" 2>&1, pg_regress_main.c), so a failing statement in the test file itself reports at column 0:

ERROR:  42P01: relation "nonexistent_top" does not exist

while one inside an \i'd file — which every pgxntool test has, via setup.sql/finish.sql — carries a prefix:

psql:test/incfile.sql:1: ERROR:  42P01: relation "nonexistent_inc" does not exist

A column-anchored match would sail straight past the second. The guard is grep -qE '(^|: )ERROR: ': either position, and psql's own two spaces after the severity (hardcoded in libpq's message formatting, and identical at default and verbose verbosity — both checked). That keeps a passing test whose pgtap description merely quotes an error message out of it, which an unanchored match did not — end-to-end, a new test asserting ok 1 - ERROR: this description mentions ERROR: on purpose now gets its first expected output created instead of being permanently refused.

PGXNTOOL_ENABLE_VERIFY_RESULTS=no remains the escape hatch if a test legitimately needs error output blessed; README.asc now says so at that bullet.

Scope notes

• PostgreSQL 11 and older write -C3 context diffs, which have no @@ hunk headers at all (pretty_diff_opts, checked across REL_11_STABLE/REL_12_STABLE/HEAD — 12 was the first with -U3). Every block there lands in the blocked pile, i.e. exactly the refusal make results gave before any of this existed. Noted in the script; the CI matrix is 12-18.
• The results path is taken as the header line's last field, so a path containing spaces truncates — that fails closed (the subsequent [ -r "$file" ] fails and the block is blocked), it isn't a hole.

Known related gap, deliberately not addressed here

base.mk's other verify-results branch — PGXNTOOL_VERIFY_RESULTS_MODE=diffs, not the default — has the same class of bug: it fails unconditionally on any regression.diffs, with no not ok scan at all, so it cannot seed a new test's first expected output either. Issue #119 is scoped to verify-results-pgtap.sh, so that branch is left alone; README.asc now says so explicitly.

Test plan

• Coverage lives in the companion pgxntool-test PR: a script-level test/standard/verify-results-pgtap-script.bats (16 tests, no Make/PostgreSQL) plus an end-to-end make results test in make-test.bats.
• Every branch proven against the pre-change script as well as this one: the no-baseline fixture exits 1 ("Tests are failing") before and 0 after; the empty-regression.diffs and junk-after-a-block fixtures exit 0 before (fail-open) and 1 after; the ERROR:-in-a-description fixture exits 1 before (false positive) and 0 after.
• Full pgxntool-test test-all against this branch with pgxntool PR Gate the main test suite on test-build; add build-results; require ON_ERROR_STOP in test/install #109 merged in locally: 273/273 pass, 0 skipped.
• CI on both PRs is red for a reason unrelated to this change: pgxntool-test master already carries check-stale-expected's installcheck dependency can run installcheck before install on a fresh tree #79's tests for PR Gate the main test suite on test-build; add build-results; require ON_ERROR_STOP in test/install #109, which is still open, so 14 tests fail against pgxntool master regardless of this PR (the same 14 locally and on CI). It can't go green until Gate the main test suite on test-build; add build-results; require ON_ERROR_STOP in test/install #109 lands.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f59c594d-41bf-48ab-b949-f98798f3ff24

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}