ref: second cleanup sweep — coverage/benchmark numeric bugs and duplication

CHANGELOG.md

@@ -14,6 +14,8 @@
 - 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
+- `@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 <name>` 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)

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")

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"
@@ -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,19 +1231,15 @@ 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
     local lineno=0 line
     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,19 +1275,15 @@ 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
     local lineno=0 line
     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
@@ -1824,19 +1821,19 @@ EOF
           <div class="legend-item">
             <span class="legend-color high"></span>
 EOF
-    echo "            <span>≥${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-80}% High</span>"
+    echo "            <span>≥${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH}% High</span>"
     cat <<'EOF'
           </div>
           <div class="legend-item">
             <span class="legend-color medium"></span>
 EOF
-    echo "            <span>${BASHUNIT_COVERAGE_THRESHOLD_LOW:-50}-${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-80}% Medium</span>"
+    echo "            <span>${BASHUNIT_COVERAGE_THRESHOLD_LOW:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_LOW}-${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH}% Medium</span>"
     cat <<'EOF'
           </div>
           <div class="legend-item">
             <span class="legend-color low"></span>
 EOF
-    echo "            <span>&lt;${BASHUNIT_COVERAGE_THRESHOLD_LOW:-50}% Low</span>"
+    echo "            <span>&lt;${BASHUNIT_COVERAGE_THRESHOLD_LOW:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_LOW}% Low</span>"
     cat <<'EOF'
           </div>
         </div>
@@ -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"

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 (#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 \
+    "${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)"
@@ -1168,7 +1176,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 +1194,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 +1218,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,34 +1261,29 @@ 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
     printf "Usage: bashunit assert \"<command>\" <assertion1> <arg1> [<assertion2> <arg2>...]\n" 1>&2
     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" \
       "${_BASHUNIT_COLOR_FAILED}" "$assertion_name" "${_BASHUNIT_COLOR_DEFAULT}" 1>&2
     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 +1297,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 +1307,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