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..135986f 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -1,22 +1,50 @@ #!/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) +# - check-stale-expected catches orphaned test/expected/*.out files (issue #14) +# - 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 +52,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 +102,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 because make test's own diff output was never + # inspected here. mkdir -p test/sql cat > test/sql/dbname.sql <<'EOF' +\set ECHO none \a \t SELECT current_database(); @@ -132,9 +126,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" { @@ -284,4 +279,156 @@ EOF assert_success } +# ============================================================================ +# verify-results / make results (issue #14) +# ============================================================================ +# +# 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 + + 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 "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" { + 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 + + 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. 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'" +} + +@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, 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