From 960846539b00ac7e7240db2c8ee69f80bd9d158f Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Thu, 6 Aug 2026 21:42:38 +0200 Subject: [PATCH 1/2] perf(assert): compare assert_within_delta in fixed point, not via bc assert_within_delta called bashunit::math::calculate twice, and each call is a subshell wrapping a `bc` (or `awk`) process. Four forks per assertion on a per-assertion path: 200 calls took 1092ms where the fork-free floor is ~108ms. They now take 165ms. The comparison is |expected - actual| <= delta, which needs no floating point at all. All three operands are padded to one decimal scale and compared as integers, in pure bash. The fast path is deliberately narrow and refuses what it cannot represent exactly -- exponent notation, a sign anywhere but the front, or enough digits to risk 64-bit overflow -- so those fall through to the existing bc/awk chain rather than getting a quietly wrong answer. Both paths were run against the full numeric suite: forcing the fixed-point path to always refuse leaves every test green, which is the check that they agree. This also fixes a bug rather than only moving it. _is_numeric accepts a leading `+`, but bc cannot parse one: `+5 - 5` returned an empty string, which compared unequal to "1", so `assert_within_delta +5 5 1` failed. The sign is now stripped once before either path, so the fallback is fixed too and not just bypassed. bashunit::math::is_le gets the same fast path; it had the identical bc > awk > strip-decimals chain for the same reason. The three helpers return through slots rather than echoing. That is not stylistic here: the caller needs the decimal count three times per assertion, and three `$( )` captures would cost more than the two bc forks the whole change exists to remove. Bash 3.0 safe: parameter expansion, `case` and integer arithmetic only. Fork budgets unchanged; compat gate green; 1661 sequential / 1620 parallel. --- CHANGELOG.md | 2 + src/assert/core.sh | 51 +++++++++++ src/util/math.sh | 143 ++++++++++++++++++++++++++++++ tests/unit/assert/numeric_test.sh | 26 ++++++ 4 files changed, 222 insertions(+) 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" +} From 65097777339a80dc7e4c8327c859c56eb32ae9a9 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Fri, 7 Aug 2026 12:18:53 +0200 Subject: [PATCH 2/2] chore(ci): retrigger pull request checks