Skip to content

Add verify-tap.sh, a standalone TAP output checker - #143

Open
jnasbyupgrade wants to merge 2 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:verify-tap
Open

jnasbyupgrade wants to merge 2 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:verify-tap

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Adds pgxntool/verify-tap.sh [--skip-non-tap] [FILE|-]..., a standalone checker that fails if TAP (pgTAP) output shows a failed test run, and makes both make results and make test use it in pgtap mode. It needs neither make nor a database, so it can be used on any TAP output, including outside pgxntool.

A stream passes only if, counting unindented lines alone (subtest output is ignored): no not ok lacks # TODO; there is no Bail out!; there is no pgTAP "Looks like you planned" line; there is exactly one plan line (1..N), before every test line or after every one; and the test-line count equals N (1..0 with no tests passes). Input with no plan line and no test lines fails, since nothing ran; with --skip-non-tap it is skipped silently instead (a TAP line is ^(not )?ok [0-9]+ or ^1\.\.[0-9]+). With no arguments, or for -, it reads stdin; each file argument is judged as its own stream. Problems go to stderr, naming the source (stdin or the path). Exit status: 0 all pass, 1 any input fails, 2 usage error or unreadable file.

The script name is provisional and may change at review.

make results change (breaking)

verify-results-pgtap.sh replaces its inline not ok/plan scan with verify-tap.sh --skip-non-tap on every test/results/*.out file, so files with no TAP line are skipped by the same rule make test uses. It therefore now also refuses on a missing, duplicate or misplaced plan line, a count mismatch and Bail out!, which catches a test that died before finishing. Plain-SQL output, the regression.diffs phase and its messages are unchanged. test/install output is never checked.

make test change (breaking)

Under PGXNTOOL_VERIFY_RESULTS_MODE=pgtap (the default), the test: recipe now runs verify-tap.sh --skip-non-tap on $(TESTOUT)/results/*.out, still prints regression.diffs, and exits non-zero if either check failed, reporting both. So make test now fails on a not ok, a missing or duplicate plan, a count mismatch or Bail out! even when that output was blessed into an expected file. An unmatched glob or empty results directory doesn't fail the run. Under diffs mode the recipe doesn't run the verifier. test/install output (written outside results/) is never passed. No new make variable.

Docs

  • README.asc: new verify-tap.sh section; verify-results/PGXNTOOL_VERIFY_RESULTS_MODE text updated, plus the list of make test checks and --skip-non-tap. README.html regenerated with asciidoctor README.asc.
  • HISTORY.asc: new STABLE section (two breaking changes plus new feature).

Follows up on #106, which is already closed.

Merge order

Merge this PR first, then Postgres-Extensions/pgxntool-test#109. This is an addition: the pgxntool-test PR adds tests for a script that only exists here, so merging it first would break CI for unrelated pgxntool-test PRs.

`pgxntool/verify-tap.sh [FILE|-]...` fails if a TAP stream shows a
failed run: a `not ok` without `# TODO`, `Bail out!`, pgTAP's "Looks
like you planned" line, a missing, duplicate or misplaced plan line, or
a test count that differs from the plan. Only unindented lines count.
It reads stdin by default, judges each file argument as its own stream,
reports problems on stderr, and exits 0, 1 (a stream failed) or 2
(usage error or unreadable file).

`make results` in `pgtap` mode now runs it on every
`test/results/*.out` file holding TAP output, replacing the inline
`not ok`/plan scan, so it also refuses on missing or duplicate plan
lines, count mismatches and `Bail out!`. Plain-SQL output and the
`regression.diffs` check are unchanged. See Postgres-Extensions#106.

Related changes in pgxntool-test:
- Direct-script tests for `verify-tap.sh`
- `verify-results-pgtap.sh` test showing a missing plan line blocks
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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: 283d6964-516d-4add-a580-b5b9993dc5ba

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

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

Add `--skip-non-tap` to `verify-tap.sh`: an input with no TAP line at
all (`^(not )?ok [0-9]+` or `^1\.\.[0-9]+`) is skipped silently instead
of failing. Without the flag, behavior is unchanged.

`verify-results-pgtap.sh` now passes every `test/results/*.out` file to
`verify-tap.sh --skip-non-tap` instead of filtering for TAP files itself,
so it and `make test` share one selection rule.

Under `PGXNTOOL_VERIFY_RESULTS_MODE=pgtap` (the default), the `test:`
recipe runs `verify-tap.sh --skip-non-tap` on `$(TESTOUT)/results/*.out`,
still prints `regression.diffs`, and fails if either check failed,
reporting both. A `not ok`, a missing or duplicate plan, a count
mismatch or `Bail out!` now fails `make test` even when that output was
blessed into an expected file. An unmatched glob or empty results
directory doesn't fail the run, `diffs` mode skips the verifier, and
`test/install` output is never checked.

README.asc, README.html and HISTORY.asc are updated to match.

Related changes in pgxntool-test:
- `--skip-non-tap` cases in the direct-script tests
- `make -n test` checks that the verifier runs under `pgtap` only
- End-to-end case: a blessed `not ok` fails `make test`
jnasbyupgrade added a commit to jnasbyupgrade/pgxntool-test that referenced this pull request Oct 16, 2026
Add tests for pgxntool commit 409787f
(Postgres-Extensions/pgxntool#143):
- `verify-tap.sh --skip-non-tap` skips input with no TAP lines
- `make test` runs `verify-tap.sh --skip-non-tap` on results under the
  default `pgtap` mode, not under `diffs`

`verify-tap-script.bats` folds `--skip-non-tap` into the existing no-TAP
test: a plain-SQL file passes silently with the flag, a failing TAP file
beside it is still judged wherever the flag goes, and stdin works too.

`make-test.bats` adds `make -n test` checks for both modes to the
existing mode-selection test, plus one end-to-end case: a `not ok`
blessed with `make results PGXNTOOL_ENABLE_VERIFY_RESULTS=no` makes
`make test` fail with no `regression.diffs`.
Comment thread verify-tap.sh
/^1\.\.[0-9]/ { plans++; plan_nr = NR; planned = substr($1, 4) + 0 }
END {
src = ENVIRON["SRC"]
if (tests + plans == 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: --skip-non-tap silently swallows a bare Bail out! stream, which is the only mode either caller (base.mk's test: recipe and verify-results-pgtap.sh) ever uses.

tests/plans are only incremented by ^(not )?ok [0-9] / ^1\.\.[0-9] lines (see

pgxntool/verify-tap.sh

Lines 66 to 74 in 409787f

tests++
if (!first_test) first_test = NR
last_test = NR
}
/^not ok/ && !/# TODO/ { notok = notok $0 "\n" }
/^Bail out!/ { bail = bail $0 "\n" }
/^# Looks like you planned/ { looks = looks $0 "\n" }
/^1\.\.[0-9]/ { plans++; plan_nr = NR; planned = substr($1, 4) + 0 }
END {
). A stream whose only content is Bail out! ... — e.g. a no_plan() pgTAP test that bails before its first assertion, since the plan line is only emitted by finish() — has tests == 0 && plans == 0. With --skip-non-tap set, this gate exit 0s immediately, before the bail != "" check a few lines down (

pgxntool/verify-tap.sh

Lines 86 to 90 in 409787f

if (bail != "") {
print "ERROR: pgtap run bailed out in " src
printf "%s", bail
failed = 1
}
) is ever reached. So make test passes and make results is willing to bless a bailed-out run — directly contradicting the guarantee stated in this script's own header, README.asc, and HISTORY.asc that "there is no Bail out! line". The same gate also lets a bare # Looks like you planned line through unflagged.

Suggested change
if (tests + plans == 0) {
if (tests + plans == 0 && notok == "" && bail == "" && looks == "") {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant