From d52ce57fba347beecb6ab33b93b64679d14f0a13 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 12 Oct 2026 18:54:50 -0500 Subject: [PATCH 01/10] Test that make pgtle fails when a non-last extension fails Add a multi-extension test that makes pgtle.sh fail for the first extension only (a directory sits where its output file goes) and asserts that `make pgtle` itself exits non-zero. With the old `;`-joined recipe only the last extension's status counted, so this failure was masked. Covers Postgres-Extensions/pgxntool#140; needs the recipe fix in Postgres-Extensions/pgxntool#142. Co-Authored-By: Claude Opus 5.5 --- test/standard/multi-extension.bats | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/standard/multi-extension.bats b/test/standard/multi-extension.bats index 656a717..01bcbb1 100644 --- a/test/standard/multi-extension.bats +++ b/test/standard/multi-extension.bats @@ -164,4 +164,25 @@ setup() { assert_success } +@test "make pgtle fails when pgtle.sh fails for a non-last extension" { + run make print-_PGXNTOOL_EXTENSIONS + assert_success + local list=${output#*set to \"} exts + read -ra exts <<< "${list%\"}" + local first_ext=${exts[0]} + [ ${#exts[@]} -ge 2 ] || error "need at least 2 extensions, got: $output" + debug 2 "first extension: $first_ext" + + local ranges=(pg_tle/*/) + [ -d "${ranges[0]}" ] || error "no pg_tle// directory from the earlier make pgtle" + local range=${ranges[0]} + + # A directory where pgtle.sh writes its output file makes it fail for the + # first extension only; the make call's own exit status is what's asserted. + rm -rf pg_tle + mkdir -p "${range}${first_ext}.sql" + run make pgtle + assert_failure +} + # vi: expandtab sw=2 ts=2 From f464c813e23172671b3f7d3ba9dfe44e6c408a99 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 14:11:47 -0500 Subject: [PATCH 02/10] Test that make results and results-build fail when a copy fails Add two tests to test/standard/make-test.bats that make a `cp` into the expected-output directory fail by putting a directory where cp writes, which fails even as root. The `make results` test makes a copy other than the last one fail, and runs `make -o verify-results results` so only the copy loop runs. The `results-build` test runs the real target. Without the fix, both targets exit 0 after the failed copy. Covers Postgres-Extensions/pgxntool#140; needs the recipe fix in https://github.com/Postgres-Extensions/pgxntool/pull/142. --- test/standard/make-test.bats | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/standard/make-test.bats b/test/standard/make-test.bats index 9ab4169..33cc706 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -21,6 +21,8 @@ # 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) +# - `make results` and `results-build` fail when any copy into expected/ +# fails (pgxntool#140) load ../lib/helpers @@ -753,6 +755,22 @@ EOF rm -f test/results/pgtap_fail.out } +@test "make results fails when a copy other than the last one fails (pgxntool#140)" { + # A directory at cp's destination makes cp fail even as root; the zz_ file + # guarantees the failing copy isn't the last one. + mkdir -p test/results test/expected/0_cp_fail.out/0_cp_fail.out + echo "copy fails" > test/results/0_cp_fail.out + echo "copy succeeds" > test/results/zz_cp_after.out + + # -o verify-results skips the test run, leaving only the copy loop. + run make -o verify-results results + rm -rf test/expected/0_cp_fail.out + rm -f test/expected/zz_cp_after.out test/results/0_cp_fail.out test/results/zz_cp_after.out + + assert_failure + assert_contains "$output" "cannot overwrite directory" +} + # ============================================================================ # results-build (issue #108) # ============================================================================ @@ -810,4 +828,20 @@ EOF git checkout -- test/build/simple_build_test.sql } +@test "results-build fails when a copy fails (pgxntool#140)" { + skip_if_no_postgres + + # A directory at cp's destination makes cp fail even as root. + mkdir -p test/build/results test/build/expected/0_cp_fail.out/0_cp_fail.out + echo "copy fails" > test/build/results/0_cp_fail.out + + run make results-build + rm -rf test/build/expected/0_cp_fail.out + rm -f test/build/results/0_cp_fail.out + + assert_failure + assert_contains "$output" "cannot overwrite directory" + assert_not_contains "$output" "not blessing stale output" +} + # vi: expandtab sw=2 ts=2 From abb5e87e643c81d2cdeebf2c0bbb393bac175728 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 14:13:45 -0500 Subject: [PATCH 03/10] Drop a results-build test assertion that matched make's command echo make echoes results-build's unprefixed `$(MAKE) -C . test-build || ...` recipe line, and that text contains the message the assertion checked was absent. The `cp` error assertion already proves the copy loop ran. --- test/standard/make-test.bats | 1 - 1 file changed, 1 deletion(-) diff --git a/test/standard/make-test.bats b/test/standard/make-test.bats index 33cc706..892f401 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -841,7 +841,6 @@ EOF assert_failure assert_contains "$output" "cannot overwrite directory" - assert_not_contains "$output" "not blessing stale output" } # vi: expandtab sw=2 ts=2 From 703c87e691c6bdacaaaf90b6fc069897a745caae Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 15:38:53 -0500 Subject: [PATCH 04/10] Add an inventory test for the loops in pgxntool's *.mk files make-loops.bats fails whenever pgxntool's *.mk files have a shell loop or $(foreach) that test/lib/pgxntool-loops.txt doesn't list, or stop having one it lists. A recipe loop hides the failure of every iteration but the last unless each iteration ends in `|| exit 1`, so the failure message says to first write a test that makes a non-last iteration fail, and only then add the entry naming that test. The test also checks that every named test exists. Loops are keyed by file, enclosing context (rule target, variable, define body, or top level) and kind, never by line number or nearby text, so unrelated edits next to a loop don't trip it. test/lib/scan-make-loops.awk finds them by reading the source rather than `make -p`, which omits recipes in inactive conditional branches. A second test runs the scanner on a fixture holding the loop shapes it must find and the comment and prose shapes it must not count. The install-schedule and print-pgtle loops have no failure test yet and are listed as untested. Related to Postgres-Extensions/pgxntool#140. --- test/lib/pgxntool-loops.txt | 38 +++++++ test/lib/scan-make-loops.awk | 190 ++++++++++++++++++++++++++++++++++ test/standard/make-loops.bats | 165 +++++++++++++++++++++++++++++ 3 files changed, 393 insertions(+) create mode 100644 test/lib/pgxntool-loops.txt create mode 100644 test/lib/scan-make-loops.awk create mode 100755 test/standard/make-loops.bats diff --git a/test/lib/pgxntool-loops.txt b/test/lib/pgxntool-loops.txt new file mode 100644 index 0000000..826e342 --- /dev/null +++ b/test/lib/pgxntool-loops.txt @@ -0,0 +1,38 @@ +# Every loop in pgxntool's *.mk files, with the test proving that a failing +# iteration fails make. A new loop needs that test BEFORE it gets an entry +# here: write a test that makes a non-last iteration fail and asserts make +# exits non-zero, confirm it fails with the loop's `|| exit 1` removed, then +# add the entry naming it. +# +# test/standard/make-loops.bats fails on any loop missing from this list, any +# entry with no matching loop, and any coverage test that doesn't exist. +# +# Format: four tab-separated fields per line. Lines starting with # and blank +# lines are ignored; order doesn't matter. +# path under pgxntool/ +# where the loop is, never a line number: +# rule: a recipe or rule line; is the +# unexpanded text before the rule's ':' +# var: a variable assignment +# define: a define body +# toplevel any other line, e.g. $(foreach ...$(eval ...)) +# for, while or until (shell), or foreach (make) +# test:: +# make-time: only for a $(foreach) that runs inside make +# and emits no shell commands +# untested: grandfathered entries only, never a new loop +# A context with two loops of the same kind gets two lines. + +base.mk rule:$(_PGXNTOOL_INSTALL_SCHEDULE) for untested:grandfathered; an iteration can only fail appending to the schedule file that the `>` just before the loop created, so there is no practical way to fail a non-last one +base.mk rule:check-duplicate-docs foreach make-time:expands to $(warning) calls; the recipe itself runs only `:` +base.mk rule:pgtle for test:standard/multi-extension.bats:make pgtle fails when pgtle.sh fails for a non-last extension +base.mk rule:print-pgtle foreach untested:grandfathered; no test makes a non-last extension's iteration fail (each iteration exits 1 on a missing file or a failed cat) +base.mk rule:results for test:standard/make-test.bats:make results fails when a copy other than the last one fails (pgxntool#140) +base.mk rule:results-build for test:standard/make-test.bats:results-build fails when a copy fails (pgxntool#140) +base.mk toplevel foreach make-time:$(eval)s an ASCIIDOC_template rule for each of ASCIIDOC_EXTS +base.mk var:ASCIIDOC_FILES foreach make-time:builds a file list (outer $(foreach)) +base.mk var:ASCIIDOC_FILES foreach make-time:builds a file list (inner $(foreach)) +base.mk var:ASCIIDOC_HTML foreach make-time:builds a file list +base.mk var:DOCS foreach make-time:builds a file list +base.mk var:_PGXNTOOL_DUPLICATE_DOCS foreach make-time:builds a name list +base.mk var:_pgxntool_docs_named foreach make-time:builds a file list diff --git a/test/lib/scan-make-loops.awk b/test/lib/scan-make-loops.awk new file mode 100644 index 0000000..af42e0f --- /dev/null +++ b/test/lib/scan-make-loops.awk @@ -0,0 +1,190 @@ +# Lists the loops in one makefile, one "\t\t" line each. +# test/standard/make-loops.bats says what counts as a loop and why; the header +# of test/lib/pgxntool-loops.txt describes the context keys. +# +# Usage: awk -v file= -f scan-make-loops.awk +# +# Every logical line is searched for both kinds of loop; classifying the line +# (recipe, define body, assignment, rule header, directive, other) only picks +# its context key and which text is searched: +# - recipe and define body lines: all of it, minus shell comment lines +# - assignments: the value, whose start counts as command position +# - rule headers: the prerequisites, plus any `; recipe` (taken from the raw +# line, since make doesn't strip comments from it) +# - everything else: the line minus its make comment + +BEGIN { + # Shell command position: start of the text, after a separator or opening + # character (quotes included, since `sh -c "..."` bodies are real shell), + # after a keyword that starts a command list, or at the start of $(shell). + CMDPOS = "(^|[;&|(){!`\"']|(^|[^A-Za-z0-9_])(do|then|else|elif)[ \t]|\\$+[({]shell[ \t])[ \t]*" + # `for NAME` must be followed by in/;/do, so prose ("for a reason") and + # awk's `for(i in A)` don't match. + LOOP = "(for[ \t]+[A-Za-z_][A-Za-z0-9_]*([ \t]+in([ \t;]|$)|[ \t]*;|[ \t]+do([ \t;]|$))|for[ \t]*\\(\\(|while([ \t({!]|$)|until([ \t({!]|$))" + SHELL_LOOP = CMDPOS LOOP +} + +# Physical lines are buffered until one doesn't end in an odd number of +# backslashes. phys[] keeps them apart because a shell comment ends at its own +# physical line even when that line is continued. +{ + if (match($0, /\\+$/) && RLENGTH % 2 == 1) { + phys[++np] = substr($0, 1, length($0) - 1) + next + } + phys[++np] = $0 + logical() + np = 0 +} +END { if (np) logical() } + +function logical( i, raw, r, t, p, s, name, key) { + raw = phys[1] + for (i = 2; i <= np; i++) raw = raw " " phys[i] + + # Like make, only lines that don't start with a tab can nest or end a define. + if (defdepth) { + if (raw !~ /^\t/) { + if (np == 1 && raw ~ /^[ \t]*endef([ \t#]|$)/) { defdepth--; return } + if (raw ~ /^[ \t]*((override|export|private)[ \t]+)*define([ \t]|$)/) defdepth++ + } + recipe("define:" defname) + return + } + + # A tab-led line is a recipe line only while a rule's recipe can follow; + # elsewhere make parses it as an ordinary line (e.g. an indented assignment). + if (inrule && raw ~ /^\t/) { recipe(ctx); return } + + r = raw + sub(/^[ \t]+/, "", r) + p = find0(r, "#") + t = p ? substr(r, 1, p - 1) : r + if (t ~ /^[ \t]*$/) return + + if (t ~ /^((override|export|private)[ \t]+)*define([ \t]|$)/) { + name = t + sub(/^((override|export|private)[ \t]+)*define[ \t]*/, "", name) + sub(/[ \t]*(:::=|::=|:=|[+?!]?=)[ \t]*$/, "", name) + defname = norm(name) + defdepth = 1 + inrule = 0 + return + } + + # Conditionals don't end a rule's recipe; the other directives do. + if (t ~ /^(ifeq|ifneq|ifdef|ifndef|else|endif)([ \t(]|$)/) { + search(t, "toplevel") + return + } + if (t ~ /^(-?include|sinclude|-?load|vpath|unexport|undefine)([ \t]|$)/) { + search(t, "toplevel") + inrule = 0 + return + } + + p = find0(t, ":=") + if (p && (substr(t, p, 1) == "=" || substr(t, p) ~ /^(:=|::=|:::=)/)) { + name = substr(t, 1, p - 1) + if (substr(t, p, 1) == "=") sub(/[+?!]$/, "", name) + sub(/^((override|export|private)[ \t]+)+/, "", name) + key = "var:" norm(name) + search_foreach(t, key) + match(substr(t, p), /^:*=/) + search_shell(substr(t, p + RLENGTH), key) + inrule = 0 + return + } + + if (p) { + ctx = "rule:" norm(substr(t, 1, p - 1)) + inrule = 1 + s = find0(substr(r, p + 1), ";#") + if (s && substr(r, p + s, 1) == ";") { + search(substr(r, 1, p + s - 1), ctx) + search(substr(r, p + s + 1), ctx) + } else { + search(t, ctx) + } + return + } + + search(t, "toplevel") + inrule = 0 +} + +# A recipe line, or a define body line (which make may later run as one). +# A physical line whose first non-blank character (after the @+- prefixes, on +# a recipe's first line) is # is a shell comment: skipped, but its newline +# still ends the command before it, hence the ";". +function recipe(key, i, p, sh) { + sh = "" + for (i = 1; i <= np; i++) { + p = phys[i] + if (i == 1) sub(/^[ \t@+-]*/, "", p) + else sub(/^[ \t]+/, "", p) + sh = sh (p ~ /^#/ ? " ;" : " " p) + } + search(sh, key) +} + +function search(s, key) { + search_foreach(s, key) + search_shell(s, key) +} + +function search_foreach(s, key) { + while (match(s, /\$+[({]foreach[ \t]/)) { + emit(key, "foreach") + s = substr(s, RSTART + RLENGTH) + } +} + +function search_shell(s, key, at, m) { + while (match(s, SHELL_LOOP)) { + at = RSTART + m = substr(s, RSTART, RLENGTH) + match(m, /for|while|until/) + m = substr(m, RSTART) + emit(key, m ~ /^for/ ? "for" : m ~ /^while/ ? "while" : "until") + # Resume just past the keyword's first character; the "_" keeps the + # remainder's start from counting as command position. + s = "_" substr(s, at + RSTART) + } +} + +function emit(key, kind) { + print file "\t" key "\t" kind +} + +# Position of the first character of s that is in set and outside any +# $(...)/${...} reference, or 0. $$ and backslash-escaped characters never +# match. +function find0(s, set, i, n, c, d) { + n = length(s) + d = 0 + for (i = 1; i <= n; i++) { + c = substr(s, i, 1) + if (c == "\\") { i++; continue } + if (d) { + if (c == "(" || c == "{") d++ + else if (c == ")" || c == "}") d-- + continue + } + if (c == "$") { + c = substr(s, i + 1, 1) + if (c == "(" || c == "{") d++ + i++ + continue + } + if (index(set, c)) return i + } + return 0 +} + +function norm(s) { + gsub(/[ \t]+/, " ", s) + sub(/^ /, "", s) + sub(/ $/, "", s) + return s +} diff --git a/test/standard/make-loops.bats b/test/standard/make-loops.bats new file mode 100755 index 0000000..34a7ada --- /dev/null +++ b/test/standard/make-loops.bats @@ -0,0 +1,165 @@ +#!/usr/bin/env bats + +# Test: inventory of the loops in pgxntool's *.mk files +# +# Fails whenever pgxntool's *.mk files have a loop that +# test/lib/pgxntool-loops.txt doesn't list, or stop having one it lists, so a +# new loop can't land without a test that makes one of its non-last iterations +# fail: a recipe loop hides the failure of every iteration but the last unless +# each one ends in `|| exit 1` or the iterations are chained with &&. Needs no +# pg_config, foundation env, or Postgres. +# +# test/lib/scan-make-loops.awk reads the source text rather than make's +# database (`make -p`, which make-variables.bats uses), because make -p shows +# only recipes in active conditional branches: with default settings the +# diffs-mode verify-results recipe is absent. A missed loop is the one failure +# this test can't afford. +# +# Loops are keyed by file, enclosing context and kind, never by line number or +# neighbouring text, so edits near a loop don't trip the test. +# +# Shell keywords count only at command position, but quoted strings aren't +# stripped (`sh -c "..."` bodies are real shell), so text that opens a quoted +# string or an embedded awk program with "while " or "for x in" is counted +# too. That overmatch is stable, so it's accepted. +# +# Not scanned: recipe text that the *.mk.sh generators echo into meta.mk and +# control.mk. They emit no loops today. + +load ../lib/helpers + +setup_file() { + setup_topdir + load_test_env "make-loops" +} + +setup() { + load_test_env "make-loops" + SCANNER="$TOPDIR/test/lib/scan-make-loops.awk" +} + +# Print "\t\t" for each loop in pgxntool's *.mk files, +# being the path under pgxntool/. +pgxntool_loops() { + local files f + files=$(cd "$PGXNREPO" && find . -name .git -prune -o -name '*.mk' -print | LC_ALL=C sort) || return + [ -n "$files" ] || error "no *.mk files found under $PGXNREPO" || return + for f in $files; do + awk -v file="${f#./}" -f "$SCANNER" "$PGXNREPO/$f" || return + done +} + +@test "every loop in pgxntool's *.mk files is listed in test/lib/pgxntool-loops.txt" { + local rel=test/lib/pgxntool-loops.txt + local actual entries listed new stale problems='' + actual=$(pgxntool_loops) + actual=$(LC_ALL=C sort <<< "$actual") + entries=$(awk '!/^#/ && !/^[[:space:]]*$/' "$TOPDIR/$rel") + listed=$(cut -f1-3 <<< "$entries" | LC_ALL=C sort) + + # comm on sorted input compares as multisets: two loops of one kind in one + # context need two entries. + new=$(LC_ALL=C comm -23 <(echo "$actual") <(echo "$listed")) + stale=$(LC_ALL=C comm -13 <(echo "$actual") <(echo "$listed")) + + [ -z "$new" ] || problems+=" +New loop(s) in pgxntool's *.mk files not listed in $rel: +$(sed 's/^/ /' <<< "$new") +A shell loop (or \$(foreach) that generates commands) in a recipe hides the failure of every iteration but the last unless each iteration ends in \`|| exit 1\` or the iterations are chained with &&. +Do these IN ORDER: + 1. Write a test that makes a NON-LAST iteration of this loop fail and asserts make exits non-zero. Confirm it catches the bug: temporarily remove the loop's \`|| exit 1\`, and the test must fail. + 2. Only then add the entry to $rel, naming that test in the coverage column (test::<@test name>). +Do NOT add the entry just to make this test pass; this list exists to force step 1. +Use make-time: only for a \$(foreach) that runs inside make and emits no shell commands. Never use untested: for a new loop. +" + [ -z "$stale" ] || problems+=" +Listed in $rel but no longer in pgxntool's *.mk files -- remove: +$(sed 's/^/ /' <<< "$stale") +If a loop moved to another context (e.g. a renamed target), fix its entry instead, and check that its coverage test still exercises the loop where it is now. +" + + local file context kind coverage extra path name bad='' + while IFS=$'\t' read -r file context kind coverage extra; do + [ -z "$extra" ] || { bad+=" more than four tab-separated fields: $file $context $kind"$'\n'; continue; } + case "$kind" in + for|while|until|foreach) ;; + *) bad+=" unknown kind '$kind': $file $context"$'\n' ;; + esac + case "$coverage" in + test:?*:?*) + path=${coverage#test:} + name=${path#*:} + path=${path%%:*} + grep -Fq -- "@test \"$name\"" "$TOPDIR/test/$path" 2>/dev/null || + bad+=" test/$path has no @test \"$name\" (covering $file $context $kind) -- if it was renamed, update the entry; if it was removed, the loop needs a new failure test"$'\n' + ;; + make-time:?*) + [ "$kind" = foreach ] || + bad+=" make-time: is only for \$(foreach), not a shell $kind loop: $file $context"$'\n' + ;; + untested:?*) ;; + *) bad+=" coverage must be test::<@test name>, make-time: or untested:: $file $context $kind"$'\n' ;; + esac + done <<< "$entries" + [ -z "$bad" ] || problems+=" +Problems with entries in $rel: +$bad" + + [ -z "$problems" ] || error "${problems#$'\n'}" +} + +@test "scan-make-loops.awk finds each loop shape and skips comments and prose" { + # Lines starting with ">" start with a tab once written out. + local fixture="$BATS_TEST_TMPDIR/fixture.mk" + awk '{ sub(/^>/, "\t") } 1' > "$fixture" <<'EOF' +# make comment: for f in a b; do $(foreach x,y,z); done +RUN_ALL = $(foreach e,a b,./run.sh $(e);) +LOOP_CMD = for f in a b; do echo $$f; done +HASH := $(subst x,#,y) $(foreach z,a,b) +copy: +>@for f in a b; do cp $$f out/ || exit 1; done +>@ls | while read f; do echo $$f; done +>@if true; then for f in a b; do echo $$f; done; fi +>@# disabled: true; for f in a b; do echo $$f; done +>@awk '{for(i in A) print A[i]}' /dev/null +>@echo "for a reason" >&2 +gen: +>@$(foreach e,a b,./run.sh $(e);) +use-helper: +>@$(RUN_ALL) +inline: ; for f in a b; do echo $$f; done +$(SRCS:.c=.o): +>@echo hi \ +># comment line inside a continued command \ +>for f in a b; do echo $$f; done +INDENTED = 1 +>INDENTED_LOOP := $(foreach x,a b,$(x)) +define TEMPLATE +$(1)-copy: +>for f in $(1); do cp $$$$f out/; done +endef +ifeq (never,taken) +untaken: +>for f in a b; do echo $$f; done +endif +EOF + + local want got + want='var:RUN_ALL foreach +var:LOOP_CMD for +var:HASH foreach +rule:copy for +rule:copy while +rule:copy for +rule:gen foreach +rule:inline for +rule:$(SRCS:.c=.o) for +var:INDENTED_LOOP foreach +define:TEMPLATE for +rule:untaken for' + got=$(awk -v file=fixture.mk -f "$SCANNER" "$fixture" | cut -f2- | tr '\t' ' ') + [ "$got" = "$want" ] || error "scanner output differs from expected: +$(diff -u <(echo "$want") <(echo "$got"))" +} + +# vi: expandtab sw=2 ts=2 From 837da7aa1c9354f551bd1db7c5cc5027471c2451 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 15:59:30 -0500 Subject: [PATCH 05/10] Test that print-pgtle fails when cat fails for a non-last extension A PATH-shadowed cat fails only on the first extension's pg_tle//.sql and passes every other call to the real cat, since the pgtle prerequisite runs pgtle.sh, which also uses cat. Without the `|| exit 1` after print-pgtle's per-extension cat, the loop goes on to the next extension and make exits 0. The print-pgtle entry in test/lib/pgxntool-loops.txt now names this test instead of being untested. Related to Postgres-Extensions/pgxntool#140. --- test/lib/pgxntool-loops.txt | 2 +- test/standard/multi-extension.bats | 38 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/test/lib/pgxntool-loops.txt b/test/lib/pgxntool-loops.txt index 826e342..4b1057a 100644 --- a/test/lib/pgxntool-loops.txt +++ b/test/lib/pgxntool-loops.txt @@ -26,7 +26,7 @@ base.mk rule:$(_PGXNTOOL_INSTALL_SCHEDULE) for untested:grandfathered; an iteration can only fail appending to the schedule file that the `>` just before the loop created, so there is no practical way to fail a non-last one base.mk rule:check-duplicate-docs foreach make-time:expands to $(warning) calls; the recipe itself runs only `:` base.mk rule:pgtle for test:standard/multi-extension.bats:make pgtle fails when pgtle.sh fails for a non-last extension -base.mk rule:print-pgtle foreach untested:grandfathered; no test makes a non-last extension's iteration fail (each iteration exits 1 on a missing file or a failed cat) +base.mk rule:print-pgtle foreach test:standard/multi-extension.bats:make print-pgtle fails when cat fails for a non-last extension base.mk rule:results for test:standard/make-test.bats:make results fails when a copy other than the last one fails (pgxntool#140) base.mk rule:results-build for test:standard/make-test.bats:results-build fails when a copy fails (pgxntool#140) base.mk toplevel foreach make-time:$(eval)s an ASCIIDOC_template rule for each of ASCIIDOC_EXTS diff --git a/test/standard/multi-extension.bats b/test/standard/multi-extension.bats index 01bcbb1..9fce089 100644 --- a/test/standard/multi-extension.bats +++ b/test/standard/multi-extension.bats @@ -164,6 +164,44 @@ setup() { assert_success } +@test "make print-pgtle fails when cat fails for a non-last extension" { + run make print-_PGXNTOOL_EXTENSIONS + assert_success + local list=${output#*set to \"} exts + read -ra exts <<< "${list%\"}" + local first_ext=${exts[0]} + [ ${#exts[@]} -ge 2 ] || error "need at least 2 extensions, got: $output" + debug 2 "first extension: $first_ext" + + # A fixed target version keeps print-pgtle from querying a live pg_tle. + local version=1.4.0 pgtle_dir + pgtle_dir=$(pgxntool/pgtle.sh --get-dir "$version") || error "pgtle.sh --get-dir $version failed" + local target="$pgtle_dir/$first_ext.sql" + local marker="$BATS_TEST_TMPDIR/fake-cat-matched" + + # Fails only on the file print-pgtle cats for the first extension; every + # other call goes to the real cat, since the pgtle prerequisite's pgtle.sh + # uses cat too. + local fakebin="$BATS_TEST_TMPDIR/fakebin" real_cat + real_cat=$(type -P cat) + mkdir -p "$fakebin" + cat > "$fakebin/cat" <&2 + exit 1 +fi +exec $(printf %q "$real_cat") "\$@" +EOF + chmod +x "$fakebin/cat" + + PATH="$fakebin:$PATH" run make print-pgtle PGXNTOOL_PGTLE_TARGET_VERSION="$version" + assert_failure + [ -f "$marker" ] || error "fake cat never saw $target; output: $output" + assert_contains "$output" "fake cat: failing on $target" +} + @test "make pgtle fails when pgtle.sh fails for a non-last extension" { run make print-_PGXNTOOL_EXTENSIONS assert_success From 68b9dd9b866964c7d1ef96f5e620c0d750c1e524 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 16:55:42 -0500 Subject: [PATCH 06/10] Fingerprint each loop's line in the loop inventory test make-loops.bats now also fails when the logical line containing a listed loop changes in anything but whitespace, so a loop can't lose its `|| exit 1`, gain a `-` prefix, or gain a command after `done` that replaces its exit status without someone rechecking it. Each entry in test/lib/pgxntool-loops.txt gains a fingerprint field: the cksum CRC of that line, with continuations joined and each run of blanks collapsed to one space. There is no check for `|| exit 1` or && itself; any change to the text trips the test. Loops are still matched by file, context and kind. A key with as many loops as entries but different fingerprints is reported as a changed loop, with the line as it is now and, in order, what to recheck before updating the fingerprint. The new-loop message also prints the fingerprint a new entry needs. scan-make-loops.awk prints the normalized line as a fourth field. New fixture tests check that the fingerprint ignores whitespace and indentation, changes when `|| exit 1` is removed from a continuation line or a `-` prefix is added, and ignores edits to the neighbouring recipe lines, and that the comparison reports a changed loop rather than a new and a stale one. Related to Postgres-Extensions/pgxntool#140. --- test/lib/pgxntool-loops.txt | 63 +++++----- test/lib/scan-make-loops.awk | 7 +- test/standard/make-loops.bats | 217 ++++++++++++++++++++++++++++++---- 3 files changed, 233 insertions(+), 54 deletions(-) diff --git a/test/lib/pgxntool-loops.txt b/test/lib/pgxntool-loops.txt index 4b1057a..e674eb9 100644 --- a/test/lib/pgxntool-loops.txt +++ b/test/lib/pgxntool-loops.txt @@ -2,37 +2,44 @@ # iteration fails make. A new loop needs that test BEFORE it gets an entry # here: write a test that makes a non-last iteration fail and asserts make # exits non-zero, confirm it fails with the loop's `|| exit 1` removed, then -# add the entry naming it. +# add the entry naming it. A changed fingerprint means the loop's line +# changed: recheck that a failing iteration still fails make, and that the +# coverage still holds, BEFORE updating it. # # test/standard/make-loops.bats fails on any loop missing from this list, any -# entry with no matching loop, and any coverage test that doesn't exist. +# entry with no matching loop, any loop whose line no longer matches its +# entry's fingerprint, and any coverage test that doesn't exist. It prints the +# fingerprint of each new or changed loop. # -# Format: four tab-separated fields per line. Lines starting with # and blank +# Format: five tab-separated fields per line. Lines starting with # and blank # lines are ignored; order doesn't matter. -# path under pgxntool/ -# where the loop is, never a line number: -# rule: a recipe or rule line; is the -# unexpanded text before the rule's ':' -# var: a variable assignment -# define: a define body -# toplevel any other line, e.g. $(foreach ...$(eval ...)) -# for, while or until (shell), or foreach (make) -# test:: -# make-time: only for a $(foreach) that runs inside make -# and emits no shell commands -# untested: grandfathered entries only, never a new loop +# path under pgxntool/ +# where the loop is, never a line number: +# rule: a recipe or rule line; is the +# unexpanded text before the rule's ':' +# var: a variable assignment +# define: a define body +# toplevel any other line, e.g. $(foreach ...$(eval ...)) +# for, while or until (shell), or foreach (make) +# cksum's CRC of the logical line containing the loop, with +# continuations joined and each run of blanks collapsed to +# one space; loops on one line share it +# test:: +# make-time: only for a $(foreach) that runs inside make +# and emits no shell commands +# untested: grandfathered entries only, never a new loop # A context with two loops of the same kind gets two lines. -base.mk rule:$(_PGXNTOOL_INSTALL_SCHEDULE) for untested:grandfathered; an iteration can only fail appending to the schedule file that the `>` just before the loop created, so there is no practical way to fail a non-last one -base.mk rule:check-duplicate-docs foreach make-time:expands to $(warning) calls; the recipe itself runs only `:` -base.mk rule:pgtle for test:standard/multi-extension.bats:make pgtle fails when pgtle.sh fails for a non-last extension -base.mk rule:print-pgtle foreach test:standard/multi-extension.bats:make print-pgtle fails when cat fails for a non-last extension -base.mk rule:results for test:standard/make-test.bats:make results fails when a copy other than the last one fails (pgxntool#140) -base.mk rule:results-build for test:standard/make-test.bats:results-build fails when a copy fails (pgxntool#140) -base.mk toplevel foreach make-time:$(eval)s an ASCIIDOC_template rule for each of ASCIIDOC_EXTS -base.mk var:ASCIIDOC_FILES foreach make-time:builds a file list (outer $(foreach)) -base.mk var:ASCIIDOC_FILES foreach make-time:builds a file list (inner $(foreach)) -base.mk var:ASCIIDOC_HTML foreach make-time:builds a file list -base.mk var:DOCS foreach make-time:builds a file list -base.mk var:_PGXNTOOL_DUPLICATE_DOCS foreach make-time:builds a name list -base.mk var:_pgxntool_docs_named foreach make-time:builds a file list +base.mk rule:$(_PGXNTOOL_INSTALL_SCHEDULE) for 2140132506 untested:grandfathered; an iteration can only fail appending to the schedule file that the `>` just before the loop created, so there is no practical way to fail a non-last one +base.mk rule:check-duplicate-docs foreach 3110729658 make-time:expands to $(warning) calls; the recipe itself runs only `:` +base.mk rule:pgtle for 3542486984 test:standard/multi-extension.bats:make pgtle fails when pgtle.sh fails for a non-last extension +base.mk rule:print-pgtle foreach 2102040647 test:standard/multi-extension.bats:make print-pgtle fails when cat fails for a non-last extension +base.mk rule:results for 2360069330 test:standard/make-test.bats:make results fails when a copy other than the last one fails (pgxntool#140) +base.mk rule:results-build for 2438470538 test:standard/make-test.bats:results-build fails when a copy fails (pgxntool#140) +base.mk toplevel foreach 4171952040 make-time:$(eval)s an ASCIIDOC_template rule for each of ASCIIDOC_EXTS +base.mk var:ASCIIDOC_FILES foreach 3923359060 make-time:builds a file list (outer $(foreach)) +base.mk var:ASCIIDOC_FILES foreach 3923359060 make-time:builds a file list (inner $(foreach)) +base.mk var:ASCIIDOC_HTML foreach 2225361476 make-time:builds a file list +base.mk var:DOCS foreach 510455491 make-time:builds a file list +base.mk var:_PGXNTOOL_DUPLICATE_DOCS foreach 3031171085 make-time:builds a name list +base.mk var:_pgxntool_docs_named foreach 2395367995 make-time:builds a file list diff --git a/test/lib/scan-make-loops.awk b/test/lib/scan-make-loops.awk index af42e0f..9b2fc35 100644 --- a/test/lib/scan-make-loops.awk +++ b/test/lib/scan-make-loops.awk @@ -1,4 +1,6 @@ -# Lists the loops in one makefile, one "\t\t" line each. +# Lists the loops in one makefile, one "\t\t\t" line +# each, being the whole logical line containing the loop with each run +# of blanks collapsed to one space; make-loops.bats fingerprints it. # test/standard/make-loops.bats says what counts as a loop and why; the header # of test/lib/pgxntool-loops.txt describes the context keys. # @@ -41,6 +43,7 @@ END { if (np) logical() } function logical( i, raw, r, t, p, s, name, key) { raw = phys[1] for (i = 2; i <= np; i++) raw = raw " " phys[i] + logical_text = norm(raw) # Like make, only lines that don't start with a tab can nest or end a define. if (defdepth) { @@ -154,7 +157,7 @@ function search_shell(s, key, at, m) { } function emit(key, kind) { - print file "\t" key "\t" kind + print file "\t" key "\t" kind "\t" logical_text } # Position of the first character of s that is in set and outside any diff --git a/test/standard/make-loops.bats b/test/standard/make-loops.bats index 34a7ada..032cc0c 100755 --- a/test/standard/make-loops.bats +++ b/test/standard/make-loops.bats @@ -3,11 +3,13 @@ # Test: inventory of the loops in pgxntool's *.mk files # # Fails whenever pgxntool's *.mk files have a loop that -# test/lib/pgxntool-loops.txt doesn't list, or stop having one it lists, so a -# new loop can't land without a test that makes one of its non-last iterations -# fail: a recipe loop hides the failure of every iteration but the last unless -# each one ends in `|| exit 1` or the iterations are chained with &&. Needs no -# pg_config, foundation env, or Postgres. +# test/lib/pgxntool-loops.txt doesn't list, stop having one it lists, or change +# the text of the line containing one, so a new loop can't land without a test +# that makes one of its non-last iterations fail, and a changed one can't land +# without a fresh look at whether it still fails make: a recipe loop hides the +# failure of every iteration but the last unless each one ends in `|| exit 1` +# or the iterations are chained with &&. Needs no pg_config, foundation env, or +# Postgres. # # test/lib/scan-make-loops.awk reads the source text rather than make's # database (`make -p`, which make-variables.bats uses), because make -p shows @@ -16,7 +18,10 @@ # this test can't afford. # # Loops are keyed by file, enclosing context and kind, never by line number or -# neighbouring text, so edits near a loop don't trip the test. +# neighbouring text, so edits near a loop don't trip the test. Each entry also +# holds a fingerprint of the logical line containing the loop, which any edit +# to that line other than whitespace changes: that line is what make sees the +# exit status of, including any command after `done` that replaces the loop's. # # Shell keywords count only at command position, but quoted strings aren't # stripped (`sh -c "..."` bodies are real shell), so text that opens a quoted @@ -38,53 +43,154 @@ setup() { SCANNER="$TOPDIR/test/lib/scan-make-loops.awk" } -# Print "\t\t" for each loop in pgxntool's *.mk files, -# being the path under pgxntool/. +# Reads scan-make-loops.awk output and inserts each loop's fingerprint, the CRC +# that cksum prints for its text, before the text. +loop_fingerprints() { + local file context kind text crc + while IFS=$'\t' read -r file context kind text; do + [ -n "$file" ] || continue + crc=$(printf '%s\n' "$text" | cksum) || return + printf '%s\t%s\t%s\t%s\t%s\n' "$file" "$context" "$kind" "${crc%% *}" "$text" + done +} + +# Print "\t\t\t\t" for each loop in +# pgxntool's *.mk files, being the path under pgxntool/. pgxntool_loops() { - local files f + local files f loops='' files=$(cd "$PGXNREPO" && find . -name .git -prune -o -name '*.mk' -print | LC_ALL=C sort) || return [ -n "$files" ] || error "no *.mk files found under $PGXNREPO" || return for f in $files; do - awk -v file="${f#./}" -f "$SCANNER" "$PGXNREPO/$f" || return + loops+=$(awk -v file="${f#./}" -f "$SCANNER" "$PGXNREPO/$f") || return + loops+=$'\n' done + loop_fingerprints <<< "$loops" +} + +# compare_loops : both are "\t\t\t +# \t..." lines, and has each loop's text in field 5. +# Prints sorted records, tab-separated, keyed by the first three fields: +# new per loop beyond the key's entry count +# unlisted +# per loop of such a key whose fingerprint no entry has +# stale per entry beyond the key's loop count +# changed +# per loop whose fingerprint no entry has, when the counts +# match, paired in input order with an unmatched entry +compare_loops() { + awk -F '\t' ' + NF == 0 { next } + { key = $1 FS $2 FS $3; keys[key] } + FILENAME == ARGV[1] { entry[key, ++entry_count[key]] = $4; next } + { loop[key, ++loop_count[key]] = $4; text[key, loop_count[key]] = $5 } + END { + for (key in keys) { + entries = entry_count[key] + 0 + loops = loop_count[key] + 0 + for (i = 1; i <= entries; i++) left[key, entry[key, i]]++ + unmatched_loops = 0 + for (i = 1; i <= loops; i++) { + if (left[key, loop[key, i]] > 0) left[key, loop[key, i]]-- + else unmatched_loop[++unmatched_loops] = i + } + unmatched_entries = 0 + for (i = 1; i <= entries; i++) { + if (left[key, entry[key, i]] > 0) { + left[key, entry[key, i]]-- + unmatched_entry[++unmatched_entries] = entry[key, i] + } + } + if (loops > entries) { + for (i = entries; i < loops; i++) print "new" FS key + for (i = 1; i <= unmatched_loops; i++) + print "unlisted" FS key FS loop[key, unmatched_loop[i]] FS text[key, unmatched_loop[i]] + } else if (loops < entries) { + for (i = loops; i < entries; i++) print "stale" FS key + } else { + for (i = 1; i <= unmatched_loops; i++) + print "changed" FS key FS loop[key, unmatched_loop[i]] FS text[key, unmatched_loop[i]] FS unmatched_entry[i] + } + } + }' <(printf '%s\n' "$1") <(printf '%s\n' "$2") | LC_ALL=C sort +} + +# fixture_fingerprint : the fingerprint of the text's only loop. +# A ">" at the start of a line stands for a tab. +fixture_fingerprint() { + local fixture="$BATS_TEST_TMPDIR/fingerprint.mk" loops + awk '{ sub(/^>/, "\t") } 1' > "$fixture" <<< "$1" || return + loops=$(awk -v file=fixture.mk -f "$SCANNER" "$fixture") || return + loops=$(loop_fingerprints <<< "$loops") || return + case $loops in + ''|*$'\n'*) error "expected exactly one loop in the fixture, got: +$loops"; return ;; + esac + cut -f4 <<< "$loops" } @test "every loop in pgxntool's *.mk files is listed in test/lib/pgxntool-loops.txt" { local rel=test/lib/pgxntool-loops.txt - local actual entries listed new stale problems='' + local actual entries differences problems='' actual=$(pgxntool_loops) - actual=$(LC_ALL=C sort <<< "$actual") entries=$(awk '!/^#/ && !/^[[:space:]]*$/' "$TOPDIR/$rel") - listed=$(cut -f1-3 <<< "$entries" | LC_ALL=C sort) + differences=$(compare_loops "$entries" "$actual") - # comm on sorted input compares as multisets: two loops of one kind in one - # context need two entries. - new=$(LC_ALL=C comm -23 <(echo "$actual") <(echo "$listed")) - stale=$(LC_ALL=C comm -13 <(echo "$actual") <(echo "$listed")) + local tag file context kind fingerprint text listed tab=$'\t' + local new='' unlisted='' stale='' changed='' changed_fingerprints='' + while IFS=$'\t' read -r tag file context kind fingerprint text listed; do + case "$tag" in + new) new+=" $file$tab$context$tab$kind"$'\n' ;; + unlisted) unlisted+=" $file$tab$context$tab$kind$tab$fingerprint$tab$text"$'\n' ;; + stale) stale+=" $file$tab$context$tab$kind"$'\n' ;; + changed) + changed+=" $file$tab$context$tab$kind$tab(listed fingerprint: $listed) + The line containing it is now: $text + To see what changed, run this in pgxntool: git log -p -- $file (or git diff -- $file) +" + changed_fingerprints+=" $file$tab$context$tab$kind$tab$fingerprint"$'\n' + ;; + esac + done <<< "$differences" [ -z "$new" ] || problems+=" New loop(s) in pgxntool's *.mk files not listed in $rel: -$(sed 's/^/ /' <<< "$new") +${new%$'\n'} A shell loop (or \$(foreach) that generates commands) in a recipe hides the failure of every iteration but the last unless each iteration ends in \`|| exit 1\` or the iterations are chained with &&. Do these IN ORDER: 1. Write a test that makes a NON-LAST iteration of this loop fail and asserts make exits non-zero. Confirm it catches the bug: temporarily remove the loop's \`|| exit 1\`, and the test must fail. - 2. Only then add the entry to $rel, naming that test in the coverage column (test::<@test name>). + 2. Only then add the entry to $rel, with the loop's fingerprint from the list below and that test in the coverage column (test::<@test name>). Do NOT add the entry just to make this test pass; this list exists to force step 1. Use make-time: only for a \$(foreach) that runs inside make and emits no shell commands. Never use untested: for a new loop. +Fingerprints of the unlisted loops, each followed by the line containing it: +${unlisted%$'\n'} +" + [ -z "$changed" ] || problems+=" +Changed loop(s) in pgxntool's *.mk files: the line containing each one no longer matches the fingerprint listed in $rel. +${changed%$'\n'} +Do these IN ORDER: + 1. Read the loop as it is now and confirm that a failure in ANY iteration, not just the last, still fails make: each iteration ends in \`|| exit 1\` or the iterations are chained with &&, there is no \`-\` prefix, and nothing after \`done\` replaces the loop's exit status. + 2. For a test: entry, confirm the named test still exercises the loop as changed: temporarily remove the loop's error handling, and the test must fail. For untested: or make-time:, recheck that the stated reason still holds. + 3. Only then update the fingerprint. +New fingerprint(s), for step 3: +${changed_fingerprints%$'\n'} +Do NOT update the fingerprint just to make this test pass. " [ -z "$stale" ] || problems+=" Listed in $rel but no longer in pgxntool's *.mk files -- remove: -$(sed 's/^/ /' <<< "$stale") +${stale%$'\n'} If a loop moved to another context (e.g. a renamed target), fix its entry instead, and check that its coverage test still exercises the loop where it is now. " - local file context kind coverage extra path name bad='' - while IFS=$'\t' read -r file context kind coverage extra; do - [ -z "$extra" ] || { bad+=" more than four tab-separated fields: $file $context $kind"$'\n'; continue; } + local coverage extra path name bad='' + while IFS=$'\t' read -r file context kind fingerprint coverage extra; do + [ -z "$extra" ] || { bad+=" more than five tab-separated fields: $file $context $kind"$'\n'; continue; } case "$kind" in for|while|until|foreach) ;; *) bad+=" unknown kind '$kind': $file $context"$'\n' ;; esac + case "$fingerprint" in + ''|*[!0-9]*) bad+=" fingerprint '$fingerprint' isn't a number: $file $context $kind"$'\n' ;; + esac case "$coverage" in test:?*:?*) path=${coverage#test:} @@ -108,6 +214,42 @@ $bad" [ -z "$problems" ] || error "${problems#$'\n'}" } +@test "compare_loops tells a changed loop from a new or a stale one" { + local entries loops want got + entries=$(tr '|' '\t' <<'EOF' +f.mk|rule:same|for|1|test:x +f.mk|rule:changed|for|2|test:x +f.mk|rule:pair|foreach|7|make-time:x +f.mk|rule:pair|foreach|7|make-time:x +f.mk|rule:fewer|for|3|test:x +f.mk|rule:fewer|for|3|test:x +f.mk|rule:more|for|4|test:x +EOF +) + loops=$(tr '|' '\t' <<'EOF' +f.mk|rule:same|for|1|same text +f.mk|rule:changed|for|5|changed text +f.mk|rule:pair|foreach|8|pair text +f.mk|rule:pair|foreach|8|pair text +f.mk|rule:fewer|for|3|fewer text +f.mk|rule:more|for|4|more text +f.mk|rule:more|for|6|added text +EOF +) + want=$(tr '|' '\t' <<'EOF' +changed|f.mk|rule:changed|for|5|changed text|2 +changed|f.mk|rule:pair|foreach|8|pair text|7 +changed|f.mk|rule:pair|foreach|8|pair text|7 +new|f.mk|rule:more|for +stale|f.mk|rule:fewer|for +unlisted|f.mk|rule:more|for|6|added text +EOF +) + got=$(compare_loops "$entries" "$loops") + [ "$got" = "$want" ] || error "compare_loops output differs from expected: +$(diff -u <(echo "$want") <(echo "$got"))" +} + @test "scan-make-loops.awk finds each loop shape and skips comments and prose" { # Lines starting with ">" start with a tab once written out. local fixture="$BATS_TEST_TMPDIR/fixture.mk" @@ -157,9 +299,36 @@ rule:$(SRCS:.c=.o) for var:INDENTED_LOOP foreach define:TEMPLATE for rule:untaken for' - got=$(awk -v file=fixture.mk -f "$SCANNER" "$fixture" | cut -f2- | tr '\t' ' ') + got=$(awk -v file=fixture.mk -f "$SCANNER" "$fixture" | cut -f2,3 | tr '\t' ' ') [ "$got" = "$want" ] || error "scanner output differs from expected: $(diff -u <(echo "$want") <(echo "$got"))" } +@test "a loop's fingerprint covers its whole logical line and ignores only whitespace" { + local base='copy: +>@echo before +>@for f in a b; do \ +> cp $$f out/ || exit 1; \ +>done +>@echo after' + local original respaced no_exit ignored before after + original=$(fixture_fingerprint "$base") + case $original in + ''|*[!0-9]*) error "fingerprint '$original' isn't a number"; return ;; + esac + # Blanks become runs of spaces and tabs, and every recipe line is indented further. + respaced=$(fixture_fingerprint "$(awk '{ sub(/^>/, ">\t "); gsub(/ /, " \t ") } 1' <<< "$base")") + no_exit=$(fixture_fingerprint "${base/ || exit 1/}") + ignored=$(fixture_fingerprint "${base/>@for/>-@for}") + before=$(fixture_fingerprint "${base/echo before/echo other; echo before}") + after=$(fixture_fingerprint "${base/echo after/echo other}") + + [ "$respaced" = "$original" ] || error "a whitespace-only change altered the fingerprint" + [ "$no_exit" != "$original" ] || + error "removing \`|| exit 1\` from a continuation line left the fingerprint unchanged" + [ "$ignored" != "$original" ] || error "adding a \`-\` prefix left the fingerprint unchanged" + [ "$before" = "$original" ] || error "editing the recipe line before the loop's altered its fingerprint" + [ "$after" = "$original" ] || error "editing the recipe line after the loop's altered its fingerprint" +} + # vi: expandtab sw=2 ts=2 From 250f97b924fc71ef57c901d12d9b524d37589c7c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 17:07:11 -0500 Subject: [PATCH 07/10] Derive pgtle test dir without earlier-test state; share extension parse --- test/standard/make-test.bats | 3 ++- test/standard/multi-extension.bats | 31 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/test/standard/make-test.bats b/test/standard/make-test.bats index 892f401..9120af6 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -831,7 +831,8 @@ EOF @test "results-build fails when a copy fails (pgxntool#140)" { skip_if_no_postgres - # A directory at cp's destination makes cp fail even as root. + # A directory at cp's destination makes cp fail even as root; the template's + # build_check.out and simple_build_test.out sort after 0_cp_fail.out, so it isn't the last copy. mkdir -p test/build/results test/build/expected/0_cp_fail.out/0_cp_fail.out echo "copy fails" > test/build/results/0_cp_fail.out diff --git a/test/standard/multi-extension.bats b/test/standard/multi-extension.bats index 9fce089..e31c6e5 100644 --- a/test/standard/multi-extension.bats +++ b/test/standard/multi-extension.bats @@ -164,18 +164,25 @@ setup() { assert_success } -@test "make print-pgtle fails when cat fails for a non-last extension" { +# Sets first_ext and pgtle_dir (the 1.4.0 range directory) in the caller's scope. +load_first_ext_and_pgtle_dir() { run make print-_PGXNTOOL_EXTENSIONS assert_success local list=${output#*set to \"} exts read -ra exts <<< "${list%\"}" - local first_ext=${exts[0]} + first_ext=${exts[0]} [ ${#exts[@]} -ge 2 ] || error "need at least 2 extensions, got: $output" debug 2 "first extension: $first_ext" + pgtle_dir=$(pgxntool/pgtle.sh --get-dir 1.4.0) || error "pgtle.sh --get-dir 1.4.0 failed" +} + +# Leaves pg_tle/ generated by make pgtle; must run before the "make pgtle fails" test. +@test "make print-pgtle fails when cat fails for a non-last extension" { + local first_ext pgtle_dir + load_first_ext_and_pgtle_dir # A fixed target version keeps print-pgtle from querying a live pg_tle. - local version=1.4.0 pgtle_dir - pgtle_dir=$(pgxntool/pgtle.sh --get-dir "$version") || error "pgtle.sh --get-dir $version failed" + local version=1.4.0 local target="$pgtle_dir/$first_ext.sql" local marker="$BATS_TEST_TMPDIR/fake-cat-matched" @@ -202,23 +209,15 @@ EOF assert_contains "$output" "fake cat: failing on $target" } +# Leaves a directory at pg_tle//.sql, which breaks print-pgtle; must run after it. @test "make pgtle fails when pgtle.sh fails for a non-last extension" { - run make print-_PGXNTOOL_EXTENSIONS - assert_success - local list=${output#*set to \"} exts - read -ra exts <<< "${list%\"}" - local first_ext=${exts[0]} - [ ${#exts[@]} -ge 2 ] || error "need at least 2 extensions, got: $output" - debug 2 "first extension: $first_ext" - - local ranges=(pg_tle/*/) - [ -d "${ranges[0]}" ] || error "no pg_tle// directory from the earlier make pgtle" - local range=${ranges[0]} + local first_ext pgtle_dir + load_first_ext_and_pgtle_dir # A directory where pgtle.sh writes its output file makes it fail for the # first extension only; the make call's own exit status is what's asserted. rm -rf pg_tle - mkdir -p "${range}${first_ext}.sql" + mkdir -p "$pgtle_dir/$first_ext.sql" run make pgtle assert_failure } From 430c8f5bfeeed4e560bf13ca9435d13336e758b5 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 17:12:21 -0500 Subject: [PATCH 08/10] Count a loop keyword outside a define body only when a do follows scan-make-loops.awk reported text like `echo "while building foo"` or `$(error bad value (for instance; see docs))` as a loop, and the inventory had no legal entry for it. Outside define bodies, a for/while/until at command position now counts only when a `do` follows it in the searched text, after one of ;&)} or right after `for NAME`: a recipe line, assignment value or rule line is one shell command line, so a real loop's `do` is on it. Define bodies keep the old rule, since one shell may run a whole define with `do` on a later line. Text that still matches but isn't a loop gets the new `not-a-loop:` coverage, which is rejected for a $(foreach). The new-loop failure message lists it and make-time: as the only exceptions to writing a failure test first, and is now the one place the steps are written out; the headers of make-loops.bats and pgxntool-loops.txt point to it. The scanner fixture gains five such prose lines, which must not be reported; four loop shapes, which must (`while ...; do ... < list`, `until`, `for f do`, `while { ...; } do`); and a define body line with "while" in an echo, which is reported because define bodies stay strict. Related to Postgres-Extensions/pgxntool#140. --- test/lib/pgxntool-loops.txt | 11 +++-- test/lib/scan-make-loops.awk | 15 +++++-- test/standard/make-loops.bats | 77 +++++++++++++++++++++++------------ 3 files changed, 67 insertions(+), 36 deletions(-) diff --git a/test/lib/pgxntool-loops.txt b/test/lib/pgxntool-loops.txt index e674eb9..450bb7e 100644 --- a/test/lib/pgxntool-loops.txt +++ b/test/lib/pgxntool-loops.txt @@ -1,10 +1,6 @@ # Every loop in pgxntool's *.mk files, with the test proving that a failing -# iteration fails make. A new loop needs that test BEFORE it gets an entry -# here: write a test that makes a non-last iteration fail and asserts make -# exits non-zero, confirm it fails with the loop's `|| exit 1` removed, then -# add the entry naming it. A changed fingerprint means the loop's line -# changed: recheck that a failing iteration still fails make, and that the -# coverage still holds, BEFORE updating it. +# iteration fails make. The steps for adding or updating an entry are in +# test/standard/make-loops.bats's failure message. # # test/standard/make-loops.bats fails on any loop missing from this list, any # entry with no matching loop, any loop whose line no longer matches its @@ -27,6 +23,9 @@ # test:: # make-time: only for a $(foreach) that runs inside make # and emits no shell commands +# not-a-loop: only for a shell for/while/until match that +# reading the line shows isn't a loop; +# says what the text is # untested: grandfathered entries only, never a new loop # A context with two loops of the same kind gets two lines. diff --git a/test/lib/scan-make-loops.awk b/test/lib/scan-make-loops.awk index 9b2fc35..dcb2958 100644 --- a/test/lib/scan-make-loops.awk +++ b/test/lib/scan-make-loops.awk @@ -14,6 +14,8 @@ # - rule headers: the prerequisites, plus any `; recipe` (taken from the raw # line, since make doesn't strip comments from it) # - everything else: the line minus its make comment +# A for/while/until at command position counts only when a `do` follows it in +# the searched text, except in a define body, where every one counts. BEGIN { # Shell command position: start of the text, after a separator or opening @@ -24,6 +26,10 @@ BEGIN { # awk's `for(i in A)` don't match. LOOP = "(for[ \t]+[A-Za-z_][A-Za-z0-9_]*([ \t]+in([ \t;]|$)|[ \t]*;|[ \t]+do([ \t;]|$))|for[ \t]*\\(\\(|while([ \t({!]|$)|until([ \t({!]|$))" SHELL_LOOP = CMDPOS LOOP + # The `do` that a real loop's keyword must be followed by outside a define + # body: after a separator, or right after `for NAME`. + DO_AFTER = "[;&)}][ \t]*do([ \t]|$)" + FOR_NAME_DO = "^for[ \t]+[A-Za-z_][A-Za-z0-9_]*[ \t]+do([ \t;]|$)" } # Physical lines are buffered until one doesn't end in an odd number of @@ -143,16 +149,19 @@ function search_foreach(s, key) { } } -function search_shell(s, key, at, m) { +function search_shell(s, key, at, m, kind) { while (match(s, SHELL_LOOP)) { at = RSTART m = substr(s, RSTART, RLENGTH) match(m, /for|while|until/) + at += RSTART - 1 + kind = substr(m, RSTART, RLENGTH) m = substr(m, RSTART) - emit(key, m ~ /^for/ ? "for" : m ~ /^while/ ? "while" : "until") + if (key ~ /^define:/ || substr(s, at + RLENGTH) ~ DO_AFTER || m ~ FOR_NAME_DO) + emit(key, kind) # Resume just past the keyword's first character; the "_" keeps the # remainder's start from counting as command position. - s = "_" substr(s, at + RSTART) + s = "_" substr(s, at + 1) } } diff --git a/test/standard/make-loops.bats b/test/standard/make-loops.bats index 032cc0c..2621eaa 100755 --- a/test/standard/make-loops.bats +++ b/test/standard/make-loops.bats @@ -2,31 +2,30 @@ # Test: inventory of the loops in pgxntool's *.mk files # -# Fails whenever pgxntool's *.mk files have a loop that -# test/lib/pgxntool-loops.txt doesn't list, stop having one it lists, or change -# the text of the line containing one, so a new loop can't land without a test -# that makes one of its non-last iterations fail, and a changed one can't land -# without a fresh look at whether it still fails make: a recipe loop hides the -# failure of every iteration but the last unless each one ends in `|| exit 1` -# or the iterations are chained with &&. Needs no pg_config, foundation env, or -# Postgres. +# Fails when pgxntool's *.mk files have a loop test/lib/pgxntool-loops.txt +# doesn't list, lack one it lists, or change the line containing one. A recipe +# loop hides the failure of every iteration but the last unless each one ends +# in `|| exit 1` or the iterations are chained with &&. The steps to take for +# each kind of failure are in the failure message. Needs no pg_config, +# foundation env, or Postgres. # -# test/lib/scan-make-loops.awk reads the source text rather than make's -# database (`make -p`, which make-variables.bats uses), because make -p shows -# only recipes in active conditional branches: with default settings the -# diffs-mode verify-results recipe is absent. A missed loop is the one failure -# this test can't afford. +# test/lib/scan-make-loops.awk reads the source text, not make's database +# (`make -p`), which shows only recipes in active conditional branches: with +# default settings it lacks the diffs-mode verify-results recipe. A missed loop +# is the one failure this test can't afford. # -# Loops are keyed by file, enclosing context and kind, never by line number or -# neighbouring text, so edits near a loop don't trip the test. Each entry also -# holds a fingerprint of the logical line containing the loop, which any edit -# to that line other than whitespace changes: that line is what make sees the -# exit status of, including any command after `done` that replaces the loop's. +# Loops are keyed by file, context and kind, never by line number, so edits +# near a loop don't trip the test. Each entry's fingerprint covers the whole +# logical line containing the loop, since that line's exit status, including +# any command after `done`, is what make sees. # -# Shell keywords count only at command position, but quoted strings aren't -# stripped (`sh -c "..."` bodies are real shell), so text that opens a quoted -# string or an embedded awk program with "while " or "for x in" is counted -# too. That overmatch is stable, so it's accepted. +# A for/while/until counts only at shell command position, and outside define +# bodies only with a `do` after it: a recipe line, assignment value or rule +# line is one shell command line, so a real loop's `do` follows on that logical +# line. A define body may be handed whole to one shell, with `do` on a later +# line, so there every keyword at command position counts. Quoted strings +# aren't stripped (`sh -c "..."` bodies are real shell); text that still +# matches but isn't a loop gets a `not-a-loop:` entry. # # Not scanned: recipe text that the *.mk.sh generators echo into meta.mk and # control.mk. They emit no loops today. @@ -158,9 +157,12 @@ ${new%$'\n'} A shell loop (or \$(foreach) that generates commands) in a recipe hides the failure of every iteration but the last unless each iteration ends in \`|| exit 1\` or the iterations are chained with &&. Do these IN ORDER: 1. Write a test that makes a NON-LAST iteration of this loop fail and asserts make exits non-zero. Confirm it catches the bug: temporarily remove the loop's \`|| exit 1\`, and the test must fail. - 2. Only then add the entry to $rel, with the loop's fingerprint from the list below and that test in the coverage column (test::<@test name>). + 2. Only after that test exists and passes, add the entry to $rel, with the loop's fingerprint from the list below and that test in the coverage column (test::<@test name>). Do NOT add the entry just to make this test pass; this list exists to force step 1. -Use make-time: only for a \$(foreach) that runs inside make and emits no shell commands. Never use untested: for a new loop. +The only exceptions, each still an entry with the fingerprint below: + - make-time:, only for a \$(foreach) that runs inside make and emits no shell commands. + - not-a-loop:, only for a for/while/until match you have READ and confirmed is not shell code that runs a loop (e.g. the word \"while\" inside an echoed message); says what the text is. If any part of the line runs a loop, it is a loop: go back to step 1. +Never use untested: for a new loop. Fingerprints of the unlisted loops, each followed by the line containing it: ${unlisted%$'\n'} " @@ -169,7 +171,7 @@ Changed loop(s) in pgxntool's *.mk files: the line containing each one no longer ${changed%$'\n'} Do these IN ORDER: 1. Read the loop as it is now and confirm that a failure in ANY iteration, not just the last, still fails make: each iteration ends in \`|| exit 1\` or the iterations are chained with &&, there is no \`-\` prefix, and nothing after \`done\` replaces the loop's exit status. - 2. For a test: entry, confirm the named test still exercises the loop as changed: temporarily remove the loop's error handling, and the test must fail. For untested: or make-time:, recheck that the stated reason still holds. + 2. For a test: entry, confirm the named test still exercises the loop as changed: temporarily remove the loop's error handling, and the test must fail. For untested: or make-time:, recheck that the stated reason still holds. For not-a-loop:, reread the line and confirm the match still isn't a loop; if the line now runs one, it needs a failure test (see the new-loop steps) and a test: entry. 3. Only then update the fingerprint. New fingerprint(s), for step 3: ${changed_fingerprints%$'\n'} @@ -203,8 +205,12 @@ If a loop moved to another context (e.g. a renamed target), fix its entry instea [ "$kind" = foreach ] || bad+=" make-time: is only for \$(foreach), not a shell $kind loop: $file $context"$'\n' ;; + not-a-loop:?*) + [ "$kind" != foreach ] || + bad+=" not-a-loop: is only for a shell for/while/until match, never a \$(foreach): $file $context"$'\n' + ;; untested:?*) ;; - *) bad+=" coverage must be test::<@test name>, make-time: or untested:: $file $context $kind"$'\n' ;; + *) bad+=" coverage must be test::<@test name>, make-time:, not-a-loop: or untested:: $file $context $kind"$'\n' ;; esac done <<< "$entries" [ -z "$bad" ] || problems+=" @@ -251,7 +257,9 @@ $(diff -u <(echo "$want") <(echo "$got"))" } @test "scan-make-loops.awk finds each loop shape and skips comments and prose" { - # Lines starting with ">" start with a tab once written out. + # Lines starting with ">" start with a tab once written out. The echo and + # $(error) lines name a keyword with no `do` after it, so only the define + # body's one counts. local fixture="$BATS_TEST_TMPDIR/fixture.mk" awk '{ sub(/^>/, "\t") } 1' > "$fixture" <<'EOF' # make comment: for f in a b; do $(foreach x,y,z); done @@ -265,6 +273,15 @@ copy: >@# disabled: true; for f in a b; do echo $$f; done >@awk '{for(i in A) print A[i]}' /dev/null >@echo "for a reason" >&2 +>@echo "while building foo" +>@echo "ERROR: failed (while installing)" +>@echo "see notes (for example; the README)" +>$(error bad value (for instance; see docs)) +>@echo "until then, nothing" +>@while read f; do echo $$f; done < list +>@until false; do break; done +>@for f do echo $$f; done +>@while { true; } do break; done gen: >@$(foreach e,a b,./run.sh $(e);) use-helper: @@ -279,6 +296,7 @@ INDENTED = 1 define TEMPLATE $(1)-copy: >for f in $(1); do cp $$$$f out/; done +>@echo "while building foo" endef ifeq (never,taken) untaken: @@ -293,11 +311,16 @@ var:HASH foreach rule:copy for rule:copy while rule:copy for +rule:copy while +rule:copy until +rule:copy for +rule:copy while rule:gen foreach rule:inline for rule:$(SRCS:.c=.o) for var:INDENTED_LOOP foreach define:TEMPLATE for +define:TEMPLATE while rule:untaken for' got=$(awk -v file=fixture.mk -f "$SCANNER" "$fixture" | cut -f2,3 | tr '\t' ' ') [ "$got" = "$want" ] || error "scanner output differs from expected: From 6e73edce1ac870b5f5002be362dc601b76d12f15 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 17:12:21 -0500 Subject: [PATCH 09/10] Check that make pgtle writes every extension's pg_tle file "make generates versioned SQL files" now also checks that each pg_tle// directory make pgtle creates holds both ext_alpha.sql and ext_beta.sql, and that it creates at least one. Fixes #71. --- test/standard/multi-extension.bats | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/standard/multi-extension.bats b/test/standard/multi-extension.bats index e31c6e5..a69e59e 100644 --- a/test/standard/multi-extension.bats +++ b/test/standard/multi-extension.bats @@ -118,6 +118,14 @@ setup() { # trimming (issues #25, #26). run make pgtle assert_success + local dir found=0 + for dir in pg_tle/*/; do + [ -d "$dir" ] || break + found=1 + [ -f "${dir}ext_alpha.sql" ] || error "make pgtle did not generate ${dir}ext_alpha.sql" + [ -f "${dir}ext_beta.sql" ] || error "make pgtle did not generate ${dir}ext_beta.sql" + done + [ "$found" = 1 ] || error "make pgtle generated no pg_tle// directory" } @test "sql/ext_alpha--1.0.0.sql is generated" { From e6e1f8a4871360c76ad1ff41ba1c96b15bdd8021 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 14 Oct 2026 17:24:11 -0500 Subject: [PATCH 10/10] Pin the ) in the loop scanner's do rule; tidy the pgtle tests The scanner fixture gains `while (false) do` and `for ((i=0;i<2;i++)) do`, which must be reported, so dropping `)` from the characters a loop's `do` may follow now fails it. The comment on that character class lists them rather than calling them separators. In multi-extension.bats, load_first_ext_and_pgtle_dir also sets pgtle_version, which the print-pgtle test passes to make instead of repeating 1.4.0, and that test's comment keeps only its ordering constraint. Related to Postgres-Extensions/pgxntool#140. --- test/lib/scan-make-loops.awk | 2 +- test/standard/make-loops.bats | 4 ++++ test/standard/multi-extension.bats | 18 ++++++++++-------- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/test/lib/scan-make-loops.awk b/test/lib/scan-make-loops.awk index dcb2958..1b6c658 100644 --- a/test/lib/scan-make-loops.awk +++ b/test/lib/scan-make-loops.awk @@ -27,7 +27,7 @@ BEGIN { LOOP = "(for[ \t]+[A-Za-z_][A-Za-z0-9_]*([ \t]+in([ \t;]|$)|[ \t]*;|[ \t]+do([ \t;]|$))|for[ \t]*\\(\\(|while([ \t({!]|$)|until([ \t({!]|$))" SHELL_LOOP = CMDPOS LOOP # The `do` that a real loop's keyword must be followed by outside a define - # body: after a separator, or right after `for NAME`. + # body: after one of ; & ) }, or right after `for NAME`. DO_AFTER = "[;&)}][ \t]*do([ \t]|$)" FOR_NAME_DO = "^for[ \t]+[A-Za-z_][A-Za-z0-9_]*[ \t]+do([ \t;]|$)" } diff --git a/test/standard/make-loops.bats b/test/standard/make-loops.bats index 2621eaa..65ec09a 100755 --- a/test/standard/make-loops.bats +++ b/test/standard/make-loops.bats @@ -282,6 +282,8 @@ copy: >@until false; do break; done >@for f do echo $$f; done >@while { true; } do break; done +>@while (false) do break; done +>@for ((i=0;i<2;i++)) do echo $$i; done gen: >@$(foreach e,a b,./run.sh $(e);) use-helper: @@ -315,6 +317,8 @@ rule:copy while rule:copy until rule:copy for rule:copy while +rule:copy while +rule:copy for rule:gen foreach rule:inline for rule:$(SRCS:.c=.o) for diff --git a/test/standard/multi-extension.bats b/test/standard/multi-extension.bats index a69e59e..a5c5d9a 100644 --- a/test/standard/multi-extension.bats +++ b/test/standard/multi-extension.bats @@ -172,7 +172,8 @@ setup() { assert_success } -# Sets first_ext and pgtle_dir (the 1.4.0 range directory) in the caller's scope. +# Sets first_ext, pgtle_version and pgtle_dir (pgtle_version's range +# directory) in the caller's scope. load_first_ext_and_pgtle_dir() { run make print-_PGXNTOOL_EXTENSIONS assert_success @@ -181,16 +182,16 @@ load_first_ext_and_pgtle_dir() { first_ext=${exts[0]} [ ${#exts[@]} -ge 2 ] || error "need at least 2 extensions, got: $output" debug 2 "first extension: $first_ext" - pgtle_dir=$(pgxntool/pgtle.sh --get-dir 1.4.0) || error "pgtle.sh --get-dir 1.4.0 failed" + pgtle_version=1.4.0 + pgtle_dir=$(pgxntool/pgtle.sh --get-dir "$pgtle_version") || + error "pgtle.sh --get-dir $pgtle_version failed" } -# Leaves pg_tle/ generated by make pgtle; must run before the "make pgtle fails" test. +# Must run before the "make pgtle fails" test, which leaves print-pgtle broken. @test "make print-pgtle fails when cat fails for a non-last extension" { - local first_ext pgtle_dir + local first_ext pgtle_version pgtle_dir load_first_ext_and_pgtle_dir - # A fixed target version keeps print-pgtle from querying a live pg_tle. - local version=1.4.0 local target="$pgtle_dir/$first_ext.sql" local marker="$BATS_TEST_TMPDIR/fake-cat-matched" @@ -211,7 +212,8 @@ exec $(printf %q "$real_cat") "\$@" EOF chmod +x "$fakebin/cat" - PATH="$fakebin:$PATH" run make print-pgtle PGXNTOOL_PGTLE_TARGET_VERSION="$version" + # A fixed target version keeps print-pgtle from querying a live pg_tle. + PATH="$fakebin:$PATH" run make print-pgtle PGXNTOOL_PGTLE_TARGET_VERSION="$pgtle_version" assert_failure [ -f "$marker" ] || error "fake cat never saw $target; output: $output" assert_contains "$output" "fake cat: failing on $target" @@ -219,7 +221,7 @@ EOF # Leaves a directory at pg_tle//.sql, which breaks print-pgtle; must run after it. @test "make pgtle fails when pgtle.sh fails for a non-last extension" { - local first_ext pgtle_dir + local first_ext pgtle_version pgtle_dir load_first_ext_and_pgtle_dir # A directory where pgtle.sh writes its output file makes it fail for the