From 50d01876e7f1d39aaa1d78c28c49b3e1871abaab Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 14 May 2026 15:45:45 -0500 Subject: [PATCH 1/4] Add tests for pgxntool commit 20c2552 (fix results ordering, whitespace, overrides) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests/updates for pgxntool commit 20c2552: - Fix `results:` ordering: `verify-results` now runs after `test` (not before) - Fix `parse_control_file` trailing whitespace stripping (multiple spaces/tabs) - Add `override` to `PGXNTOOL_ENABLE_*` so command-line values are normalized - `make-results.bats`: new test verifying `verify-results` blocks `make results` when tests are failing; updated comments - `test-verify-results.bats`: remove `make results` assertion (now requires PostgreSQL since `make results` runs `make test` first); add clarifying comments - New `04-pgtle.bats` test: `pgtle.sh` handles multiple spaces and tab before trailing comment in control files - `template-multi-extension/ext_beta.control`: use multiple spaces before trailing comment to exercise whitespace trimming - `multi-extension.bats`: update comments for multiple-spaces testing - `test.md` agent: fix debug level description (BATS 1–5 scale vs `lib.sh` 10–50) - `gather-repo-info.sh`: add sanity check for invalid debug levels Co-Authored-By: Claude --- .claude/agents/test.md | 4 +- .../skills/commit/scripts/gather-repo-info.sh | 37 +++++++++++++++++++ template-multi-extension/ext_beta.control | 2 +- test/sequential/04-pgtle.bats | 27 ++++++++++++++ test/standard/make-results.bats | 20 +++++++--- test/standard/multi-extension.bats | 12 +++--- test/standard/test-verify-results.bats | 11 +++--- 7 files changed, 93 insertions(+), 20 deletions(-) diff --git a/.claude/agents/test.md b/.claude/agents/test.md index db0a630..9c549e6 100644 --- a/.claude/agents/test.md +++ b/.claude/agents/test.md @@ -232,7 +232,7 @@ DEBUG=2 test/bats/bin/bats tests/01-meta.bats # Debug output test/bats/bin/bats --verbose tests/01-meta.bats # BATS verbose mode ``` -**Debug levels**: 10 (critical), 20 (significant), 30 (general), 40 (verbose), 50+ (maximum) +**Debug levels (BATS test infrastructure, `helpers.bash`)**: 1 (critical/pollution detection), 2 (test flow/major ops), 3 (state detail), 5 (maximum). Note: this 1–5 scale is separate from the pgxntool lib.sh `debug` function which uses multiples of 10 (30=general, 40=verbose, 50=maximum). The two systems are independent. --- @@ -247,7 +247,7 @@ Tests set these automatically (from `tests/helpers.bash`): - `PGXNBRANCH` - Branch to use (defaults to `master`) - `TEST_TEMPLATE` - Template directory (defaults to `${TOPDIR}/template`) - `PG_LOCATION` - PostgreSQL installation path -- `DEBUG` - Debug level (0-5) +- `DEBUG` - Debug level for BATS test helpers (0–5 scale) --- diff --git a/.claude/skills/commit/scripts/gather-repo-info.sh b/.claude/skills/commit/scripts/gather-repo-info.sh index 4147c30..251ad42 100755 --- a/.claude/skills/commit/scripts/gather-repo-info.sh +++ b/.claude/skills/commit/scripts/gather-repo-info.sh @@ -57,6 +57,37 @@ gather_info() { ) } +# check_debug_levels: scan pgxntool shell scripts for invalid debug levels. +# Valid levels are single-digit (0-9) or multiples of 10 (10, 20, 30, ...). +# Two-digit levels that are NOT multiples of 10 (e.g., 11, 22, 25) are flagged. +check_debug_levels() { + local pgxntool_path="$1" + local found_invalid=0 + + while IFS= read -r -d '' sh_file; do + while IFS= read -r match; do + # Extract the level number from "debug NN" + local level + level=$(echo "$match" | grep -oE 'debug [0-9]+' | grep -oE '[0-9]+$') + # Skip single-digit levels (always valid) + [ ${#level} -le 1 ] && continue + # Flag multi-digit levels that are not multiples of 10 + if [ $((level % 10)) -ne 0 ]; then + if [ "$found_invalid" -eq 0 ]; then + echo "### SANITY: Invalid debug levels detected" + found_invalid=1 + fi + echo " $sh_file: debug $level" + fi + done < <(grep -nE '\bdebug [0-9]+\b' "$sh_file" 2>/dev/null || true) + done < <(find "$pgxntool_path" -maxdepth 1 -name "*.sh" -print0 2>/dev/null) + + if [ "$found_invalid" -eq 0 ]; then + echo "### SANITY: debug levels OK (all single-digit or multiples of 10)" + fi + echo +} + if [ $# -eq 0 ]; then echo "Usage: gather-repo-info.sh [ ...]" >&2 exit 1 @@ -73,3 +104,9 @@ for repo in "$@"; do echo "---" echo done + +# Run debug level check for pgxntool (first arg is expected to be pgxntool path) +if [ $# -ge 1 ] && [ -f "$1/lib.sh" ]; then + echo "## Sanity Checks" + check_debug_levels "$1" +fi diff --git a/template-multi-extension/ext_beta.control b/template-multi-extension/ext_beta.control index 8797783..9a92de0 100644 --- a/template-multi-extension/ext_beta.control +++ b/template-multi-extension/ext_beta.control @@ -1,3 +1,3 @@ comment = 'Beta extension for multi-extension testing' -default_version = '2.5.0' # DO NOT REMOVE: trailing comment exercises parse_control_file comment handling (issue #25) +default_version = '2.5.0' # DO NOT REMOVE: multiple spaces before comment exercises parse_control_file whitespace trimming (issues #25, #26) requires = 'plpgsql' diff --git a/test/sequential/04-pgtle.bats b/test/sequential/04-pgtle.bats index af55920..2287524 100644 --- a/test/sequential/04-pgtle.bats +++ b/test/sequential/04-pgtle.bats @@ -228,6 +228,33 @@ teardown_file() { rm -f empty.control } +@test "pgtle: parse_control_file handles whitespace before trailing comment" { + # Multiple spaces and a tab before a comment would leave stray whitespace after + # comment removal if only a single space was stripped (issue #26 follow-up to #25). + # Verify parse_control_file correctly strips all trailing whitespace so pgtle.sh + # finds the versioned SQL file and succeeds. + + # Multiple spaces before comment + printf "default_version = '7.7.7' # multiple spaces\n" > wspace_test.control + echo "SELECT 1;" > sql/wspace_test--7.7.7.sql + + run "$TEST_REPO/pgxntool/pgtle.sh" --extension wspace_test --pgtle-version 1.5.0+ + assert_success + assert_file_exists "pg_tle/1.5.0+/wspace_test.sql" + + # Tab before comment + printf "default_version = '6.6.6'\t# tab before comment\n" > tab_test.control + echo "SELECT 1;" > sql/tab_test--6.6.6.sql + + run "$TEST_REPO/pgxntool/pgtle.sh" --extension tab_test --pgtle-version 1.5.0+ + assert_success + assert_file_exists "pg_tle/1.5.0+/tab_test.sql" + + # Cleanup + rm -f wspace_test.control sql/wspace_test--7.7.7.sql pg_tle/1.5.0+/wspace_test.sql + rm -f tab_test.control sql/tab_test--6.6.6.sql pg_tle/1.5.0+/tab_test.sql +} + @test "pgtle: warning on module_pathname in control" { # Create a C extension control file echo "comment = 'C extension'" > cext.control diff --git a/test/standard/make-results.bats b/test/standard/make-results.bats index 874cd12..41f1e01 100755 --- a/test/standard/make-results.bats +++ b/test/standard/make-results.bats @@ -11,8 +11,6 @@ # 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. -# - verify-results behavior (blocking make results when tests fail) is tested -# separately in test-verify-results.bats. load ../lib/helpers @@ -73,11 +71,21 @@ setup() { [ -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 to fix the expected output. - # Must disable verify-results: the previous test created a mismatch, which caused - # make test to generate regression.diffs. verify-results blocks make results when - # regression.diffs exists, so we bypass it here to test the fix-mismatch workflow. + # 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 } diff --git a/test/standard/multi-extension.bats b/test/standard/multi-extension.bats index e0fcf51..656a717 100644 --- a/test/standard/multi-extension.bats +++ b/test/standard/multi-extension.bats @@ -16,9 +16,10 @@ # - sql/ext_beta.sql # - META.in.json (with provides for both extensions) # -# Note: ext_beta.control intentionally has a trailing comment on default_version. -# This exercises pgtle.sh's parse_control_file to catch the quote-before-comment bug -# (issue #25). Do not remove the comment from ext_beta.control. +# Note: ext_beta.control intentionally has MULTIPLE spaces before the trailing comment +# on default_version. This exercises parse_control_file's whitespace trimming to catch +# both the single-space-only bug (issue #25) and the multiple-spaces bug (issue #26). +# Do not reduce the spaces or remove the comment from ext_beta.control. load ../lib/helpers @@ -112,8 +113,9 @@ setup() { run make all assert_success - # Also verify pgtle generation works — ext_beta.control has a trailing comment on - # default_version to exercise parse_control_file's comment handling (issue #25). + # Also verify pgtle generation works — ext_beta.control has multiple spaces before + # a trailing comment on default_version to exercise parse_control_file whitespace + # trimming (issues #25, #26). run make pgtle assert_success } diff --git a/test/standard/test-verify-results.bats b/test/standard/test-verify-results.bats index 7a110e0..c9217be 100644 --- a/test/standard/test-verify-results.bats +++ b/test/standard/test-verify-results.bats @@ -35,7 +35,11 @@ setup() { assert_not_contains "$output" "installcheck" } -@test "regression.diffs blocks both verify-results and make results" { +@test "regression.diffs blocks verify-results" { + # This test only verifies the verify-results target directly. The broader behavior + # of make results being blocked after a failing test run is tested in + # make-results.bats (which requires PostgreSQL) because the fix for the ordering + # bug means make results now runs make test first, requiring a live database. cat > test/regression.diffs <<'EOF' *** test/expected/test.out --- test/results/test.out @@ -51,11 +55,6 @@ EOF assert_contains "$output" "Tests are failing" assert_contains "$output" "Cannot run 'make results'" - # make results itself should also be blocked - run make results - assert_failure - assert_contains "$output" "Cannot run 'make results'" - # State left for next test (regression.diffs still present) } From e6953037283d8482a78a799fe19bdd44bf898f3f Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 15 May 2026 14:45:57 -0500 Subject: [PATCH 2/4] Separate test debug from script debug; add commit-time level check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates for pgxntool commit 6e707bb (clarify debug() level scheme): - `lib.sh` comment updated to range-based scheme (1-9, 10-19, 20-29...) `test/lib/helpers.bash`: `debug()` now checks `$TESTDEBUG` instead of `$DEBUG`, so test infrastructure verbosity and pgxntool script verbosity are independently controlled. Use `TESTDEBUG=N` for test runs, `DEBUG=N` for pgxntool scripts. `.claude/skills/commit/SKILL.md`: Added Step 2b — advisory scan of debug level distribution in changed pgxntool `.sh` files at commit time. Flags scripts with loops but only single-digit levels, or 5+ calls all at the same level. `.claude/agents/test.md`, `.claude/settings.json`: Updated all `DEBUG=` references to `TESTDEBUG=`; corrected stale debug level descriptions to match helpers.bash. `.claude/skills/test/scripts/run-tests.sh`: Derive `PGPORT` from `PGCLUSTER` at startup so pg_regress and raw PostgreSQL binaries use the correct cluster port. Co-Authored-By: Claude --- .claude/agents/test.md | 13 +++++++------ .claude/settings.json | 20 ++++++++++---------- .claude/skills/commit/SKILL.md | 12 ++++++++++++ .claude/skills/test/scripts/run-tests.sh | 10 ++++++++++ test/lib/helpers.bash | 6 ++++-- 5 files changed, 43 insertions(+), 18 deletions(-) diff --git a/.claude/agents/test.md b/.claude/agents/test.md index 9c549e6..89fb46b 100644 --- a/.claude/agents/test.md +++ b/.claude/agents/test.md @@ -35,7 +35,7 @@ make clean-envs && test/bats/bin/bats tests/04-pgtle.bats ✅ **DO THIS**: ```bash test/bats/bin/bats tests/04-pgtle.bats # Auto-rebuilds if needed -DEBUG=5 test/bats/bin/bats tests/04-pgtle.bats # For investigation +TESTDEBUG=5 test/bats/bin/bats tests/04-pgtle.bats # For investigation ``` **ONLY clean when debugging the cleanup mechanism itself** and you MUST document what cleanup failure you're investigating. @@ -228,11 +228,11 @@ test/bats/bin/bats tests/test-pgtle-install.bats ### Debugging ```bash -DEBUG=2 test/bats/bin/bats tests/01-meta.bats # Debug output +TESTDEBUG=2 test/bats/bin/bats tests/01-meta.bats # Debug output test/bats/bin/bats --verbose tests/01-meta.bats # BATS verbose mode ``` -**Debug levels (BATS test infrastructure, `helpers.bash`)**: 1 (critical/pollution detection), 2 (test flow/major ops), 3 (state detail), 5 (maximum). Note: this 1–5 scale is separate from the pgxntool lib.sh `debug` function which uses multiples of 10 (30=general, 40=verbose, 50=maximum). The two systems are independent. +**Debug levels** (set via `TESTDEBUG`): 1 (pollution/critical), 2 (major workflow), 3 (detailed state), 5 (verbose internals) --- @@ -247,7 +247,8 @@ Tests set these automatically (from `tests/helpers.bash`): - `PGXNBRANCH` - Branch to use (defaults to `master`) - `TEST_TEMPLATE` - Template directory (defaults to `${TOPDIR}/template`) - `PG_LOCATION` - PostgreSQL installation path -- `DEBUG` - Debug level for BATS test helpers (0–5 scale) +- `TESTDEBUG` - Test infrastructure debug level (0-5); controls helpers.bash debug() output +- `DEBUG` - pgxntool script debug level; controls debug() in pgxntool/lib.sh (independent of TESTDEBUG) --- @@ -292,7 +293,7 @@ test/bats/bin/bats tests/04-pgtle.bats # Auto-rebuilds foundation via ensure_fo ### Debugging Test Failures 1. Read test output (which assertion failed?) -2. Use DEBUG mode: `DEBUG=5 test/bats/bin/bats tests/test-name.bats` +2. Use TESTDEBUG mode: `TESTDEBUG=5 test/bats/bin/bats tests/test-name.bats` 3. Inspect environment: `cd .envs/sequential/repo && ls -la` 4. Check state markers: `ls .envs/sequential/.bats-state/` 5. **Work top-down**: Fix earliest failure first (downstream failures often cascade) @@ -361,7 +362,7 @@ make test test/bats/bin/bats tests/04-pgtle.bats # Debug -DEBUG=5 test/bats/bin/bats tests/04-pgtle.bats +TESTDEBUG=5 test/bats/bin/bats tests/04-pgtle.bats # Test infrastructure make test-recursion diff --git a/.claude/settings.json b/.claude/settings.json index e89011d..0c153c8 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -2,17 +2,17 @@ "permissions": { "allow": [ "Bash(make:*)", - "Bash(DEBUG=1 make:*)", - "Bash(DEBUG=2 make:*)", - "Bash(DEBUG=3 make:*)", - "Bash(DEBUG=4 make:*)", - "Bash(DEBUG=5 make:*)", + "Bash(TESTDEBUG=1 make:*)", + "Bash(TESTDEBUG=2 make:*)", + "Bash(TESTDEBUG=3 make:*)", + "Bash(TESTDEBUG=4 make:*)", + "Bash(TESTDEBUG=5 make:*)", "Bash(test/bats/bin/bats:*)", - "Bash(DEBUG=1 test/bats/bin/bats:*)", - "Bash(DEBUG=2 test/bats/bin/bats:*)", - "Bash(DEBUG=3 test/bats/bin/bats:*)", - "Bash(DEBUG=4 test/bats/bin/bats:*)", - "Bash(DEBUG=5 test/bats/bin/bats:*)", + "Bash(TESTDEBUG=1 test/bats/bin/bats:*)", + "Bash(TESTDEBUG=2 test/bats/bin/bats:*)", + "Bash(TESTDEBUG=3 test/bats/bin/bats:*)", + "Bash(TESTDEBUG=4 test/bats/bin/bats:*)", + "Bash(TESTDEBUG=5 test/bats/bin/bats:*)", "Edit", "WebFetch", diff --git a/.claude/skills/commit/SKILL.md b/.claude/skills/commit/SKILL.md index 43a2eed..ea560f4 100644 --- a/.claude/skills/commit/SKILL.md +++ b/.claude/skills/commit/SKILL.md @@ -41,6 +41,18 @@ bash .claude/skills/commit/scripts/gather-repo-info.sh ../pgxntool . Review output. Identify which repos have changes. +### 2b. Check Debug Level Distribution (pgxntool .sh changes only) + +If `../pgxntool/*.sh` files are in the diff, scan each changed file: + +1. Collect all `debug N` call levels +2. Note whether the file contains loops (`for`, `while`) +3. Flag if: file has loops AND every debug call uses a single-digit level (1-9) +4. Flag if: file has 5+ debug calls AND they all use the exact same level + +This is **advisory only** — present any findings in Step 5 and let the user decide. +If nothing looks off, say nothing (don't add noise to the common case). + ### 3. Check HISTORY.asc (pgxntool changes only) Read `../pgxntool/HISTORY.asc`. Determine if changes are significant and user-visible: diff --git a/.claude/skills/test/scripts/run-tests.sh b/.claude/skills/test/scripts/run-tests.sh index 9805e47..13dd90d 100755 --- a/.claude/skills/test/scripts/run-tests.sh +++ b/.claude/skills/test/scripts/run-tests.sh @@ -8,6 +8,16 @@ set -euo pipefail +# Set PGPORT from PGCLUSTER if not already set, so pg_regress and other raw +# PostgreSQL binaries use the correct cluster (Debian pg_wrapper handles +# PGCLUSTER but raw binaries only respect PGPORT). +if [ -z "${PGPORT:-}" ] && [ -n "${PGCLUSTER:-}" ]; then + _port=$(pg_lsclusters -h 2>/dev/null | awk -v c="$PGCLUSTER" \ + 'BEGIN{split(c,a,"/"); v=a[1]; n=a[2]} $1==v && $2==n {print $3; exit}') + [ -n "$_port" ] && export PGPORT=$_port + unset _port +fi + # Determine log directory (per-project, based on working directory) LOG_DIR="/tmp/pgxntool-test-logs$(pwd | sed 's/\//_/g')" mkdir -p "$LOG_DIR" diff --git a/test/lib/helpers.bash b/test/lib/helpers.bash index b797bc1..7983fc3 100644 --- a/test/lib/helpers.bash +++ b/test/lib/helpers.bash @@ -70,13 +70,15 @@ error() { # Debug output function # Usage: debug LEVEL "message" -# Outputs message if DEBUG >= LEVEL +# Outputs message if TESTDEBUG >= LEVEL +# Uses TESTDEBUG (not DEBUG) to avoid conflating test infrastructure verbosity +# with pgxntool script verbosity — set them independently as needed. debug() { local level=$1 shift local message="$*" - if [ "${DEBUG:-0}" -ge "$level" ]; then + if [ "${TESTDEBUG:-0}" -ge "$level" ]; then out -f "DEBUG[$level]: $message" fi } From 847290e721ea839faf42c2e26cef2a22ee0fd30f Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 14 Jul 2026 16:00:32 -0500 Subject: [PATCH 3/4] Drop check_debug_levels: non-multiples-of-10 are valid The commit-time check flagged any two-digit level not divisible by 10, contradicting the debug() scheme (levels are anchors, not strict multiples -- 11/15/25 are fine). Overkill; removed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../skills/commit/scripts/gather-repo-info.sh | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/.claude/skills/commit/scripts/gather-repo-info.sh b/.claude/skills/commit/scripts/gather-repo-info.sh index 251ad42..4147c30 100755 --- a/.claude/skills/commit/scripts/gather-repo-info.sh +++ b/.claude/skills/commit/scripts/gather-repo-info.sh @@ -57,37 +57,6 @@ gather_info() { ) } -# check_debug_levels: scan pgxntool shell scripts for invalid debug levels. -# Valid levels are single-digit (0-9) or multiples of 10 (10, 20, 30, ...). -# Two-digit levels that are NOT multiples of 10 (e.g., 11, 22, 25) are flagged. -check_debug_levels() { - local pgxntool_path="$1" - local found_invalid=0 - - while IFS= read -r -d '' sh_file; do - while IFS= read -r match; do - # Extract the level number from "debug NN" - local level - level=$(echo "$match" | grep -oE 'debug [0-9]+' | grep -oE '[0-9]+$') - # Skip single-digit levels (always valid) - [ ${#level} -le 1 ] && continue - # Flag multi-digit levels that are not multiples of 10 - if [ $((level % 10)) -ne 0 ]; then - if [ "$found_invalid" -eq 0 ]; then - echo "### SANITY: Invalid debug levels detected" - found_invalid=1 - fi - echo " $sh_file: debug $level" - fi - done < <(grep -nE '\bdebug [0-9]+\b' "$sh_file" 2>/dev/null || true) - done < <(find "$pgxntool_path" -maxdepth 1 -name "*.sh" -print0 2>/dev/null) - - if [ "$found_invalid" -eq 0 ]; then - echo "### SANITY: debug levels OK (all single-digit or multiples of 10)" - fi - echo -} - if [ $# -eq 0 ]; then echo "Usage: gather-repo-info.sh [ ...]" >&2 exit 1 @@ -104,9 +73,3 @@ for repo in "$@"; do echo "---" echo done - -# Run debug level check for pgxntool (first arg is expected to be pgxntool path) -if [ $# -ge 1 ] && [ -f "$1/lib.sh" ]; then - echo "## Sanity Checks" - check_debug_levels "$1" -fi From 86b3a8794137201c9868cd270ef81752d03d56cd Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 14 Jul 2026 17:22:37 -0500 Subject: [PATCH 4/4] test: fix template expected output and align verify-results tests The template's base.sql/pgxntool-test.sql lacked \set ECHO none, so pg_regress echoed SQL input into results and the expected files didn't match. Dormant until pgxntool's verify-results began re-running the suite (the ordering fix in the paired pgxntool PR), which surfaced it. Add \set ECHO none and regenerate expected output. Also update test-verify-results.bats to assert the new "verify-results depends on test" behavior instead of the old ordering. Co-Authored-By: Claude Opus 4.8 (1M context) --- template/test/expected/base.out | 1 + template/test/expected/pgxntool-test.out | 2 ++ template/test/sql/base.sql | 3 ++ template/test/sql/pgxntool-test.sql | 5 +++ test/standard/test-verify-results.bats | 44 ++++++++++++------------ 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/template/test/expected/base.out b/template/test/expected/base.out index ff9c724..45ea463 100644 --- a/template/test/expected/base.out +++ b/template/test/expected/base.out @@ -1,2 +1,3 @@ +\set ECHO none single_ext_ok diff --git a/template/test/expected/pgxntool-test.out b/template/test/expected/pgxntool-test.out index 7ecdd9a..03532f1 100644 --- a/template/test/expected/pgxntool-test.out +++ b/template/test/expected/pgxntool-test.out @@ -1,2 +1,4 @@ +\set ECHO none 1..1 ok 1 +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/template/test/sql/base.sql b/template/test/sql/base.sql index 202a341..e8157a7 100644 --- a/template/test/sql/base.sql +++ b/template/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/template/test/sql/pgxntool-test.sql b/template/test/sql/pgxntool-test.sql index 891080c..259cd7b 100644 --- a/template/test/sql/pgxntool-test.sql +++ b/template/test/sql/pgxntool-test.sql @@ -1,3 +1,8 @@ +\set ECHO none +-- DO NOT REMOVE: pg_regress runs psql with echo-all, so without this every +-- input line (including all of setup.sql) is echoed into the output and never +-- matches expected/pgxntool-test.out. This mirrors the standard pgxntool pgtap +-- convention (every test file starts with \set ECHO none). \i test/pgxntool/setup.sql SELECT plan(1); diff --git a/test/standard/test-verify-results.bats b/test/standard/test-verify-results.bats index c9217be..3279e28 100644 --- a/test/standard/test-verify-results.bats +++ b/test/standard/test-verify-results.bats @@ -5,10 +5,10 @@ # Validates that verify-results correctly blocks make results when tests are # failing, detects pgtap failures, and can be disabled. # -# Separate from make-results.bats because verify-results is pure file-checking -# logic that doesn't require PostgreSQL. Keeping it in its own suite means these -# tests always run, even when PostgreSQL is unavailable (make-results.bats skips -# entirely in that case). +# 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 @@ -29,37 +29,37 @@ setup() { assert_success } -@test "verify-results has no dependencies on test execution" { +@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_not_contains "$output" "installcheck" + assert_contains "$output" "installcheck" } -@test "regression.diffs blocks verify-results" { - # This test only verifies the verify-results target directly. The broader behavior - # of make results being blocked after a failing test run is tested in - # make-results.bats (which requires PostgreSQL) because the fix for the ordering - # bug means make results now runs make test first, requiring a live database. - cat > test/regression.diffs <<'EOF' -*** test/expected/test.out ---- test/results/test.out -*************** -*** 1 **** -! expected ---- 1 ---- -! actual -EOF +@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'" - # State left for next test (regression.diffs still present) + # 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" { - # regression.diffs still present from previous test + # 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.