From 46dc143ddbdd3ff0dcfcb70289cc43b75ac172d1 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Mon, 27 Jul 2026 21:44:30 +0200 Subject: [PATCH] fix(doubles): fail call assertions on a name that was never spied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Call assertions resolved their state from a dynamically-named global. When the name was never spied that global is unset, the call count reads as 0, and the assertion behaved as if the command existed and simply had not been invoked. `assert_not_called tuoch` therefore passed while asserting nothing — worst on the two assertions agents reach for to prove a guard clause held. Track registration alongside the call count and fail with a distinct message naming the cause. `assert_not_called` / `_times 0` on a registered spy still pass; an unmocked spy fails like an unknown name. `tests/acceptance/mock_test.sh` proved clear_mocks by asserting 0 calls on a cleared spy, which is exactly the case now rejected; it asserts the new failure instead, which is the stronger signal. Also drop the two references to `assert_fails` in `.claude/` — no such assertion exists in the framework. Closes #895 --- .claude/rules/testing.md | 9 +++- .claude/skills/add-assertion/SKILL.md | 3 +- CHANGELOG.md | 1 + docs/ai-agents.md | 3 ++ docs/public/bashunit-skill.md | 4 ++ docs/test-doubles.md | 12 ++++++ src/test_doubles.sh | 59 ++++++++++++++++++++++++--- tests/acceptance/mock_test.sh | 7 +++- tests/functional/doubles_test.sh | 42 +++++++++++++++++++ 9 files changed, 131 insertions(+), 9 deletions(-) diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index b9c617b5..03ba6e4c 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -33,7 +33,14 @@ assert_file_not_exists "$path" assert_directory_exists "$path" assert_array_contains "value" "${array[@]}" assert_array_not_contains "value" "${array[@]}" -assert_fails "assert_equals 'a' 'b'" +``` + +There is no `assert_fails`. To assert that an assertion *fails*, capture its output and +compare it with the renderer: + +```bash +assert_same "$(bashunit::console_results::print_failed_test "My test" "a" "but got " "b")" \ + "$(assert_same "a" "b")" ``` ## Test Doubles diff --git a/.claude/skills/add-assertion/SKILL.md b/.claude/skills/add-assertion/SKILL.md index 4bf4355d..810a9bf4 100644 --- a/.claude/skills/add-assertion/SKILL.md +++ b/.claude/skills/add-assertion/SKILL.md @@ -33,7 +33,8 @@ Read `src/assertions.sh` and `tests/unit/assert_test.sh` to understand: For each test in inventory, follow RED -> GREEN -> REFACTOR: 1. **Basic success case** — assertion passes with valid input -2. **Failure case** — assertion fails correctly, use `assert_fails` +2. **Failure case** — assertion fails correctly: capture its output and compare it with + `bashunit::console_results::print_failed_test` (there is no `assert_fails`) 3. **Edge cases** — empty input, special characters, nested structures, malformed data ### 4. Integration diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ae0196a..93526da9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - The `--parallel` unsupported-OS warning no longer claims Alpine is excluded ### Fixed +- Call assertions (`assert_not_called`, `assert_have_been_called*`) fail with `was never registered as a spy` instead of reporting zero calls when the name was never spied — a typo used to pass silently (#895) - The per-argument form a spy records was written with a literal `$'\x1f'` separator instead of the byte, so it could not be compared against (#894) - `--parallel` no longer discards worker stderr written outside a test body; it renders as a `Stderr from ` block (#864) - The minimum-bash gate compares the minor version and parses suffixed versions; the floor is unchanged at **Bash 3.0+** diff --git a/docs/ai-agents.md b/docs/ai-agents.md index 1567d19b..04941214 100644 --- a/docs/ai-agents.md +++ b/docs/ai-agents.md @@ -114,6 +114,9 @@ actually make against this API: - Spy assertion argument order is inconsistent — check, don't guess: `assert_have_been_called_times ` but `assert_have_been_called_with [call_index]`. +- A call assertion on a name that was never passed to `bashunit::spy` fails with + `was never registered as a spy`. Spies do not survive across tests, so register the + spy inside the test that asserts on it. - `assert_have_been_called_with` joins the expected arguments with spaces, so it cannot tell `cmd "a b"` from `cmd a b` — a quoting bug passes it. Use `assert_have_been_called_with_args ...` (no `call_index`) to compare diff --git a/docs/public/bashunit-skill.md b/docs/public/bashunit-skill.md index bce8ab90..b7879174 100644 --- a/docs/public/bashunit-skill.md +++ b/docs/public/bashunit-skill.md @@ -165,6 +165,10 @@ failing_command || ec=$? # correct local out; out=$(failing_command) # WRONG under set -e ``` +**Spies do not survive across tests, and a call assertion on an unregistered name fails** +with `was never registered as a spy` (it used to report zero calls, so `assert_not_called` +with a typo passed while asserting nothing). Register the spy in the test that asserts on it. + **`assert_have_been_called_with` joins arguments with spaces**, so it cannot see argument boundaries: `touch "a b"` also satisfies `assert_have_been_called_with touch "a" "b"`. When an argument may contain a space — any path — use `assert_have_been_called_with_args`, which diff --git a/docs/test-doubles.md b/docs/test-doubles.md index 98bfae25..fbf1be6f 100644 --- a/docs/test-doubles.md +++ b/docs/test-doubles.md @@ -127,6 +127,18 @@ function test_example() { ``` ::: +::: tip Call assertions require a registered spy +Every `assert_have_been_called*` / `assert_not_called` assertion fails when the name it targets was never passed to `bashunit::spy` — or was removed by `bashunit::unmock`, or belongs to a previous test: + +``` +✗ Failed: My test + Expected 'tuoch' + was never registered as a spy; call it first with 'bashunit::spy tuoch' +``` + +Without that check a typo would report zero calls, so `assert_not_called tuoch` would pass while asserting nothing. Spies are cleared between tests, so spy inside the test that asserts on them. +::: + ## assert_have_been_called > `assert_have_been_called "spy"` diff --git a/src/test_doubles.sh b/src/test_doubles.sh index 0cf23d5e..e73ddc88 100644 --- a/src/test_doubles.sh +++ b/src/test_doubles.sh @@ -8,21 +8,37 @@ declare -a _BASHUNIT_MOCKED_FUNCTIONS=() # resolves `_BASHUNIT_SPY_foo_TIMES_FILE` instead. _BASHUNIT_SPY_TIMES_OUT=0 +_BASHUNIT_SPY_REGISTERED_OUT=false # Reads the recorded call count for $1 into _BASHUNIT_SPY_TIMES_OUT (0 when -# never spied/called). Shared by the call-count assertions instead of each -# repeating the "resolve times file, cat it, default to 0" sequence. +# never called) and whether $1 is a live spy into _BASHUNIT_SPY_REGISTERED_OUT. +# Shared by the call-count assertions instead of each repeating the "resolve +# times file, cat it, default to 0" sequence. function bashunit::spy::times_to_slot() { local command="$1" local variable variable="$(bashunit::helper::normalize_variable_name "$command")" local file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE" _BASHUNIT_SPY_TIMES_OUT=0 + _BASHUNIT_SPY_REGISTERED_OUT=false + if [ -n "${!file_var-}" ]; then + _BASHUNIT_SPY_REGISTERED_OUT=true + fi if [ -f "${!file_var-}" ]; then _BASHUNIT_SPY_TIMES_OUT=$(cat "${!file_var}" 2>/dev/null || builtin echo 0) fi } +# Fails the calling assertion because $1 is not a registered spy. Reporting +# "never called" for a name that was never spied — a typo, or a spy that was +# unmocked — makes the assertion vacuous while still printing green. +# Arguments: $1 - command, $2 - test label +function bashunit::spy::fail_unregistered() { + bashunit::state::add_assertions_failed + bashunit::console_results::print_failed_test "$2" "$1" \ + "was never registered as a spy; call it first with" "bashunit::spy $1" +} + _BASHUNIT_SPY_SERIALIZED_OUT="" # Serializes the given arguments into _BASHUNIT_SPY_SERIALIZED_OUT exactly the @@ -137,6 +153,11 @@ function assert_have_been_called() { local times=$_BASHUNIT_SPY_TIMES_OUT local label="${2:-$(bashunit::helper::normalize_test_function_name "${FUNCNAME[1]}")}" + if [ "$_BASHUNIT_SPY_REGISTERED_OUT" = false ]; then + bashunit::spy::fail_unregistered "$command" "$label" + return + fi + if [ "$times" -eq 0 ]; then bashunit::state::add_assertions_failed bashunit::console_results::print_failed_test "${label}" "${command}" "to have been called" "once" @@ -166,6 +187,14 @@ function assert_have_been_called_with() { local variable variable="$(bashunit::helper::normalize_variable_name "$command")" local file_var="_BASHUNIT_SPY_${variable}_PARAMS_FILE" + local label + label="$(bashunit::helper::normalize_test_function_name "${FUNCNAME[1]}")" + + if [ -z "${!file_var-}" ]; then + bashunit::spy::fail_unregistered "$command" "$label" + return + fi + local line="" if [ -f "${!file_var-}" ]; then if [ -n "$index" ]; then @@ -180,8 +209,7 @@ function assert_have_been_called_with() { if [ "$expected" != "$raw" ]; then bashunit::state::add_assertions_failed - bashunit::console_results::print_failed_test "$(bashunit::helper::normalize_test_function_name \ - "${FUNCNAME[1]}")" "$expected" "but got " "$raw" + bashunit::console_results::print_failed_test "$label" "$expected" "but got " "$raw" return fi @@ -198,6 +226,14 @@ function assert_have_been_called_with_args() { local variable variable="$(bashunit::helper::normalize_variable_name "$command")" local file_var="_BASHUNIT_SPY_${variable}_PARAMS_FILE" + local label + label="$(bashunit::helper::normalize_test_function_name "${FUNCNAME[1]}")" + + if [ -z "${!file_var-}" ]; then + bashunit::spy::fail_unregistered "$command" "$label" + return + fi + local line="" if [ -f "${!file_var-}" ]; then line=$(tail -n 1 "${!file_var}" 2>/dev/null || true) @@ -208,8 +244,8 @@ function assert_have_been_called_with_args() { if [ "$expected" != "$actual" ]; then bashunit::state::add_assertions_failed - bashunit::console_results::print_failed_test "$(bashunit::helper::normalize_test_function_name \ - "${FUNCNAME[1]}")" "${expected//$'\x1f'/ }" "but got " "${actual//$'\x1f'/ }" + bashunit::console_results::print_failed_test "$label" \ + "${expected//$'\x1f'/ }" "but got " "${actual//$'\x1f'/ }" return fi @@ -222,6 +258,12 @@ function assert_have_been_called_times() { bashunit::spy::times_to_slot "$command" local times=$_BASHUNIT_SPY_TIMES_OUT local label="${3:-$(bashunit::helper::normalize_test_function_name "${FUNCNAME[1]}")}" + + if [ "$_BASHUNIT_SPY_REGISTERED_OUT" = false ]; then + bashunit::spy::fail_unregistered "$command" "$label" + return + fi + if [ "$times" -ne "$expected_count" ]; then bashunit::state::add_assertions_failed bashunit::console_results::print_failed_test "${label}" "${command}" \ @@ -248,6 +290,11 @@ function assert_have_been_called_nth_with() { bashunit::spy::times_to_slot "$command" local times=$_BASHUNIT_SPY_TIMES_OUT + if [ "$_BASHUNIT_SPY_REGISTERED_OUT" = false ]; then + bashunit::spy::fail_unregistered "$command" "$label" + return + fi + if [ "$nth" -gt "$times" ]; then bashunit::state::add_assertions_failed bashunit::console_results::print_failed_test "${label}" \ diff --git a/tests/acceptance/mock_test.sh b/tests/acceptance/mock_test.sh index 40a483a9..8a07c507 100644 --- a/tests/acceptance/mock_test.sh +++ b/tests/acceptance/mock_test.sh @@ -16,5 +16,10 @@ function test_runner_clear_mocks_first() { function test_runner_clear_mocks_second() { assert_not_equals "foo" "$(ls)" - assert_have_been_called_times 0 ps + # The spy registered by the first test is gone, so a call assertion on `ps` + # now reports an unregistered spy rather than zero calls. + assert_same \ + "$(bashunit::console_results::print_failed_test "Runner clear mocks second" "ps" \ + "was never registered as a spy; call it first with" "bashunit::spy ps")" \ + "$(assert_have_been_called_times 0 ps)" } diff --git a/tests/functional/doubles_test.sh b/tests/functional/doubles_test.sh index 4ba6b86a..f4128314 100644 --- a/tests/functional/doubles_test.sh +++ b/tests/functional/doubles_test.sh @@ -127,6 +127,48 @@ function test_spy_call_with_args_detects_wrong_argument_boundaries() { "$(assert_have_been_called_with_args spy_boundary_command "a" "b")" } +function test_assert_not_called_fails_when_the_command_was_never_spied() { + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Assert not called fails when the command was never spied" \ + "never_spied_command" "was never registered as a spy; call it first with" \ + "bashunit::spy never_spied_command")" \ + "$(assert_not_called never_spied_command)" +} + +function test_call_assertions_fail_when_the_command_was_never_spied() { + local label="Call assertions fail when the command was never spied" + local expected + expected="$(bashunit::console_results::print_failed_test "$label" \ + "never_spied_command" "was never registered as a spy; call it first with" \ + "bashunit::spy never_spied_command")" + + assert_same "$expected" "$(assert_have_been_called never_spied_command)" + assert_same "$expected" "$(assert_have_been_called_times 0 never_spied_command)" + assert_same "$expected" "$(assert_have_been_called_with never_spied_command "x")" + assert_same "$expected" "$(assert_have_been_called_with_args never_spied_command "x")" + assert_same "$expected" "$(assert_have_been_called_nth_with 1 never_spied_command "x")" +} + +function test_call_assertions_fail_after_the_spy_was_unmocked() { + bashunit::spy spy_to_be_unmocked + bashunit::unmock spy_to_be_unmocked + + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Call assertions fail after the spy was unmocked" \ + "spy_to_be_unmocked" "was never registered as a spy; call it first with" \ + "bashunit::spy spy_to_be_unmocked")" \ + "$(assert_not_called spy_to_be_unmocked)" +} + +function test_assert_not_called_still_passes_on_a_registered_spy() { + bashunit::spy spy_never_invoked + + assert_empty "$(assert_not_called spy_never_invoked)" + assert_empty "$(assert_have_been_called_times 0 spy_never_invoked)" +} + function test_spy_on_echo_does_not_hang() { source ./tests/functional/fixtures/echo_function.sh bashunit::spy echo