From 8f8f772fe23be94d78605877ac2249215890fa13 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Fri, 14 Aug 2026 11:17:17 +0200 Subject: [PATCH] fix(doubles): the misuse advice must name a helper that exists Passing a quoted command line where a name belongs reported "is not a usable command name for mock; pass arguments after it, as in 'mock ls -l'". A bare `mock` is `command not found` -- the rule this API's docs stress hardest -- so following the advice got the reader nowhere. The example was wrong for every caller too: spy takes a single name, `mock ls -l` means "mock ls with -l as the body", and mock_sequence expects answers rather than arguments. The real mistake is quoting a command line as the name, so the advice is now to name the command alone; mock already forwards the call's own arguments to the replacement. Run the recommended form in the tests rather than only string-matching the message, since string-matching is what let this survive. Those tests assert on behaviour, not on captured output: under --simple the rendered failure is a one-character marker. Closes #1229 --- CHANGELOG.md | 1 + src/doubles/mock.sh | 11 +++++++--- src/doubles/spy.sh | 2 +- tests/functional/doubles_test.sh | 37 ++++++++++++++++++++++++++++++-- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 218bdfa1..4ca7bfbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ - Performance: `--coverage` is roughly 5x faster and `--coverage-report-html` roughly 19x — a run over this repo went from 16.2s to 2.9s, and a 128-file HTML report from 58.7s to 3.1s. The report phase emits each format in one awk invocation per run instead of Bash loops and forks per file and per row, and the capture path writes records straight to disk, normalizes a path with one fork instead of four, and reads each cache once (#1092, #1096, #1098, #1099, #1102, #1104, #1110, #1117) ### Fixed +- The mock/spy misuse message names a helper that exists. Passing a quoted command line where a name belongs advised `mock ls -l`, but a bare `mock` is `command not found` — the rule this API's docs stress hardest — and "pass arguments after it" described none of the three callers. It now names `bashunit::mock`/`bashunit::spy`/`bashunit::mock_sequence` and tells you to name the command alone (#1229) - A JSON test skipped because `jq` is missing is reported under its own name instead of `bashunit::assert_json::require_jq`. On a machine without `jq` every JSON test rendered under that one internal name, so `--show-skipped` could not tell you which tests had not run (#1223) - `assert_exec "cmd" --exit 1` works under `--strict`: the command ran with `eval` and its status was read on the next line, so `set -e` aborted the test before the assertion — the one assertion whose job is checking an exit code could not check a failing one. Successful commands were unaffected, which is why it went unnoticed (#1207) - `bashunit bench` reports `No benchmarks found` and exits non-zero instead of printing a header and exiting 0 when the path does not exist, or holds no `bench_` function — a typo left a CI benchmark job green having measured nothing (#1199) diff --git a/src/doubles/mock.sh b/src/doubles/mock.sh index 4ac77e11..d846ffc8 100644 --- a/src/doubles/mock.sh +++ b/src/doubles/mock.sh @@ -70,8 +70,13 @@ function bashunit::doubles::refuse_unusable_name() { # Through fail_with, like an assertion: it labels the failure with the test # name and counts it, so the misuse is visible in a default run rather than # only under --strict. + # $fn is the namespaced helper: a bare `mock` is `command not found`, so + # advice built from the short name sends the reader nowhere (#1229). The + # example names the command alone, which is true for all three callers -- + # spy takes only a name, and mock forwards the call's own arguments to the + # replacement, so the name never carries them. bashunit::assert::fail_with "" "$command" \ - "is not a usable command name for $fn; pass arguments after it, as in" "$fn ls -l" + "is not a usable command name for $fn; name the command alone, as in" "$fn ls" return 0 ;; esac @@ -84,7 +89,7 @@ function bashunit::mock() { local command=$1 shift - if bashunit::doubles::refuse_unusable_name "mock" "$command"; then + if bashunit::doubles::refuse_unusable_name "bashunit::mock" "$command"; then return 1 fi @@ -120,7 +125,7 @@ function bashunit::mock_sequence() { local command=$1 shift - if bashunit::doubles::refuse_unusable_name "mock_sequence" "$command"; then + if bashunit::doubles::refuse_unusable_name "bashunit::mock_sequence" "$command"; then return 1 fi diff --git a/src/doubles/spy.sh b/src/doubles/spy.sh index 279d0f25..ef6b1817 100644 --- a/src/doubles/spy.sh +++ b/src/doubles/spy.sh @@ -195,7 +195,7 @@ function bashunit::spy() { local command=$1 local exit_code_or_impl="${2:-}" - if bashunit::doubles::refuse_unusable_name "spy" "$command"; then + if bashunit::doubles::refuse_unusable_name "bashunit::spy" "$command"; then return 1 fi diff --git a/tests/functional/doubles_test.sh b/tests/functional/doubles_test.sh index f28c8fa2..6547a2fe 100644 --- a/tests/functional/doubles_test.sh +++ b/tests/functional/doubles_test.sh @@ -358,17 +358,50 @@ function test_a_destructive_command_is_never_reached() { function test_mock_refuses_a_name_with_arguments() { assert_same \ "$(bashunit::console_results::print_failed_test "Mock refuses a name with arguments" \ - "ls -l" "is not a usable command name for mock; pass arguments after it, as in" "mock ls -l")" \ + "ls -l" "is not a usable command name for bashunit::mock; name the command alone, as in" \ + "bashunit::mock ls")" \ "$(bashunit::mock "ls -l" echo hi)" } function test_spy_refuses_a_name_with_shell_syntax() { assert_same \ "$(bashunit::console_results::print_failed_test "Spy refuses a name with shell syntax" \ - "foo;bar" "is not a usable command name for spy; pass arguments after it, as in" "spy ls -l")" \ + "foo;bar" "is not a usable command name for bashunit::spy; name the command alone, as in" \ + "bashunit::spy ls")" \ "$(bashunit::spy "foo;bar")" } +# The advice named a bare `mock`, which is `command not found` -- the exact rule +# this API's docs stress hardest (#1229). The two tests above pin the wording +# (both sides go through the same renderer, so they hold in every output mode); +# these run the form that wording recommends, which is what string-matching +# alone could never do. +# +# Do not capture the rendered failure here to inspect it: under `--simple` +# `print_line` emits a one-character marker, so the message would be "F". +function test_the_recommended_form_names_the_command_alone() { + bashunit::mock ls echo hi + + assert_same "hi" "$(ls)" +} + +# mock forwards the call's own arguments to the replacement, which is why the +# name never needs to carry them -- the premise of the advice above. +function test_a_mocked_command_still_receives_its_arguments() { + bashunit::mock ls echo hi + + assert_same "hi -l" "$(ls -l)" +} + +# spy takes a single name, so the recommended form is its whole interface. +function test_a_spy_named_alone_records_a_call_with_arguments() { + bashunit::spy ls + + ls -l + + assert_have_been_called ls +} + # Narrow on purpose: these are legal function names in bash and legitimate # commands to mock, so the guard must not reject them. function test_mock_accepts_names_bash_allows() {