From ecf192601842aabcecc0074cbdb7a8bbcd4fa509 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 8 Aug 2026 14:32:10 +0200 Subject: [PATCH] fix(doubles): a non-numeric spy count leaked a raw shell error Closes #984. assert_have_been_called_times my_cmd 1 ./src/doubles/assertions.sh: line 180: [: my_cmd: integer expression expected The count reached `[ "$times" -ne "$expected_count" ]` unchecked, so bash printed a diagnostic naming bashunit's own source file and line. The user's mistake was swapping two arguments; what they were shown was the framework's internals. assert_have_been_called_nth_with had the identical exposure on $nth. Both now report a usage error through the channel #989 introduced, naming the argument order that was got wrong: assert_have_been_called_times expects a numeric count first (expected_count, command), got 'my_cmd' The guard runs before the spy lookup, deliberately. Placed after it, a swapped call was diagnosed as "'1' was never registered as a spy" -- true, and a symptom. The ordering mistake is the cause, and naming it directly is what turns the failure into an instruction. The other two things this issue asked for were already in place and did not need changing. docs/assertions.md already cross-references the order difference from both entries, and the existing fail_unregistered path already names what was read as the spy. Checked before writing anything: the only gap left was the crash. 1706 sequential / 1665 parallel; baseline + 1, RED first. --- CHANGELOG.md | 1 + src/assert/core.sh | 13 +++++++++++ src/doubles/assertions.sh | 22 +++++++++++++++++++ tests/acceptance/bashunit_spy_usage_test.sh | 17 ++++++++++++++ .../fixtures/spy_usage/non_numeric.sh | 12 ++++++++++ 5 files changed, 65 insertions(+) create mode 100644 tests/acceptance/bashunit_spy_usage_test.sh create mode 100644 tests/acceptance/fixtures/spy_usage/non_numeric.sh 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 +}