From 9e6325038d36a4db212f3988e4d2f543c00f9ea8 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 8 Aug 2026 14:49:25 +0200 Subject: [PATCH 1/2] fix(runner): a quoted shell-error phrase is no longer a runtime error Closes #992. detect_runtime_error decided whether a failing test also hit a *shell* error by scanning its captured output for phrases like "command not found". That capture includes bashunit's own rendering of the failure, so any message quoting one of those strings was misread and the test reported twice -- once as Failed, once as Error -- for a single cause. It is reachable without touching the framework. A test whose subject is error handling naturally has these strings in its output: asserting that a script prints a useful message, capturing stderr from a command that failed. When such a test fails, the phrase is in the capture and the failure is misclassified. Passing tests were never affected, which is why this stayed invisible until a test started failing -- exactly when clear reporting matters. The discriminator is the prefix bash puts on its own diagnostics: /tmp/rd2.sh: line 2: /etc/passwd: Permission denied Checked across command-not-found, unbound variable, permission denied, no such file, syntax error, division by 0, readonly variable and integer-expression- expected, from inside a sourced test file. All carry ": line N: ". bashunit's own output never does. Two groups, because they are prefixed for different reasons. Parser and expansion diagnostics require the prefix on the same line as the phrase. "killed", "segmentation fault" and "cannot allocate memory" come from job control rather than the parser and can arrive without it, so they keep the old whole-capture match; none of them appears in bashunit's own output, so they carry no false-positive risk. The first attempt applied the prefix rule to everything and broke two existing contracts -- one pinning "process: killed" and one pinning the extracted message's shape. Both were right to fail: the extraction still runs over the whole capture, not the matched line, so the message is byte-identical to before. This also lifts the constraint #991 worked around: assert_true could not say "command not found" without triggering the duplicate. Verified by restoring that wording and confirming no Error appears. The phrasing stays "unknown command" because it is already documented and released, not because it has to be; the comment in assert/core.sh says so now. 1708 sequential / 1667 parallel; baseline + 2, RED first. --- CHANGELOG.md | 1 + src/assert/core.sh | 10 +-- src/runner/diagnostics.sh | 61 ++++++++++++++++--- ...hunit_runtime_error_classification_test.sh | 30 +++++++++ .../runtime_error/quotes_a_diagnostic.sh | 10 +++ .../fixtures/runtime_error/real_error.sh | 6 ++ 6 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 tests/acceptance/bashunit_runtime_error_classification_test.sh create mode 100644 tests/acceptance/fixtures/runtime_error/quotes_a_diagnostic.sh create mode 100644 tests/acceptance/fixtures/runtime_error/real_error.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index f07ed27d..737d345c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Internal: Split `src/runner.sh` and `src/coverage.sh` into focused modules with no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925) ### Fixed +- A failing test whose output quotes a shell-error phrase is no longer also reported as a runtime `Error`. The classifier scanned the whole capture -- which includes bashunit's own failure rendering -- for strings like `command not found`, so one cause was reported twice. It now requires the source-and-line prefix bash puts on a real diagnostic (#992) - `assert_have_been_called_times` and `assert_have_been_called_nth_with` report a usage error when their numeric argument is not a number, instead of leaking `[: my_cmd: integer expression expected` from inside bashunit. The common cause is swapping the count and the spy, which the message now names (#984) - `assert_false` no longer passes when the command does not exist. Exit code 127 is non-zero, so a typo in the command name satisfied the assertion while testing nothing; 127 and 126 now fail both `assert_true` and `assert_false`, because they mean the command never ran - `assert_within_delta` accepts a leading `+` on any operand (#979) diff --git a/src/assert/core.sh b/src/assert/core.sh index 71d25557..45eefa33 100755 --- a/src/assert/core.sh +++ b/src/assert/core.sh @@ -137,10 +137,12 @@ _BASHUNIT_ASSERT_EXIT_DESC_OUT="" # word rather than evaluated, and the failure used to point nowhere near the # cause. # -# The wording deliberately avoids the literal phrase "command not found". -# runner/diagnostics.sh classifies a test as a runtime error by scanning its -# output for that exact string, so using it here made every such failure report -# as both Failed and Error for the one cause. +# The wording avoided the literal phrase "command not found" because +# runner/diagnostics.sh used to classify a test as a runtime error by scanning +# its output for that exact string, which made every such failure report as both +# Failed and Error. That constraint is gone -- the classifier now requires a +# shell diagnostic's source-and-line prefix as well (#992). The phrasing stays +# as it is because it is already documented and released, not because it must. # Arguments: $1 - exit code, $2 - the command as written ## function bashunit::assert::_describe_exit_code() { diff --git a/src/runner/diagnostics.sh b/src/runner/diagnostics.sh index acf66b27..f79a8aab 100644 --- a/src/runner/diagnostics.sh +++ b/src/runner/diagnostics.sh @@ -75,20 +75,67 @@ $usage_after" return fi + # Conditions the shell reports without a source-and-line prefix, because they + # come from job control rather than the parser. Matched anywhere in the + # capture, as before. + case "$runtime_output" in + *"killed"* | *"segmentation fault"* | *"cannot allocate memory"*) + local runtime_error="${runtime_output#*: }" + _BASHUNIT_RUNNER_RUNTIME_ERROR_OUT="${runtime_error//$'\n'/}" + return + ;; + esac + + # Everything else is a diagnostic bash emits with its source and line + # ("file.sh: line 12: foo: command not found"). The cheap gate first: most + # failing tests contain none of these phrases and pay one glob. case "$runtime_output" in *"command not found"* | *"unbound variable"* | *"permission denied"* | \ *"no such file or directory"* | *"syntax error"* | *"bad substitution"* | \ - *"division by 0"* | *"cannot allocate memory"* | *"bad file descriptor"* | \ - *"segmentation fault"* | *"illegal option"* | *"argument list too long"* | \ - *"readonly variable"* | *"missing keyword"* | *"killed"* | \ + *"division by 0"* | *"bad file descriptor"* | \ + *"illegal option"* | *"argument list too long"* | \ + *"readonly variable"* | *"missing keyword"* | \ *"cannot execute binary file"* | *"invalid arithmetic operator"* | \ *"ambiguous redirect"* | *"integer expression expected"* | \ *"too many arguments"* | *"value too great"* | \ - *"not a valid identifier"* | *"unexpected EOF"*) - local runtime_error="${runtime_output#*: }" - _BASHUNIT_RUNNER_RUNTIME_ERROR_OUT="${runtime_error//$'\n'/}" - ;; + *"not a valid identifier"* | *"unexpected EOF"*) ;; + *) return ;; esac + + # A phrase alone is not enough. The capture also carries bashunit's own + # rendering of the failure, and a test whose subject is error handling will + # legitimately quote one of these strings as data -- both used to be misread + # as runtime errors and reported twice, as Failed and as Error, for one cause. + # Requiring the prefix on the same line separates what the shell said from what + # we said about it; bashunit's own output never carries it. + local line + while IFS= read -r line; do + case "$line" in + *": line "[0-9]*": "*) ;; + *) continue ;; + esac + + case "$line" in + *"command not found"* | *"unbound variable"* | *"permission denied"* | \ + *"no such file or directory"* | *"syntax error"* | *"bad substitution"* | \ + *"division by 0"* | *"bad file descriptor"* | \ + *"illegal option"* | *"argument list too long"* | \ + *"readonly variable"* | *"missing keyword"* | \ + *"cannot execute binary file"* | *"invalid arithmetic operator"* | \ + *"ambiguous redirect"* | *"integer expression expected"* | \ + *"too many arguments"* | *"value too great"* | \ + *"not a valid identifier"* | *"unexpected EOF"*) + # Extract from the whole capture, not the matched line: the message shape + # (leading source stripped, newlines removed) is pinned by + # tests/unit/runner/diagnostics_test.sh. + local runtime_error="${runtime_output#*: }" + _BASHUNIT_RUNNER_RUNTIME_ERROR_OUT="${runtime_error//$'\n'/}" + return + ;; + esac + done <&1) || exit_code=$? + + assert_same 1 "$exit_code" + assert_contains "✗ Failed: Quotes a shell diagnostic as data" "$output" + assert_not_contains "✗ Error: Quotes a shell diagnostic as data" "$output" +} + +# The regression guard: a test that really does hit a shell error must still be +# classified as one. +function test_a_real_shell_error_is_still_reported() { + local fixture=tests/acceptance/fixtures/runtime_error/real_error.sh + local output exit_code=0 + + output=$(NO_COLOR=1 ./bashunit --no-parallel --skip-env-file "$fixture" 2>&1) || exit_code=$? + + assert_same 1 "$exit_code" + assert_contains "✗ Error: Hits a real shell error" "$output" + assert_contains "command not found" "$output" +} diff --git a/tests/acceptance/fixtures/runtime_error/quotes_a_diagnostic.sh b/tests/acceptance/fixtures/runtime_error/quotes_a_diagnostic.sh new file mode 100644 index 00000000..0f4b8b44 --- /dev/null +++ b/tests/acceptance/fixtures/runtime_error/quotes_a_diagnostic.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash + +# A test whose subject is error handling: its output legitimately contains the +# text of a shell diagnostic, as data. The test fails on its assertion, but +# nothing went wrong at runtime -- it must not also be reported as an Error. +function test_quotes_a_shell_diagnostic_as_data() { + local captured="bash: some_tool: command not found" + + assert_same "expected something else" "$captured" +} diff --git a/tests/acceptance/fixtures/runtime_error/real_error.sh b/tests/acceptance/fixtures/runtime_error/real_error.sh new file mode 100644 index 00000000..0361872b --- /dev/null +++ b/tests/acceptance/fixtures/runtime_error/real_error.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash + +function test_hits_a_real_shell_error() { + definitely_not_a_real_command_xyz + assert_same 1 1 +} From 0b986b8e2bd33652996160b6974dbd5e4606e683 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 8 Aug 2026 14:53:49 +0200 Subject: [PATCH 2/2] test(runner): pin the runtime-error guard to LC_ALL=C bash translates its diagnostics and the classifier's phrase list is English only, so under the Spanish and Japanese CI locales a genuine 'command not found' is never recognised. That is pre-existing -- the list has always been English -- and not what this test is about, so it forces the C locale and keeps testing the classification mechanics. The localization gap is filed separately. --- .../bashunit_runtime_error_classification_test.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/acceptance/bashunit_runtime_error_classification_test.sh b/tests/acceptance/bashunit_runtime_error_classification_test.sh index bae6b0ca..a14529c6 100644 --- a/tests/acceptance/bashunit_runtime_error_classification_test.sh +++ b/tests/acceptance/bashunit_runtime_error_classification_test.sh @@ -18,11 +18,17 @@ function test_quoting_a_diagnostic_is_not_a_runtime_error() { # The regression guard: a test that really does hit a shell error must still be # classified as one. +# +# Pinned to LC_ALL=C on purpose. bash translates its diagnostics, and the phrase +# list this classifier matches on is English only, so under a Spanish or +# Japanese locale a genuine "command not found" is not recognised at all. That +# is a pre-existing limitation, not something this test should assert about; +# forcing the C locale keeps it testing the classification mechanics. function test_a_real_shell_error_is_still_reported() { local fixture=tests/acceptance/fixtures/runtime_error/real_error.sh local output exit_code=0 - output=$(NO_COLOR=1 ./bashunit --no-parallel --skip-env-file "$fixture" 2>&1) || exit_code=$? + output=$(LC_ALL=C NO_COLOR=1 ./bashunit --no-parallel --skip-env-file "$fixture" 2>&1) || exit_code=$? assert_same 1 "$exit_code" assert_contains "✗ Error: Hits a real shell error" "$output"