Cover verify-results-pgtap.sh's regression.diffs classification

[jnasbyupgrade]

Test coverage for pgxntool PR Postgres-Extensions/pgxntool#121, which lets make results seed the first expected output for a brand-new test (pgxntool issue #119).

New test/standard/verify-results-pgtap-script.bats owns verify-results-pgtap.sh's decision logic, invoking the script directly against a scratch directory with no Make, no foundation environment and no PostgreSQL — the layering CLAUDE.md prescribes, modeled on check-stale-expected-script.bats. It covers:

• a brand-new test with no baseline (a single @@ -0,0 hunk) now passing
• a real mismatch against an existing baseline still blocking, including when a brand-new test sits in the same regression.diffs
• an unblessed file whose output holds a SQL error still blocking, in both of psql's real renderings: column 0 (the test file itself, which pg_regress feeds psql on stdin) and the psql:<file>:<line>: prefix an \i'd file produces — a column-anchored match would miss the second, which is what setup.sql/finish.sql generate
• a passing test whose pgtap description merely mentions ERROR: being blessed rather than permanently refused
• an empty-but-existing regression.diffs blocking (pg_regress bailed before comparing anything; test/results/ holds a stale run)
• blocks that aren't a clean no-baseline shape: a header with no hunks, and one with only some @@ -0,0 hunks
• unrecognized content after an otherwise-blessable block blocking the whole file — while diff's own \ No newline at end of file marker still doesn't
• classification driven by unprefixed diff /@@ headers, not by diffed content that looks like a header (both directions: a +@@ ... line inside added content, and a deleted -- comment rendering as --- comment)
• the pre-existing not ok / # TODO / plan-mismatch scan

make-test.bats keeps only what genuinely needs a real pg_regress run: an end-to-end test that make results seeds a brand-new test's first expected output — asserting the no expected output yet message, so it can't pass on a file that appeared some other way — and one wiring test that make verify-results invokes the script and surfaces its failure. Its TODO and plan-mismatch cases moved into the script-level file rather than paying for a full make verify-results run each to re-test logic decided entirely inside the script.

Test plan

• test/standard/verify-results-pgtap-script.bats: 16/16 pass.
• Full test-all against a local pgxntool with pgxntool PR #109 merged in: 273/273 pass, 0 failed, 0 skipped, including every test added or changed here.
• Each new fail-closed test verified to actually discriminate, by running its fixture against the pre-fix script too: the empty-regression.diffs and junk-after-a-block fixtures exit 0 there (fail-open) and 1 now; the ERROR:-in-a-description fixture exits 1 there (false positive) and 0 now.
• Full test-all against pgxntool master as it stands today: 14 failures, 0 skipped. All 14 are pre-existing and unrelated to this PR, and CI reproduces exactly the same 14 — they are the tests merged in Cover test-build ordering, build-results, and test/install ON_ERROR_STOP #79 for pgxntool PR #109 (check-test-install-error-stop.sh, build-results, test-build ordering, the dist manifest entry for that script, and test/install marker state). That PR is still open, so the script and targets those tests exercise don't exist in pgxntool master yet; the script-level ones fail with exit 127, "No such file or directory". CI here cannot go green until #109 lands, independently of this PR.

[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: 7c24f005-19a4-47f0-bab7-43d5dbb6fb46

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

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[jnasbyupgrade]

Except we're calling load_test_env; doesn't that in fact create a foundation environment?

[jnasbyupgrade]

(To be clear, it would be nice to avoid foundation if that's reasonable, since it's faster.)

[jnasbyupgrade]

[Claude] load_test_env here only calls create_env (makes test/.envs/<name>/.bats-state and a .env of path exports) and then setup_pgxntool_vars to resolve PGXNREPO. There is no git init, template copy, setup.sh, or PostgreSQL, so this file is already foundation-free (same pattern as check-stale-expected-script.bats). I ran the file and confirmed test/.envs/verify-results-pgtap-script/ contains only .bats-state and .env, with no repo dir (the foundation env does have one). Added a one-line comment at both load_test_env calls in dcea5dc. This also covers your follow-up (4179007811): nothing further to avoid.