diff --git a/CHANGELOG.md b/CHANGELOG.md index bdc51ad3..6899ee13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,13 @@ ## Unreleased ### Changed +- `assert_within_delta` compares in fixed-point integer arithmetic instead of shelling out to `bc`/`awk` twice per call, falling back to the old chain for operands it cannot represent exactly. 200 calls went from 1092ms to 165ms, against a 108ms fork-free floor - Spies are substantially cheaper. `assert_have_been_called` and the spy call counter dropped their `cat` and command-substitution forks in favour of the `read` builtin and existing return-slot helpers: a 200-call spy test went from 1111ms to 170ms, against a 111ms fork-free floor - `assert_contains_ignore_case` folds case with `shopt -s nocasematch` on Bash 3.1+ instead of two `tr` subprocesses, and falls back to `tr` only on Bash 3.0. Roughly 10x faster in a run dominated by that assertion (300 calls: 1419ms -> 131ms), with identical results including non-ASCII folding - 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_within_delta` accepts a leading `+` on any operand. `bashunit::assert::_is_numeric` allowed it but `bc` cannot parse it, so `assert_within_delta +5 5 1` compared against an empty string and failed - `BASHUNIT_SHARD_INDEX` / `BASHUNIT_SHARD_TOTAL` set directly (for example in `.bashunitrc`) are now validated. Only the `--shard` flag path parsed them, so a zero or non-numeric total reached raw arithmetic and printed a bare `division by 0` shell error while still exiting 0, and an out-of-range index silently reported `No tests found` - The `assert_date_*` assertions no longer accept unparseable input. They discarded `bashunit::date::to_epoch`'s failure signal, so a raw non-numeric string reached integer comparison: it either crashed with a bare shell error or coerced to epoch 0, which made two equally-invalid values compare equal — `assert_date_within_delta "" "" "5"` passed - `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 diff --git a/src/assert/core.sh b/src/assert/core.sh index 158bcc3a..6a57fc6f 100755 --- a/src/assert/core.sh +++ b/src/assert/core.sh @@ -881,6 +881,57 @@ function assert_within_delta() { return fi + # A leading `+` is valid to _is_numeric but not to bc, which returns an empty + # string for `+5 - 5` and made the comparison below fail. Stripped once here so + # both the fixed-point path and the bc/awk fallback see a plain number. + expected=${expected#+} + actual=${actual#+} + delta=${delta#+} + + # Fork-free path: bring all three operands to one decimal scale, then compare + # as integers. The bc/awk chain below costs a subshell plus a process, twice, + # on a per-assertion path. bc also cannot parse a leading `+`, which + # _is_numeric accepts, so `assert_within_delta +5 5 1` used to fail with an + # empty comparison result rather than pass. + local scale expected_places actual_places delta_places + bashunit::math::decimals_to_slot "$expected" + expected_places=$_BASHUNIT_MATH_DECIMALS_OUT + bashunit::math::decimals_to_slot "$actual" + actual_places=$_BASHUNIT_MATH_DECIMALS_OUT + bashunit::math::decimals_to_slot "$delta" + delta_places=$_BASHUNIT_MATH_DECIMALS_OUT + scale=$expected_places + if [ "$actual_places" -gt "$scale" ]; then + scale=$actual_places + fi + if [ "$delta_places" -gt "$scale" ]; then + scale=$delta_places + fi + + local padded_expected padded_actual padded_delta + bashunit::math::pad_to_slot "$expected" "$scale" + padded_expected=$_BASHUNIT_MATH_PADDED_OUT + bashunit::math::pad_to_slot "$actual" "$scale" + padded_actual=$_BASHUNIT_MATH_PADDED_OUT + bashunit::math::pad_to_slot "$delta" "$scale" + padded_delta=$_BASHUNIT_MATH_PADDED_OUT + + if bashunit::math::scale_pair_to_slots "$padded_expected" "$padded_actual"; then + local scaled_diff=$((_BASHUNIT_MATH_SCALED_L_OUT - _BASHUNIT_MATH_SCALED_R_OUT)) + if [ "$scaled_diff" -lt 0 ]; then + scaled_diff=$((-scaled_diff)) + fi + if bashunit::math::scale_pair_to_slots "$padded_delta" "$padded_expected"; then + if [ "$scaled_diff" -gt "$_BASHUNIT_MATH_SCALED_L_OUT" ]; then + bashunit::assert::fail_with "" "${actual}" "to be within ${delta} of" "${expected}" + return + fi + + bashunit::state::add_assertions_passed + return + fi + fi + local diff diff="$(bashunit::math::calculate "$expected - $actual")" case "$diff" in diff --git a/src/util/math.sh b/src/util/math.sh index 4831b2a7..d5cf3880 100644 --- a/src/util/math.sh +++ b/src/util/math.sh @@ -32,6 +32,143 @@ function bashunit::math::calculate() { echo "$result" } +_BASHUNIT_MATH_PADDED_OUT="" + +## +# Pads $1 to exactly $2 decimal places into _BASHUNIT_MATH_PADDED_OUT, so a set +# of operands can be scaled against one shared power of ten. Pure string work, +# no fork and no arithmetic, so it is safe on any operand shape. +# Arguments: $1 - decimal operand, $2 - target number of decimal places +## +function bashunit::math::pad_to_slot() { + local value=$1 + local places=$2 + + case "$value" in + *.*) ;; + *) value="${value}." ;; + esac + + local frac=${value#*.} + while [ ${#frac} -lt "$places" ]; do + frac="${frac}0" + done + + _BASHUNIT_MATH_PADDED_OUT="${value%%.*}.$frac" +} + +_BASHUNIT_MATH_DECIMALS_OUT=0 + +## +# Number of decimal places in $1 into _BASHUNIT_MATH_DECIMALS_OUT, or 0 when it +# has none. A slot rather than an echo: the caller needs this three times per +# assertion, and three `$( )` captures would cost more than the two `bc` forks +# this whole path exists to avoid. +# Arguments: $1 - decimal operand +## +function bashunit::math::decimals_to_slot() { + local frac + case "$1" in + *.*) + frac=${1#*.} + _BASHUNIT_MATH_DECIMALS_OUT=${#frac} + ;; + *) _BASHUNIT_MATH_DECIMALS_OUT=0 ;; + esac +} + +_BASHUNIT_MATH_SCALED_L_OUT="" +_BASHUNIT_MATH_SCALED_R_OUT="" + +## +# Scales two decimal operands to a common integer scale so they can be compared +# with plain `[ ]` arithmetic, writing them into +# _BASHUNIT_MATH_SCALED_L_OUT / _BASHUNIT_MATH_SCALED_R_OUT. No fork: the +# alternative is `bc` or `awk`, and both this and bashunit::math::is_le sit on a +# per-assertion path where that costs a subshell plus a process. +# +# Deliberately narrow. It handles a plain decimal with an optional sign and +# nothing else, and refuses anything it cannot represent exactly in 64-bit +# integer arithmetic, so callers keep their existing bc/awk chain as a fallback +# rather than this quietly returning a wrong answer. +# +# Arguments: $1 - left operand, $2 - right operand +# Returns: 0 and sets both slots, 1 when the pair must go to the fallback +## +function bashunit::math::scale_pair_to_slots() { + local left=$1 right=$2 + + # Exponent notation and anything non-numeric goes to the fallback. + case "$left$right" in + '' | *[!0-9.+-]* | *e* | *E*) return 1 ;; + esac + + local left_sign=1 right_sign=1 + case "$left" in + -*) left_sign=-1 left=${left#-} ;; + +*) left=${left#+} ;; + esac + case "$right" in + -*) right_sign=-1 right=${right#-} ;; + +*) right=${right#+} ;; + esac + # A sign anywhere but the front is not a plain decimal. + case "$left$right" in + *-* | *+*) return 1 ;; + esac + + local left_int left_frac right_int right_frac + case "$left" in + *.*) left_int=${left%%.*} left_frac=${left#*.} ;; + *) left_int=$left left_frac="" ;; + esac + case "$right" in + *.*) right_int=${right%%.*} right_frac=${right#*.} ;; + *) right_int=$right right_frac="" ;; + esac + # A second dot survives the split above. + case "$left_int$left_frac$right_int$right_frac" in + *.*) return 1 ;; + esac + + left_int=${left_int:-0} + right_int=${right_int:-0} + + # Pad the shorter fraction so both sides share one scale. + while [ ${#left_frac} -lt ${#right_frac} ]; do left_frac="${left_frac}0"; done + while [ ${#right_frac} -lt ${#left_frac} ]; do right_frac="${right_frac}0"; done + + # 18 digits keeps the scaled value inside a signed 64-bit integer. + if [ $((${#left_int} + ${#left_frac})) -gt 18 ] || + [ $((${#right_int} + ${#right_frac})) -gt 18 ]; then + return 1 + fi + + # Strip leading zeros; $(( )) reads a leading zero as octal. + while [ ${#left_int} -gt 1 ]; do + case "$left_int" in 0*) left_int=${left_int#0} ;; *) break ;; esac + done + while [ ${#right_int} -gt 1 ]; do + case "$right_int" in 0*) right_int=${right_int#0} ;; *) break ;; esac + done + local left_frac_value=${left_frac:-0} right_frac_value=${right_frac:-0} + while [ ${#left_frac_value} -gt 1 ]; do + case "$left_frac_value" in 0*) left_frac_value=${left_frac_value#0} ;; *) break ;; esac + done + while [ ${#right_frac_value} -gt 1 ]; do + case "$right_frac_value" in 0*) right_frac_value=${right_frac_value#0} ;; *) break ;; esac + done + + local power=1 i=0 + while [ "$i" -lt ${#left_frac} ]; do + power=$((power * 10)) + i=$((i + 1)) + done + + _BASHUNIT_MATH_SCALED_L_OUT=$((left_sign * (left_int * power + left_frac_value))) + _BASHUNIT_MATH_SCALED_R_OUT=$((right_sign * (right_int * power + right_frac_value))) +} + ## # Numeric <= comparison that tolerates decimal operands. Plain `[ -le ]` # exits 2 ("integer expression expected") on a fractional value instead of @@ -46,6 +183,12 @@ function bashunit::math::is_le() { local left="$1" local right="$2" + # Fork-free for plain decimals, which is nearly all of them. + if bashunit::math::scale_pair_to_slots "$left" "$right"; then + [ "$_BASHUNIT_MATH_SCALED_L_OUT" -le "$_BASHUNIT_MATH_SCALED_R_OUT" ] + return + fi + if bashunit::dependencies::has_bc; then [ "$(echo "$left <= $right" | bc)" = "1" ] return diff --git a/tests/unit/assert/numeric_test.sh b/tests/unit/assert/numeric_test.sh index ab9166b4..7b7c8e81 100644 --- a/tests/unit/assert/numeric_test.sh +++ b/tests/unit/assert/numeric_test.sh @@ -189,3 +189,29 @@ function test_unsuccessful_assert_within_delta_with_a_non_numeric_value() { "abc 105 3" "to all be numeric" "but got a non-numeric value")" \ "$(assert_within_delta "abc" "105" "3")" } + +# bc cannot parse a leading `+`, but bashunit::assert::_is_numeric accepts one, +# so this pair used to reach the comparison, get an empty result back, and fail +# the assertion. The fixed-point path handles the sign itself. +function test_assert_within_delta_accepts_a_leading_plus() { + assert_within_delta "+5" "5" "1" + assert_within_delta "5" "+5" "1" +} + +# The fixed-point path deliberately refuses operands it cannot represent exactly +# in 64-bit integer arithmetic and hands them to the bc/awk chain. This one has +# more digits than that path allows, so it exercises the fallback rather than +# the fast path -- and must still give the same answer. +function test_assert_within_delta_falls_back_for_very_high_precision() { + assert_within_delta "1.00000000000000000001" "1.00000000000000000002" "0.1" +} + +function test_assert_within_delta_compares_mixed_precision_operands() { + assert_within_delta "100" "100.0001" "0.001" + assert_within_delta "1.000" "1" "0" + assert_within_delta "3.14159" "3.1416" "0.0001" +} + +function test_assert_within_delta_handles_negative_operands() { + assert_within_delta "-2.5" "-2.4" "0.2" +}