fix(runner): a quoted shell-error phrase is no longer a runtime error

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)

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() {

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 <<EOF
+$runtime_output
+EOF
 }
 
 ##

tests/acceptance/bashunit_runtime_error_classification_test.sh

@@ -0,0 +1,36 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+# The classifier scans a failing test's captured output for shell-error phrases.
+# That output also carries bashunit's own rendering, so a failure message -- or
+# a test that legitimately quotes a diagnostic -- used to be misread as a
+# runtime error and reported twice, as Failed and as Error, for one cause.
+function test_quoting_a_diagnostic_is_not_a_runtime_error() {
+  local fixture=tests/acceptance/fixtures/runtime_error/quotes_a_diagnostic.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 "✗ 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.
+#
+# 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=$(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"
+  assert_contains "command not found" "$output"
+}

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"
+}

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
+}