diff --git a/CHANGELOG.md b/CHANGELOG.md index 03777d97..a392162f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased ### Fixed +- Every run cleans up its run-output scratch directory on exit (test runs, `--version`/`--help`/subcommand exits, and Ctrl-C); previously each invocation leaked one directory under `$TMPDIR/bashunit/run/`. Sourcing errors are captured in that directory too, saving a `mktemp` and an `rm` fork per test file - `bashunit::helper::get_function_line_number` no longer disables `extdebug` for its caller: it enables the option only inside a subshell (also dropping an `awk` fork) - `--test-timeout` no longer intermittently reports a fast test as timed out; the watchdog now signals by pid and skips a test that already completed diff --git a/src/env.sh b/src/env.sh index 3c5f4c17..4980bf9e 100644 --- a/src/env.sh +++ b/src/env.sh @@ -509,6 +509,28 @@ BASHUNIT_TEMP_DIR="${TMPDIR:-/tmp}/bashunit/tmp" # Create both scratch directories in a single `mkdir -p` fork. mkdir -p "$_BASHUNIT_RUN_OUTPUT_DIR" "$BASHUNIT_TEMP_DIR" 2>/dev/null || true +# Removes this run's scratch directory (guarded like parallel::cleanup so a +# broken variable can never turn the rm loose elsewhere). Called at the end of +# a run and on SIGINT; without it every invocation leaks one directory. +function bashunit::env::cleanup_run_output_dir() { + local target="$_BASHUNIT_RUN_OUTPUT_DIR" + case "$target" in + */bashunit/run/*) + rm -rf "$target" + ;; + *) + bashunit::internal_log "env::cleanup_run_output_dir" "refused unsafe path:$target" + return 1 + ;; + esac +} + +# Cover early-exit paths (--version, --help, doc, init, ...). The test-run path +# replaces this trap in main.sh and calls the cleanup explicitly instead; child +# subshells never inherit EXIT traps, so a parallel worker cannot remove the +# directory mid-run. +trap 'bashunit::env::cleanup_run_output_dir' EXIT + if bashunit::env::is_dev_mode_enabled; then bashunit::internal_log "info" "Dev log enabled" "file:$BASHUNIT_DEV_LOG" fi diff --git a/src/main.sh b/src/main.sh index 7bb2bb92..0b031c14 100644 --- a/src/main.sh +++ b/src/main.sh @@ -901,6 +901,9 @@ function bashunit::main::exec_tests() { # Persist this run's failing tests so a later --rerun-failed can replay them. bashunit::rerun::persist + # The rerun cache is read from the run dir above, so clean up only after it. + bashunit::env::cleanup_run_output_dir + bashunit::internal_log "Finished tests" "exit_code:$exit_code" exit $exit_code } @@ -945,6 +948,7 @@ function bashunit::main::cleanup() { if bashunit::parallel::is_enabled; then bashunit::parallel::cleanup fi + bashunit::env::cleanup_run_output_dir exit 1 } @@ -962,6 +966,7 @@ function bashunit::main::handle_stop_on_failure_sync() { if bashunit::parallel::is_enabled; then bashunit::parallel::cleanup fi + bashunit::env::cleanup_run_output_dir exit 1 } diff --git a/src/runner.sh b/src/runner.sh index 4b190bd2..e0572d7a 100755 --- a/src/runner.sh +++ b/src/runner.sh @@ -392,8 +392,11 @@ function bashunit::runner::load_test_files() { scripts_ids[scripts_ids_count]="${BASHUNIT_CURRENT_SCRIPT_ID}" scripts_ids_count=$((scripts_ids_count + 1)) bashunit::internal_log "Loading file" "$test_file" + # Files are sourced sequentially in this loop (parallel workers fork after), + # so a fixed path in the run dir is safe: `2>` truncates it per file and the + # run-dir cleanup removes it, saving a mktemp and an rm fork per file. local source_err_file source_err source_status - source_err_file="$(bashunit::temp_file "source_err")" + source_err_file="$_BASHUNIT_RUN_OUTPUT_DIR/source_err" # shellcheck source=/dev/null source "$test_file" 2>"$source_err_file" source_status=$? @@ -401,7 +404,6 @@ function bashunit::runner::load_test_files() { if [ -s "$source_err_file" ]; then source_err="$(cat "$source_err_file")" fi - rm -f "$source_err_file" # A non-zero source status, or a syntax-error line on stderr, means the file # failed to load. Match the captured stderr with `case` (no grep fork). local source_failed=false diff --git a/tests/acceptance/bashunit_run_forks_test.sh b/tests/acceptance/bashunit_run_forks_test.sh index 68758036..abb39224 100644 --- a/tests/acceptance/bashunit_run_forks_test.sh +++ b/tests/acceptance/bashunit_run_forks_test.sh @@ -157,3 +157,28 @@ function test_running_a_test_file_stays_within_the_awk_fork_budget() { assert_less_or_equal_than 3 "$awk_forks" } + +# Regression guard: a run must remove its run-output scratch directory on exit. +# The directory (introduced in #801 under $TMPDIR/bashunit/run/) had no cleanup, +# leaking one empty directory per bashunit invocation. Point the nested run at a +# private TMPDIR so the check is deterministic and parallel-safe. +function test_run_removes_its_run_output_dir() { + local dir + dir="$(bashunit::temp_dir)" + local fixture="$dir/leak_probe_test.sh" + printf 'function test_ok() { assert_true true; }\n' >"$fixture" + + TMPDIR="$dir" ./bashunit --no-parallel "$fixture" >/dev/null 2>&1 + # Early-exit paths (no test run) must clean up via the EXIT trap. + TMPDIR="$dir" ./bashunit --version >/dev/null 2>&1 + + local leftover=0 + if [ -d "$dir/bashunit/run" ]; then + local entry + for entry in "$dir/bashunit/run"/*/*; do + [ -e "$entry" ] && leftover=$((leftover + 1)) + done + fi + + assert_equals 0 "$leftover" +}