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