Skip to content

fix(runner): runtime errors are detected by string-matching the test's own output #992

Description

@Chemaclass

Summary

bashunit::runner::_classify_runtime_error (src/runner/diagnostics.sh) decides whether a failing test also suffered a shell error by scanning its captured output for a list of literal phrases:

case "$runtime_output" in
*"command not found"* | *"unbound variable"* | *"permission denied"* | \
  *"no such file or directory"* | *"syntax error"* | *"bad substitution"* | \
  … )

The output being scanned is the test's whole captured stream, which includes text bashunit itself wrote. So any failure message that happens to quote one of these phrases is misread as a shell error, and the test is reported twice — once as Failed, once as Error — for one cause.

How it surfaced

While implementing #982 I made assert_true report command not found: <arg> instead of a bare exit code: 127. That is the natural wording, and it immediately produced:

✗ Failed: True missing
✗ Error: True missing

I worked around it by choosing wording that avoids the phrase (unknown command:), which is why #982 does not carry this bug. But the workaround is a landmine for the next person: the most descriptive phrasing for a not-found command is the one that must not be used, and nothing in the code says so except a comment I added at the call site.

The general case

The same collision is reachable without any framework change. A test whose subject is error handling will naturally have these strings in its output — asserting that a script prints a useful message, capturing stderr from a command that failed, snapshotting an error path:

function test_reports_a_helpful_error() {
  local out; out=$(./my-installer --bogus 2>&1)
  assert_contains "SOMETHING"  "$out"     # $out contains "command not found"
}

When that assertion fails, the failure output carries the phrase and the test is additionally flagged as a runtime error. Passing tests are unaffected — the classifier only runs on the failure path — so this is invisible until a test starts failing, which is exactly when clear reporting matters most.

Why this is worth fixing rather than documenting

The double report is not just noise. Failed and Error mean different things in this framework — one is "your assertion did not hold", the other is "your test could not run properly" — and conflating them sends the reader looking for a broken test when the test is fine.

Proposal

The root problem is that one string carries two things: what the command under test emitted, and what bashunit rendered about it. Options, roughly in order of preference:

  1. Classify from the command's output only, before bashunit's own failure rendering is appended. If the two are separable at capture time, this removes the ambiguity entirely rather than narrowing it.
  2. Anchor the patterns. Real shell diagnostics arrive as bash: line N: foo: command not found — prefixed with a source and line. Matching that shape instead of a bare substring would ignore prose that merely mentions the phrase. Narrower, still heuristic.
  3. Use the exit code where one is available. 127/126 are unambiguous and need no string matching. This would not cover every entry in the list, but it would cover the most common ones.

Whatever is chosen, the comment at the assert_true call site in src/assert/core.sh explaining why the phrase is avoided should be removed as part of it.

Constraints

  • Bash 3.0+; case and parameter expansion only, no new syntax surface.
  • This runs on the per-test failure path — no new fork. See .claude/rules/perf-fork-budget.md.
  • Failure output is compared verbatim across the acceptance suite, so any change to what is classified will move snapshots; regenerate deliberately and read the diff.
  • The classifier's existing behaviour on genuine shell errors must not regress — those are the reason it exists.

Acceptance criteria

  • A failing assertion whose message or captured output contains command not found is reported as Failed only, not also Error
  • A test that genuinely hits a shell error is still reported as Error — regression covered by a test
  • Reproduced first: a test that fails while its output legitimately quotes one of the listed phrases
  • The assert_true wording constraint in src/assert/core.sh is lifted, or the comment explaining it is updated to say it is no longer required
  • make sa · make lint · ./bashunit --parallel --simple --strict tests/ · bash build.sh bin -v

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions