From f17c776887952b17cb13a51603298415490ea15c Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 8 Aug 2026 15:25:31 +0200 Subject: [PATCH] feat(assert): assert_true and assert_false accept a command with arguments Closes #994. assert_true test -d /tmp assert_true grep -q foo ./file assert_false test -d /nope The argument used to be run as a single command word. Anything with arguments was looked up as a command whose name contained spaces, so the natural spelling was the broken one and the workaround was an `eval ` prefix nobody had documented until #993. Arguments are now passed through as arguments. Nothing is re-parsed, so a path containing a space survives -- which the eval form cannot promise. Additive by construction: with exactly one argument the old path runs unchanged, including the literal `true`/`false`/`0`/`1` fast path and the `eval ` prefix. So every existing call means what it meant. The literal check is guarded on the argument count, because `assert_true true --version` is the command named true, not the boolean. The exit code is captured with `|| exit_code=$?` rather than a bare call: under --strict a failing command as a statement would abort the whole test. The alternative of splitting the single-argument string on whitespace was rejected. It would silently change what an existing string means, break arguments that legitimately contain spaces, and make the assertion sometimes-word-splitting. Passing real arguments needs no re-parsing at all. Verified across all eight shapes: literal true, literal 0, bare function, eval prefix, variadic, variadic with a space in a path, variadic assert_false, and `true --version` where the literal and the command collide. 1713 sequential / 1672 parallel; baseline + 5, RED first. --- CHANGELOG.md | 1 + docs/assertions.md | 25 +++-- src/assert/core.sh | 105 ++++++++++++------ ...it_should_display_all_assert_docs.snapshot | 11 +- tests/unit/assert/basic_test.sh | 39 +++++++ 5 files changed, 126 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 737d345c..b3ee13c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased ### Added +- `assert_true` and `assert_false` accept a command with its arguments — `assert_true test -d /tmp`. Arguments are passed through rather than re-parsed, so a value containing a space survives. A single argument keeps its previous meaning exactly, so existing calls are unaffected (#994) - `assert_is_symlink`, `assert_is_not_symlink` and `assert_symlink_to` assert on a symbolic link itself. Every other filesystem assertion follows the link, so a link and its target were indistinguishable and a dangling link read as "does not exist" (#981) ### Added diff --git a/docs/assertions.md b/docs/assertions.md index c3f16f10..de9c15f2 100644 --- a/docs/assertions.md +++ b/docs/assertions.md @@ -34,21 +34,24 @@ to narrow it (`bashunit doc json`). | **Manual failure** | [bashunit::fail](#bashunit-fail) | ## assert_true -> `assert_true bool|function|command` +> `assert_true bool|function|command [args...]` -The argument is run as a **single command word**, so it must be a bare command or -function name with no arguments. Anything with arguments — including a bracketed -condition of the form used inside `if` — is treated as one long command name, and -the assertion fails with `unknown command`. - -Prefix with `eval` to run anything more than a bare name: +Pass a command with its arguments as separate arguments: ```bash -assert_true "my_function" # bare name: works -assert_true "eval test -d /tmp" # arguments: needs eval -assert_true "eval grep -q foo file" # arguments: needs eval +assert_true test -d /tmp +assert_true grep -q foo ./file +assert_true my_function ``` +Arguments are passed through untouched, so a value containing a space survives. + +A **single** argument keeps its older meaning: it is run as one command word, so +`assert_true "test -d /tmp"` looks for a command with that whole name and fails +with `unknown command`. Quote the whole thing only with an `eval` prefix — +`assert_true "eval test -d /tmp"` — or, better, drop the quotes and use the form +above. + A purpose-built assertion is usually clearer still — `assert_directory_exists` rather than a hand-rolled `test -d`. @@ -83,7 +86,7 @@ function mock_false() { ::: ## assert_false -> `assert_false bool|function|command` +> `assert_false bool|function|command [args...]` Reports an error if the argument result in a falsy value: `false` or `1`. diff --git a/src/assert/core.sh b/src/assert/core.sh index 45eefa33..c03a287b 100755 --- a/src/assert/core.sh +++ b/src/assert/core.sh @@ -127,6 +127,33 @@ function bashunit::fail() { bashunit::console_results::print_failure_message "${label}" "$message" } +_BASHUNIT_ASSERT_BOOL_EXIT_OUT=0 + +## +# Runs the subject of assert_true / assert_false and leaves its exit code in +# _BASHUNIT_ASSERT_BOOL_EXIT_OUT. +# +# With more than one argument the arguments are passed through as arguments -- +# no re-parsing, so a path containing a space survives. With exactly one they go +# through run_command_or_eval, which is the historical behaviour: a bare command +# word, or an `eval `-prefixed string. Keeping that split is what makes the +# variadic form purely additive. +# +# The `|| exit_code=$?` capture is deliberate: a bare failing command as a +# statement would abort the whole test under --strict (`set -e`). +## +function bashunit::assert::_run_bool_subject() { + local exit_code=0 + + if [ $# -gt 1 ]; then + "$@" >/dev/null 2>&1 || exit_code=$? + else + bashunit::run_command_or_eval "$1" || exit_code=$? + fi + + _BASHUNIT_ASSERT_BOOL_EXIT_OUT=$exit_code +} + _BASHUNIT_ASSERT_EXIT_DESC_OUT="" ## @@ -158,25 +185,28 @@ function assert_true() { local actual="$1" - # Check for expected literal values first - case "$actual" in - "") - bashunit::handle_bool_assertion_failure "true or 0" "$actual" - return - ;; - "true" | "0") - bashunit::state::add_assertions_passed - return - ;; - "false" | "1") - bashunit::handle_bool_assertion_failure "true or 0" "$actual" - return - ;; - esac + # The literal values only mean themselves when they are the whole subject; + # with arguments following, "true" is the command named true. + if [ $# -eq 1 ]; then + case "$actual" in + "") + bashunit::handle_bool_assertion_failure "true or 0" "$actual" + return + ;; + "true" | "0") + bashunit::state::add_assertions_passed + return + ;; + "false" | "1") + bashunit::handle_bool_assertion_failure "true or 0" "$actual" + return + ;; + esac + fi - # Run command or eval and check the exit code - bashunit::run_command_or_eval "$actual" - local exit_code=$? + bashunit::assert::_run_bool_subject "$@" + local exit_code=$_BASHUNIT_ASSERT_BOOL_EXIT_OUT + actual="$*" if [ "$exit_code" -ne 0 ]; then bashunit::assert::_describe_exit_code "$exit_code" "$actual" @@ -192,25 +222,28 @@ function assert_false() { local actual="$1" - # Check for expected literal values first - case "$actual" in - "") - bashunit::handle_bool_assertion_failure "false or 1" "$actual" - return - ;; - "false" | "1") - bashunit::state::add_assertions_passed - return - ;; - "true" | "0") - bashunit::handle_bool_assertion_failure "false or 1" "$actual" - return - ;; - esac + # As in assert_true: the literal values only mean themselves when they are the + # whole subject. + if [ $# -eq 1 ]; then + case "$actual" in + "") + bashunit::handle_bool_assertion_failure "false or 1" "$actual" + return + ;; + "false" | "1") + bashunit::state::add_assertions_passed + return + ;; + "true" | "0") + bashunit::handle_bool_assertion_failure "false or 1" "$actual" + return + ;; + esac + fi - # Run command or eval and check the exit code - bashunit::run_command_or_eval "$actual" - local exit_code=$? + bashunit::assert::_run_bool_subject "$@" + local exit_code=$_BASHUNIT_ASSERT_BOOL_EXIT_OUT + actual="$*" # 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 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 8b113545..3d150e4f 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 @@ -1,18 +1,13 @@ ## assert_true -------------- -> `assert_true bool|function|command` +> `assert_true bool|function|command args...` -The argument is run as a **single command word**, so it must be a bare command or -function name with no arguments. Anything with arguments — including a bracketed -condition of the form used inside `if` — is treated as one long command name, and -the assertion fails with `unknown command`. - -Prefix with `eval` to run anything more than a bare name: +Pass a command with its arguments as separate arguments: ## assert_false -------------- -> `assert_false bool|function|command` +> `assert_false bool|function|command args...` Reports an error if the argument result in a falsy value: `false` or `1`. diff --git a/tests/unit/assert/basic_test.sh b/tests/unit/assert/basic_test.sh index 2872cbf8..7fee8d9c 100644 --- a/tests/unit/assert/basic_test.sh +++ b/tests/unit/assert/basic_test.sh @@ -217,3 +217,42 @@ function test_assert_false_fails_when_the_command_does_not_exist() { "unknown command: definitely_not_a_command")" \ "$(assert_false "definitely_not_a_command")" } + +# Arguments as real arguments. The single-argument forms are untouched: one +# argument still means "run this as a command word", so every existing call +# behaves exactly as before. +function test_assert_true_accepts_a_command_with_arguments() { + assert_empty "$(assert_true test -d /tmp)" +} + +function test_assert_true_variadic_fails_when_the_command_fails() { + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Assert true variadic fails when the command fails" \ + "command or function with zero exit code" "but got " \ + "exit code: 1")" \ + "$(assert_true test -d /definitely/not/a/directory)" +} + +function test_assert_false_accepts_a_command_with_arguments() { + assert_empty "$(assert_false test -d /definitely/not/a/directory)" +} + +function test_assert_true_variadic_does_not_re_parse_its_arguments() { + local dir + dir=$(bashunit::temp_dir) + # A path containing a space survives, because it is passed as one argument + # rather than re-split out of a single string. + mkdir -p "$dir/two words" + + assert_empty "$(assert_true test -d "$dir/two words")" +} + +function test_assert_true_variadic_reports_a_missing_command() { + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Assert true variadic reports a missing command" \ + "command or function with zero exit code" "but got " \ + "unknown command: definitely_not_a_command --flag")" \ + "$(assert_true definitely_not_a_command --flag)" +}