From 29d4d99cf1c69c1e16bb6eda0c087fd5f97c2274 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 8 Aug 2026 13:58:18 +0200 Subject: [PATCH] feat(assert): name the problem instead of printing exit code 127 Closes #982. assert_true and assert_false ran their argument as a command and reported a bare number when it failed: Expected 'command or function with zero exit code' but got 'exit code: 127' 127 is the shell's not-found code and 126 its not-executable code. Both are now named, with the argument that produced them, so the failure points at its cause. The case that motivated this is not an exotic one. `assert_true "[ -d /tmp ]"` is valid bash that works in an `if`, and it produces exactly this failure, because the argument is run as a command word rather than evaluated. A reader seeing `exit code: 127` has no reason to suspect the shape of their argument and will go and look at /tmp first. assert_false had the worse half of the same problem. It failed only on exit code 0, so 127 counted as "non-zero, therefore false" and a typo in the command name satisfied the assertion: assert_false "definitley_not_a_command" # passed That is a false pass -- the assertion reported success while running nothing. 126 and 127 now fail both assertions, because they mean the command never ran, which is neither true nor false. The wording avoids the literal phrase "command not found" on purpose. runner/diagnostics.sh classifies a test as a runtime error by scanning its output for that exact string, so the obvious phrasing made every one of these failures report as both Failed and Error for a single cause. Found by writing it the obvious way first and watching the duplicate appear. The underlying fragility -- the framework detecting shell errors by string-matching its own output stream -- is filed separately. The docs describe the bracket trap without using brackets: `bashunit doc` strips them while rendering, so the first draft rendered as `assert_true " -d /tmp "` in the CLI, which teaches the wrong lesson. Snapshot regenerated. 1697 sequential / 1656 parallel; baseline + 3, all RED first. --- CHANGELOG.md | 2 + docs/assertions.md | 6 +++ src/assert/core.sh | 44 ++++++++++++++++--- ...it_should_display_all_assert_docs.snapshot | 6 +++ tests/unit/assert/basic_test.sh | 34 ++++++++++++++ 5 files changed, 86 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69142dca..9f5aa18e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Named snapshot assertions support multiple snapshots per test; mismatches show the resolved path and `--snapshot-update` hint (#986) ### Changed +- `assert_true` / `assert_false` name the problem instead of printing a bare number: exit code 127 now reports `unknown command: ` and 126 `not executable: `. The most common cause is passing a test expression like `[ -d /tmp ]`, which is run as a command word - Core comparison assertions report missing required arguments as usage errors instead of comparing against empty values (#983) - Performance: Literal snapshots bypass placeholder regex processing unless they contain a placeholder (about 13x faster) (#985) - Performance: `assert_within_delta` uses fixed-point arithmetic for common values, with a `bc`/`awk` fallback for unsupported inputs (about 6.6x faster) (#979) @@ -14,6 +15,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 +- `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) - Invalid `BASHUNIT_SHARD_INDEX` / `BASHUNIT_SHARD_TOTAL` values now fail with a clear error instead of reaching raw arithmetic or reporting no tests - Date assertions reject unparseable values instead of crashing or treating them as epoch 0 diff --git a/docs/assertions.md b/docs/assertions.md index 6bdfba93..3591065d 100644 --- a/docs/assertions.md +++ b/docs/assertions.md @@ -36,6 +36,12 @@ to narrow it (`bashunit doc json`). ## assert_true > `assert_true bool|function|command` +Takes a **command**, not a test expression. A bracketed condition of the form +used inside `if` is run as a command word, so the shell reports exit code 127 and +the assertion fails with `unknown command`. Use the `test` builtin instead — +`assert_true "test -d /tmp"` — or a purpose-built assertion such as +`assert_directory_exists`. + Reports an error if the argument result in a truthy value: `true` or `0`. - [assert_false](#assert-false) is similar but different. diff --git a/src/assert/core.sh b/src/assert/core.sh index ecc9d569..59beb74c 100755 --- a/src/assert/core.sh +++ b/src/assert/core.sh @@ -114,6 +114,30 @@ function bashunit::fail() { bashunit::console_results::print_failure_message "${label}" "$message" } +_BASHUNIT_ASSERT_EXIT_DESC_OUT="" + +## +# Describes a failing exit code for assert_true / assert_false. 127 and 126 are +# the two codes the shell reserves for "I could not run this at all", and a bare +# number tells the reader nothing: the most natural shell idiom, +# `assert_true "[ -d /tmp ]"`, hits 127 because the argument is run as a command +# 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. +# Arguments: $1 - exit code, $2 - the command as written +## +function bashunit::assert::_describe_exit_code() { + case "$1" in + 127) _BASHUNIT_ASSERT_EXIT_DESC_OUT="unknown command: $2" ;; + 126) _BASHUNIT_ASSERT_EXIT_DESC_OUT="not executable: $2" ;; + *) _BASHUNIT_ASSERT_EXIT_DESC_OUT="exit code: $1" ;; + esac +} + function assert_true() { bashunit::assert::should_skip && return 0 @@ -140,7 +164,9 @@ function assert_true() { local exit_code=$? if [ "$exit_code" -ne 0 ]; then - bashunit::handle_bool_assertion_failure "command or function with zero exit code" "exit code: $exit_code" + bashunit::assert::_describe_exit_code "$exit_code" "$actual" + bashunit::handle_bool_assertion_failure \ + "command or function with zero exit code" "$_BASHUNIT_ASSERT_EXIT_DESC_OUT" else bashunit::state::add_assertions_passed fi @@ -171,11 +197,17 @@ function assert_false() { bashunit::run_command_or_eval "$actual" local exit_code=$? - if [ "$exit_code" -eq 0 ]; then - bashunit::handle_bool_assertion_failure "command or function with non-zero exit code" "exit code: $exit_code" - else - bashunit::state::add_assertions_passed - fi + # 127/126 mean the command never ran. Treating "did not run" as "returned + # false" let a typo in the command name satisfy this assertion while testing + # nothing, so those are failures here as well as in assert_true. + case "$exit_code" in + 0 | 126 | 127) + bashunit::assert::_describe_exit_code "$exit_code" "$actual" + bashunit::handle_bool_assertion_failure \ + "command or function with non-zero exit code" "$_BASHUNIT_ASSERT_EXIT_DESC_OUT" + ;; + *) bashunit::state::add_assertions_passed ;; + esac } function bashunit::run_command_or_eval() { diff --git a/tests/acceptance/snapshots/bashunit_test_sh.test_bashunit_should_display_all_assert_docs.snapshot b/tests/acceptance/snapshots/bashunit_test_sh.test_bashunit_should_display_all_assert_docs.snapshot index cdcc7766..53f416a7 100644 --- a/tests/acceptance/snapshots/bashunit_test_sh.test_bashunit_should_display_all_assert_docs.snapshot +++ b/tests/acceptance/snapshots/bashunit_test_sh.test_bashunit_should_display_all_assert_docs.snapshot @@ -2,6 +2,12 @@ -------------- > `assert_true bool|function|command` +Takes a **command**, not a test expression. A bracketed condition of the form +used inside `if` is run as a command word, so the shell reports exit code 127 and +the assertion fails with `unknown command`. Use the `test` builtin instead — +`assert_true "test -d /tmp"` — or a purpose-built assertion such as +`assert_directory_exists`. + Reports an error if the argument result in a truthy value: `true` or `0`. - assert_false is similar but different. diff --git a/tests/unit/assert/basic_test.sh b/tests/unit/assert/basic_test.sh index 01102c2e..2872cbf8 100644 --- a/tests/unit/assert/basic_test.sh +++ b/tests/unit/assert/basic_test.sh @@ -183,3 +183,37 @@ function test_assert_not_same_with_custom_label() { "$(bashunit::console_results::print_failed_test "my custom label" "foo" "to not be" "foo")" \ "$(assert_not_same "foo" "foo" "my custom label")" } + +# Exit code 127 is the shell's not-found code. Reporting the bare number +# gives the reader nothing to act on -- and the most natural shell idiom, +# a `[ ... ]` test expression, produces exactly this because assert_true runs +# its argument as a command word rather than evaluating it. +function test_assert_true_reports_a_missing_command_by_name() { + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Assert true reports a missing command by name" \ + "command or function with zero exit code" "but got " \ + "unknown command: definitely_not_a_command")" \ + "$(assert_true "definitely_not_a_command")" +} + +function test_assert_true_reports_a_bracket_expression_as_not_found() { + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Assert true reports a bracket expression as not found" \ + "command or function with zero exit code" "but got " \ + "unknown command: [ -d /tmp ]")" \ + "$(assert_true "[ -d /tmp ]")" +} + +# assert_false only failed when the exit code was 0, so a command that does not +# exist satisfied it: 127 is non-zero, therefore "false". A typo in the command +# name made the assertion pass while testing nothing. +function test_assert_false_fails_when_the_command_does_not_exist() { + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Assert false fails when the command does not exist" \ + "command or function with non-zero exit code" "but got " \ + "unknown command: definitely_not_a_command")" \ + "$(assert_false "definitely_not_a_command")" +}