From 311c55c5d92a21e24109db23335e1122bcf78c89 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 9 Aug 2026 19:58:25 +0200 Subject: [PATCH] perf(coverage): stop forking grep per source line in the report phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profiling --coverage showed the report, not the capture engine, was half the wall time and identical for both engines: is_executable_line fell back to a `grep -E` fork for every line it could not classify in pure Bash, and every tracked line is classified twice per run (precompute_file_stats, then report_lcov). 286 source lines cost 1055 grep forks. The combined regex is replaced by `case` globs reproducing it exactly, quirks included: inside a POSIX bracket expression a backslash is a literal member of the set, so `[^\)]` also excludes `\` and `[\{\}]` also matches a bare line continuation. Verified by running both implementations over the same 41173 lines across 395 files on Bash 3.2 and 5.3 with zero disagreements, with the differential harness mutation-tested. Also surfaces the engine: --verbose reports the one in use, and an explicit BASHUNIT_COVERAGE_ENGINE=xtrace that the running Bash cannot honour now warns instead of being dropped silently — the common case on macOS. Coverage totals are unchanged and still match between --parallel and sequential for each engine. Closes #1005 --- .claude/rules/architecture-map.md | 2 +- .claude/rules/perf-fork-budget.md | 23 ++++ CHANGELOG.md | 6 + adrs/adr-010-src-module-directories.md | 5 +- docs/coverage.md | 34 ++++- src/config/env.sh | 1 + src/coverage/config.sh | 31 +++++ src/coverage/lines.sh | 105 +++++++++++----- src/coverage/report_text.sh | 21 ++++ .../bashunit_coverage_forks_test.sh | 103 ++++++++++++++++ tests/unit/coverage/engine_test.sh | 116 ++++++++++++++++++ 11 files changed, 409 insertions(+), 38 deletions(-) create mode 100644 tests/acceptance/bashunit_coverage_forks_test.sh diff --git a/.claude/rules/architecture-map.md b/.claude/rules/architecture-map.md index 0d8b4346..e5517246 100644 --- a/.claude/rules/architecture-map.md +++ b/.claude/rules/architecture-map.md @@ -105,7 +105,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end). | `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) | | `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches | | `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` | -| `coverage/lines.sh` | executable-line classification (`_NONEXEC_PATTERN`) and hit-data reading | +| `coverage/lines.sh` | executable-line classification (fork-free; the report phase's hot path) and hit-data reading | | `coverage/stats.sh` | percentages, the precomputed per-file stats cache, threshold gate | | `coverage/functions.sh` | function definitions and their line spans | | `coverage/branches.sh` | branch extraction + hit computation; **one file on purpose** — the `_branch_*` helpers mutate `extract_branches`'s locals via dynamic scoping | diff --git a/.claude/rules/perf-fork-budget.md b/.claude/rules/perf-fork-budget.md index 0e82d9ba..5170e3a8 100644 --- a/.claude/rules/perf-fork-budget.md +++ b/.claude/rules/perf-fork-budget.md @@ -57,6 +57,7 @@ fork class; they are the RED test of the TDD cycle. | probe fork + first-use fork | probe does the real work and seeds the return slot | #802 (clock perl) | | per-worker `mkdir -p` | parent pre-creates before spawning workers (`[ -d ]` inside a worker races its siblings) | #813 | | sanitize-args pipeline on empty input | guard: skip the pipeline when the input is empty | #813 | +| `printf \| grep -cE` per line as a classifier fallback | `case` globs translating each regex alternation | #1005 (286 lines cost 1055 forks) | `shopt -s extdebug` for `declare -F`: enable it **inside the capture subshell only** — toggling it in the caller's shell clobbers caller state (#808). @@ -90,6 +91,28 @@ sourcing `src/` (irreducible without lazy-loading, rejected in #798). (decision-cache miss: grep|head + dirname) — bounded by file count, nightly non-gating workflow, not worth chasing. +**Don't assume the engine is the cost.** Profiling `--coverage` for #1005 found +the *report* phase was ~50-60% of wall time and identical for both engines: the +per-line executable/non-executable classifier fell back to a `grep -E` fork for +every line it could not decide in pure Bash, and every tracked line is classified +twice per run (`precompute_file_stats`, then `report_lcov`). Budget guarded by +`tests/acceptance/bashunit_coverage_forks_test.sh`, expressed as "fork count does +not grow with source-line count" rather than an absolute number. + +Two traps when replacing a regex classifier with `case` globs — both silently +change coverage numbers rather than erroring: + +- Inside a POSIX bracket expression a backslash is a **literal member of the + set**, so `[^\)]` excludes `\` as well as `)`. `x=$(foo)` and + `x=$(printf '%s\n')` therefore classify differently. +- `[\{\}]` likewise matches a lone `\`, so a bare line continuation counts as a + brace-only line. + +Verify such a rewrite by running both implementations over the same input +(`git show HEAD:` + `declare -f | sed` to alias the old one) across every +`git ls-files '*.sh'` line, and mutation-test the harness itself — a differential +that cannot fail proves nothing. + **Parallel 10-test file run (CI's mode):** ~11 forks — 3 `mkdir`, 4 `rm`, 3 `awk` (#813; was 61). The per-test result file is named by a per-suite ordinal the single-threaded dispatcher assigns just before each `&` (the fork diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f93f21f..bd2fece1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +### Added +- The coverage engine in use is reported by `--verbose`, and an explicit `BASHUNIT_COVERAGE_ENGINE=xtrace` that the running Bash cannot honour now warns instead of being silently ignored (#1005) + +### Changed +- Performance: `--coverage` is about 1.6x to 2.3x faster. Executable-line classification no longer forks `grep` per source line, which was roughly half of a coverage run's wall time and affected both engines equally (#1005) + ### Fixed - Report formats are no longer empty under `--parallel`. `--report-junit`, `--report-tap`, `--report-json`, `--report-html` and `--log-junit` all recorded zero tests, because the rows were collected inside the per-test worker and nothing rebuilt them in the parent (#1004) diff --git a/adrs/adr-010-src-module-directories.md b/adrs/adr-010-src-module-directories.md index 76d70eb7..37c5e7e7 100644 --- a/adrs/adr-010-src-module-directories.md +++ b/adrs/adr-010-src-module-directories.md @@ -136,8 +136,9 @@ Beyond the aggregator rule, five traps have cost real time. The first was found * **Order-dependent file-scope initialisers must stay contiguous and in order.** `_BASHUNIT_COVERAGE_XTRACE_PS4` expands `$_BASHUNIT_COVERAGE_XTRACE_FS` at file - scope, and `_NONEXEC_PATTERN` is built across seven successive lines. Grep for - file-scope assignments that read another file-scope variable before splitting. + scope. (`_NONEXEC_PATTERN`, built across seven successive lines, was the other + example until #1005 replaced it with pure-Bash matching.) Grep for file-scope + assignments that read another file-scope variable before splitting. * **`.gitignore` can silently swallow a new module directory.** `coverage/` was unanchored, so it also matched `src/coverage/` and excluded all twelve new files from git — `git status` showed no `??` entries at all, and the split would have diff --git a/docs/coverage.md b/docs/coverage.md index df5d1540..663fc7e1 100644 --- a/docs/coverage.md +++ b/docs/coverage.md @@ -40,17 +40,39 @@ Coverage report written to: coverage/lcov.info ## How It Works -bashunit uses Bash's built-in `DEBUG` trap mechanism to track line execution: +bashunit records which lines ran, then classifies and reports them: -1. **Trap Setup**: When coverage is enabled, a DEBUG trap is set that fires before every command execution -2. **Line Recording**: Each executed line's file path and line number are recorded -3. **Filtering**: Only files matching your coverage paths (and not excluded) are tracked -4. **Aggregation**: After tests complete, hit data is aggregated and reported +1. **Capture**: every executed line's file path and line number is recorded, either from a `DEBUG` trap or from `xtrace` — see [Tracing engine](#tracing-engine) +2. **Filtering**: only files matching your coverage paths (and not excluded) are tracked +3. **Aggregation**: after tests complete, hit data is aggregated +4. **Reporting**: each source line is classified as executable or not, and the executable ones are matched against the hits ::: tip Performance -The DEBUG trap adds overhead to test execution. For large test suites, consider running coverage periodically rather than on every test run. +Coverage roughly doubles to quadruples wall-clock time, depending on Bash version +and engine. Cost is split fairly evenly between capture and reporting, and the +reporting half is the same whichever engine ran. If that is too slow to keep on +while you work, narrow `BASHUNIT_COVERAGE_PATHS` — both halves scale with the +number of tracked source lines, not with the size of the test suite. ::: +### Seeing which engine ran + +`--verbose` prints the engine that was actually used: + +```bash +./bashunit --coverage --verbose tests/ +# Coverage engine: xtrace +``` + +An explicit `BASHUNIT_COVERAGE_ENGINE=xtrace` that the running Bash cannot honour +is reported rather than silently ignored: + +``` +Warning: coverage engine 'xtrace' needs Bash 4.1+ (running 3.2); using 'trap'. +``` + +`auto` falling back to `trap` is normal and is not warned about. + ## Configuration ### Command Line Options diff --git a/src/config/env.sh b/src/config/env.sh index 6d38bd6b..21bc4964 100644 --- a/src/config/env.sh +++ b/src/config/env.sh @@ -637,6 +637,7 @@ function bashunit::env::print_verbose() { "BASHUNIT_COVERAGE_REPORT" "BASHUNIT_COVERAGE_REPORT_HTML" "BASHUNIT_COVERAGE_MIN" + "BASHUNIT_COVERAGE_ENGINE" ) local max_length=0 diff --git a/src/coverage/config.sh b/src/coverage/config.sh index 65a52d49..a6ca24ed 100644 --- a/src/coverage/config.sh +++ b/src/coverage/config.sh @@ -143,3 +143,34 @@ function bashunit::coverage::resolve_engine() { *) echo "trap" ;; esac } + +## +# The engine this run is actually using. +# Prefers the value resolved once in init and exported to every worker, so a +# worker reports what it ran rather than re-deciding. +# Returns: prints "xtrace" or "trap" +## +function bashunit::coverage::engine_in_use() { + if [ -n "${_BASHUNIT_COVERAGE_ENGINE_RESOLVED:-}" ]; then + echo "$_BASHUNIT_COVERAGE_ENGINE_RESOLVED" + return 0 + fi + bashunit::coverage::resolve_engine +} + +## +# Whether an explicit engine request could not be honoured. +# Only an explicit `xtrace` counts: `auto` resolving to trap is the documented +# fallback, not an ignored request. macOS ships Bash 3.2, so this is the common +# case there and used to be entirely silent (#1005). +# Returns: 0 when downgraded, 1 otherwise +## +function bashunit::coverage::engine_was_downgraded() { + if [ "${BASHUNIT_COVERAGE_ENGINE:-auto}" != "xtrace" ]; then + return 1 + fi + if bashunit::coverage::xtrace_is_supported; then + return 1 + fi + return 0 +} diff --git a/src/coverage/lines.sh b/src/coverage/lines.sh index f11fc82f..6fd21379 100644 --- a/src/coverage/lines.sh +++ b/src/coverage/lines.sh @@ -3,32 +3,19 @@ # Static line classification and reading recorded hit data. -# Pre-compiled combined regex of all non-executable line patterns. -# Collapses multiple grep subshells into a single invocation per line for performance. -# Each alternation is fully self-anchored so semantics match the original per-pattern checks. -# Patterns covered (in order): -# - comment-only lines (including shebang) -# - function declarations (but not single-line functions with a body) -# - brace-only lines -# - control flow keywords (then, else, fi, do, done, esac, in, ;;, ;;&, ;&) -# - loop terminators with redirection/pipe/fd (e.g. "done < file", "done | sort") -# - case patterns like "--option)" or "*) # comment" -# - standalone ) for arrays/subshells -_BASHUNIT_COVERAGE_NONEXEC_PATTERN='^[[:space:]]*#' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'|^[[:space:]]*(function[[:space:]]+)?' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'[a-zA-Z_][a-zA-Z0-9_:]*' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'[[:space:]]*\(\)[[:space:]]*\{?[[:space:]]*$' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'|^[[:space:]]*[\{\}][[:space:]]*$' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'|^[[:space:]]*' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'(then|else|fi|do|done|esac|in|;;|;;&|;&)' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'[[:space:]]*(#.*)?$' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'|^[[:space:]]*done' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'[[:space:]]+[^[:space:]#].*$' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'|^[[:space:]]*[^\)]+\)[[:space:]]*(#.*)?$' -_BASHUNIT_COVERAGE_NONEXEC_PATTERN="${_BASHUNIT_COVERAGE_NONEXEC_PATTERN}"'|^[[:space:]]*\)[[:space:]]*(#.*)?$' - - # Check if a line is executable (used by get_executable_lines and report_lcov) +# +# Every tracked line is classified twice per run (once by precompute_file_stats, +# once by report_lcov), so this is the report phase's hottest path and stays +# fork-free. It used to fall back to one `grep -E` per unclassified line against +# a combined regex; 286 source lines cost 1055 grep forks and roughly half of a +# `--coverage` run's wall time (#1005). The pure-Bash rules below reproduce that +# regex exactly, quirks included — see the comments on the individual cases. +# +# Non-executable lines are: comments and the shebang, brace-only lines, a bare +# line continuation, control-flow keywords, loop terminators with a redirection +# or pipe, function declarations, and case patterns. +# # Arguments: line content, line number # Returns: 0 if executable, 1 if not function bashunit::coverage::is_executable_line() { @@ -47,22 +34,82 @@ function bashunit::coverage::is_executable_line() { local trimmed="${stripped%"$_trail"}" case "$trimmed" in - '#'*) return 1 ;; # Comments (including shebang) - '{' | '}') return 1 ;; # Braces only + '#'*) return 1 ;; # Comments (including shebang) + '{' | '}' | [\\]) return 1 ;; # Braces only, or a bare line continuation esac + # A keyword may butt straight up against a trailing comment (`done#note`), + # so end the token at a `#` as well as at whitespace. local first="${trimmed%%[[:space:]]*}" + first="${first%%'#'*}" case "$first" in 'then' | 'else' | 'fi' | 'do' | 'done' | 'esac' | 'in' | ';;' | ';;&' | ';&' | ')') local rest="${trimmed#"$first"}" local _rl="${rest%%[![:space:]]*}" rest="${rest#"$_rl"}" case "$rest" in '' | '#'*) return 1 ;; esac + # A loop terminator still terminates a loop when a redirection or a pipe + # follows it: `done < file`, `done | sort`. Reaching here means `done` was + # followed by whitespace and something that is not a comment. + if [ "$first" = 'done' ]; then + return 1 + fi ;; esac - # Fallback: grep for complex patterns (function declarations, case patterns, done+redirection) - [ "$(printf '%s' "$line" | "$GREP" -cE "$_BASHUNIT_COVERAGE_NONEXEC_PATTERN" || true)" -gt 0 ] && return 1 + # Function declarations: `[function ]name()` with an optional trailing `{`. + # No trailing comment is allowed, matching the pattern this replaced. + case "$trimmed" in + *'()'*) + local fn_rest="$trimmed" + case "$fn_rest" in + 'function'[[:space:]]*) + fn_rest="${fn_rest#function}" + fn_rest="${fn_rest#"${fn_rest%%[![:space:]]*}"}" + ;; + esac + case "$fn_rest" in + *'{') + fn_rest="${fn_rest%'{'}" + fn_rest="${fn_rest%"${fn_rest##*[![:space:]]}"}" + ;; + esac + case "$fn_rest" in + *'()') + local fn_name="${fn_rest%'()'}" + fn_name="${fn_name%"${fn_name##*[![:space:]]}"}" + case "$fn_name" in + [a-zA-Z_]*) + case "${fn_name#?}" in + *[!a-zA-Z0-9_:]*) : ;; + *) return 1 ;; + esac + ;; + esac + ;; + esac + ;; + esac + + # Case patterns: something, then `)`, then end of line or a comment — + # `--option)`, `*) # note`. The pattern this replaced spelled the leading + # segment `[^\)]+`, and inside a POSIX bracket expression a backslash is a + # literal member of the set, so a backslash anywhere before that `)` + # suppressed the match. `x=$(foo)` is therefore classified non-executable + # while `x=$(printf '%s\n')` is executable; both are preserved here. + case "$line" in + *')'*) + local cp_before="${line%%')'*}" + case "$cp_before" in + '' | *[\\]*) : ;; + *) + local cp_after="${line#*')'}" + cp_after="${cp_after#"${cp_after%%[![:space:]]*}"}" + case "$cp_after" in '' | '#'*) return 1 ;; esac + ;; + esac + ;; + esac return 0 } diff --git a/src/coverage/report_text.sh b/src/coverage/report_text.sh index 27595e87..465585ff 100644 --- a/src/coverage/report_text.sh +++ b/src/coverage/report_text.sh @@ -2,6 +2,26 @@ # Terminal coverage report. +## +# Say which engine ran, and never let an ignored explicit request pass silently. +# +# The warning is unconditional because an ignored BASHUNIT_COVERAGE_ENGINE is a +# misconfiguration the user cannot otherwise see: on macOS's Bash 3.2 an explicit +# `xtrace` was accepted and dropped with no output at all (#1005). The engine +# line itself is verbose-only, since `auto` resolving to trap is normal. +## +function bashunit::coverage::print_engine_notice() { + if bashunit::coverage::engine_was_downgraded; then + printf "%sWarning: coverage engine 'xtrace' needs Bash 4.1+ (running %s.%s); using 'trap'.%s\n" \ + "$_BASHUNIT_COLOR_INCOMPLETE" "${BASH_VERSINFO[0]}" "${BASH_VERSINFO[1]}" \ + "$_BASHUNIT_COLOR_DEFAULT" + fi + + if bashunit::env::is_verbose_enabled; then + printf "Coverage engine: %s\n" "$(bashunit::coverage::engine_in_use)" + fi +} + function bashunit::coverage::report_text() { if ! bashunit::env::is_coverage_enabled; then return 0 @@ -12,6 +32,7 @@ function bashunit::coverage::report_text() { local has_files=false echo "" + bashunit::coverage::print_engine_notice echo "Coverage Report" echo "---------------" diff --git a/tests/acceptance/bashunit_coverage_forks_test.sh b/tests/acceptance/bashunit_coverage_forks_test.sh new file mode 100644 index 00000000..f4413fb0 --- /dev/null +++ b/tests/acceptance/bashunit_coverage_forks_test.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Regression guard for the coverage *report* phase. +# +# Profiling #1005 showed the report, not the capture engine, was half the cost +# of a `--coverage` run: `is_executable_line` fell back to a `grep -E` fork for +# every line it could not classify in pure Bash, and every tracked line is +# classified twice per run (once by precompute_file_stats, once by report_lcov). +# 286 source lines cost 1055 grep forks; the classification is pure Bash now. +# +# The budget is expressed as "does not grow with the number of source lines" +# rather than an absolute count: that is the property that was fixed, and it +# does not need retuning when an unrelated grep is added elsewhere. + +# Writes a fixture pair into $1: a test file, plus a source file of $2 lines +# whose content exercises every branch of the classifier (comments, function +# declarations, case patterns, loop terminators, continuations). +function _coverage_fixture() { + local dir="$1" + local body_lines="$2" + + { + echo '#!/usr/bin/env bash' + echo 'function covered_fn() {' + echo ' local total=0' + local i + for i in $(seq 1 "$body_lines"); do + echo " # comment $i" + echo " total=\$((total + $i))" + echo " case \"\$total\" in" + echo " ${i}) total=\$((total + 0)) ;;" + echo " *) : ;;" + echo " esac" + done + echo ' echo "$total"' + echo '}' + # Deliberately not named *_test.sh: that is a default BASHUNIT_COVERAGE_EXCLUDE + # pattern, and an excluded file is never classified at all. + } >"$dir/libcov.sh" + + { + echo "source \"$dir/libcov.sh\"" + echo 'function test_covers_source() { assert_not_empty "$(covered_fn)"; }' + } >"$dir/coverage_forks_test.sh" +} + +# Runs a --coverage run with a `grep` PATH shim and echoes the fork count. +function _count_grep_forks_for_coverage() { + local dir="$1" + local real_grep="$2" + local count_file="$dir/grep_count" + + { + echo '#!/usr/bin/env bash' + echo "echo x >> \"$count_file\"" + echo "exec \"$real_grep\" \"\$@\"" + } >"$dir/grep" + chmod +x "$dir/grep" + : >"$count_file" + + PATH="$dir:$PATH" \ + BASHUNIT_COVERAGE_PATHS="$dir" \ + BASHUNIT_COVERAGE_REPORT="$dir/lcov.info" \ + ./bashunit --no-parallel --coverage "$dir/coverage_forks_test.sh" >/dev/null 2>&1 || true + + local forks=0 + if [ -f "$count_file" ]; then + forks="$(grep -c . "$count_file" || true)" + fi + echo "$forks" +} + +function test_coverage_report_does_not_fork_grep_per_source_line() { + if bashunit::check_os::is_windows; then + bashunit::skip "PATH shims are unreliable under Git Bash" && return + fi + + local real_grep + real_grep="$(command -v grep)" + + # Canonicalise: bashunit::temp_dir can yield a doubled slash (TMPDIR already + # ends in one), and BASHUNIT_COVERAGE_PATHS is prefix-matched against paths + # that coverage has already canonicalised — a mismatch tracks nothing at all + # and the census would silently measure an empty run. + local small_dir large_dir + small_dir="$(cd "$(bashunit::temp_dir)" && pwd)" + large_dir="$(cd "$(bashunit::temp_dir)" && pwd)" + + # 6 lines of source per unit: the large fixture has ~240 more lines to + # classify than the small one, which used to cost ~480 extra grep forks + # (every line is classified twice per run). + _coverage_fixture "$small_dir" 2 + _coverage_fixture "$large_dir" 42 + + local small_forks large_forks + small_forks="$(_count_grep_forks_for_coverage "$small_dir" "$real_grep")" + large_forks="$(_count_grep_forks_for_coverage "$large_dir" "$real_grep")" + + # Allow a small constant slack for the per-file greps that legitimately + # remain (hit-data extraction), but nothing proportional to line count. + assert_less_than 20 "$((large_forks - small_forks))" +} diff --git a/tests/unit/coverage/engine_test.sh b/tests/unit/coverage/engine_test.sh index 22b7a07b..50b6ad4c 100644 --- a/tests/unit/coverage/engine_test.sh +++ b/tests/unit/coverage/engine_test.sh @@ -6,13 +6,16 @@ # on every host, independent of the Bash the suite happens to run under. _ORIG_COVERAGE_ENGINE="" +_ORIG_VERBOSE="" function set_up() { _ORIG_COVERAGE_ENGINE="${BASHUNIT_COVERAGE_ENGINE:-}" + _ORIG_VERBOSE="${BASHUNIT_VERBOSE:-}" } function tear_down() { BASHUNIT_COVERAGE_ENGINE="$_ORIG_COVERAGE_ENGINE" + BASHUNIT_VERBOSE="$_ORIG_VERBOSE" } function _pretend_xtrace_supported() { @@ -87,3 +90,116 @@ function test_xtrace_support_probe_matches_the_running_bash() { assert_equals "$expected" "$actual" } + +# On macOS's system Bash 3.2 an explicit BASHUNIT_COVERAGE_ENGINE=xtrace used to +# be accepted and ignored, with nothing said about it (#1005). +function test_an_explicit_xtrace_request_is_a_downgrade_when_unsupported() { + _pretend_xtrace_unsupported + BASHUNIT_COVERAGE_ENGINE="xtrace" + + local exit_code=0 + bashunit::coverage::engine_was_downgraded || exit_code=$? + + assert_equals 0 "$exit_code" +} + +function test_an_honoured_xtrace_request_is_not_a_downgrade() { + _pretend_xtrace_supported + BASHUNIT_COVERAGE_ENGINE="xtrace" + + local exit_code=0 + bashunit::coverage::engine_was_downgraded || exit_code=$? + + assert_equals 1 "$exit_code" +} + +# `auto` picking trap is the documented fallback, not an ignored request. +function test_auto_falling_back_to_trap_is_not_a_downgrade() { + _pretend_xtrace_unsupported + BASHUNIT_COVERAGE_ENGINE="auto" + + local exit_code=0 + bashunit::coverage::engine_was_downgraded || exit_code=$? + + assert_equals 1 "$exit_code" +} + +function test_an_explicit_trap_request_is_not_a_downgrade() { + _pretend_xtrace_unsupported + BASHUNIT_COVERAGE_ENGINE="trap" + + local exit_code=0 + bashunit::coverage::engine_was_downgraded || exit_code=$? + + assert_equals 1 "$exit_code" +} + +function test_engine_in_use_reports_the_resolved_engine() { + _pretend_xtrace_supported + BASHUNIT_COVERAGE_ENGINE="auto" + local previous="${_BASHUNIT_COVERAGE_ENGINE_RESOLVED:-}" + _BASHUNIT_COVERAGE_ENGINE_RESOLVED="" + + local actual + actual="$(bashunit::coverage::engine_in_use)" + _BASHUNIT_COVERAGE_ENGINE_RESOLVED="$previous" + + assert_equals "xtrace" "$actual" +} + +# The resolved engine is picked once in init and inherited by every worker, so +# it wins over re-resolving: a worker must report what it actually ran. +function test_engine_in_use_prefers_the_engine_resolved_at_init() { + _pretend_xtrace_supported + BASHUNIT_COVERAGE_ENGINE="auto" + local previous="${_BASHUNIT_COVERAGE_ENGINE_RESOLVED:-}" + _BASHUNIT_COVERAGE_ENGINE_RESOLVED="trap" + + local actual + actual="$(bashunit::coverage::engine_in_use)" + _BASHUNIT_COVERAGE_ENGINE_RESOLVED="$previous" + + assert_equals "trap" "$actual" +} + +function test_engine_notice_warns_when_an_explicit_request_was_downgraded() { + _pretend_xtrace_unsupported + BASHUNIT_COVERAGE_ENGINE="xtrace" + BASHUNIT_VERBOSE="false" + + local output + output="$(bashunit::coverage::print_engine_notice)" + + assert_contains "xtrace" "$output" + assert_contains "trap" "$output" +} + +function test_engine_notice_is_silent_when_auto_falls_back() { + _pretend_xtrace_unsupported + BASHUNIT_COVERAGE_ENGINE="auto" + BASHUNIT_VERBOSE="false" + + assert_empty "$(bashunit::coverage::print_engine_notice)" +} + +function test_engine_notice_is_silent_when_the_request_was_honoured() { + _pretend_xtrace_supported + BASHUNIT_COVERAGE_ENGINE="xtrace" + BASHUNIT_VERBOSE="false" + + assert_empty "$(bashunit::coverage::print_engine_notice)" +} + +function test_engine_notice_reports_the_engine_when_verbose() { + _pretend_xtrace_supported + BASHUNIT_COVERAGE_ENGINE="auto" + BASHUNIT_VERBOSE="true" + local previous="${_BASHUNIT_COVERAGE_ENGINE_RESOLVED:-}" + _BASHUNIT_COVERAGE_ENGINE_RESOLVED="xtrace" + + local output + output="$(bashunit::coverage::print_engine_notice)" + _BASHUNIT_COVERAGE_ENGINE_RESOLVED="$previous" + + assert_contains "xtrace" "$output" +}