diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fdd8263..f07ed27d 100644 --- a/CHANGELOG.md +++ b/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 +- `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) - Invalid `BASHUNIT_SHARD_INDEX` / `BASHUNIT_SHARD_TOTAL` values now fail with a clear error instead of reaching raw arithmetic or reporting no tests diff --git a/src/assert/core.sh b/src/assert/core.sh index 59beb74c..71d25557 100755 --- a/src/assert/core.sh +++ b/src/assert/core.sh @@ -22,6 +22,19 @@ function bashunit::assert::should_skip() { # Emits a machine-detectable assertion usage error. The runner strips the # prefix and reports the message through the existing Error channel. +## +# Emits a machine-detectable assertion usage error for an argument of the wrong +# *shape*, as opposed to a missing one. Same prefix as usage_error, so the +# runner strips it and reports through the existing Error channel. +# Arguments: $1 - assertion name, $2 - the rest of the sentence +## +function bashunit::assert::usage_error_detail() { + local assertion=$1 + local detail=$2 + + printf 'bashunit: assertion usage error: %s %s\n' "$assertion" "$detail" >&2 +} + function bashunit::assert::usage_error() { local assertion=$1 local required=$2 diff --git a/src/doubles/assertions.sh b/src/doubles/assertions.sh index b694a17f..ae5d81b3 100644 --- a/src/doubles/assertions.sh +++ b/src/doubles/assertions.sh @@ -164,6 +164,18 @@ function assert_have_been_called_with_any() { function assert_have_been_called_times() { local expected_count=$1 local command=$2 + + # Guarded before the spy lookup so a swapped call is diagnosed as the ordering + # mistake it is, rather than as "the count you passed is not a registered + # spy". Without this the value reaches `[ "$times" -ne "$expected_count" ]` + # and bash prints "integer expression expected" from inside bashunit. + case "$expected_count" in + '' | *[!0-9]*) + bashunit::assert::usage_error_detail "${FUNCNAME[0]}" \ + "expects a numeric count first (expected_count, command), got '$expected_count'" + return 2 + ;; + esac bashunit::spy::times_to_slot "$command" local times=$_BASHUNIT_SPY_TIMES_OUT local label="${3:-}" @@ -192,6 +204,16 @@ function assert_have_been_called_times() { function assert_have_been_called_nth_with() { local nth=$1 + + # Same guard as assert_have_been_called_times: $nth reaches an integer + # comparison below. + case "$nth" in + '' | *[!0-9]*) + bashunit::assert::usage_error_detail "${FUNCNAME[0]}" \ + "expects a numeric call index first (nth, command, expected_args), got '$nth'" + return 2 + ;; + esac local command=$2 shift 2 local expected="$*" diff --git a/tests/acceptance/bashunit_spy_usage_test.sh b/tests/acceptance/bashunit_spy_usage_test.sh new file mode 100644 index 00000000..572e878c --- /dev/null +++ b/tests/acceptance/bashunit_spy_usage_test.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +set -euo pipefail + +function test_non_numeric_spy_count_is_a_usage_error() { + local fixture=tests/acceptance/fixtures/spy_usage/non_numeric.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 "✗ Error: Swapped count and spy" "$output" + assert_contains \ + "assert_have_been_called_times expects a numeric count first (expected_count, command), got 'my_cmd'" \ + "$output" + # The raw shell diagnostic must not reach the user. + assert_not_contains "integer expression expected" "$output" +} diff --git a/tests/acceptance/fixtures/spy_usage/non_numeric.sh b/tests/acceptance/fixtures/spy_usage/non_numeric.sh new file mode 100644 index 00000000..901f171e --- /dev/null +++ b/tests/acceptance/fixtures/spy_usage/non_numeric.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +function my_cmd() { echo x; } + +# The spy and the count swapped: assert_have_been_called_times takes the count +# first. Without a guard this reaches `[ 1 -ne my_cmd ]` and leaks a raw +# "integer expression expected" from inside bashunit. +function test_swapped_count_and_spy() { + bashunit::spy my_cmd + my_cmd >/dev/null + assert_have_been_called_times my_cmd 1 +}