From 40c79186856d7ef10392d2d247c91b2d43910786 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 31 Jul 2026 16:04:12 -0500 Subject: [PATCH 1/2] Combine make-test/make-results/verify-results into one suite `test`, `results`, and `verify-results` are one tightly-coupled pipeline -- results/verify-results only exist to safely update the output test checks against -- so test/standard/make-test.bats, make-results.bats, and test-verify-results.bats are merged into one file (make-test.bats survives; the other two are deleted) sharing a single foundation-copied environment instead of three, per CLAUDE.md's directive to minimize state changes/setup cost in the suite. The two separate mismatch-creation steps the old files used (test/expected/pgxntool-test.out vs base.out) are consolidated into one shared mismatch; the two entry points (`make results` and `make verify-results` directly) and all three pgtap-detection cases stay as distinct tests since they're genuinely separate coverage. Surveyed the rest of test/standard/*.bats for the same "fragmented pipeline" pattern: none qualify -- the closest candidates (test-test-build.bats vs test-test-install.bats, dist-clean.bats vs 02-dist.bats, and check-stale-expected-script.bats vs make-test.bats) are independently toggleable features or deliberate layer splits (script decision-logic tests vs Makefile-wiring tests), not fragments of one pipeline. Deleted 5 dead tests (test/output/test/input directory handling) left behind when pgxntool dropped .source file support (issues #9/#22): grepped base.mk and confirmed it never references test/output or test/input anywhere -- $(TESTOUT) defaults to test/results, a different path. These tests, and the defensive `rm -rf test/output` added for issue #49, were purely working around pg_regress's own incidental optional *.source-file directory convention, which is pg_regress's behavior to own, not pgxntool's to re-verify. Added a real structural proof for issue #79 (installcheck must run after install): `make -p -n installcheck` dumps Make's parsed prerequisite database independent of runtime scheduling, and confirms `install` is listed as installcheck's prerequisite. Verified this check actually discriminates: it lists `install` against the fixed base.mk and omits it against the unfixed one. The previous behavioral test (`make uninstall && make test`) is kept as a secondary sanity check, since Make's actual scheduling order for two unrelated same-target prerequisites could have made it pass even against a broken base.mk purely by luck -- it doesn't prove the fix on its own, only the make -p check does. Trimmed two overly long inline comments per review: the issue #49 exit-code assertion now just says "Ensure make exited non-zero"; the confusing pseudo-syntax quote in the results-blocked test was replaced with one section-header comment explaining results/verify-results/$(TEST_DEPS) once. Also fixed a comment that was already factually wrong post-fix (claimed verify-results depends on `test`, which base.mk no longer does). Fixed the same missing `\set ECHO none` issue as the main template (see the paired pgxntool-test fix-79-49 branch) in template-multi-extension/test/sql/base.sql -- latent since #49's bug hid it, caught now that make test actually reports failure. This branch is based on current master and needs no pgxntool code change of its own, but its new/updated assertions exercise base.mk behavior that only exists on the pgxntool fix-79-49 branch (Postgres-Extensions/pgxntool#83) -- this repo's own CI falls back to pgxntool master when no same-named paired branch exists, so this PR's CI will not go green until pgxntool#83 merges. Co-Authored-By: Claude Sonnet 5 --- .../test/expected/base.out | 1 + template-multi-extension/test/sql/base.sql | 3 + test/lib/foundation.bats | 2 +- test/sequential/00-validate-tests.bats | 8 +- test/standard/make-results.bats | 99 ------ test/standard/make-test.bats | 317 +++++++++++++++--- test/standard/test-verify-results.bats | 118 ------- 7 files changed, 272 insertions(+), 276 deletions(-) delete mode 100755 test/standard/make-results.bats delete mode 100644 test/standard/test-verify-results.bats diff --git a/template-multi-extension/test/expected/base.out b/template-multi-extension/test/expected/base.out index 7169d4a..06dd87a 100644 --- a/template-multi-extension/test/expected/base.out +++ b/template-multi-extension/test/expected/base.out @@ -1,2 +1,3 @@ +\set ECHO none multi_ext_ok diff --git a/template-multi-extension/test/sql/base.sql b/template-multi-extension/test/sql/base.sql index f32747d..744d8ec 100644 --- a/template-multi-extension/test/sql/base.sql +++ b/template-multi-extension/test/sql/base.sql @@ -1,3 +1,6 @@ +\set ECHO none +-- DO NOT REMOVE: pg_regress runs psql with echo-all, so without this the SQL +-- input is echoed into the output and never matches expected/base.out. \a \t SELECT pg_sleep(.1); diff --git a/test/lib/foundation.bats b/test/lib/foundation.bats index 8ea3124..1143411 100644 --- a/test/lib/foundation.bats +++ b/test/lib/foundation.bats @@ -29,7 +29,7 @@ # # All other tests depend on this foundation: # - Sequential tests (01-meta, 02-dist, 03-setup-final) build on this base -# - Independent tests (doc, make-results) copy this base to their own environment +# - Independent tests (doc, make-test) copy this base to their own environment # # The foundation is created once in .envs/foundation/ and then copied to other # test environments for speed. Run `make foundation` to rebuild from scratch. diff --git a/test/sequential/00-validate-tests.bats b/test/sequential/00-validate-tests.bats index 8c86249..c0d8d9a 100755 --- a/test/sequential/00-validate-tests.bats +++ b/test/sequential/00-validate-tests.bats @@ -191,7 +191,7 @@ teardown_file() { done } -@test "only make-results tests call make results" { +@test "only make-test tests call make results" { # make results overwrites expected output files, so it should only be used # in tests specifically designed to test that functionality. local test_root="$BATS_TEST_DIRNAME/.." @@ -205,8 +205,8 @@ teardown_file() { fi assert_success # Any status other than 0 or 1 is a real grep error - # Filter out the tests that are allowed to call make results - run grep -v -e 'make-results\.bats$' -e 'test-verify-results\.bats$' <<< "$output" + # Filter out the test that is allowed to call make results + run grep -v -e 'make-test\.bats$' <<< "$output" # grep returns 1 when all lines are filtered out -- that's the success case if [ "$status" -eq 1 ]; then @@ -217,7 +217,7 @@ teardown_file() { # If we get here, there are violations (grep returned 0, meaning matches remain) echo "FAIL: The following tests call 'make results' but should not:" >&2 echo "$output" >&2 - echo "Only make-results.bats and test-verify-results.bats may call 'make results'" >&2 + echo "Only make-test.bats may call 'make results'" >&2 return 1 } diff --git a/test/standard/make-results.bats b/test/standard/make-results.bats deleted file mode 100755 index 41f1e01..0000000 --- a/test/standard/make-results.bats +++ /dev/null @@ -1,99 +0,0 @@ -#!/usr/bin/env bats - -# Test: make results functionality -# -# Tests that make results correctly updates expected output files: -# - Modifies expected output to create a mismatch -# - Verifies make test detects the mismatch -# - Runs make results to update expected output -# - Verifies make test now passes -# -# Not tested here: -# - make results when already up-to-date (idempotent case). Safe to re-run, -# but the extra make test invocation adds non-trivial time for little value. - -load ../lib/helpers - -setup_file() { - setup_topdir - - # Independent test - gets its own isolated environment with foundation TEST_REPO - load_test_env "make-results" - ensure_foundation "$TEST_DIR" - - # Every test in this file requires PostgreSQL. Skip expensive setup if unavailable. - if ! check_postgres_available; then - return 0 - fi - - cd "$TEST_REPO" - - # State modification: Ensure expected output exists. - # The template should already have it, but guard against it being missing or empty. - if [ ! -f "test/expected/pgxntool-test.out" ] || [ ! -s "test/expected/pgxntool-test.out" ]; then - make results - fi - - # State modification: Ensure expected output is committed to git. - # Later tests create a mismatch and check git status to verify it, - # which only works if the baseline is committed. - local status_output - status_output=$(git status --porcelain test/expected/pgxntool-test.out) - if [ -n "$status_output" ]; then - git add test/expected/pgxntool-test.out - git commit -m "Add baseline expected output" - fi -} - -setup() { - skip_if_no_postgres - load_test_env "make-results" - cd "$TEST_REPO" -} - -@test "can modify expected output to create mismatch" { - # Add a blank line to create a difference - echo >> test/expected/pgxntool-test.out - - # Verify file was modified (should show as modified since it's committed) - run git status --porcelain test/expected/pgxntool-test.out - [ -n "$output" ] - echo "$output" | grep -qE "^.M" -} - -@test "make test shows diff with modified expected output" { - # Run make test (should show diffs due to mismatch) - # Note: make test doesn't exit non-zero due to .IGNORE: installcheck - run make test - - # Check that diff output was produced (either in output or test/output directory exists) - # test/output is created when tests fail - [ -d "test/output" ] || echo "$output" | grep -q "diff" -} - -@test "make results is blocked by verify-results when tests fail" { - # The previous test ran make test which failed (mismatch), creating regression.diffs. - # With the fixed ordering (results: test verify-results), make results re-runs - # make test first (still fails → regression.diffs), then verify-results blocks. - # This test verifies that verify-results correctly intercepts failing results. - run make results - assert_failure - assert_contains "$output" "Cannot run 'make results'" -} - -@test "make results updates expected output" { - # Run make results with verify-results disabled to actually fix the mismatch. - # regression.diffs still exists from the previous test's make test run. - # Disabling verify-results lets make results run make test and copy the fresh - # results to expected/, fixing the mismatch. - run make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results - assert_success -} - -@test "make test succeeds after make results" { - # Now make test should pass - run make test - assert_success -} - -# vi: expandtab sw=2 ts=2 diff --git a/test/standard/make-test.bats b/test/standard/make-test.bats index be161a6..f45106a 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -1,22 +1,53 @@ #!/usr/bin/env bats -# Test: make test framework +# Test: make test / make results / verify-results # -# Tests that the test framework works correctly: -# - Creates test/output directory when needed -# - Uses test/output for expected outputs -# - Doesn't recreate output when directories removed +# These three targets are one tightly-coupled pipeline -- `make results` and +# `make verify-results` only exist to safely update the expected output that +# `make test` checks against -- so they share a single file and a single +# foundation-copied environment rather than three. Covers: +# - `make test` succeeds against the template's known-good state +# - EXTRA_CLEAN targets the real $(TESTOUT)/results/ directory (issue #7) +# - unique per-directory database naming (REGRESS_DBNAME) +# - installcheck always runs after install, even when pulled in indirectly +# (issue #79) +# - check-stale-expected catches orphaned test/expected/*.out files (issue #14) +# - `make test` exits non-zero on a real regression.diffs mismatch (issue #49) +# - verify-results blocks `make results` when tests are failing, detects +# pgtap failures, and can be disabled load ../lib/helpers setup_file() { - # Set TOPDIR setup_topdir - # Independent test - gets its own isolated environment with foundation TEST_REPO load_test_env "make-test" ensure_foundation "$TEST_DIR" + + # The verify-results/results tests below need a committed baseline expected + # output file so they can create a detectable mismatch against it. Skip + # this if PostgreSQL is unavailable -- `make results` needs a live server, + # and those tests skip themselves via skip_if_no_postgres anyway. + if check_postgres_available; then + cd "$TEST_REPO" + + # State modification: Ensure expected output exists. + # The template should already have it, but guard against it being missing or empty. + if [ ! -f "test/expected/pgxntool-test.out" ] || [ ! -s "test/expected/pgxntool-test.out" ]; then + make results + fi + + # State modification: Ensure expected output is committed to git. + # The verify-results/results tests below create a mismatch and check git + # status to verify it, which only works if the baseline is committed. + local status_output + status_output=$(git status --porcelain test/expected/pgxntool-test.out) + if [ -n "$status_output" ]; then + git add test/expected/pgxntool-test.out + git commit -m "Add baseline expected output" + fi + fi } setup() { @@ -24,54 +55,11 @@ setup() { cd "$TEST_REPO" } -@test "test/output directory does not exist initially" { - # Skip if already exists from previous run - if [ -d "test/output" ]; then - skip "test/output already exists" - fi - - assert_dir_not_exists "test/output" -} - -@test "make test creates test/output directory" { - # Skip if already exists - if [ -d "test/output" ]; then - skip "test/output already exists" - fi - - # pg_regress does NOT create input/ or output/ directories - they are optional - # INPUT directories. We need to create it ourselves for this test. - mkdir -p test/output - - # Verify directory was created - assert_dir_exists "test/output" -} - -@test "test/output is a directory" { - assert_dir_exists "test/output" -} - @test "make test succeeds" { run make test assert_success } -@test "can remove test directories" { - # Remove input and output - rm -rf test/input test/output - - assert_dir_not_exists "test/output" -} - -@test "make test doesn't recreate output when directories removed" { - # After removing directories, output should not be recreated - # We only care that the directory doesn't get recreated, not that tests pass - run make test - - # test/output should NOT exist (correct behavior) - assert_dir_not_exists "test/output" -} - @test "repository is still functional" { # Basic sanity check assert_file_exists "Makefile" @@ -117,10 +105,19 @@ setup() { @test "unique db name: create test SQL file and expected output" { skip_if_no_postgres - # Create a simple SQL test that queries the current database name - # Use \t and \a so output is just the bare value (no headers/formatting) + # Create a simple SQL test that queries the current database name. + # \set ECHO none is required: pg_regress always runs test files through + # `psql -a` (echo mode -- see psql_start_test() in pg_regress_main.c), so + # without it every input line, including \a/\t themselves, gets echoed + # verbatim into the output before it runs. This is the same convention + # template/test/sql/base.sql already documents and relies on -- hand-writing + # just the bare value here (without \set ECHO none) previously produced an + # expected file that could never match pg_regress's real output; that + # mismatch went unnoticed only because of issue #49 (make test always + # exiting 0 regardless of regression.diffs). mkdir -p test/sql cat > test/sql/dbname.sql <<'EOF' +\set ECHO none \a \t SELECT current_database(); @@ -132,9 +129,10 @@ EOF expected_dbname=$(make print-REGRESS_DBNAME 2>&1 | sed -n 's/.*set to "\(.*\)"/\1/p') [ -n "$expected_dbname" ] || error "Could not extract REGRESS_DBNAME from make" - # Manually create the expected output file + # Expected output is just the echoed `\set ECHO none` line itself (read + # while ECHO was still "all") followed by the query's own bare result. mkdir -p test/expected - printf '%s\n' "$expected_dbname" > test/expected/dbname.out + printf '\\set ECHO none\n%s\n' "$expected_dbname" > test/expected/dbname.out } @test "unique db name: make test passes" { @@ -144,6 +142,60 @@ EOF assert_success } +# Test: installcheck must run after install, even when pulled in indirectly +# (issue #79) +# +# `.IGNORE: installcheck` plus `installcheck: $(TEST_RESULT_FILES) ...` gives +# installcheck its own prerequisite chain, separate from `install`. `test`'s +# TEST_DEPS lists `install installcheck` (and, when enabled, +# check-stale-expected, which itself depends on installcheck -- see issue +# #14 below) as independent, unordered prerequisites of `test` -- Make does +# not guarantee unrelated same-target prerequisites build left-to-right. +# Nothing stopped installcheck's own chain (pulled in either directly or via +# check-stale-expected's dependency on it) from running before `install` +# ever did. On a genuinely uninstalled tree, pg_regress then runs against a +# database missing the extension, and every SQL test fails with "schema ... +# does not exist". base.mk now has an explicit `installcheck: install` edge +# (mirroring test-build's own `test-build: install` edge) so install always +# runs first, regardless of what pulls installcheck in. + +@test "installcheck's parsed prerequisite list includes install (issue #79, structural proof)" { + # make -p dumps Make's internal parsed-rule database, independent of + # runtime scheduling order. This proves the `installcheck: install` edge + # genuinely exists in base.mk -- unlike a real `make test` run, which could + # theoretically pass even on a broken base.mk purely by luck of Make's + # scheduling order for unrelated same-target prerequisites (see the + # behavioral test below). + run make -p -n installcheck + assert_success + + local prereq_line + prereq_line=$(echo "$output" | awk '/^installcheck:/{print; exit}') + [ -n "$prereq_line" ] || error "installcheck rule not found in 'make -p' database dump" + + echo "$prereq_line" | grep -qw install || \ + error "installcheck's parsed prerequisite list does not include 'install': $prereq_line" +} + +@test "make test succeeds from a genuinely uninstalled state (issue #79)" { + skip_if_no_postgres + + # The `make -p` test above is the primary proof for this issue (the + # dependency edge genuinely exists in the parsed makefile). This test is a + # complementary real-world sanity check of the whole pipeline: on a + # genuinely uninstalled tree, does pg_regress actually find the extension + # already installed by the time it runs? `make uninstall` forces that + # precondition regardless of what any earlier test in this file already + # installed on the shared PostgreSQL instance -- the original bug was + # historically masked in exactly that way. + run make uninstall + assert_success + + run make test + assert_success + assert_not_contains "$output" "does not exist" +} + # Test: check-stale-expected (issue #14) # # `make test` never caught a stale test/expected/*.out left behind after a @@ -284,4 +336,161 @@ EOF assert_success } +# ============================================================================ +# verify-results / make results (issues #14, #49) +# ============================================================================ +# +# `results` depends on `verify-results` (when enabled), which depends on +# $(TEST_DEPS) -- testdeps/install/installcheck/etc -- directly, NOT on the +# `test` target itself (see base.mk's comment above `verify-results: +# $(TEST_DEPS)`). So `make results` reruns the suite by pulling in those +# prerequisites fresh, rather than by invoking `test`. This matters because +# `test`'s own recipe now exits 1 as soon as it sees regression.diffs (issue +# #49's fix, tested below) -- if `results`/`verify-results` depended on +# `test` instead, that exit would abort the chain before verify-results ever +# got to inspect the diff and report it. + +@test "verify-results succeeds with clean template state" { + skip_if_no_postgres + + run make verify-results + assert_success +} + +@test "verify-results pulls in installcheck via TEST_DEPS, not by depending on test" { + # Confirm the dependency is wired: a dry-run of verify-results must + # include the installcheck recipe pulled in via $(TEST_DEPS) (see the + # section header above for why it's $(TEST_DEPS) and not `test` itself). + run make -n verify-results 2>&1 + assert_success + assert_contains "$output" "installcheck" +} + +@test "can modify expected output to create mismatch" { + skip_if_no_postgres + + # Add a blank line to create a difference. This mismatch stays in place + # through the rest of this section -- "make results updates expected + # output" below is what finally fixes it. + echo >> test/expected/pgxntool-test.out + + # Verify file was modified (should show as modified since it's committed) + run git status --porcelain test/expected/pgxntool-test.out + [ -n "$output" ] + echo "$output" | grep -qE "^.M" +} + +@test "make test shows diff with modified expected output, and exits non-zero (issue #49)" { + skip_if_no_postgres + + run make test + # Ensure make exited non-zero + assert_failure + + # Confirms the pre-existing cat behavior wasn't lost while adding the exit. + echo "$output" | grep -q "diff" +} + +@test "make results is blocked by verify-results when tests fail" { + skip_if_no_postgres + + # The previous test's mismatch is still in place, so verify-results's + # rerun of $(TEST_DEPS) fails again and blocks make results. + run make results + assert_failure + assert_contains "$output" "Cannot run 'make results'" +} + +@test "verify-results blocks when invoked directly, not just via results" { + skip_if_no_postgres + + # Same mismatch as above, but this test invokes `make verify-results` + # directly instead of going through `results` -- a distinct entry point + # worth covering on its own, since `results` only reaches verify-results + # as a prerequisite. + run make verify-results + assert_failure + assert_contains "$output" "Tests are failing" + assert_contains "$output" "Cannot run 'make results'" +} + +@test "verify-results can be disabled" { + # With verify-results disabled, results depends only on $(TEST_DEPS), so + # its dry-run never mentions the verify-results block message. We check + # for the actual block message rather than the bare string + # "verify-results", since that could spuriously match make's own + # "Entering directory" banners depending on the environment's path. + run make -n results PGXNTOOL_ENABLE_VERIFY_RESULTS=no 2>&1 + assert_success + assert_not_contains "$output" "Cannot run 'make results'" +} + +@test "make results updates expected output" { + skip_if_no_postgres + + # Run make results with verify-results disabled to actually fix the mismatch. + # Disabling verify-results lets make results run the suite and copy the + # fresh results to expected/, fixing the mismatch. + run make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results + assert_success +} + +@test "make test succeeds after make results" { + skip_if_no_postgres + + # Now make test should pass + run make test + assert_success +} + +@test "verify-results detects pgtap failures in result files" { + skip_if_no_postgres + + mkdir -p test/results + cat > test/results/pgtap_fail.out <<'EOF' +1..2 +ok 1 - passing test +not ok 2 - failing test +EOF + + run make verify-results + assert_failure + assert_contains "$output" "pgtap failure detected" + + rm -f test/results/pgtap_fail.out +} + +@test "verify-results ignores pgtap TODO failures" { + skip_if_no_postgres + + mkdir -p test/results + cat > test/results/pgtap_todo.out <<'EOF' +1..1 +not ok 1 - known issue # TODO fix later +EOF + + run make verify-results + assert_success + + rm -f test/results/pgtap_todo.out +} + +@test "verify-results detects pgtap plan mismatch" { + skip_if_no_postgres + + mkdir -p test/results + cat > test/results/pgtap_plan.out <<'EOF' +1..3 +ok 1 - test one +ok 2 - test two +# Looks like you planned 3 tests but ran 2 +EOF + + run make verify-results + assert_failure + assert_contains "$output" "pgtap plan mismatch" + + rm -f test/results/pgtap_plan.out +} + # vi: expandtab sw=2 ts=2 diff --git a/test/standard/test-verify-results.bats b/test/standard/test-verify-results.bats deleted file mode 100644 index 3279e28..0000000 --- a/test/standard/test-verify-results.bats +++ /dev/null @@ -1,118 +0,0 @@ -#!/usr/bin/env bats - -# Test: verify-results feature -# -# Validates that verify-results correctly blocks make results when tests are -# failing, detects pgtap failures, and can be disabled. -# -# NOTE: pgxntool makes verify-results depend on test (verify-results: test), so -# `make verify-results` re-runs the suite and therefore requires PostgreSQL. The -# pgtap-detection cases below plant files in test/results/ (which survive the -# fresh test run) rather than a regression.diffs (which the fresh run overwrites). - -load ../lib/helpers - -setup_file() { - setup_topdir - - load_test_env "verify-results" - ensure_foundation "$TEST_DIR" -} - -setup() { - load_test_env "verify-results" - cd "$TEST_REPO" -} - -@test "verify-results succeeds with clean template state" { - run make verify-results - assert_success -} - -@test "verify-results depends on test execution" { - # pgxntool makes verify-results depend on test (verify-results: test) so that - # `make results` re-runs the tests and checks the FRESH regression.diffs, even - # under make -j. Confirm the dependency is wired: a dry-run of verify-results - # must include the installcheck recipe pulled in via the test target. - run make -n verify-results 2>&1 - assert_success - assert_contains "$output" "installcheck" -} - -@test "verify-results blocks when a test actually fails" { - # verify-results depends on test, so `make verify-results` re-runs the suite. - # A planted regression.diffs would just be overwritten by that fresh run, so - # force a REAL failure by corrupting an expected file, then confirm - # verify-results refuses to let make results proceed. - echo >> test/expected/base.out - - run make verify-results - assert_failure - assert_contains "$output" "Tests are failing" - assert_contains "$output" "Cannot run 'make results'" - - # Restore clean state for the tests that follow. - run git checkout -- test/expected/base.out - assert_success - rm -f test/regression.diffs -} - -@test "verify-results can be disabled" { - # With verify-results disabled, results depends only on test, so its dry-run - # never mentions the verify-results block message. - run make -n results PGXNTOOL_ENABLE_VERIFY_RESULTS=no 2>&1 - assert_success - # Check that verify-results recipe commands are not in the dry-run output. - # Note: we cannot check for the string "verify-results" directly because the - # test environment directory path contains "verify-results" and appears in - # make's "Entering directory" messages. - assert_not_contains "$output" "Cannot run 'make results'" - - rm -f test/regression.diffs -} - -@test "verify-results detects pgtap failures in result files" { - mkdir -p test/results - cat > test/results/pgtap_fail.out <<'EOF' -1..2 -ok 1 - passing test -not ok 2 - failing test -EOF - - run make verify-results - assert_failure - assert_contains "$output" "pgtap failure detected" - - rm -f test/results/pgtap_fail.out -} - -@test "verify-results ignores pgtap TODO failures" { - mkdir -p test/results - cat > test/results/pgtap_todo.out <<'EOF' -1..1 -not ok 1 - known issue # TODO fix later -EOF - - run make verify-results - assert_success - - rm -f test/results/pgtap_todo.out -} - -@test "verify-results detects pgtap plan mismatch" { - mkdir -p test/results - cat > test/results/pgtap_plan.out <<'EOF' -1..3 -ok 1 - test one -ok 2 - test two -# Looks like you planned 3 tests but ran 2 -EOF - - run make verify-results - assert_failure - assert_contains "$output" "pgtap plan mismatch" - - rm -f test/results/pgtap_plan.out -} - -# vi: expandtab sw=2 ts=2 From 82c0c7c5a7c003400dd60127f1cf84e129090ed5 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 31 Jul 2026 16:25:39 -0500 Subject: [PATCH 2/2] make-test.bats: remove #79/#49/#35 fix-dependent content The reorg in 40c7918 accidentally pulled in test content that only makes sense once pgxntool PR #83 (fix-79-49) lands, defeating the purpose of keeping this reorg mergeable against current pgxntool master on its own: - Delete the two tests asserting the `installcheck: install` edge (issue #79), which doesn't exist in base.mk yet. - Revert "make test shows diff..." to not assert a non-zero exit (issue #49's fix, unmerged); current master's `test` recipe still never exits non-zero on a regression.diffs mismatch. - Revert the verify-results/results section's comments and one test name back to describing current master's real wiring (`verify-results: test`), not the `verify-results: $(TEST_DEPS)` wiring #83 would introduce. Verified against a fresh clone of real, unmodified Postgres-Extensions/pgxntool master: full suite green except one unrelated, pre-existing failure in test/sequential/04-pgtle.bats caused by this branch's base being behind pgxntool-test's own upstream master (which already carries bea9b18, a fix for pgxntool's unrelated PGTLE_VERSION->PGXNTOOL_PGTLE_VERSION rename in #85). --- test/standard/make-test.bats | 100 +++++++---------------------------- 1 file changed, 19 insertions(+), 81 deletions(-) diff --git a/test/standard/make-test.bats b/test/standard/make-test.bats index f45106a..135986f 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -9,10 +9,7 @@ # - `make test` succeeds against the template's known-good state # - EXTRA_CLEAN targets the real $(TESTOUT)/results/ directory (issue #7) # - unique per-directory database naming (REGRESS_DBNAME) -# - installcheck always runs after install, even when pulled in indirectly -# (issue #79) # - check-stale-expected catches orphaned test/expected/*.out files (issue #14) -# - `make test` exits non-zero on a real regression.diffs mismatch (issue #49) # - verify-results blocks `make results` when tests are failing, detects # pgtap failures, and can be disabled @@ -113,8 +110,8 @@ setup() { # template/test/sql/base.sql already documents and relies on -- hand-writing # just the bare value here (without \set ECHO none) previously produced an # expected file that could never match pg_regress's real output; that - # mismatch went unnoticed only because of issue #49 (make test always - # exiting 0 regardless of regression.diffs). + # mismatch went unnoticed because make test's own diff output was never + # inspected here. mkdir -p test/sql cat > test/sql/dbname.sql <<'EOF' \set ECHO none @@ -142,60 +139,6 @@ EOF assert_success } -# Test: installcheck must run after install, even when pulled in indirectly -# (issue #79) -# -# `.IGNORE: installcheck` plus `installcheck: $(TEST_RESULT_FILES) ...` gives -# installcheck its own prerequisite chain, separate from `install`. `test`'s -# TEST_DEPS lists `install installcheck` (and, when enabled, -# check-stale-expected, which itself depends on installcheck -- see issue -# #14 below) as independent, unordered prerequisites of `test` -- Make does -# not guarantee unrelated same-target prerequisites build left-to-right. -# Nothing stopped installcheck's own chain (pulled in either directly or via -# check-stale-expected's dependency on it) from running before `install` -# ever did. On a genuinely uninstalled tree, pg_regress then runs against a -# database missing the extension, and every SQL test fails with "schema ... -# does not exist". base.mk now has an explicit `installcheck: install` edge -# (mirroring test-build's own `test-build: install` edge) so install always -# runs first, regardless of what pulls installcheck in. - -@test "installcheck's parsed prerequisite list includes install (issue #79, structural proof)" { - # make -p dumps Make's internal parsed-rule database, independent of - # runtime scheduling order. This proves the `installcheck: install` edge - # genuinely exists in base.mk -- unlike a real `make test` run, which could - # theoretically pass even on a broken base.mk purely by luck of Make's - # scheduling order for unrelated same-target prerequisites (see the - # behavioral test below). - run make -p -n installcheck - assert_success - - local prereq_line - prereq_line=$(echo "$output" | awk '/^installcheck:/{print; exit}') - [ -n "$prereq_line" ] || error "installcheck rule not found in 'make -p' database dump" - - echo "$prereq_line" | grep -qw install || \ - error "installcheck's parsed prerequisite list does not include 'install': $prereq_line" -} - -@test "make test succeeds from a genuinely uninstalled state (issue #79)" { - skip_if_no_postgres - - # The `make -p` test above is the primary proof for this issue (the - # dependency edge genuinely exists in the parsed makefile). This test is a - # complementary real-world sanity check of the whole pipeline: on a - # genuinely uninstalled tree, does pg_regress actually find the extension - # already installed by the time it runs? `make uninstall` forces that - # precondition regardless of what any earlier test in this file already - # installed on the shared PostgreSQL instance -- the original bug was - # historically masked in exactly that way. - run make uninstall - assert_success - - run make test - assert_success - assert_not_contains "$output" "does not exist" -} - # Test: check-stale-expected (issue #14) # # `make test` never caught a stale test/expected/*.out left behind after a @@ -337,18 +280,12 @@ EOF } # ============================================================================ -# verify-results / make results (issues #14, #49) +# verify-results / make results (issue #14) # ============================================================================ # -# `results` depends on `verify-results` (when enabled), which depends on -# $(TEST_DEPS) -- testdeps/install/installcheck/etc -- directly, NOT on the -# `test` target itself (see base.mk's comment above `verify-results: -# $(TEST_DEPS)`). So `make results` reruns the suite by pulling in those -# prerequisites fresh, rather than by invoking `test`. This matters because -# `test`'s own recipe now exits 1 as soon as it sees regression.diffs (issue -# #49's fix, tested below) -- if `results`/`verify-results` depended on -# `test` instead, that exit would abort the chain before verify-results ever -# got to inspect the diff and report it. +# pgxntool makes verify-results depend on test (verify-results: test), so +# `make results` re-runs the tests and checks the FRESH regression.diffs, +# even under make -j. @test "verify-results succeeds with clean template state" { skip_if_no_postgres @@ -357,10 +294,12 @@ EOF assert_success } -@test "verify-results pulls in installcheck via TEST_DEPS, not by depending on test" { - # Confirm the dependency is wired: a dry-run of verify-results must - # include the installcheck recipe pulled in via $(TEST_DEPS) (see the - # section header above for why it's $(TEST_DEPS) and not `test` itself). +@test "verify-results depends on test execution" { + # pgxntool makes verify-results depend on test (verify-results: test) so + # that `make results` re-runs the tests and checks the FRESH + # regression.diffs, even under make -j. Confirm the dependency is wired: a + # dry-run of verify-results must include the installcheck recipe pulled in + # via the test target. run make -n verify-results 2>&1 assert_success assert_contains "$output" "installcheck" @@ -380,22 +319,21 @@ EOF echo "$output" | grep -qE "^.M" } -@test "make test shows diff with modified expected output, and exits non-zero (issue #49)" { +@test "make test shows diff with modified expected output" { skip_if_no_postgres + # Run make test (should show diffs due to mismatch). + # Note: make test doesn't exit non-zero due to .IGNORE: installcheck. run make test - # Ensure make exited non-zero - assert_failure - # Confirms the pre-existing cat behavior wasn't lost while adding the exit. echo "$output" | grep -q "diff" } @test "make results is blocked by verify-results when tests fail" { skip_if_no_postgres - # The previous test's mismatch is still in place, so verify-results's - # rerun of $(TEST_DEPS) fails again and blocks make results. + # The previous test's mismatch is still in place. verify-results depends + # on test, so its rerun of the suite fails again and blocks make results. run make results assert_failure assert_contains "$output" "Cannot run 'make results'" @@ -415,8 +353,8 @@ EOF } @test "verify-results can be disabled" { - # With verify-results disabled, results depends only on $(TEST_DEPS), so - # its dry-run never mentions the verify-results block message. We check + # With verify-results disabled, results depends only on test, so its + # dry-run never mentions the verify-results block message. We check # for the actual block message rather than the bare string # "verify-results", since that could spuriously match make's own # "Entering directory" banners depending on the environment's path.