Test make test's pgTap output check and pgtap-only mode

test/lib/dist-expected-files.txt

@@ -90,6 +90,7 @@ pgxntool/WHAT_IS_THIS
 pgxntool/test/
 pgxntool/test/bin/
 pgxntool/test/bin/check-stale-expected.sh
+pgxntool/test/bin/check-tap-output.sh
 pgxntool/test/bin/check-test-install-error-stop.sh
 pgxntool/test/deps.sql
 pgxntool/test/pgxntool/

test/lib/pgxntool-variables.txt

@@ -64,12 +64,14 @@ legacy   TEST_RESULT_FILES
 legacy   TEST_SQL_FILES
 internal _PGXNTOOL_BASE_MK_INCLUDED
 internal _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT
+internal _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT
 internal _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT
 internal _PGXNTOOL_CONTROL_FILES
 internal _PGXNTOOL_DUPLICATE_DOCS
 internal _PGXNTOOL_EXTENSIONS
 internal _PGXNTOOL_INSTALL_SCHEDULE
 internal _PGXNTOOL_POST_TAG_VERSION_BUMP_SCRIPT
+internal _PGXNTOOL_TEST_INSTALL_OUTPUT_FILES
 internal _pgxntool_docs_named
 internal _pgxntool_validate_choice
 legacy   pgxntool-sync-local

test/standard/check-tap-output-script.bats

@@ -0,0 +1,168 @@
+#!/usr/bin/env bats
+
+# Test: check-tap-output.sh - pure script-logic unit tests
+#
+# These tests invoke test/bin/check-tap-output.sh directly on scratch files --
+# no foundation environment, no `make`, no PostgreSQL. They own all of its
+# decision logic: what counts as TAP output, and which TAP output fails.
+#
+# make-test.bats covers only base.mk's wiring (which files `make test` passes
+# in, the PGXNTOOL_VERIFY_RESULTS_MODE=diffs skip, exit-status propagation)
+# plus one real end-to-end failure; verify-results-pgtap-script.bats covers
+# verify-results-pgtap.sh's use of this script.
+
+load ../lib/helpers
+load ../lib/assertions
+
+setup_file() {
+  setup_topdir
+  load_test_env "check-tap-output-script"  # only resolves PGXNREPO; creates no foundation
+}
+
+setup() {
+  load_test_env "check-tap-output-script"  # only resolves PGXNREPO; creates no foundation
+  export SCRIPT="$PGXNREPO/test/bin/check-tap-output.sh"
+  export OUTDIR="$BATS_TEST_TMPDIR/out"
+  mkdir -p "$OUTDIR"
+}
+
+# Write $2 as the content of $OUTDIR/$1.out.
+write_out() {
+  printf '%s\n' "$2" > "$OUTDIR/$1.out"
+}
+
+@test "check-tap-output.sh: passes clean TAP output, no_plan() (plan last), a skip_all 1..0, and TODO failures" {
+  write_out clean '\set ECHO none
+1..2
+ok 1
+ok 2 - described
+# TRANSACTION INTENTIONALLY LEFT OPEN!'
+  write_out noplan '\set ECHO none
+ok 1
+ok 2 - described
+1..2
+# TRANSACTION INTENTIONALLY LEFT OPEN!'
+  write_out skipall '1..0 # Skipped: nothing to do here'
+  write_out todo '1..2
+ok 1
+not ok 2 - known issue # TODO fix later'
+
+  run "$SCRIPT" "$OUTDIR/clean.out" "$OUTDIR/noplan.out" "$OUTDIR/skipall.out" "$OUTDIR/todo.out"
+  assert_success
+  [ -z "$output" ] || error "expected no output, got: $output"
+}
+
+@test "check-tap-output.sh: passes non-TAP files and ignores nonexistent arguments" {
+  # Plain SQL output is not TAP even if it mentions pgTap's phrases mid-line,
+  # and an unmatched shell glob arrives as a literal, nonexistent path.
+  write_out plain '\set ECHO none
+
+ x
+---
+ not ok here
+(1 row)'
+
+  run "$SCRIPT" "$OUTDIR/plain.out" "$OUTDIR/missing.out" "$OUTDIR/nomatch/*.out"
+  assert_success
+
+  run "$SCRIPT"
+  assert_success
+
+  # --list names just the TAP files, by the same definition, and checks
+  # nothing: a failing TAP file is still listed with a zero exit.
+  write_out failing '1..1
+not ok 1'
+  run "$SCRIPT" --list "$OUTDIR/plain.out" "$OUTDIR/failing.out" "$OUTDIR/missing.out"
+  assert_success
+  [ "$output" = "$OUTDIR/failing.out" ]
+}
+
+@test "check-tap-output.sh: fails on a not ok without TODO" {
+  write_out failing '1..2
+ok 1
+not ok 2 - broken'
+
+  run "$SCRIPT" "$OUTDIR/failing.out"
+  assert_failure_with_status 1
+  assert_contains "$output" "pgtap failure detected in $OUTDIR/failing.out"
+  assert_contains "$output" "not ok 2 - broken"
+}
+
+@test "check-tap-output.sh: fails on pgTap's 'Looks like you planned' line" {
+  write_out planned '1..3
+ok 1
+ok 2
+# Looks like you planned 3 tests but ran 2'
+
+  run "$SCRIPT" "$OUTDIR/planned.out"
+  assert_failure_with_status 1
+  assert_contains "$output" "pgtap plan mismatch in $OUTDIR/planned.out"
+  assert_contains "$output" "Looks like you planned 3 tests but ran 2"
+  # pgTap's own line already says it; no second count message.
+  assert_not_contains "$output" "but found"
+}
+
+@test "check-tap-output.sh: fails when there is no plan line" {
+  # A no_plan() test that died before finish() printed its plan.
+  write_out noplan 'ok 1
+ok 2'
+
+  run "$SCRIPT" "$OUTDIR/noplan.out"
+  assert_failure_with_status 1
+  assert_contains "$output" "$OUTDIR/noplan.out has no plan line"
+}
+
+@test "check-tap-output.sh: fails on more than one plan line" {
+  write_out twoplans '1..1
+ok 1
+1..1'
+
+  run "$SCRIPT" "$OUTDIR/twoplans.out"
+  assert_failure_with_status 1
+  assert_contains "$output" "$OUTDIR/twoplans.out has 2 plan lines"
+}
+
+@test "check-tap-output.sh: fails when the test count differs from the plan" {
+  # A plan(N) test that died partway through: no pgTap mismatch line at all.
+  write_out short '1..3
+ok 1
+ERROR:  boom'
+  write_out long '1..1
+ok 1
+ok 2'
+
+  run "$SCRIPT" "$OUTDIR/short.out"
+  assert_failure_with_status 1
+  assert_contains "$output" "plan mismatch in $OUTDIR/short.out: planned 3 tests but found 1"
+
+  run "$SCRIPT" "$OUTDIR/long.out"
+  assert_failure_with_status 1
+  assert_contains "$output" "planned 1 tests but found 2"
+}
+
+@test "check-tap-output.sh: counts only top-level lines, so subtests don't skew the plan" {
+  write_out subtest '1..1
+    1..2
+    ok 1
+    not ok 2
+ok 1 - subtest'
+
+  run "$SCRIPT" "$OUTDIR/subtest.out"
+  assert_success
+}
+
+@test "check-tap-output.sh: reports every failing file and fails if any one fails" {
+  write_out good '1..1
+ok 1'
+  write_out bad1 '1..1
+not ok 1'
+  write_out bad2 'ok 1'
+
+  run "$SCRIPT" "$OUTDIR/good.out" "$OUTDIR/bad1.out" "$OUTDIR/bad2.out"
+  assert_failure_with_status 1
+  assert_contains "$output" "bad1.out"
+  assert_contains "$output" "bad2.out"
+  assert_not_contains "$output" "good.out"
+}
+
+# vi: expandtab ts=2 sw=2

test/standard/make-test.bats

@@ -20,7 +20,7 @@
 # - verify-results blocks `make results` when tests are failing, lets a
 #   brand-new test's first expected output through (issue #119), and can be
 #   disabled. Its own pass/fail logic lives in verify-results-pgtap-script.bats
-# - PGXNTOOL_VERIFY_RESULTS_MODE is validated (pgtap/diffs, case-insensitive)
+# - PGXNTOOL_VERIFY_RESULTS_MODE is validated (pgtap/pgtap-only/diffs, case-insensitive)
 
 load ../lib/helpers
 
@@ -559,6 +559,68 @@ EOF
   assert_success
 }
 
+# Test: TAP output check in the `test` recipe (Postgres-Extensions/pgxntool#106)
+#
+# Which TAP output fails is check-tap-output.sh's own logic, covered in
+# check-tap-output-script.bats. These cover base.mk's wiring only: the files
+# the recipe passes in, the PGXNTOOL_VERIFY_RESULTS_MODE=diffs skip, and that
+# the script's exit status decides `make test`. The one real end-to-end case
+# (a blessed `not ok`) is in the verify-results section below, since seeding it
+# needs `make results`.
+
+@test "test and verify-results pass results/*.out and the test/install actual output to the TAP check" {
+  # The template has test/install/create_install_marker.sql; its actual output
+  # is $(TESTDIR)/install/<name>.out, whatever TESTOUT is.
+  run make -n test
+  assert_success
+  assert_contains "$output" "test/bin/check-tap-output.sh test/results/*.out test/install/create_install_marker.out"
+
+  run make -n verify-results
+  assert_success
+  assert_contains "$output" "verify-results-pgtap.sh test test/install/create_install_marker.out"
+
+  # pgtap-only: verify-results-pgtap.sh, not a plain regression.diffs check,
+  # judges `make test`'s diffs, after the same TAP check.
+  run make -n test PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only
+  assert_success
+  assert_contains "$output" "test/bin/check-tap-output.sh test/results/*.out test/install/create_install_marker.out"
+  assert_contains "$output" "verify-results-pgtap.sh --pgtap-only --for-test test || status=1"
+  assert_not_contains "$output" "cat test/regression.diffs"
+
+  run make -n verify-results PGXNTOOL_VERIFY_RESULTS_MODE=pgtap-only
+  assert_success
+  assert_contains "$output" "verify-results-pgtap.sh --pgtap-only test test/install/create_install_marker.out"
+
+  run make -n test PGXNTOOL_VERIFY_RESULTS_MODE=diffs
+  assert_success
+  assert_not_contains "$output" "check-tap-output.sh"
+  assert_contains "$output" "cat test/regression.diffs"
+}
+
+@test "make test runs the TAP check stub, takes its exit status, and skips it in diffs mode" {
+  skip_if_no_postgres
+
+  local marker="$BATS_TEST_TMPDIR/check-tap-output-invoked"
+  local stub_script
+
+  stub_script=$(make_stub_script fail-stub 5 "STUB SENTINEL MESSAGE")
+  run make test _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT="$stub_script"
+  assert_failure
+  assert_contains "$output" "STUB SENTINEL MESSAGE"
+
+  stub_script=$(make_stub_script pass-stub 0 "" "$marker")
+  run make test _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT="$stub_script"
+  assert_success
+  assert_file_exists "$marker"
+
+  # diffs mode: never invoked at all, not merely tolerated.
+  rm -f "$marker"
+  stub_script=$(make_stub_script disabled-stub 1 "" "$marker")
+  run make test PGXNTOOL_VERIFY_RESULTS_MODE=diffs _PGXNTOOL_CHECK_TAP_OUTPUT_SCRIPT="$stub_script"
+  assert_success
+  assert_file_not_exists "$marker"
+}
+
 # ============================================================================
 # verify-results / make results (issues #14, #49)
 # ============================================================================
@@ -648,13 +710,13 @@ EOF
   assert_not_contains "$output" "Cannot run 'make results'"
 }
 
-@test "PGXNTOOL_VERIFY_RESULTS_MODE accepts pgtap/diffs case-insensitively" {
+@test "PGXNTOOL_VERIFY_RESULTS_MODE accepts pgtap/pgtap-only/diffs case-insensitively" {
   run make print-PGXNTOOL_VERIFY_RESULTS_MODE
   assert_success
   assert_contains "$output" 'set to "pgtap"'
 
   local value expected
-  for value in pgtap pgTap PGTAP diffs Diffs DIFFS; do
+  for value in pgtap pgTap PGTAP pgtap-only PgTap-Only diffs Diffs DIFFS; do
     expected=$(echo "$value" | tr '[:upper:]' '[:lower:]')
     run make print-PGXNTOOL_VERIFY_RESULTS_MODE "PGXNTOOL_VERIFY_RESULTS_MODE=$value"
     assert_success
@@ -667,7 +729,7 @@ EOF
   for value in bogus pgtapp "pgtap diffs" ""; do
     run make print-PGXNTOOL_VERIFY_RESULTS_MODE "PGXNTOOL_VERIFY_RESULTS_MODE=$value"
     assert_failure
-    assert_contains "$output" "PGXNTOOL_VERIFY_RESULTS_MODE must be one of: pgtap diffs; got \"$value\""
+    assert_contains "$output" "PGXNTOOL_VERIFY_RESULTS_MODE must be one of: pgtap pgtap-only diffs; got \"$value\""
   done
 }
 
@@ -733,6 +795,29 @@ EOF
   rm -f test/sql/brand_new.sql test/expected/brand_new.out test/results/brand_new.out
 }
 
+@test "make test fails on a not ok that was blessed into expected output (Postgres-Extensions/pgxntool#106)" {
+  skip_if_no_postgres
+
+  # Actual output matches expected byte for byte, so regression.diffs stays
+  # absent; only the TAP check can fail this. Blessing it needs the
+  # PGXNTOOL_ENABLE_VERIFY_RESULTS=no escape hatch, which must keep working
+  # because the TAP check lives in the `test` recipe, not TEST_DEPS.
+  sed -e 's/, 3$/, 4/' test/sql/pgxntool-test.sql > test/sql/blessed_fail.sql
+  grep -q ', 4$' test/sql/blessed_fail.sql || error "failed to create a failing test from test/sql/pgxntool-test.sql"
+
+  run make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results
+  assert_success
+  assert_file_exists test/expected/blessed_fail.out
+  grep -q '^not ok' test/expected/blessed_fail.out || error "blessed_fail.out has no 'not ok' line"
+
+  run make test
+  assert_failure
+  assert_contains "$output" "pgtap failure detected in test/results/blessed_fail.out"
+  assert_file_not_exists test/regression.diffs
+
+  rm -f test/sql/blessed_fail.sql test/expected/blessed_fail.out test/results/blessed_fail.out
+}
+
 @test "verify-results propagates a pgtap failure the script detects" {
   skip_if_no_postgres
 

test/standard/test-test-install.bats

@@ -5,6 +5,7 @@
 # Tests the complete test/install lifecycle:
 # - Auto-detection: enabled when test/install/ has SQL files
 # - Schedule generation with ../install/ relative paths
+# - The $(TESTOUT)/install symlink used when TESTOUT differs from TESTDIR
 # - Core contract: install state persists into main test suite
 # - Disabling via PGXNTOOL_ENABLE_TEST_INSTALL
 # - PGXNTOOL_ENABLE_TEST_INSTALL=yes erroring when test/install/ is empty
@@ -45,6 +46,23 @@ setup() {
   # Schedule should reference install files with relative path
   run grep "../install/" test/install/schedule
   assert_success
+
+  # Those ../install/ paths resolve against TESTOUT for actual output but
+  # against TESTDIR for expected output, so a separate TESTOUT gets a
+  # symlink back to TESTDIR's install directory.
+  local out="$BATS_TEST_TMPDIR/testout"
+  run make _test-install-output-link TESTOUT="$out"
+  assert_success
+  [ -L "$out/install" ]
+  [ "$(readlink "$out/install")" = "$TEST_REPO/test/install" ]
+
+  # Anything else already at that path is left alone, and fails the build.
+  rm "$out/install"
+  mkdir "$out/install"
+  run make _test-install-output-link TESTOUT="$out"
+  assert_failure
+  echo "$output" | grep -q "exists and is not a symlink to $TEST_REPO/test/install"
+  [ -d "$out/install" ] && [ ! -L "$out/install" ]
 }
 
 @test "install schedule orders files by name, not filesystem/creation order (issue #111)" {