Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

### Changed
- Faster test execution: removed avoidable subshell forks from the per-test hot path (runtime-error detection, subshell-output decode, name normalization, test-id generation, retry-count lookup, and the passing-test line are now fork-free), and per-test temp-file cleanup no longer forks `rm` when the test created none (`rm` 102 -> 2 on a 100-test file). No behaviour change (#764)
- Per-test execution time now defaults to `auto` (`BASHUNIT_SHOW_EXECUTION_TIME=true|false|auto`): shown when the shell has a fork-free clock (Bash 5.0+, GNU `date`), hidden when measuring would fork an interpreter (e.g. Bash 3.2 on macOS, which falls back to `perl`). This removes ~2 `perl` forks per test on those shells. `--profile`, `--verbose`, reports, and `=true` still measure. See `adrs/adr-008-auto-skip-per-test-timing.md` (#765)
- Faster test execution: each test file's data-provider annotations are scanned once and cached, replacing a per-test `grep`+`sed` probe with a pure-bash lookup (no behaviour change) (#763)
- Faster test execution: skip the base64 encode fork for a test's empty title, hook message and output (the common case), and short-circuit decoding of empty fields (no behaviour change) (#762)
Expand Down
13 changes: 9 additions & 4 deletions src/console_results.sh
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,11 @@ function bashunit::console_results::print_successful_test() {
local duration=${1:-"0"}
shift

# Pure-bash concatenation (the printf only did %s substitution) to avoid a
# $(...) fork per passing test (#764).
local line
if [ -z "$*" ]; then
line=$(printf "%s✓ Passed%s: %s" "$_BASHUNIT_COLOR_PASSED" "$_BASHUNIT_COLOR_DEFAULT" "$test_name")
line="${_BASHUNIT_COLOR_PASSED}✓ Passed${_BASHUNIT_COLOR_DEFAULT}: ${test_name}"
else
local quoted_args=""
local arg
Expand All @@ -240,8 +242,7 @@ function bashunit::console_results::print_successful_test() {
quoted_args="$quoted_args, '$arg'"
fi
done
line=$(printf "%s✓ Passed%s: %s (%s)" \
"$_BASHUNIT_COLOR_PASSED" "$_BASHUNIT_COLOR_DEFAULT" "$test_name" "$quoted_args")
line="${_BASHUNIT_COLOR_PASSED}✓ Passed${_BASHUNIT_COLOR_DEFAULT}: ${test_name} (${quoted_args})"
fi

# Retry annotation (e.g. " (retry 1/2)") set by the runner when a test only
Expand Down Expand Up @@ -371,7 +372,11 @@ function bashunit::console_results::snapshot_line_diff() {

# Explicit empty-array init so referencing the arrays is safe under `set -u`
# on Bash 4.4+ (Bash 3.x is lenient; newer Bash treats an unset array as unbound).
local expected_lines=() actual_lines=()
# Declare and assign separately: bash 3.0 does not expand a compound array
# assignment attached to `local`, it stores the literal "()" as element 0.
local expected_lines actual_lines
expected_lines=()
actual_lines=()
local _line=""
local i=0
while IFS= read -r _line || [ -n "$_line" ]; do
Expand Down
18 changes: 12 additions & 6 deletions src/env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,20 @@ function bashunit::env::test_timeout_secs() {
# Prints the number of extra attempts for a failed test (0 = no retry).
# A non-numeric value is treated as 0.
##
function bashunit::env::retry_count() {
# Validates BASHUNIT_RETRY into the integer global _BASHUNIT_RETRY_VALIDATED.
# In-shell (no fork) so the per-test hot path can read the global instead of
# capturing retry_count in a $(...) subshell every test (#764).
_BASHUNIT_RETRY_VALIDATED=0
function bashunit::env::resolve_retry_count() {
case "${BASHUNIT_RETRY:-0}" in
'' | *[!0-9]*)
printf '%s' "0"
return
;;
'' | *[!0-9]*) _BASHUNIT_RETRY_VALIDATED=0 ;;
*) _BASHUNIT_RETRY_VALIDATED="${BASHUNIT_RETRY:-0}" ;;
esac
printf '%s' "${BASHUNIT_RETRY:-0}"
}

function bashunit::env::retry_count() {
bashunit::env::resolve_retry_count
printf '%s' "$_BASHUNIT_RETRY_VALIDATED"
}

function bashunit::env::is_random_order_enabled() {
Expand Down
12 changes: 11 additions & 1 deletion src/globals.sh
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,17 @@ function bashunit::temp_dir() {
function bashunit::cleanup_testcase_temp_files() {
bashunit::internal_log "cleanup_testcase_temp_files"
if [ -n "${BASHUNIT_CURRENT_TEST_ID:-}" ]; then
rm -rf "$BASHUNIT_TEMP_DIR/${BASHUNIT_CURRENT_TEST_ID}"_*
# Probe the glob in pure bash first: most tests create no temp file, so
# skipping the rm avoids a fork per test (#764). A non-matching glob either
# stays literal (nullglob off) or yields an empty array (nullglob on);
# ${matches[0]:-} handles both under set -u, and [ -e ] is false in each
# case. temp_file runs in a $(...) subshell, so a global flag could not
# reach this trap; checking the filesystem is the only reliable signal.
# Declare and assign separately: bash 3.0 does not expand a compound array
# assignment attached to `local`, it stores the literal "(glob)" instead.
local matches
matches=("$BASHUNIT_TEMP_DIR/${BASHUNIT_CURRENT_TEST_ID}"_*)
[ -e "${matches[0]:-}" ] && rm -rf "${matches[@]}"
fi
}

Expand Down
16 changes: 10 additions & 6 deletions src/helpers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ function bashunit::helper::normalize_test_function_name() {
local original_fn_name="${1-}"
local interpolated_fn_name="${2-}"

local custom_title
custom_title="$(bashunit::state::get_test_title)"
# Read the reserved-namespace state globals directly (the accessors just echo
# them) to avoid a nested subshell fork on this per-test hot path (#764).
local custom_title="${_BASHUNIT_TEST_TITLE:-}"
if [ -n "$custom_title" ]; then
echo "$custom_title"
return
Expand All @@ -51,8 +52,7 @@ function bashunit::helper::normalize_test_function_name() {
if [ -z "${interpolated_fn_name-}" ]; then
case "${original_fn_name}" in
*"::"*)
local state_interpolated_fn_name
state_interpolated_fn_name="$(bashunit::state::get_current_test_interpolated_function_name)"
local state_interpolated_fn_name="${_BASHUNIT_CURRENT_TEST_INTERPOLATED_NAME:-}"

if [ -n "$state_interpolated_fn_name" ]; then
interpolated_fn_name="$state_interpolated_fn_name"
Expand Down Expand Up @@ -549,6 +549,10 @@ function bashunit::helper::get_function_line_number() {
echo "$line_number"
}

# Writes a sanitized, process-unique id into _BASHUNIT_HELPER_ID_OUT.
# Return-slot form so the per-test caller avoids a $(...) capture fork (#764).
# Arguments: $1 basename
_BASHUNIT_HELPER_ID_OUT=""
function bashunit::helper::generate_id() {
local basename="$1"
# Inline normalize_variable_name + random_str to avoid two forks per call.
Expand All @@ -565,9 +569,9 @@ function bashunit::helper::generate_id() {
for ((_i = 0; _i < 6; _i++)); do
_suffix="$_suffix${_chars:RANDOM%${#_chars}:1}"
done
echo "${sanitized}_$$_${_suffix}"
_BASHUNIT_HELPER_ID_OUT="${sanitized}_$$_${_suffix}"
else
echo "${sanitized}_$$"
_BASHUNIT_HELPER_ID_OUT="${sanitized}_$$"
fi
}

Expand Down
50 changes: 34 additions & 16 deletions src/runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ function bashunit::runner::source_login_shell_profiles() {
function bashunit::runner::export_test_identity() {
local test_file=$1
local fn_name=$2
export BASHUNIT_CURRENT_TEST_ID="$(bashunit::helper::generate_id "$fn_name")"
bashunit::helper::generate_id "$fn_name"
export BASHUNIT_CURRENT_TEST_ID="$_BASHUNIT_HELPER_ID_OUT"
bashunit::runner::resolve_test_location "$test_file" "$fn_name"
export _BASHUNIT_TEST_LOCATION
if [ "${_BASHUNIT_COVERAGE_ON:-0}" = 1 ]; then
Expand Down Expand Up @@ -137,6 +138,8 @@ _BASHUNIT_RUNNER_COUNTS_SKIPPED_OUT=0
_BASHUNIT_RUNNER_COUNTS_INCOMPLETE_OUT=0
_BASHUNIT_RUNNER_COUNTS_SNAPSHOT_OUT=0
_BASHUNIT_RUNNER_COUNTS_EXIT_CODE_OUT=0
_BASHUNIT_RUNNER_RUNTIME_ERROR_OUT=""
_BASHUNIT_RUNNER_SUBSHELL_OUTPUT_OUT=""
# Suffix appended to a passed-test line when it only passed after retrying.
_BASHUNIT_RETRY_NOTE=""

Expand Down Expand Up @@ -209,8 +212,13 @@ function bashunit::runner::record_profile() {
printf '%s\t%s\t%s\n' "$duration" "$test_name" "$test_file" >>"$PROFILE_OUTPUT_PATH"
}

# Writes the detected runtime-error message (empty when none) into
# _BASHUNIT_RUNNER_RUNTIME_ERROR_OUT. Return-slot form avoids a per-test fork
# on the hot path (#764).
# Arguments: $1 runtime_output
function bashunit::runner::detect_runtime_error() {
local runtime_output=$1
_BASHUNIT_RUNNER_RUNTIME_ERROR_OUT=""
case "$runtime_output" in
*"command not found"* | *"unbound variable"* | *"permission denied"* | \
*"no such file or directory"* | *"syntax error"* | *"bad substitution"* | \
Expand All @@ -222,11 +230,9 @@ function bashunit::runner::detect_runtime_error() {
*"too many arguments"* | *"value too great"* | \
*"not a valid identifier"* | *"unexpected EOF"*)
local runtime_error="${runtime_output#*: }"
printf '%s' "${runtime_error//$'\n'/}"
return 0
_BASHUNIT_RUNNER_RUNTIME_ERROR_OUT="${runtime_error//$'\n'/}"
;;
esac
printf ''
}

##
Expand Down Expand Up @@ -364,7 +370,8 @@ function bashunit::runner::load_test_files() {
continue
fi
unset BASHUNIT_CURRENT_TEST_ID
export BASHUNIT_CURRENT_SCRIPT_ID="$(bashunit::helper::generate_id "${test_file}")"
bashunit::helper::generate_id "${test_file}"
export BASHUNIT_CURRENT_SCRIPT_ID="$_BASHUNIT_HELPER_ID_OUT"
scripts_ids[scripts_ids_count]="${BASHUNIT_CURRENT_SCRIPT_ID}"
scripts_ids_count=$((scripts_ids_count + 1))
bashunit::internal_log "Loading file" "$test_file"
Expand Down Expand Up @@ -489,7 +496,8 @@ function bashunit::runner::load_bench_files() {
for bench_file in "${files[@]+"${files[@]}"}"; do
[ -f "$bench_file" ] || continue
unset BASHUNIT_CURRENT_TEST_ID
export BASHUNIT_CURRENT_SCRIPT_ID="$(bashunit::helper::generate_id "${bench_file}")"
bashunit::helper::generate_id "${bench_file}"
export BASHUNIT_CURRENT_SCRIPT_ID="$_BASHUNIT_HELPER_ID_OUT"
# shellcheck source=/dev/null
source "$bench_file"
# Update function cache after sourcing new bench file
Expand Down Expand Up @@ -1074,8 +1082,8 @@ function bashunit::runner::run_test() {

local test_execution_result
local timed_out="false"
local retry_max
retry_max=$(bashunit::env::retry_count)
bashunit::env::resolve_retry_count
local retry_max=$_BASHUNIT_RETRY_VALIDATED
local retries_used=0
local measure_duration=false
bashunit::runner::needs_test_duration && measure_duration=true
Expand All @@ -1094,8 +1102,8 @@ function bashunit::runner::run_test() {
fi

local attempt_runtime_output="${test_execution_result%%##ASSERTIONS_*}"
local attempt_runtime_error
attempt_runtime_error=$(bashunit::runner::detect_runtime_error "$attempt_runtime_output")
bashunit::runner::detect_runtime_error "$attempt_runtime_output"
local attempt_runtime_error=$_BASHUNIT_RUNNER_RUNTIME_ERROR_OUT
bashunit::runner::extract_result_counts "$test_execution_result"
# Mirror the commit-phase failure test exactly (runtime error, non-zero exit,
# or a failed assertion); snapshot/incomplete/skipped/risky are not failures.
Expand Down Expand Up @@ -1127,7 +1135,8 @@ function bashunit::runner::run_test() {
"$test_file" "$fn_name" "$duration" "$test_execution_result"
fi

local subshell_output=$(bashunit::runner::decode_subshell_output "$test_execution_result")
bashunit::runner::decode_subshell_output "$test_execution_result"
local subshell_output=$_BASHUNIT_RUNNER_SUBSHELL_OUTPUT_OUT

if [ -n "$subshell_output" ]; then
bashunit::runner::extract_subshell_type "$subshell_output"
Expand All @@ -1139,10 +1148,11 @@ function bashunit::runner::run_test() {
fi
fi

local runtime_output="${test_execution_result%%##ASSERTIONS_*}"

local runtime_error
runtime_error=$(bashunit::runner::detect_runtime_error "$runtime_output")
# Reuse the final attempt's values (the loop always runs at least once and
# its locals persist in this function scope), instead of recomputing and
# forking detect_runtime_error a second time (#764).
local runtime_output=$attempt_runtime_output
local runtime_error=$attempt_runtime_error

# parse_result accumulates _BASHUNIT_TEST_EXIT_CODE; reset it so each test's
# exit code is read in isolation (a non-zero/timed-out test must not poison
Expand Down Expand Up @@ -1362,12 +1372,20 @@ function bashunit::runner::cleanup_on_exit() {
bashunit::state::export_subshell_context
}

# Writes the decoded subshell output into _BASHUNIT_RUNNER_SUBSHELL_OUTPUT_OUT.
# The empty case (a passing test with no captured output) short-circuits with
# no subshell at all; only the non-empty path pays the base64 fork (#762/#764).
# Arguments: $1 test_execution_result
function bashunit::runner::decode_subshell_output() {
local test_execution_result="$1"

local test_output_base64="${test_execution_result##*##TEST_OUTPUT=}"
test_output_base64="${test_output_base64%%##*}"
bashunit::helper::decode_base64 "$test_output_base64"
if [ -z "$test_output_base64" ] || [ "$test_output_base64" = "_BASHUNIT_EMPTY_" ]; then
_BASHUNIT_RUNNER_SUBSHELL_OUTPUT_OUT=""
return
fi
_BASHUNIT_RUNNER_SUBSHELL_OUTPUT_OUT="$(bashunit::helper::decode_base64 "$test_output_base64")"
}

function bashunit::runner::is_simple_progress_output() {
Expand Down
5 changes: 4 additions & 1 deletion src/watch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ function bashunit::watch::is_available() {
function bashunit::watch::run() {
local path="${1:-.}"
shift
local extra_args=("$@")
# Declare and assign separately: bash 3.0 does not expand a compound array
# assignment attached to `local`, it collapses "$@" into one literal element.
local extra_args
extra_args=("$@")

local tool
tool=$(bashunit::watch::is_available)
Expand Down
17 changes: 17 additions & 0 deletions tests/unit/bash_compatibility_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env bash

# Bash 3.0 does not expand a compound array assignment attached to `local`:
# `local arr=(a b)` stores the literal string "(a b)" as a single element
# instead of building the array. Every bash >= 3.2 does the right thing, so
# this only ever breaks on the Bash 3.0 jobs, and silently (see #764).
# Declare and assign on separate lines instead:
#
# local arr
# arr=(a b)
#
function test_src_has_no_compound_array_assignment_attached_to_local() {
local offenders
offenders=$(grep -rnE '^[[:space:]]*local[[:space:]]+[A-Za-z_][A-Za-z0-9_]*=\(' src/ || true)

assert_empty "$offenders"
}
20 changes: 20 additions & 0 deletions tests/unit/env_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,26 @@ function test_retry_count_treats_non_numeric_as_zero() {
assert_equals "0" "$result"
}

function test_resolve_retry_count_writes_configured_value_to_global() {
local original="${BASHUNIT_RETRY:-0}"
export BASHUNIT_RETRY="3"

bashunit::env::resolve_retry_count

export BASHUNIT_RETRY="$original"
assert_equals "3" "$_BASHUNIT_RETRY_VALIDATED"
}

function test_resolve_retry_count_writes_zero_for_non_numeric_to_global() {
local original="${BASHUNIT_RETRY:-0}"
export BASHUNIT_RETRY="abc"

bashunit::env::resolve_retry_count

export BASHUNIT_RETRY="$original"
assert_equals "0" "$_BASHUNIT_RETRY_VALIDATED"
}

function test_is_tap_output_enabled_when_format_is_tap() {
local original="$BASHUNIT_OUTPUT_FORMAT"
export BASHUNIT_OUTPUT_FORMAT="tap"
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/globals_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,35 @@ function test_globals_temp_dir_in_test_function() {
assert_directory_not_exists "$temp_dir"
}

function test_globals_cleanup_testcase_skips_and_preserves_others_when_none_created() {
local original_id="${BASHUNIT_CURRENT_TEST_ID:-}"
# A test id with no temp files of its own exercises the pure-bash skip path.
export BASHUNIT_CURRENT_TEST_ID="ghost_${$}_noexist"
local unrelated="$BASHUNIT_TEMP_DIR/unrelated_${$}.keep"
: >"$unrelated"

bashunit::cleanup_testcase_temp_files

assert_file_exists "$unrelated"
rm -f "$unrelated"
export BASHUNIT_CURRENT_TEST_ID="$original_id"
}

function test_globals_cleanup_testcase_skip_path_is_safe_under_nullglob() {
local original_id="${BASHUNIT_CURRENT_TEST_ID:-}"
export BASHUNIT_CURRENT_TEST_ID="ghost_${$}_noexist"

# With nullglob on the non-matching glob yields an empty array; the skip must
# not trip set -u on ${matches[0]}.
shopt -s nullglob
bashunit::cleanup_testcase_temp_files
local status=$?
shopt -u nullglob

assert_successful_code "$status"
export BASHUNIT_CURRENT_TEST_ID="$original_id"
}

function test_globals_temp_dir_and_file_in_script() {
assert_directory_exists "$SCRIPT_TEMP_DIR"
assert_file_exists "$SCRIPT_TEMP_FILE"
Expand Down
Loading
Loading