testmark-bash

Merge branch 'error-provenance' into high-road. This has been sanity-defying.

commit 60745a01d91a39783c4880774fc497aed327f59c · parent 0c4d3ffbad · parent 29dc7c3cab · browse this tree
warpfork, 2026-10-25 10:33:52 UTC

We tried to get realistic line numbers in many different ways, and *gave up*.

The final straw for me was realizing that bash's own extdebug internals
for reporting line numbers *gets things wrong*.

I'm uncomfortable with *so many* of the compromises made during this arc,
but at this point I think it's also "these are bugs in bash" and
are things that we're just going to attempt to overlook
because there's no other realistic way for this project to move on
while being... in bash.

diff --git a/README.md b/README.md
index d58e490..435921a 100644
--- a/README.md
+++ b/README.md
@@ -76,6 +76,7 @@ Each case runs it in a fresh bash, so it's a clean shell:
 
 - stdin is the trigger hunk's body, exactly.
 - `set -euo pipefail` is on.  A failing command stops the case, and the failure shows up in its output.
+  Bash's own error messages there are labeled with the function's name, nothing more, so they read the same wherever the test runs.
 - The working directory is the test script's, so a relative path like `./mytool` works, wherever the test is run from.
 - It sees exported variables (like `PATH`), but no other variables, and no other functions.
   If it calls helpers, pass them to `testmarktest_provide helper1 helper2` first.
diff --git a/docs/hacking/bash-pitfalls.md b/docs/hacking/bash-pitfalls.md
index 043cb1c..2cc902d 100644
--- a/docs/hacking/bash-pitfalls.md
+++ b/docs/hacking/bash-pitfalls.md
@@ -158,6 +158,38 @@ We've been unable to use `set -e` as a strict mode (unfortunately).
 
 That means an awful lot of `cmd || return 1` is required.
 
+The reason is a suppression rule with long reach.
+`set -e` is ignored for any command whose failure is being tested:
+the condition of an `if` or `while`, a command under `!`,
+or any command of an `&&`/`||` list except the last.
+So far, so documented.
+The reach is that the suppression extends dynamically into every function *called* from such a position,
+where `set -e` can neither be re-enabled nor even detected as off.
+Library code must expect calls like `testmark_check ... || status=$?`,
+so every function here has to assume `set -e` does nothing for it.
+
+The suppression is also inherited by forked children, where it likewise can't be shaken off.
+Measured, on bash 4.4 and 5.2, with `set -e` on and `f() { false; echo survived; }`:
+does `survived` print?
+
+| where `f` runs                    | called plainly | called from a tested context |
+| ---                               | ---            | ---                          |
+| the current shell                 | no             | yes                          |
+| a subshell `( f )`                | no             | yes                          |
+| an async command `f &`            | no             | yes                          |
+| a command substitution `$(f)`     | yes            | yes                          |
+| a process substitution `<(f)`     | yes            | yes                          |
+| a fresh bash, running `set -e; f` | no             | no                           |
+
+(The substitutions don't deliver a working `set -e` even when called plainly;
+for command substitution that's documented behavior, bash clearing `-e` there outside posix mode.)
+
+Only a process boundary sheds the suppression.
+This is why the runner gives every case a fresh bash process:
+a case is promised a working `set -euo pipefail`,
+and since callers legitimately write `testmarktest_run docs || exit 1`,
+no fork of the harness's shell could keep that promise.
+
 ### treat unset variables as fatal
 
 We turned `set -u` on in our library code, as a strict mode to catch as many bugs as possible.
diff --git a/lib/testmark.bash b/lib/testmark.bash
index 4d62194..2fd5523 100644
--- a/lib/testmark.bash
+++ b/lib/testmark.bash
@@ -106,6 +106,10 @@ testmark_check() { _testmark_call _testmark_check "$@"; }
 ##
 ## FN runs under `set -euo pipefail`.  It sees exported variables (except BASH_ENV, so its bash starts clean), but no other variables, and no functions but itself and those given to testmarktest_provide.
 ##
+## Bash's own runtime error messages from inside these functions (a command not found, an unbound variable) are relabeled in the output:
+## `FN: message`, with no path and no line number, so the text is the same wherever the test runs.
+## Set TESTMARK_RAW_ERRORS, to any value, to keep bash's text verbatim instead.
+##
 ## - @param $1 GLOB: a bash pattern, matched against whole hunk names; `*` matches `/` too.
 ## - @param $2 FN: the name of a defined function.
 testmarktest_rule_basic() { _testmark_call _testmarktest_rule_basic "$@"; }
diff --git a/lib/testmark/run.bash b/lib/testmark/run.bash
index ff22f6d..5b0997a 100644
--- a/lib/testmark/run.bash
+++ b/lib/testmark/run.bash
@@ -6,6 +6,9 @@ _testmarktest_globs=()
 _testmarktest_fns=()
 # Functions sent into every case, besides the rule's own.
 _testmarktest_provided=()
+# A case's script sources function i from pinned fd (this base + i): the case's own function, then the provided ones, in order.
+# _testmarktest_script assigns the pins; _testmarktest_relabel turns them back into function names.
+_testmarktest_fdbase=10
 # The directory of the script being run, absolute; empty if bash isn't running a script file.
 # Set when the library is sourced, so a later `cd` can't move it.
 _testmarktest_dir=
@@ -47,6 +50,12 @@ _testmarktest_provide() {
 		_testmark_err "usage: testmarktest_provide FN..."
 		return 1
 	}
+	# The pins count up from _testmarktest_fdbase and must stay clear of the fds bash gives
+	# process substitutions (63, counting down), so a case holds at most 50 functions.
+	((${#_testmarktest_provided[@]} + $# + 1 <= 50)) || {
+		_testmark_err "testmarktest_provide: a case can hold at most 50 functions"
+		return 1
+	}
 	local fn
 	for fn; do _testmarktest_need_fn testmarktest_provide "$fn" || return 1; done
 	_testmarktest_provided+=("$@")
@@ -130,6 +139,7 @@ _testmarktest_case() {
 		_testmarktest_exec "$1" "$2" "$3"
 		printf '\0%s' "$?"
 	)
+	[[ -n ${TESTMARK_RAW_ERRORS+set} ]] || _testmarktest_relabel stream "$3"
 	if ((st != 0)); then
 		[[ -z $stream || $stream == *$'\n' ]] || stream+=$'\n'
 		stream+="exit status: $st"$'\n'
@@ -150,12 +160,42 @@ _testmarktest_exec() {
 ## Prints the script a case runs: strict mode, the functions it may call, and a call to function $1.
 ## This is the one place a case's script is built.
 _testmarktest_script() {
-	local fn
+	local fn fd=$_testmarktest_fdbase
 	printf 'set -euo pipefail\n'
-	# Each function is sourced from its own process substitution, so bash's errors inside one, which land in fixtures,
-	# give line numbers within that function, whatever else this script holds.
+	# Each function is sourced from its own pinned fd, so bash's errors inside one name a source the
+	# harness chose, whatever fds the environment holds open; _testmarktest_relabel maps it back.
 	for fn in "$1" "${_testmarktest_provided[@]}"; do
-		printf 'source <(printf %%s %q)\n' "$(declare -f -- "$fn")"
+		printf 'source /dev/fd/%d %d< <(printf %%s %q)\n' "$fd" "$fd" "$(declare -f -- "$fn")"
+		fd=$((fd + 1))
 	done
 	printf '%q\n' "$1"
 }
+
+## Relabels bash's own runtime error messages in a case's output, held by the variable named $1:
+## a line beginning `/dev/fd/F: line N: `, where F is the pin of case function $2 or of a provided function,
+## trades that prefix for the function's name, becoming `FN: ` and the rest.
+## The line number is dropped on purpose: it counts lines of `declare -f` output, text that exists in no file.
+_testmarktest_relabel() {
+	local -n _tmr_stream=$1
+	local -a fns=("$2" "${_testmarktest_provided[@]}")
+	local out='' rest=$_tmr_stream line len body num i
+	while :; do
+		line=${rest%%$'\n'*}
+		len=${#line}
+		if [[ $line == /dev/fd/* ]]; then
+			for i in "${!fns[@]}"; do
+				body="/dev/fd/$((_testmarktest_fdbase + i)): line "
+				[[ $line == "$body"* ]] || continue
+				body=${line#"$body"}
+				num=${body%%:*}
+				[[ -n $num && $num != *[!0-9]* && $body == "$num: "* ]] && line="${fns[i]}: ${body#"$num": }"
+				break
+			done
+		fi
+		out+=$line
+		((len < ${#rest})) || break
+		out+=$'\n'
+		rest=${rest:len+1}
+	done
+	_tmr_stream=$out
+}
diff --git a/tests/test-run.sh b/tests/test-run.sh
index 3cfd6ab..58b5cd9 100755
--- a/tests/test-run.sh
+++ b/tests/test-run.sh
@@ -7,7 +7,7 @@ cd "$(dirname "$0")"
 # shellcheck source=tests/lib.sh
 source ./lib.sh
 
-unset TESTMARK_REGEN
+unset TESTMARK_REGEN TESTMARK_RAW_ERRORS
 mkdir t
 
 ## Writes t/doc.md and t/test.sh, runs the script from $work, and checks what it printed and its status.
@@ -149,7 +149,7 @@ testmarktest_run doc.md' "PASS  doc.md: a/in (exercise_p)
 
 --- status 0"
 
-run_case "nounset is on in a case" "$(one $'/dev/fd/61: line 3: nope: unbound variable\nexit status: 1\n')" '
+run_case "nounset is on in a case; bash's error is relabeled with the function's name" "$(one $'exercise_u: nope: unbound variable\nexit status: 1\n')" '
 exercise_u() { echo "$nope"; }
 testmarktest_rule_basic "*/in" exercise_u
 testmarktest_run doc.md' "PASS  doc.md: a/in (exercise_u)
@@ -157,6 +157,23 @@ testmarktest_run doc.md' "PASS  doc.md: a/in (exercise_u)
 
 --- status 0"
 
+run_case "held-open fds don't change a case's error text" "$(one $'exercise_u: nope: unbound variable\nexit status: 1\n')" '
+exec 10>/dev/null 61>/dev/null 62>/dev/null 63>/dev/null
+exercise_u() { echo "$nope"; }
+testmarktest_rule_basic "*/in" exercise_u
+testmarktest_run doc.md' "PASS  doc.md: a/in (exercise_u)
+--- stderr
+
+--- status 0"
+
+run_case "TESTMARK_RAW_ERRORS keeps bash's own text" "$(one $'/dev/fd/10: line 3: nope: unbound variable\nexit status: 1\n')" '
+exercise_u() { echo "$nope"; }
+testmarktest_rule_basic "*/in" exercise_u
+TESTMARK_RAW_ERRORS=1 testmarktest_run doc.md' "PASS  doc.md: a/in (exercise_u)
+--- stderr
+
+--- status 0"
+
 run_case "the caller's set -e doesn't stop a run partway" '[testmark]:# (a/in)
 ```
 ```
@@ -224,7 +241,7 @@ testmarktest_run doc.md' "PASS  doc.md: a/in (exercise_lc)
 
 --- status 0"
 
-run_case "other functions don't cross, the library included" "$(one $'/dev/fd/61: line 4: helper: command not found\nexit status: 127\n')" '
+run_case "other functions don't cross, the library included" "$(one $'exercise_h: helper: command not found\nexit status: 127\n')" '
 helper() { echo helped; }
 exercise_h() { declare -F | grep testmark || :; helper; }
 testmarktest_rule_basic "*/in" exercise_h
@@ -233,7 +250,7 @@ testmarktest_run doc.md' "PASS  doc.md: a/in (exercise_h)
 
 --- status 0"
 
-run_case "an error in a provided function counts lines from that function's start" "$(one $'/dev/fd/61: line 3: nope_cmd: command not found\nexit status: 127\n')" '
+run_case "an error in a provided function is labeled with that function's name" "$(one $'helper: nope_cmd: command not found\nexit status: 127\n')" '
 helper() {
 	nope_cmd
 }