From 40937fd0a7ac0cdcda24bfc816e9eb13d5d7e648 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Thu, 13 Aug 2026 23:08:36 +0200 Subject: [PATCH] fix(coverage): distinguish 'nothing tracked' from 'nothing covered' A mistyped --coverage-paths yields Total: 0/0 and the gate reported 'Coverage 0% is below minimum 80%'. That 0% is arithmetic, not a measurement: nothing was found to measure. The message sends the reader to their tests when the cause is the paths, and in CI it fails for a reason the output cannot act on. Report the two cases apart, and keep failing either way -- a misconfigured run quietly satisfying an 80% gate is worse than either message. The gate could not see the executable total because get_percentage echoes, so every caller wraps it in $( ) and a return slot set inside dies with that subshell -- the same boundary as #1145 and #1147. Totals now come from a slot-setting helper the gate calls directly; get_percentage stays the echoing wrapper its callers expect. Closes #1171 --- CHANGELOG.md | 1 + src/coverage/stats.sh | 45 +++++++++++-- .../bashunit_coverage_nothing_tracked_test.sh | 67 +++++++++++++++++++ 3 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 tests/acceptance/bashunit_coverage_nothing_tracked_test.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index fcb9a9f4..ef65ebef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Performance: `--coverage` is roughly 5x faster and `--coverage-report-html` roughly 19x — a run over this repo went from 16.2s to 2.9s, and a 128-file HTML report from 58.7s to 3.1s. The report phase emits each format in one awk invocation per run instead of Bash loops and forks per file and per row, and the capture path writes records straight to disk, normalizes a path with one fork instead of four, and reads each cache once (#1092, #1096, #1098, #1099, #1102, #1104, #1110, #1117) ### Fixed +- A coverage run that tracked no executable line at all no longer reports it as `Coverage 0% is below minimum N%`: that 0% is arithmetic rather than a measurement, and the message sent the reader to their tests when the cause is almost always a `--coverage-paths` that matched nothing. The gate still fails, now naming the real problem (#1171) - A run whose scratch directory goes missing now says so once, on stderr, naming the directory, instead of recovering silently — the recovery added for that case left no trace at all, which is worse for diagnosis than the misleading error it replaced (#1167) - A run's scratch-directory cleanup can no longer widen to every concurrent run's: the safety guard accepted any path containing `/bashunit/run/`, which includes the per-OS parent that all runs on a machine share, so a run whose random token came out empty would have deleted the others' directories along with its own. Both the run and the parallel trees now require the token segment (#1165) - A run survives its scratch directory going missing instead of reporting a valid test file as unsourceable: `source "$file" 2>"$dir/source_err"` fails on the *redirect* when the directory is gone, which bash reports as the command failing, so the runner printed `Failed to source '' (exit 1, ..., no stderr)` and named the one thing that was not at fault (#1163) diff --git a/src/coverage/stats.sh b/src/coverage/stats.sh index 1ff4b295..b595c738 100644 --- a/src/coverage/stats.sh +++ b/src/coverage/stats.sh @@ -10,6 +10,11 @@ _BASHUNIT_COVERAGE_COLOR_OUT="" # Sets _BASHUNIT_COVERAGE_CLASS_OUT to high/medium/low for a percentage. # Arguments: $1 - percentage ## +# Executable-line total behind the last get_percentage call, so the threshold +# gate can tell "nothing covered" from "nothing to cover" (#1171). +_BASHUNIT_COVERAGE_TOTAL_EXEC_OUT=0 +_BASHUNIT_COVERAGE_TOTAL_HIT_OUT=0 + function bashunit::coverage::class_to_slot() { local pct="$1" if [ "$pct" -ge "${BASHUNIT_COVERAGE_THRESHOLD_HIGH:-$_BASHUNIT_DEFAULT_COVERAGE_THRESHOLD_HIGH}" ]; then @@ -236,7 +241,15 @@ function bashunit::coverage::split_stats() { _BASHUNIT_COVERAGE_SPLIT_CLASS_OUT="${rest#*:}" } -function bashunit::coverage::get_percentage() { +## +# Totals behind the last percentage, in the CALLER's shell. +# +# get_percentage echoes, so every caller wraps it in $( ) -- and a return slot +# set inside that subshell dies with it, which is why the threshold gate could +# not see the executable total (#1171). This does the work and sets the slots; +# get_percentage stays the echoing wrapper it has always been. +## +function bashunit::coverage::totals_to_slots() { local total_executable=0 local total_hit=0 @@ -259,7 +272,16 @@ function bashunit::coverage::get_percentage() { done < <(bashunit::coverage::get_tracked_files) fi - bashunit::coverage::calculate_percentage "$total_hit" "$total_executable" + # The gate needs to tell "your tests cover none of it" from "there was + # nothing to cover": both are 0%, and only the first is about coverage. + _BASHUNIT_COVERAGE_TOTAL_EXEC_OUT=$total_executable + _BASHUNIT_COVERAGE_TOTAL_HIT_OUT=$total_hit +} + +function bashunit::coverage::get_percentage() { + bashunit::coverage::totals_to_slots + bashunit::coverage::calculate_percentage \ + "$_BASHUNIT_COVERAGE_TOTAL_HIT_OUT" "$_BASHUNIT_COVERAGE_TOTAL_EXEC_OUT" } function bashunit::coverage::check_threshold() { @@ -274,13 +296,26 @@ function bashunit::coverage::check_threshold() { if bashunit::coverage::is_diff_enabled && [ -n "$_BASHUNIT_COVERAGE_DIFF_PCT_OUT" ]; then pct="$_BASHUNIT_COVERAGE_DIFF_PCT_OUT" else - pct=$(bashunit::coverage::get_percentage) + # Not `$(get_percentage)`: that subshell would take the totals with it. + bashunit::coverage::totals_to_slots + pct=$(bashunit::coverage::calculate_percentage \ + "$_BASHUNIT_COVERAGE_TOTAL_HIT_OUT" "$_BASHUNIT_COVERAGE_TOTAL_EXEC_OUT") fi if [ "$pct" -lt "$BASHUNIT_COVERAGE_MIN" ]; then local message - message=$(printf "%sCoverage %d%% is below minimum %d%%%s" \ - "$_BASHUNIT_COLOR_FAILED" "$pct" "$BASHUNIT_COVERAGE_MIN" "$_BASHUNIT_COLOR_DEFAULT") + if [ "${_BASHUNIT_COVERAGE_TOTAL_EXEC_OUT:-0}" -eq 0 ]; then + # 0% here is arithmetic, not a measurement: nothing was found to measure. + # Reporting it as low coverage sends the reader to their tests when the + # cause is almost always the paths (#1171). Still fails -- a misconfigured + # run quietly satisfying an 80% gate is worse than either message. + message=$(printf "%sCoverage gate failed: no executable lines were tracked%s\n%s" \ + "$_BASHUNIT_COLOR_FAILED" "$_BASHUNIT_COLOR_DEFAULT" \ + "Check --coverage-paths (BASHUNIT_COVERAGE_PATHS): it matched no shell file with executable code.") + else + message=$(printf "%sCoverage %d%% is below minimum %d%%%s" \ + "$_BASHUNIT_COLOR_FAILED" "$pct" "$BASHUNIT_COVERAGE_MIN" "$_BASHUNIT_COLOR_DEFAULT") + fi # Under a machine --output the gate still speaks, but on stderr: on stdout # it would sit next to the JSON or XML document and break the parser. if bashunit::env::is_machine_output_enabled; then diff --git a/tests/acceptance/bashunit_coverage_nothing_tracked_test.sh b/tests/acceptance/bashunit_coverage_nothing_tracked_test.sh new file mode 100644 index 00000000..af7d7001 --- /dev/null +++ b/tests/acceptance/bashunit_coverage_nothing_tracked_test.sh @@ -0,0 +1,67 @@ +#!/usr/bin/env bash +set -euo pipefail + +# A coverage run that tracked no executable line at all reports "Coverage 0% is +# below minimum N%", which reads as "your tests cover nothing" when the actual +# cause is usually that nothing was *found* to cover -- a mistyped +# --coverage-paths, a directory that holds no shell files, or sources that are +# all comments. The number is accurate and the diagnosis is wrong, which is the +# expensive kind of message (#1171). + +function set_up_before_script() { + ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +} + +# A project with one real source file and a passing test that touches nothing. +function _project() { # $1 = dir + mkdir -p "$1/src" + printf '%s\n' '#!/usr/bin/env bash' 'function real_fn() { echo hi; }' >"$1/src/real.sh" + printf '%s\n' '#!/usr/bin/env bash' 'function test_nothing() { assert_same 1 1; }' \ + >"$1/t_test.sh" +} + +function _run_coverage() { # $1 = dir, $@ = extra flags + local dir="$1" + shift + (cd "$dir" && "$ROOT_DIR/bashunit" --no-parallel --coverage "$@" t_test.sh 2>&1) || true +} + +function test_a_path_that_tracks_nothing_says_so_rather_than_blaming_coverage() { + local dir + dir="$(bashunit::temp_dir)" + _project "$dir" + + local output + output="$(_run_coverage "$dir" --coverage-paths srcc/ --coverage-min 80 | strip_ansi)" + + assert_contains "no executable lines" "$output" + assert_contains "coverage-paths" "$output" +} + +# The gate must still fail: a misconfigured run passing an 80% requirement +# silently is worse than either message. +function test_a_run_that_tracks_nothing_still_fails_the_minimum() { + local dir + dir="$(bashunit::temp_dir)" + _project "$dir" + + local code=0 + (cd "$dir" && "$ROOT_DIR/bashunit" --no-parallel --coverage \ + --coverage-paths srcc/ --coverage-min 80 t_test.sh >/dev/null 2>&1) || code=$? + + assert_general_error "" "" "$code" +} + +# A real path with real uncovered lines keeps the percentage message: that one +# is accurate and is what the flag exists to report. +function test_genuinely_uncovered_code_still_reports_the_percentage() { + local dir + dir="$(bashunit::temp_dir)" + _project "$dir" + + local output + output="$(_run_coverage "$dir" --coverage-paths src/ --coverage-min 80 | strip_ansi)" + + assert_contains "below minimum" "$output" + assert_not_contains "no executable lines" "$output" +}