Uh oh!
There was an error while loading. Please reload this page.
fix(assert): assert_json_equals no longer passes on unparseable JSON - #967
Merged
Conversation
./bashunit assert json_equals "not json at all" "completely different garbage" # exit 0 -- passed Both sides were piped through `jq -S '.'` with stderr discarded and the exit code ignored. jq prints nothing on invalid input, so both operands became the empty string and compared equal. Two arbitrary pieces of garbage matched each other, and a user asserting against malformed JSON got a green test. This is the worst failure shape for a test framework: not a crash, a false pass. The fix gates the comparison on jq's exit status as well as its output. Either side failing to parse is now a failure, which also means `assert_json_equals "abc" "abc"` fails where it previously passed -- correctly, since neither operand is JSON and the assertion cannot be meaningfully true. Behaviour deliberately unchanged elsewhere: `require_jq` still returns early when jq is absent, so a missing jq does not turn into "invalid input" and existing skips are preserved; key-order insensitivity via `-S` still holds. Of the two tests added, only the both-sides-invalid one reproduces the bug -- one valid side against one invalid side already failed correctly. The other is a regression guard, not a second reproduction, and is labelled as such rather than padding the count. 1635 sequential / 1594 parallel, both baseline + 2.
Uh oh!
There was an error while loading. Please reload this page.
Chemaclass added a commit
that referenced
this pull request
Aug 2, 2026
./bashunit assert date_within_delta "banana" "banana" "5" # exit 0 ./bashunit assert date_within_delta "" "" "5" # exit 0 bashunit::date::to_epoch signals an unparseable date by returning 1 and echoing the raw input back instead of a number. All five assert_date_* functions captured only the echoed value with a plain `x="$(...)"` and dropped the return code, so the raw string flowed into an unguarded integer comparison. Depending on the garbage that either crashed with a bare "integer expression expected" shell error, or coerced to 0 -- which made two equally-unparseable values compare equal to each other and pass. A false pass is the worst outcome a test framework can produce, and this is the second instance of the same shape found in this sweep, after assert_json_equals in #967. Both came from discarding an exit status while keeping the output. Also fixes to_epoch's handling of "": the all-digits fast path matched an empty string vacuously (no non-digit characters) and returned it as a valid epoch rather than letting it fall through to the parsing cascade, which rejects it. Two details worth recording. The helper returns through the fixed slot _BASHUNIT_DATE_EPOCH_OUT rather than an outvar name plus eval. bash-style.md prefers the slot and calls the eval form a last resort, and the reason applies directly here: the eval form's internal locals can be shadowed by a caller naming its outvar the same thing, which is the bug behind PR #672 -- and the style guide requires a shadowing regression test for that form. All five callers are in this file and read the slot on the next line, so one slot suffices and the hazard disappears instead of needing a test to police it. Callers chain `|| return 0`, not bare `|| return`. Every other failure path here ends in `fail_with; return`, which returns 0, so a failing assertion leaves the test function's status at 0. Propagating 1 would make runner/exec.sh's `[ "$test_exit_code" -ne 0 ]` check report the one cause as both Failed and Error. 1642 sequential / 1601 parallel; baseline + 7 new tests, all RED first.
This was referenced Aug 8, 2026
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 freeto 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
Both sides were piped through
jq -S '.'with stderr discarded and the exit code ignored. jq prints nothing on invalid input, so both operands became the empty string and compared equal. Two arbitrary pieces of garbage matched each other, and a user asserting against malformed JSON got a green test.This is the worst failure shape for a test framework: not a crash, a false pass.
💡 Changes
assert_json_equals "abc" "abc"fails where it previously passed, correctly, since neither operand is JSON and the assertion cannot be meaningfully trueDeliberately unchanged:
require_jqstill returns early when jq is absent, so a missingjqdoesn't become "invalid input" and existing skips are preserved. Key-order insensitivity via-Sstill holds — verified{"b":2,"a":1}vs{"a":1,"b":2}still passes.🧪 On the tests
Two added, but only the both-sides-invalid one reproduces the bug — one valid side against one invalid side already failed correctly before the fix. The other is a regression guard, not a second reproduction. Flagging that rather than presenting both as evidence of the same defect.
Confirmed RED before / GREEN after against the unfixed source.
✅ Verification
make sa·make lint·bash build.sh bin -v→✅ Build verified ✅· 1635 sequential / 1594 parallel-simple-strict — baseline + the 2 new tests.