From bf78c277cc6f2673bd0b609d53bdadf842b9e12a Mon Sep 17 00:00:00 2001 From: chemaclass Date: Sat, 25 Jul 2026 12:20:29 +0200 Subject: [PATCH 1/7] docs: strip step-narrating comments from the assert CLI path exec_assert/exec_multi_assert in src/main.sh had a comment restating almost every line ("Check if the function exists", "Set test title for this assertion", etc.) added when multi-assertion mode landed. Remove them along with matching Arrange-Act-Assert narration in bashunit_init_test.sh and a redundant section header in bashunit_assert_multi_test.sh. Load-bearing comments (Bash 3 array guard, #877 reference, exit-code vs output assertion branches) are left untouched. --- src/main.sh | 13 ------------- tests/acceptance/bashunit_assert_multi_test.sh | 1 - tests/acceptance/bashunit_init_test.sh | 4 ---- 3 files changed, 18 deletions(-) diff --git a/src/main.sh b/src/main.sh index 3087f5ef..1d153386 100644 --- a/src/main.sh +++ b/src/main.sh @@ -1168,7 +1168,6 @@ function bashunit::main::exec_assert() { local assert_fn=$original_assert_fn - # Check if the function exists if ! type "$assert_fn" >/dev/null 2>&1; then assert_fn="assert_$assert_fn" if ! type "$assert_fn" >/dev/null 2>&1; then @@ -1187,14 +1186,12 @@ function bashunit::main::exec_assert() { exit 1 fi - # Get the last argument safely by calculating the array length local last_index=$((args_count - 1)) local last_arg="${args[$last_index]}" local output="" local inner_exit_code=0 local bashunit_exit_code=0 - # Handle different assert_* functions case "$assert_fn" in assert_exit_code) output=$(bashunit::main::handle_assert_exit_code "$last_arg") @@ -1213,10 +1210,8 @@ function bashunit::main::exec_assert() { assert_fn="assert_same" fi - # Set a friendly test title for CLI assert command output bashunit::state::set_test_title "assert ${original_assert_fn#assert_}" - # Run the assertion function and write into stderr "$assert_fn" "${args[@]}" 1>&2 bashunit_exit_code=$? @@ -1258,7 +1253,6 @@ function bashunit::main::exec_multi_assert() { local cmd="$1" shift - # Require at least one assertion if [ $# -lt 1 ]; then printf "%sError: Multi-assertion mode requires at least one assertion.%s\n" \ "${_BASHUNIT_COLOR_FAILED}" "${_BASHUNIT_COLOR_DEFAULT}" 1>&2 @@ -1266,7 +1260,6 @@ function bashunit::main::exec_multi_assert() { return 1 fi - # Check that assertions come in pairs (assertion + arg) if [ $# -lt 2 ] || [ $(($# % 2)) -ne 0 ]; then local assertion_name="${1:-}" printf "%sError: Missing argument for assertion '%s'.%s\n" \ @@ -1274,18 +1267,15 @@ function bashunit::main::exec_multi_assert() { return 1 fi - # Execute command and capture output + exit code local stdout local cmd_exit_code stdout=$(eval "$cmd" 2>&1) cmd_exit_code=$? - # Print stdout for user visibility if [ -n "$stdout" ]; then echo "$stdout" 1>&1 fi - # Parse and execute assertions in pairs local overall_result=0 while [ $# -gt 0 ]; do local assertion_name="$1" @@ -1299,7 +1289,6 @@ function bashunit::main::exec_multi_assert() { shift 2 - # Resolve assertion function name local assert_fn="$assertion_name" if ! type "$assert_fn" &>/dev/null; then assert_fn="assert_$assertion_name" @@ -1310,10 +1299,8 @@ function bashunit::main::exec_multi_assert() { fi fi - # Set test title for this assertion bashunit::state::set_test_title "assert ${assertion_name#assert_}" - # Execute assertion with appropriate argument if bashunit::main::is_exit_code_assertion "$assertion_name"; then # Exit code assertion: pass expected value and captured exit code "$assert_fn" "$assertion_arg" "" "$cmd_exit_code" 1>&2 diff --git a/tests/acceptance/bashunit_assert_multi_test.sh b/tests/acceptance/bashunit_assert_multi_test.sh index a9ea0540..7ce52e73 100644 --- a/tests/acceptance/bashunit_assert_multi_test.sh +++ b/tests/acceptance/bashunit_assert_multi_test.sh @@ -5,7 +5,6 @@ function set_up() { export BASHUNIT_SIMPLE_OUTPUT=false } -# Test multi-assertion mode function test_multi_assert_exit_code_and_contains() { ./bashunit assert "echo 'some error' && exit 1" exit_code "1" contains "some error" 2>&1 assert_successful_code diff --git a/tests/acceptance/bashunit_init_test.sh b/tests/acceptance/bashunit_init_test.sh index c9df8888..927486b5 100644 --- a/tests/acceptance/bashunit_init_test.sh +++ b/tests/acceptance/bashunit_init_test.sh @@ -14,14 +14,10 @@ function tear_down() { } function test_bashunit_init_creates_structure() { - # switch into a clean temporary directory pushd "$TMP_DIR" >/dev/null - # generate test scaffolding "$BASHUNIT_PATH" init >/tmp/init.log - # perform the assertions assert_file_exists "tests/example_test.sh" assert_file_exists "tests/bootstrap.sh" - # return to the original working directory popd >/dev/null } From 1aef3a9e39b3fb642adc56f932191cad05982740 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 25 Jul 2026 12:22:25 +0200 Subject: [PATCH 2/7] fix(coverage): validate BASHUNIT_COVERAGE_THRESHOLD_LOW/HIGH as integers They are compared with `[ -ge ]` in the coverage class lookup, which errors instead of returning false on a non-integer value: a bad threshold leaked a raw "integer expression expected" into the coverage report and silently mis-bucketed every file's high/medium/low class. Reuses bashunit::main::require_non_negative_int_or_exit, the same gate already covering the other numeric BASHUNIT_* settings (#873). --- CHANGELOG.md | 1 + src/main.sh | 8 +++++++ .../bashunit_invalid_option_value_test.sh | 23 +++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3db6e17..4bef60b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - The `--parallel` unsupported-OS warning no longer claims Alpine is excluded: Alpine has been a supported parallel platform since the race conditions were fixed, the message was simply never updated ### Fixed +- `BASHUNIT_COVERAGE_THRESHOLD_LOW`/`BASHUNIT_COVERAGE_THRESHOLD_HIGH` (env-only, no CLI flag) now validate as non-negative integers like the other numeric settings. They are compared with `[ -ge ]` in the coverage class lookup, which errors instead of returning false on a non-integer value: a bad threshold leaked a raw `integer expression expected` into the coverage report and silently mis-bucketed every file's high/medium/low class - `bashunit assert ` with no arguments now reports a clear error and exits non-zero. It used to compute a `-1` array index, which Bash 3.x rejects, so it leaked a raw `bad array subscript` and still exited `0` — a malformed standalone assertion in a deploy script reported success (#877) - A missing `--env`/`--boot` bootstrap file is now an error instead of a green run that tested nothing: the file used to be sourced unchecked, so a typo'd path leaked a raw `No such file or directory`, skipped the entire suite and still exited `0`. The report options (`--report-json`, `--log-junit`, `--report-tap`, `--report-html`, `--log-gha`) are likewise checked before the run instead of failing silently after it — an unwritable destination produced a passing run, no report on disk, and exit `0`. `--seed` now validates as a non-negative integer like the other numeric options (#875) - The numeric options (`--jobs`, `--retry`, `--test-timeout`, `--coverage-min`) and `--output` now reject an invalid value with a clear error and a non-zero exit, instead of dropping the setting. `--jobs abc` was the worst case: `[ n -lt abc ]` errors rather than returning false, so the Bash 3.x job-slot poll never reached its `break` and the run **hung**, while Bash 4.3+ silently ignored the concurrency limit. `--coverage-min abc` leaked a raw `[: abc: integer expression expected` and still exited `0`, so a CI job never enforced its threshold. Validation runs on the resolved configuration, so it covers the `BASHUNIT_*` environment variables too (#873) diff --git a/src/main.sh b/src/main.sh index 1d153386..85e99928 100644 --- a/src/main.sh +++ b/src/main.sh @@ -69,6 +69,14 @@ function bashunit::main::validate_config_or_exit() { bashunit::main::require_non_negative_int_or_exit \ "${BASHUNIT_COVERAGE_MIN}" "BASHUNIT_COVERAGE_MIN (--coverage-min)" fi + # Env-only (no CLI flag). Compared with `[ -ge ]` in + # bashunit::coverage::get_coverage_class, which errors instead of returning + # false on a non-integer operand, leaking a raw shell error into the + # coverage report and silently mis-bucketing every file's class (#880). + bashunit::main::require_non_negative_int_or_exit \ + "${BASHUNIT_COVERAGE_THRESHOLD_LOW:-50}" "BASHUNIT_COVERAGE_THRESHOLD_LOW" + bashunit::main::require_non_negative_int_or_exit \ + "${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-80}" "BASHUNIT_COVERAGE_THRESHOLD_HIGH" if [ -n "${BASHUNIT_SEED:-}" ]; then bashunit::main::require_non_negative_int_or_exit "${BASHUNIT_SEED}" "BASHUNIT_SEED (--seed)" diff --git a/tests/acceptance/bashunit_invalid_option_value_test.sh b/tests/acceptance/bashunit_invalid_option_value_test.sh index 582f9332..1462aa06 100644 --- a/tests/acceptance/bashunit_invalid_option_value_test.sh +++ b/tests/acceptance/bashunit_invalid_option_value_test.sh @@ -72,6 +72,29 @@ function test_bashunit_fails_when_parallel_jobs_env_var_is_not_a_number() { assert_contains "BASHUNIT_PARALLEL_JOBS" "$output" } +# BASHUNIT_COVERAGE_THRESHOLD_LOW/HIGH are env-only (no CLI flag) and are +# compared with `[ -ge ]` in bashunit::coverage::get_coverage_class. A +# non-integer value used to leak a raw "integer expression expected" shell +# error into the coverage report and silently mis-bucket every file's +# high/medium/low class instead of failing fast (#880). +function test_bashunit_fails_when_coverage_threshold_high_env_var_is_not_a_number() { + local ec=0 + local output + output=$(BASHUNIT_COVERAGE_THRESHOLD_HIGH=abc ./bashunit --env "$TEST_ENV_FILE" --coverage "$TEST_FILE" 2>&1) || ec=$? + + assert_general_error "" "" "$ec" + assert_contains "BASHUNIT_COVERAGE_THRESHOLD_HIGH" "$output" +} + +function test_bashunit_fails_when_coverage_threshold_low_env_var_is_not_a_number() { + local ec=0 + local output + output=$(BASHUNIT_COVERAGE_THRESHOLD_LOW=abc ./bashunit --env "$TEST_ENV_FILE" --coverage "$TEST_FILE" 2>&1) || ec=$? + + assert_general_error "" "" "$ec" + assert_contains "BASHUNIT_COVERAGE_THRESHOLD_LOW" "$output" +} + function test_bashunit_still_accepts_valid_option_values() { local ec=0 local output From e52725f33885588c3ba97bb433bb25950774ba23 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 25 Jul 2026 12:22:38 +0200 Subject: [PATCH 3/7] fix(benchmark): compare @max_ms threshold as a float, not an integer `@max_ms` accepts a decimal (its parse_annotations regex is `[0-9.][0-9.]*`, and the average itself can already be fractional), but the status column used plain `[ "$avg" -le "$max_ms" ]`. That errors instead of comparing on a fractional operand, so a well under-budget row printed a stray "integer expression expected" line and always rendered as failing (">") regardless of the real average. Adds bashunit::math::is_le, mirroring bashunit::math::calculate's bc > awk > integer fallback chain. --- CHANGELOG.md | 1 + src/benchmark.sh | 2 +- src/math.sh | 31 +++++++++++++++++++++++++++++++ tests/unit/benchmark_test.sh | 24 ++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bef60b8..f5a1dd4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ ### Fixed - `BASHUNIT_COVERAGE_THRESHOLD_LOW`/`BASHUNIT_COVERAGE_THRESHOLD_HIGH` (env-only, no CLI flag) now validate as non-negative integers like the other numeric settings. They are compared with `[ -ge ]` in the coverage class lookup, which errors instead of returning false on a non-integer value: a bad threshold leaked a raw `integer expression expected` into the coverage report and silently mis-bucketed every file's high/medium/low class +- `@max_ms` benchmark annotations with a decimal value (e.g. `@max_ms=100.5`) no longer leak a raw `integer expression expected` into the results table and always render as failing (`>`) regardless of the actual average; the threshold comparison now tolerates fractional operands like the average itself already could - `bashunit assert ` with no arguments now reports a clear error and exits non-zero. It used to compute a `-1` array index, which Bash 3.x rejects, so it leaked a raw `bad array subscript` and still exited `0` — a malformed standalone assertion in a deploy script reported success (#877) - A missing `--env`/`--boot` bootstrap file is now an error instead of a green run that tested nothing: the file used to be sourced unchecked, so a typo'd path leaked a raw `No such file or directory`, skipped the entire suite and still exited `0`. The report options (`--report-json`, `--log-junit`, `--report-tap`, `--report-html`, `--log-gha`) are likewise checked before the run instead of failing silently after it — an unwritable destination produced a passing run, no report on disk, and exit `0`. `--seed` now validates as a non-negative integer like the other numeric options (#875) - The numeric options (`--jobs`, `--retry`, `--test-timeout`, `--coverage-min`) and `--output` now reject an invalid value with a clear error and a non-zero exit, instead of dropping the setting. `--jobs abc` was the worst case: `[ n -lt abc ]` errors rather than returning false, so the Bash 3.x job-slot poll never reached its `break` and the run **hung**, while Bash 4.3+ silently ignored the concurrency limit. `--coverage-min abc` leaked a raw `[: abc: integer expression expected` and still exited `0`, so a CI job never enforced its threshold. Validation runs on the resolved configuration, so it covers the `BASHUNIT_*` environment variables too (#873) diff --git a/src/benchmark.sh b/src/benchmark.sh index fbc1a6fd..457193c6 100644 --- a/src/benchmark.sh +++ b/src/benchmark.sh @@ -143,7 +143,7 @@ function bashunit::benchmark::print_results() { continue fi - if [ "$avg" -le "$max_ms" ]; then + if bashunit::math::is_le "$avg" "$max_ms"; then local raw="≤ ${max_ms}" local padded padded=$(printf "%14s" "$raw") diff --git a/src/math.sh b/src/math.sh index cc3df1eb..4831b2a7 100644 --- a/src/math.sh +++ b/src/math.sh @@ -32,6 +32,37 @@ function bashunit::math::calculate() { echo "$result" } +## +# Numeric <= comparison that tolerates decimal operands. Plain `[ -le ]` +# exits 2 ("integer expression expected") on a fractional value instead of +# comparing it, which silently mis-reports the wrong side as failing (see +# bashunit::benchmark::print_results, whose `@max_ms` annotation allows +# decimals). Mirrors bashunit::math::calculate's bc > awk > integer fallback +# chain. +# Arguments: $1 - left operand, $2 - right operand +# Returns: 0 when $1 <= $2, 1 otherwise +## +function bashunit::math::is_le() { + local left="$1" + local right="$2" + + if bashunit::dependencies::has_bc; then + [ "$(echo "$left <= $right" | bc)" = "1" ] + return + fi + + if bashunit::dependencies::has_awk; then + awk -v a="$left" -v b="$right" 'BEGIN { exit !(a <= b) }' + return + fi + + # Downgrade to integer comparison by stripping decimals, matching + # bashunit::math::calculate's no-bc/no-awk fallback. + left=$(echo "$left" | sed -E 's/([0-9]+)\.[0-9]+/\1/g') + right=$(echo "$right" | sed -E 's/([0-9]+)\.[0-9]+/\1/g') + [ "$left" -le "$right" ] +} + ## # Deterministically shuffles stdin lines (one item per line) with a Fisher-Yates # driven by a seeded LCG (glibc constants). Same seed + same input always yields diff --git a/tests/unit/benchmark_test.sh b/tests/unit/benchmark_test.sh index 30758da0..8ac25bd8 100755 --- a/tests/unit/benchmark_test.sh +++ b/tests/unit/benchmark_test.sh @@ -199,6 +199,30 @@ function test_print_results_outputs_failing_threshold_status() { assert_contains "> 100" "$output" } +# `@max_ms` accepts decimals (the parse_annotations regex is `[0-9.][0-9.]*`, +# and add_result's own test above stores an avg of "42.5"), but plain `[ -le ]` +# errors on a fractional operand instead of comparing it: the well-under-budget +# row below used to print a stray "integer expression expected" line and always +# fall through to the ">" (failing) branch regardless of the real average (#880). +function test_print_results_outputs_passing_threshold_status_with_decimal_max_ms() { + function bashunit::env::is_bench_mode_enabled() { return 0; } + function bashunit::env::is_simple_output_enabled() { return 1; } + function bashunit::console_results::print_execution_time() { :; } + function bashunit::print_line() { :; } + + _BASHUNIT_BENCH_NAMES=("fast_fn") + _BASHUNIT_BENCH_REVS=("1") + _BASHUNIT_BENCH_ITS=("1") + _BASHUNIT_BENCH_AVERAGES=("10") + _BASHUNIT_BENCH_MAX_MILLIS=("100.5") + + local output + output=$(bashunit::benchmark::print_results 2>&1) + + assert_contains "≤ 100.5" "$output" + assert_not_contains "integer expression expected" "$output" +} + function test_print_results_adds_newline_in_simple_mode() { function bashunit::env::is_bench_mode_enabled() { return 0; } function bashunit::env::is_simple_output_enabled() { return 0; } From b708defc128a4b1c6d4d8d9409b6cd640bdac765 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 25 Jul 2026 12:24:10 +0200 Subject: [PATCH 4/7] ref(coverage): reuse env.sh threshold defaults instead of hardcoding them get_coverage_class and the HTML legend independently hardcoded 80/50 as the BASHUNIT_COVERAGE_THRESHOLD_HIGH/LOW fallback in six places, so a future change to _BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH/LOW in env.sh (the single source of truth for every other BASHUNIT_* default) would silently drift from these copies. Reference the canonical globals instead, and pin the fallback behavior with a regression test. --- src/coverage.sh | 10 +++++----- tests/unit/coverage_helpers_test.sh | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/coverage.sh b/src/coverage.sh index 975e21eb..06eb7bba 100644 --- a/src/coverage.sh +++ b/src/coverage.sh @@ -156,9 +156,9 @@ function bashunit::coverage::get_tracked_files() { # Get coverage class (high/medium/low) based on percentage function bashunit::coverage::get_coverage_class() { local pct="$1" - if [ "$pct" -ge "${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-80}" ]; then + if [ "$pct" -ge "${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH}" ]; then echo "high" - elif [ "$pct" -ge "${BASHUNIT_COVERAGE_THRESHOLD_LOW:-50}" ]; then + elif [ "$pct" -ge "${BASHUNIT_COVERAGE_THRESHOLD_LOW:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_LOW}" ]; then echo "medium" else echo "low" @@ -1824,19 +1824,19 @@ EOF
EOF - echo " ≥${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-80}% High" + echo " ≥${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH}% High" cat <<'EOF'
EOF - echo " ${BASHUNIT_COVERAGE_THRESHOLD_LOW:-50}-${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-80}% Medium" + echo " ${BASHUNIT_COVERAGE_THRESHOLD_LOW:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_LOW}-${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH}% Medium" cat <<'EOF'
EOF - echo " <${BASHUNIT_COVERAGE_THRESHOLD_LOW:-50}% Low" + echo " <${BASHUNIT_COVERAGE_THRESHOLD_LOW:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_LOW}% Low" cat <<'EOF'
diff --git a/tests/unit/coverage_helpers_test.sh b/tests/unit/coverage_helpers_test.sh index 10bd8bf4..6c15ea61 100644 --- a/tests/unit/coverage_helpers_test.sh +++ b/tests/unit/coverage_helpers_test.sh @@ -115,6 +115,26 @@ function test_coverage_get_coverage_class_boundary_low() { assert_equals "medium" "$result" } +# _BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH/_LOW (env.sh) are the single source +# of truth for the threshold defaults. When the BASHUNIT_COVERAGE_THRESHOLD_* +# env vars are unset, get_coverage_class must fall back to those same globals +# rather than an independently hardcoded number, so the two never drift apart. +function test_coverage_get_coverage_class_falls_back_to_the_default_high_threshold_when_unset() { + local result + unset BASHUNIT_COVERAGE_THRESHOLD_HIGH + unset BASHUNIT_COVERAGE_THRESHOLD_LOW + result=$(bashunit::coverage::get_coverage_class "$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH") + assert_equals "high" "$result" +} + +function test_coverage_get_coverage_class_falls_back_to_the_default_low_threshold_when_unset() { + local result + unset BASHUNIT_COVERAGE_THRESHOLD_HIGH + unset BASHUNIT_COVERAGE_THRESHOLD_LOW + result=$(bashunit::coverage::get_coverage_class "$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_LOW") + assert_equals "medium" "$result" +} + # === Percentage calculation tests === function test_coverage_calculate_percentage_basic() { From 3c4ae5cdbd760585136e9171ebbc7e4e41d7e1fd Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 25 Jul 2026 12:24:03 +0200 Subject: [PATCH 5/7] ref(coverage): dedupe the hits_by_line loader across report writers Seven functions independently re-parsed get_all_line_hits into their own local -a hits_by_line with an identical 5-line while-loop. Bash 3.0 can't return an array from a function or copy a sparse array without losing its line-number keys, so extract the parse into bashunit::coverage::load_hits_by_line, which writes the shared _BASHUNIT_COVERAGE_HITS_BY_LINE global instead (same return-slot pattern already used elsewhere in this file). No behavior change; existing coverage unit/acceptance tests cover every call site touched. --- src/coverage.sh | 89 +++++++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 48 deletions(-) diff --git a/src/coverage.sh b/src/coverage.sh index 06eb7bba..90068a6d 100644 --- a/src/coverage.sh +++ b/src/coverage.sh @@ -610,11 +610,7 @@ function bashunit::coverage::get_hit_lines() { function bashunit::coverage::compute_file_coverage() { local file="$1" - local -a hits_by_line=() - local hit_lineno hit_count - while IFS=: read -r hit_lineno hit_count; do - [ -n "$hit_lineno" ] && hits_by_line[hit_lineno]=$hit_count - done < <(bashunit::coverage::get_all_line_hits "$file") + bashunit::coverage::load_hits_by_line "$file" local executable=0 hit=0 lineno=0 line line_hits local -a cv_lines=() @@ -628,7 +624,7 @@ function bashunit::coverage::compute_file_coverage() { ((++lineno)) bashunit::coverage::is_executable_line "$line" "$lineno" || continue ((++executable)) - line_hits=${hits_by_line[lineno]:-0} + line_hits=${_BASHUNIT_COVERAGE_HITS_BY_LINE[lineno]:-0} [ "$line_hits" -gt 0 ] && ((++hit)) done @@ -715,6 +711,26 @@ function bashunit::coverage::get_all_line_hits() { return 0 } +# Populates the shared _BASHUNIT_COVERAGE_HITS_BY_LINE array (sparse, keyed by +# line number -> hit count) from get_all_line_hits for $1. +# +# Seven call sites used to repeat this "while IFS=: read ... done < <(get_all_line_hits)" +# parse loop into their own `local -a hits_by_line`. Bash 3.0 cannot return an +# array from a function or pass one by reference, and copying a sparse array +# with `local -a x=("${src[@]}")` silently renumbers it from 0, which would +# corrupt the line-number keys -- so this loader writes one shared global +# instead (return-slot pattern, see bash-style.md). Callers must consume the +# result before the next call; there is no adjacent-call isolation. +declare -a _BASHUNIT_COVERAGE_HITS_BY_LINE +function bashunit::coverage::load_hits_by_line() { + local file="$1" + _BASHUNIT_COVERAGE_HITS_BY_LINE=() + local hl_lineno hl_count + while IFS=: read -r hl_lineno hl_count; do + [ -n "$hl_lineno" ] && _BASHUNIT_COVERAGE_HITS_BY_LINE[hl_lineno]=$hl_count + done < <(bashunit::coverage::get_all_line_hits "$file") +} + # Get all test hits for a file in one pass (performance optimization) # Output format: lineno|test_file:test_function (may have duplicates, one per hit) function bashunit::coverage::get_all_line_tests() { @@ -1012,17 +1028,18 @@ function bashunit::coverage::extract_branches() { } # Sets _BASHUNIT_ARM_TAKEN_OUT to 1 iff any executable line in -# [arm_start..arm_end] has a recorded hit, else 0. Caller must have -# populated the hits_by_line and src_lines arrays in scope; Bash 3.0 -# cannot pass arrays into a function. Result is returned via the -# global to avoid a per-arm subshell. +# [arm_start..arm_end] has a recorded hit, else 0. Reads hit counts from +# the shared _BASHUNIT_COVERAGE_HITS_BY_LINE global (see +# load_hits_by_line); caller must have populated the src_lines array in +# scope -- Bash 3.0 cannot pass arrays into a function. Result is +# returned via the global to avoid a per-arm subshell. _BASHUNIT_ARM_TAKEN_OUT=0 function bashunit::coverage::_arm_taken() { local arm_start="$1" arm_end="$2" ln for ((ln = arm_start; ln <= arm_end; ln++)); do bashunit::coverage::is_executable_line \ "${src_lines[$((ln - 1))]:-}" "$ln" || continue - if [ "${hits_by_line[$ln]:-0}" -gt 0 ]; then + if [ "${_BASHUNIT_COVERAGE_HITS_BY_LINE[$ln]:-0}" -gt 0 ]; then _BASHUNIT_ARM_TAKEN_OUT=1 return fi @@ -1039,11 +1056,7 @@ function bashunit::coverage::_arm_taken() { function bashunit::coverage::compute_branch_hits() { local file="$1" - local -a hits_by_line=() - local _hl_ln _hl_cnt - while IFS=: read -r _hl_ln _hl_cnt; do - [ -n "$_hl_ln" ] && hits_by_line[_hl_ln]=$_hl_cnt - done < <(bashunit::coverage::get_all_line_hits "$file") + bashunit::coverage::load_hits_by_line "$file" local -a src_lines=() local _sli=0 _sl @@ -1218,11 +1231,7 @@ function bashunit::coverage::report_text_uncovered() { while IFS= read -r file; do { [ -z "$file" ] || [ ! -f "$file" ]; } && continue - local -a hits_by_line=() - local _hl_ln _hl_cnt - while IFS=: read -r _hl_ln _hl_cnt; do - [ -n "$_hl_ln" ] && hits_by_line[_hl_ln]=$_hl_cnt - done < <(bashunit::coverage::get_all_line_hits "$file") + bashunit::coverage::load_hits_by_line "$file" local -a uncovered_lines=() local _ucount=0 @@ -1230,7 +1239,7 @@ function bashunit::coverage::report_text_uncovered() { while IFS= read -r line || [ -n "$line" ]; do lineno=$((lineno + 1)) bashunit::coverage::is_executable_line "$line" "$lineno" || continue - local lh="${hits_by_line[$lineno]:-0}" + local lh="${_BASHUNIT_COVERAGE_HITS_BY_LINE[$lineno]:-0}" if [ "$lh" -eq 0 ]; then uncovered_lines[_ucount]="$lineno" _ucount=$((_ucount + 1)) @@ -1266,11 +1275,7 @@ function bashunit::coverage::report_text_line_hits() { while IFS= read -r file; do { [ -z "$file" ] || [ ! -f "$file" ]; } && continue - local -a hits_by_line=() - local _hl_ln _hl_cnt - while IFS=: read -r _hl_ln _hl_cnt; do - [ -n "$_hl_ln" ] && hits_by_line[_hl_ln]=$_hl_cnt - done < <(bashunit::coverage::get_all_line_hits "$file") + bashunit::coverage::load_hits_by_line "$file" local -a hit_specs=() local _hc=0 @@ -1278,7 +1283,7 @@ function bashunit::coverage::report_text_line_hits() { while IFS= read -r line || [ -n "$line" ]; do lineno=$((lineno + 1)) bashunit::coverage::is_executable_line "$line" "$lineno" || continue - local lh="${hits_by_line[$lineno]:-0}" + local lh="${_BASHUNIT_COVERAGE_HITS_BY_LINE[$lineno]:-0}" if [ "$lh" -gt 0 ]; then hit_specs[_hc]="${lineno}:${lh}" _hc=$((_hc + 1)) @@ -1311,11 +1316,7 @@ function bashunit::coverage::report_text_functions() { functions_data=$(bashunit::coverage::extract_functions "$file") [ -z "$functions_data" ] && continue - local -a hits_by_line=() - local _hl_ln _hl_cnt - while IFS=: read -r _hl_ln _hl_cnt; do - [ -n "$_hl_ln" ] && hits_by_line[_hl_ln]=$_hl_cnt - done < <(bashunit::coverage::get_all_line_hits "$file") + bashunit::coverage::load_hits_by_line "$file" local -a file_lines=() local _fli=0 _fl @@ -1345,7 +1346,7 @@ function bashunit::coverage::report_text_functions() { bashunit::coverage::is_executable_line \ "${file_lines[$((ln - 1))]:-}" "$ln" || continue fn_executable=$((fn_executable + 1)) - [ "${hits_by_line[$ln]:-0}" -gt 0 ] && fn_hit=$((fn_hit + 1)) + [ "${_BASHUNIT_COVERAGE_HITS_BY_LINE[$ln]:-0}" -gt 0 ] && fn_hit=$((fn_hit + 1)) done fn_pct=$(bashunit::coverage::calculate_percentage "$fn_hit" "$fn_executable") @@ -1377,11 +1378,7 @@ function bashunit::coverage::report_lcov() { echo "SF:$file" - local -a hits_by_line=() - local hit_lineno hit_count - while IFS=: read -r hit_lineno hit_count; do - [ -n "$hit_lineno" ] && hits_by_line[hit_lineno]=$hit_count - done < <(bashunit::coverage::get_all_line_hits "$file") + bashunit::coverage::load_hits_by_line "$file" # Function records (FN/FNDA/FNF/FNH). Emit FN lines as we walk # and buffer the matching FNDA lines for emission after, per @@ -1396,7 +1393,7 @@ function bashunit::coverage::report_lcov() { any_hit=0 for ((fln = fn_start; fln <= fn_end; fln++)); do - if [ "${hits_by_line[$fln]:-0}" -gt 0 ]; then + if [ "${_BASHUNIT_COVERAGE_HITS_BY_LINE[$fln]:-0}" -gt 0 ]; then any_hit=1 break fi @@ -1436,7 +1433,7 @@ function bashunit::coverage::report_lcov() { ((++lineno)) bashunit::coverage::is_executable_line "$line" "$lineno" || continue ((++executable)) - local lh="${hits_by_line[$lineno]:-0}" + local lh="${_BASHUNIT_COVERAGE_HITS_BY_LINE[$lineno]:-0}" [ "$lh" -gt 0 ] && ((++hit)) echo "DA:${lineno},${lh}" done @@ -1921,11 +1918,7 @@ function bashunit::coverage::generate_file_html() { local uncovered=$((executable - hit)) # Pre-load all line hits into indexed array (performance optimization) - local -a hits_by_line=() - local _ln _cnt - while IFS=: read -r _ln _cnt; do - hits_by_line[_ln]=$_cnt - done < <(bashunit::coverage::get_all_line_hits "$file") + bashunit::coverage::load_hits_by_line "$file" # Pre-load all file lines into indexed array (avoids sed per line) local -a file_lines=() @@ -2206,7 +2199,7 @@ EOF ln_content="${file_lines[$((ln - 1))]:-}" if bashunit::coverage::is_executable_line "$ln_content" "$ln"; then ((++fn_executable)) - local ln_hits=${hits_by_line[$ln]:-0} + local ln_hits=${_BASHUNIT_COVERAGE_HITS_BY_LINE[$ln]:-0} if [ "$ln_hits" -gt 0 ]; then ((++fn_hit)) fi @@ -2269,7 +2262,7 @@ EOF if bashunit::coverage::is_executable_line "$line" "$lineno"; then # O(1) lookup from pre-loaded array - local hits=${hits_by_line[$lineno]:-0} + local hits=${_BASHUNIT_COVERAGE_HITS_BY_LINE[$lineno]:-0} if [ "$hits" -gt 0 ]; then row_class="covered" From 43397d796057862882e0a4cd133c994bc51a0c6a Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 25 Jul 2026 12:24:08 +0200 Subject: [PATCH 6/7] ref(doubles): dedupe spy call-count lookup into times_to_slot assert_have_been_called, assert_have_been_called_times and assert_have_been_called_nth_with each repeated the same "resolve the spy's times file, cat it, default to 0" sequence. Extract bashunit::spy::times_to_slot (return-slot pattern, no added forks) and call it from all three. Adds direct unit coverage for the new helper alongside the existing spy assertion tests. --- src/test_doubles.sh | 41 ++++++++++++++++++--------------- tests/unit/test_doubles_test.sh | 18 +++++++++++++++ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/test_doubles.sh b/src/test_doubles.sh index 7c227ea8..5146363e 100644 --- a/src/test_doubles.sh +++ b/src/test_doubles.sh @@ -7,6 +7,22 @@ declare -a _BASHUNIT_MOCKED_FUNCTIONS=() # A test that does `local foo_times_file=...` is harmless because the helper # resolves `_BASHUNIT_SPY_foo_TIMES_FILE` instead. +_BASHUNIT_SPY_TIMES_OUT=0 + +# Reads the recorded call count for $1 into _BASHUNIT_SPY_TIMES_OUT (0 when +# never spied/called). Shared by the call-count assertions instead of each +# repeating the "resolve times file, cat it, default to 0" sequence. +function bashunit::spy::times_to_slot() { + local command="$1" + local variable + variable="$(bashunit::helper::normalize_variable_name "$command")" + local file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE" + _BASHUNIT_SPY_TIMES_OUT=0 + if [ -f "${!file_var-}" ]; then + _BASHUNIT_SPY_TIMES_OUT=$(cat "${!file_var}" 2>/dev/null || builtin echo 0) + fi +} + function bashunit::unmock() { local command=$1 @@ -102,13 +118,8 @@ function bashunit::spy() { function assert_have_been_called() { local command=$1 - local variable - variable="$(bashunit::helper::normalize_variable_name "$command")" - local file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE" - local times=0 - if [ -f "${!file_var-}" ]; then - times=$(cat "${!file_var}" 2>/dev/null || builtin echo 0) - fi + bashunit::spy::times_to_slot "$command" + local times=$_BASHUNIT_SPY_TIMES_OUT local label="${2:-$(bashunit::helper::normalize_test_function_name "${FUNCNAME[1]}")}" if [ "$times" -eq 0 ]; then @@ -165,13 +176,8 @@ function assert_have_been_called_with() { function assert_have_been_called_times() { local expected_count=$1 local command=$2 - local variable - variable="$(bashunit::helper::normalize_variable_name "$command")" - local file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE" - local times=0 - if [ -f "${!file_var-}" ]; then - times=$(cat "${!file_var}" 2>/dev/null || builtin echo 0) - fi + bashunit::spy::times_to_slot "$command" + local times=$_BASHUNIT_SPY_TIMES_OUT local label="${3:-$(bashunit::helper::normalize_test_function_name "${FUNCNAME[1]}")}" if [ "$times" -ne "$expected_count" ]; then bashunit::state::add_assertions_failed @@ -192,15 +198,12 @@ function assert_have_been_called_nth_with() { local variable variable="$(bashunit::helper::normalize_variable_name "$command")" - local times_file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE" local file_var="_BASHUNIT_SPY_${variable}_PARAMS_FILE" local label label="$(bashunit::helper::normalize_test_function_name "${FUNCNAME[1]}")" - local times=0 - if [ -f "${!times_file_var-}" ]; then - times=$(cat "${!times_file_var}" 2>/dev/null || builtin echo 0) - fi + bashunit::spy::times_to_slot "$command" + local times=$_BASHUNIT_SPY_TIMES_OUT if [ "$nth" -gt "$times" ]; then bashunit::state::add_assertions_failed diff --git a/tests/unit/test_doubles_test.sh b/tests/unit/test_doubles_test.sh index 85a90d5f..f8bdc686 100644 --- a/tests/unit/test_doubles_test.sh +++ b/tests/unit/test_doubles_test.sh @@ -252,6 +252,24 @@ function test_spy_with_impl_calls_custom_function() { assert_same "custom output" "$output" } +function test_spy_times_to_slot_reports_zero_when_never_spied() { + bashunit::spy::times_to_slot "never_spied_command" + + assert_same "0" "$_BASHUNIT_SPY_TIMES_OUT" +} + +function test_spy_times_to_slot_reports_recorded_call_count() { + bashunit::spy ps + + ps + ps + ps + + bashunit::spy::times_to_slot ps + + assert_same "3" "$_BASHUNIT_SPY_TIMES_OUT" +} + function test_spy_with_impl_records_calls_and_delegates() { custom_ps_impl() { builtin echo "delegated" From eeae2f466190010b50b00767aff663ed6b21fb4a Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sat, 25 Jul 2026 12:38:35 +0200 Subject: [PATCH 7/7] test(coverage): pin threshold validation against an allexport .env The two threshold tests passed in isolation but failed once integrated: .env is gitignored, so it exists in the main checkout but not in the worktrees they were written in. Both .env and .env.example list BASHUNIT_COVERAGE_THRESHOLD_LOW/HIGH with an empty value, and an allexport 'source .env' turns that listing into an unconditional assignment that overrides the exported value under test (#865). CI does 'cp .env.example .env', so this would have gone red there too. Run them with --skip-env-file, and correct an issue reference that pointed at a number that did not exist. --- src/main.sh | 2 +- tests/acceptance/bashunit_invalid_option_value_test.sh | 10 +++++++--- tests/unit/benchmark_test.sh | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main.sh b/src/main.sh index 85e99928..8985345a 100644 --- a/src/main.sh +++ b/src/main.sh @@ -72,7 +72,7 @@ function bashunit::main::validate_config_or_exit() { # Env-only (no CLI flag). Compared with `[ -ge ]` in # bashunit::coverage::get_coverage_class, which errors instead of returning # false on a non-integer operand, leaking a raw shell error into the - # coverage report and silently mis-bucketing every file's class (#880). + # coverage report and silently mis-bucketing every file's class (#879). bashunit::main::require_non_negative_int_or_exit \ "${BASHUNIT_COVERAGE_THRESHOLD_LOW:-50}" "BASHUNIT_COVERAGE_THRESHOLD_LOW" bashunit::main::require_non_negative_int_or_exit \ diff --git a/tests/acceptance/bashunit_invalid_option_value_test.sh b/tests/acceptance/bashunit_invalid_option_value_test.sh index 1462aa06..ed45d650 100644 --- a/tests/acceptance/bashunit_invalid_option_value_test.sh +++ b/tests/acceptance/bashunit_invalid_option_value_test.sh @@ -76,11 +76,14 @@ function test_bashunit_fails_when_parallel_jobs_env_var_is_not_a_number() { # compared with `[ -ge ]` in bashunit::coverage::get_coverage_class. A # non-integer value used to leak a raw "integer expression expected" shell # error into the coverage report and silently mis-bucket every file's -# high/medium/low class instead of failing fast (#880). +# high/medium/low class instead of failing fast (#879). function test_bashunit_fails_when_coverage_threshold_high_env_var_is_not_a_number() { local ec=0 local output - output=$(BASHUNIT_COVERAGE_THRESHOLD_HIGH=abc ./bashunit --env "$TEST_ENV_FILE" --coverage "$TEST_FILE" 2>&1) || ec=$? + # --skip-env-file is required: .env / .env.example both list these two names, + # and an allexport `source .env` turns an empty listing into an unconditional + # assignment that overrides the exported value under test (#865). + output=$(BASHUNIT_COVERAGE_THRESHOLD_HIGH=abc ./bashunit --skip-env-file --coverage "$TEST_FILE" 2>&1) || ec=$? assert_general_error "" "" "$ec" assert_contains "BASHUNIT_COVERAGE_THRESHOLD_HIGH" "$output" @@ -89,7 +92,8 @@ function test_bashunit_fails_when_coverage_threshold_high_env_var_is_not_a_numbe function test_bashunit_fails_when_coverage_threshold_low_env_var_is_not_a_number() { local ec=0 local output - output=$(BASHUNIT_COVERAGE_THRESHOLD_LOW=abc ./bashunit --env "$TEST_ENV_FILE" --coverage "$TEST_FILE" 2>&1) || ec=$? + # See the note above: .env would otherwise override the value under test. + output=$(BASHUNIT_COVERAGE_THRESHOLD_LOW=abc ./bashunit --skip-env-file --coverage "$TEST_FILE" 2>&1) || ec=$? assert_general_error "" "" "$ec" assert_contains "BASHUNIT_COVERAGE_THRESHOLD_LOW" "$output" diff --git a/tests/unit/benchmark_test.sh b/tests/unit/benchmark_test.sh index 8ac25bd8..69cb022f 100755 --- a/tests/unit/benchmark_test.sh +++ b/tests/unit/benchmark_test.sh @@ -203,7 +203,7 @@ function test_print_results_outputs_failing_threshold_status() { # and add_result's own test above stores an avg of "42.5"), but plain `[ -le ]` # errors on a fractional operand instead of comparing it: the well-under-budget # row below used to print a stray "integer expression expected" line and always -# fall through to the ">" (failing) branch regardless of the real average (#880). +# fall through to the ">" (failing) branch regardless of the real average (#879). function test_print_results_outputs_passing_threshold_status_with_decimal_max_ms() { function bashunit::env::is_bench_mode_enabled() { return 0; } function bashunit::env::is_simple_output_enabled() { return 1; }