From 4df4b150bc4e806571361e1e49f4f5d3feea30d3 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 2 Aug 2026 12:33:10 +0200 Subject: [PATCH] fix(assert): assert_json_equals no longer passes on unparseable JSON ./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. --- CHANGELOG.md | 1 + src/assert/json.sh | 13 +++++++++---- tests/unit/assert/json_test.sh | 28 ++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 905cde45..0c893f44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Internal: `src/runner.sh` and `src/coverage.sh` are split into `src/runner/` and `src/coverage/` modules of single-responsibility files, each behind a `source`-only `index.sh` aggregator. A pure relocation, no behavior change; see [ADR-010](adrs/adr-010-src-module-directories.md) (#924, #925) ### Fixed +- `assert_json_equals` no longer reports two invalid (unparseable) JSON strings as equal. It sorted both sides with `jq -S` but never checked jq's exit code, so two differently-invalid inputs both silently sorted to an empty string and compared equal instead of failing - Parallel runs no longer lose results when two test files in different directories share a filename. Per-test results were bucketed by basename, so the second file overwrote the first — silently, with the run still green. A tests/ tree mirroring a src/ tree makes that layout ordinary (#959) - Coverage no longer counts variable assignments as functions. A line like `URL="https://${host}/api"` was reported as a function, inflating `FNF`/`FNH` in the LCOV report (17 phantom entries in bashunit's own run); when the value also contained a `|`, the malformed record aborted the LCOV writer with a raw bash arithmetic error (#936) - `build.sh` dedupes embedded files by repo-relative path. The previous basename key compared the top-level loop's relative paths against the recursion's absolute ones, so a file reached from two places could be bundled twice in the released binary; it also collided for same-named files in different directories (#923) diff --git a/src/assert/json.sh b/src/assert/json.sh index 2fb7f419..0a0be553 100644 --- a/src/assert/json.sh +++ b/src/assert/json.sh @@ -53,12 +53,17 @@ function assert_json_equals() { local expected="$1" local actual="$2" - local expected_sorted - expected_sorted=$(printf '%s' "$expected" | jq -S '.' 2>/dev/null) + # jq -S prints nothing (not an error message) on invalid JSON, so its exit + # code -- not just its output -- has to gate the comparison: two inputs that + # both fail to parse would otherwise both sort to "" and compare equal, + # reporting unparseable input as matching JSON instead of failing. + local expected_sorted actual_valid=true expected_valid=true + expected_sorted=$(printf '%s' "$expected" | jq -S '.' 2>/dev/null) || expected_valid=false local actual_sorted - actual_sorted=$(printf '%s' "$actual" | jq -S '.' 2>/dev/null) + actual_sorted=$(printf '%s' "$actual" | jq -S '.' 2>/dev/null) || actual_valid=false - if [ "$expected_sorted" != "$actual_sorted" ]; then + if [ "$expected_valid" = false ] || [ "$actual_valid" = false ] || + [ "$expected_sorted" != "$actual_sorted" ]; then bashunit::assert::fail_with "" "${expected}" "but got " "${actual}" return fi diff --git a/tests/unit/assert/json_test.sh b/tests/unit/assert/json_test.sh index 885a2016..0583cc8e 100644 --- a/tests/unit/assert/json_test.sh +++ b/tests/unit/assert/json_test.sh @@ -75,3 +75,31 @@ function test_unsuccessful_assert_json_equals() { "$expected" "but got " "$actual")" \ "$(assert_json_equals "$expected" "$actual")" } + +# jq -S silently produces empty output (not a parse-error message) on invalid +# JSON; without checking its exit code, two differently-invalid or identically +# unparseable inputs both sort to "" and compare equal, turning "not JSON at +# all" into a false pass. +function test_unsuccessful_assert_json_equals_when_expected_is_invalid_json() { + if [ "$_JQ_AVAILABLE" = false ]; then bashunit::skip "jq required"; return; fi + local expected='not json' + local actual='{"a":1}' + + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Unsuccessful assert json equals when expected is invalid json" \ + "$expected" "but got " "$actual")" \ + "$(assert_json_equals "$expected" "$actual")" +} + +function test_unsuccessful_assert_json_equals_when_both_sides_are_invalid_json() { + if [ "$_JQ_AVAILABLE" = false ]; then bashunit::skip "jq required"; return; fi + local expected='not json' + local actual='also not json' + + assert_same \ + "$(bashunit::console_results::print_failed_test \ + "Unsuccessful assert json equals when both sides are invalid json" \ + "$expected" "but got " "$actual")" \ + "$(assert_json_equals "$expected" "$actual")" +}