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

HISTORY.asc

@@ -1,5 +1,28 @@
 STABLE
 ------
+== Fix `make results` refusing to create a new test's first expected output
+`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 impossible: pgxntool creates an empty placeholder for a test
+that has no expected file (pg_regress aborts outright on a missing one), so
+pg_regress always reports a difference against it no matter how clean the
+new test's actual output is.
+
+`verify-results` now reads what `regression.diffs` actually says. A test
+that differs only from that empty placeholder is allowed through, provided
+its own pgtap output is clean and contains no SQL error; a test that differs
+from a real expected file still blocks `make results` as before. This
+applies to the default `PGXNTOOL_VERIFY_RESULTS_MODE=pgtap`;
+`PGXNTOOL_VERIFY_RESULTS_MODE=diffs` still blocks on any `regression.diffs`.
+
+An empty `regression.diffs` is now its own, separately reported refusal:
+pg_regress writes one at startup and removes it again when it finishes with
+nothing to report, so an empty one left behind means it died before
+comparing anything (unreachable server, interrupt) and `test/results/` still
+holds an earlier run's output. `make results` previously blocked that case
+only as a side effect of blocking on the file's existence, and says so
+explicitly now.
+
 == Add `PGXNTOOL_ENABLE_FS_INSTALL` and `PGXNTOOL_ENABLE_PGXN_INSTALL`
 `test`/`verify-results` unconditionally filesystem-installed the extension
 (via PGXS's `install`), and `installcheck` unconditionally auto-installed
@@ -43,7 +66,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.
 
-Issues fixed in this release: #87, #21, #55, #90
+Issues fixed in this release: #87, #21, #55, #90, #119
 
 2.3.0
 -----

README.asc

@@ -237,11 +237,13 @@ 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`:
 
-`pgtap` (the default):: Scans `test/results/*.out` for pgTAP `not ok` lines (ignoring `# TODO` items) and plan-count mismatches, then also checks `test/results/regression.diffs` for any output mismatch pgTAP itself wouldn't catch.
-`diffs`:: Only checks whether `test/results/regression.diffs` exists.
+`pgtap` (the default):: Scans `test/results/*.out` for pgTAP `not ok` lines (ignoring `# TODO` items) and plan-count mismatches, then also checks `test/regression.diffs` for any output mismatch pgTAP itself wouldn't catch. A test that has no expected output file yet is the one exception: it always shows up in `regression.diffs` (pgxntool creates an empty expected file for it, because `pg_regress` aborts on a missing one), so `make results` is allowed to create its first expected output — provided its own pgTAP output is clean and free of SQL errors. If a new test legitimately needs output containing a SQL error blessed, use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` for that one run.
+`diffs`:: Only checks whether `test/regression.diffs` exists, so it can't create a new test's first expected output; use `pgtap` mode, or `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` for that one run.
 
 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`.
 
+It also refuses when `pg_regress` didn't finish at all — an unreachable server, an interrupt — since `test/results/` then still holds an earlier run's output and blessing it would make that stale output the new expected results.
+
 To select a mode explicitly:
 ----
 # In your Makefile
@@ -729,7 +731,7 @@ Default: `pgtap`. Controls how the <<_verify_results_safeguard,verify-results sa
 
 ==== pgtap
 
-Scans `test/results/*.out` for pgTap `not ok` lines and plan mismatches, falling back to checking for `regression.diffs` too. Use this mode when your test suite uses 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.
 
 ==== diffs
 

README.html

@@ -1000,18 +1000,21 @@ <h4 id="_verify_results_safeguard"><a class="anchor" href="#_verify_results_safe
 <dl>
 <dt class="hdlist1"><code>pgtap</code> (the default)</dt>
 <dd>
-<p>Scans <code>test/results/*.out</code> for pgTAP <code>not ok</code> lines (ignoring <code># TODO</code> items) and plan-count mismatches, then also checks <code>test/results/regression.diffs</code> for any output mismatch pgTAP itself wouldn&#8217;t catch.</p>
+<p>Scans <code>test/results/*.out</code> for pgTAP <code>not ok</code> lines (ignoring <code># TODO</code> items) and plan-count mismatches, then also checks <code>test/regression.diffs</code> for any output mismatch pgTAP itself wouldn&#8217;t catch. A test that has no expected output file yet is the one exception: it always shows up in <code>regression.diffs</code> (pgxntool creates an empty expected file for it, because <code>pg_regress</code> aborts on a missing one), so <code>make results</code> 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 <code>PGXNTOOL_ENABLE_VERIFY_RESULTS=no</code> for that one run.</p>
 </dd>
 <dt class="hdlist1"><code>diffs</code></dt>
 <dd>
-<p>Only checks whether <code>test/results/regression.diffs</code> exists.</p>
+<p>Only checks whether <code>test/regression.diffs</code> exists, so it can&#8217;t create a new test&#8217;s first expected output; use <code>pgtap</code> mode, or <code>PGXNTOOL_ENABLE_VERIFY_RESULTS=no</code> for that one run.</p>
 </dd>
 </dl>
 </div>
 <div class="paragraph">
 <p>If tests are failing, <code>make results</code> prints an explanation of what failed and exits without touching <code>test/expected/</code>; fix the failures, then re-run <code>make results</code>.</p>
 </div>
 <div class="paragraph">
+<p>It also refuses when <code>pg_regress</code> didn&#8217;t finish at all — an unreachable server, an interrupt — since <code>test/results/</code> then still holds an earlier run&#8217;s output and blessing it would make that stale output the new expected results.</p>
+</div>
+<div class="paragraph">
 <p>To select a mode explicitly:</p>
 </div>
 <div class="listingblock">
@@ -1980,7 +1983,7 @@ <h3 id="_pgxntool_verify_results_mode"><a class="anchor" href="#_pgxntool_verify
 <div class="sect3">
 <h4 id="_pgtap"><a class="anchor" href="#_pgtap"></a><a class="link" href="#_pgtap">8.8.1. pgtap</a></h4>
 <div class="paragraph">
-<p>Scans <code>test/results/*.out</code> for pgTap <code>not ok</code> lines and plan mismatches, falling back to checking for <code>regression.diffs</code> too. Use this mode when your test suite uses pgTap.</p>
+<p>Scans <code>test/results/*.out</code> for pgTap <code>not ok</code> lines and plan mismatches, falling back to checking <code>regression.diffs</code> too — except for a test that has no expected output yet, whose first expected file <code>make results</code> is allowed to create. Use this mode when your test suite uses pgTap.</p>
 </div>
 </div>
 <div class="sect3">
@@ -2068,7 +2071,7 @@ <h2 id="_copyright"><a class="anchor" href="#_copyright"></a><a class="link" hre
 </div>
 <div id="footer">
 <div id="footer-text">
-Last updated 2026-09-08 16:14:24 -0500
+Last updated 2026-09-15 17:56:12 -0500
 </div>
 </div>
 </body>

verify-results-pgtap.sh

@@ -2,7 +2,8 @@
 # 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. Exits non-zero if any problems are found.
+# 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
 #
@@ -36,12 +37,111 @@ if [ $failed -ne 0 ]; then
 	exit 1
 fi
 
-# Also check regression.diffs (output mismatch even if pgtap all passed)
-if [ -r "$TESTOUT/regression.diffs" ]; then
-	echo "ERROR: Tests are failing. Cannot run 'make results'."
-	echo "Fix test failures first, then run 'make results'."
-	echo
-	echo "See $TESTOUT/regression.diffs for details:"
-	cat "$TESTOUT/regression.diffs"
+# Also check regression.diffs (output mismatch even if pgtap all passed).
+#
+# A test whose output matched ANY of its expected files - including a
+# pg_regress _N.out alternate - is left out of regression.diffs entirely, so
+# every block in here is a test that matched none of them. The one exception
+# is a test that has no expected output yet: pg_regress aborts outright on a
+# missing expected file, so base.mk touches an empty test/expected/<name>.out
+# for every test/sql/*.sql lacking one, and diffing actual output against that
+# placeholder always reports a difference. Blocking on that is unwinnable -
+# seeding that first expected file is precisely what `make results` is for -
+# so those blocks are skipped here.
+diffs="$TESTOUT/regression.diffs"
+
+# pg_regress truncates regression.diffs at startup and deletes it again on a
+# clean finish with nothing to report, so an empty one means it died before
+# comparing anything - an unreachable server, an interrupt. Whatever is in
+# results/ is then left over from some earlier run, and base.mk's `.IGNORE:
+# installcheck` means `make results` gets here anyway, so blessing it would
+# bless stale output.
+if [ -e "$diffs" ] && [ ! -s "$diffs" ]; then
+	echo "ERROR: pg_regress did not complete. Cannot run 'make results'."
+	echo "$diffs is empty, so nothing was actually compared;"
+	echo "anything in $TESTOUT/results/ is left over from an earlier run."
 	exit 1
 fi
+
+if [ -s "$diffs" ]; then
+	# pg_regress heads each block with "diff <opts> <expected> <results>" (see
+	# results_differ() in pg_regress.c), then appends the diff itself. Only
+	# those headers and diff hunk headers can start with "diff " or "@@ "
+	# unprefixed; every line carrying file content is prefixed with ' ', '+',
+	# '-' or '\'. A block whose hunks all read "@@ -0,0 +N,M @@" was diffed
+	# against an empty expected file, i.e. that placeholder. Anything that
+	# fits none of those shapes is something this has no business
+	# interpreting, so it blocks the whole file.
+	#
+	# Hunk headers only exist in unified diffs. PostgreSQL 12 was the first
+	# to write them (11 and older use -C3 context diffs), so on those every
+	# block lands in the blocked pile - the same refusal `make results` gave
+	# before any of this existed.
+	classified=$(awk '
+		function flush(   status) {
+			if (results == "") return
+			status = (hunks > 0 && unblessed) ? "unblessed" : "regression"
+			print status "\t" results
+		}
+		/^diff /   { flush(); results = $NF; unblessed = 1; hunks = 0; next }
+		/^@@ /     { hunks++; if ($0 !~ /^@@ -0,0 /) unblessed = 0; next }
+		/^[ +-]/   { next }
+		/^\\/      { next }
+		/^$/       { next }
+		           { unrecognized = 1 }
+		END        { flush(); if (unrecognized) print "unrecognized\t" FILENAME }
+	' "$diffs")
+
+	blocked=
+	blessable=
+	while IFS="$(printf '\t')" read -r status file; do
+		case $status in
+		unblessed)
+			# An unblessed file holding a hard SQL error is not a passing test
+			# the pgtap scan above was merely quiet about: ON_ERROR_STOP aborts
+			# the script, so pgtap emits neither 'not ok' nor its plan-mismatch
+			# line. Blessing that would make the error the baseline.
+			#
+			# 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.
+			if [ -r "$file" ] && ! grep -qE '(^|: )ERROR:  ' "$file"; then
+				blessable="$blessable  $file
+"
+				continue
+			fi
+			;;
+		unrecognized)
+			blocked="$blocked  (unrecognized content in $file)
+"
+			continue
+			;;
+		esac
+		blocked="$blocked  $file
+"
+	done <<EOF
+$classified
+EOF
+
+	# Nothing recognizable in a non-empty regression.diffs: refuse to guess.
+	[ -n "$classified" ] || blocked="  (unrecognized $diffs content)
+"
+
+	if [ -n "$blocked" ]; then
+		echo "ERROR: Tests are failing. Cannot run 'make results'."
+		echo "Fix test failures first, then run 'make results'."
+		echo
+		echo "Failing tests:"
+		printf '%s' "$blocked"
+		echo
+		echo "See $diffs for details:"
+		cat "$diffs"
+		exit 1
+	fi
+
+	echo "NOTE: these tests have no expected output yet; 'make results' will create it:"
+	printf '%s' "$blessable"
+fi