Test verify-tap.sh, the standalone TAP output checker

[jnasbyupgrade]

Tests for Postgres-Extensions/pgxntool#143, which adds pgxntool/verify-tap.sh [--skip-non-tap] [FILE|-]... (a standalone TAP output checker) and makes make results and make test use it in pgtap mode.

• test/standard/verify-tap-script.bats (new) invokes the script directly on scratch files and stdin, with no foundation, make or PostgreSQL: each pass/fail rule (not ok without # TODO, Bail out!, pgTAP's "Looks like you planned", missing/duplicate/misplaced plan line, count mismatch, 1..0, subtests ignored), no-TAP input with and without --skip-non-tap (including beside a failing TAP file and on stdin), stdin via a pipe and via - with diagnostics on stderr only, multiple files with one failing, and exit code 2 for usage errors or an unreadable file.
• test/standard/verify-results-pgtap-script.bats: a new case shows a missing plan line now blocks make results while plain-SQL output is left alone. The SQL-error case now gives its fixture a complete plan, so it still exercises the regression.diffs scan instead of being caught by the new count check.
• test/standard/make-test.bats: make -n test dry-runs show the test: recipe invokes verify-tap.sh --skip-non-tap under pgtap and not under diffs (folded into the existing mode-selection test), and one end-to-end case blesses a not ok with make results PGXNTOOL_ENABLE_VERIFY_RESULTS=no and shows make test then fails with no regression.diffs.
• verify-tap.sh is added to test/lib/dist-expected-files.txt and to CLAUDE.md's list of hand-invoked scripts in the user-facing API surface.

No new helpers and no template change.

Merge order

Merge Postgres-Extensions/pgxntool#143 first. This is an addition: these tests need a script that only exists on that branch, so merging this first would break CI for unrelated pgxntool-test PRs.

[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: 58adceb1-3355-41d1-af0c-6e3deea17188

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

• Autofix · 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.}

[github-actions]

Claude finished @jnasbyupgrade's task in 28s —— View job

---

Review of PR #109

I read the diff only. I couldn't read ../pgxntool/verify-tap.sh because it is outside the allowed working directory, and I didn't run the tests. Check the assertion strings against the script's actual messages once pgxntool#143 is available.

Overall: The test layering follows CLAUDE.md. Decision logic is tested directly against the script. The Makefile layer is tested with make -n. There is one end-to-end test. The coverage is thorough and I found no blocking bugs.

Suggestions

1. The PR description doesn't match the diff. It says "No new end-to-end runs", but test/standard/make-test.bats adds a new test, "make test fails on a not ok that was blessed…". That test runs make results and make test for real. It is also the only change to make-test.bats that the description doesn't mention. I think the test is justified, since only a real run proves make test invokes verify-tap.sh after pg_regress. The description should say so.

2. The blessed-not ok test builds its fixture with sed (make-test.bats, around line 770). sed 's/, 3$/, 4/' test/sql/pgxntool-test.sql depends on the exact template text. CLAUDE.md prefers putting fixture content in template/ over generating it in test code. If the pattern stops matching, the copy still passes through unchanged. The grep '^not ok 1' guard then fails with a clear message, but the failure points at the wrong cause. Consider writing the failing SQL out directly with a heredoc, or asserting that the sed changed the file.

3. Cleanup doesn't run when the test fails (make-test.bats). The rm -f for blessed_not_ok.* only runs if every assertion passes. A failure leaves blessed_not_ok.sql and its expected file in the shared foundation. Every later make test in that file then fails too, which hides the real failure. A teardown, or a cleanup that doesn't depend on the assertions passing, would fix this. The existing pgtap_fail test just above has the same pattern, so you may want to fix both together.

4. Minor: duplicated setup (verify-tap-script.bats). load_test_env is called in both setup_file and setup, and the comment says it only resolves PGXNREPO. If the per-test call is enough, drop the setup_file one.

5. Minor: coverage gaps (verify-tap-script.bats). The test for the 1..0 skip-all case only covers a # Skipped: suffix. It doesn't cover a bare 1..0, or 1..0 combined with other test lines. That may be intentional, so ignore this if so.

What looks good

• The verify-results-pgtap-script.bats fixture change (1..2 → 1..1) keeps the SQL-error case exercising the regression.diffs scan. The added Tests are failing assertions confirm it.
• dist-expected-files.txt and the CLAUDE.md API-surface list are both updated.
• The merge-order note is correct for an addition.