fix(doubles): a non-numeric spy count leaked a raw shell error - #996
Merged
Conversation
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.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Background
Related #984
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_withhad the identical exposure on$nth.💡 Changes
Both now report a usage error through the channel #989 introduced, naming the argument order that was got wrong:
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.✅ What the issue asked for that was already done
Checked before writing anything, rather than assumed:
docs/assertions.mdalready cross-references the order difference from both entries ("the spy comes first here, but second inassert_have_been_called_times")fail_unregisteredpath already names what was read as the spyThe only gap left was the crash, so that's all this changes.
🔒 Verification
1 new acceptance test, RED first.
make sa·make lint·bash build.sh bin -v→✅ Build verified ✅· 1706 sequential / 1665 parallel-simple-strict.