Skip to content

Commit c86c220

Browse files
authored
fix(runner): remove the run-output scratch dir on exit (#811)
1 parent 788cf49 commit c86c220

5 files changed

Lines changed: 57 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Unreleased
44

55
### Fixed
6+
- 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
67
- `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)
78
- `--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
89

src/env.sh

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,28 @@ BASHUNIT_TEMP_DIR="${TMPDIR:-/tmp}/bashunit/tmp"
509509
# Create both scratch directories in a single `mkdir -p` fork.
510510
mkdir -p "$_BASHUNIT_RUN_OUTPUT_DIR" "$BASHUNIT_TEMP_DIR" 2>/dev/null || true
511511

512+
# Removes this run's scratch directory (guarded like parallel::cleanup so a
513+
# broken variable can never turn the rm loose elsewhere). Called at the end of
514+
# a run and on SIGINT; without it every invocation leaks one directory.
515+
function bashunit::env::cleanup_run_output_dir() {
516+
local target="$_BASHUNIT_RUN_OUTPUT_DIR"
517+
case "$target" in
518+
*/bashunit/run/*)
519+
rm -rf "$target"
520+
;;
521+
*)
522+
bashunit::internal_log "env::cleanup_run_output_dir" "refused unsafe path:$target"
523+
return 1
524+
;;
525+
esac
526+
}
527+
528+
# Cover early-exit paths (--version, --help, doc, init, ...). The test-run path
529+
# replaces this trap in main.sh and calls the cleanup explicitly instead; child
530+
# subshells never inherit EXIT traps, so a parallel worker cannot remove the
531+
# directory mid-run.
532+
trap 'bashunit::env::cleanup_run_output_dir' EXIT
533+
512534
if bashunit::env::is_dev_mode_enabled; then
513535
bashunit::internal_log "info" "Dev log enabled" "file:$BASHUNIT_DEV_LOG"
514536
fi

src/main.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,9 @@ function bashunit::main::exec_tests() {
901901
# Persist this run's failing tests so a later --rerun-failed can replay them.
902902
bashunit::rerun::persist
903903

904+
# The rerun cache is read from the run dir above, so clean up only after it.
905+
bashunit::env::cleanup_run_output_dir
906+
904907
bashunit::internal_log "Finished tests" "exit_code:$exit_code"
905908
exit $exit_code
906909
}
@@ -945,6 +948,7 @@ function bashunit::main::cleanup() {
945948
if bashunit::parallel::is_enabled; then
946949
bashunit::parallel::cleanup
947950
fi
951+
bashunit::env::cleanup_run_output_dir
948952
exit 1
949953
}
950954

@@ -962,6 +966,7 @@ function bashunit::main::handle_stop_on_failure_sync() {
962966
if bashunit::parallel::is_enabled; then
963967
bashunit::parallel::cleanup
964968
fi
969+
bashunit::env::cleanup_run_output_dir
965970
exit 1
966971
}
967972

src/runner.sh

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,16 +392,18 @@ function bashunit::runner::load_test_files() {
392392
scripts_ids[scripts_ids_count]="${BASHUNIT_CURRENT_SCRIPT_ID}"
393393
scripts_ids_count=$((scripts_ids_count + 1))
394394
bashunit::internal_log "Loading file" "$test_file"
395+
# Files are sourced sequentially in this loop (parallel workers fork after),
396+
# so a fixed path in the run dir is safe: `2>` truncates it per file and the
397+
# run-dir cleanup removes it, saving a mktemp and an rm fork per file.
395398
local source_err_file source_err source_status
396-
source_err_file="$(bashunit::temp_file "source_err")"
399+
source_err_file="$_BASHUNIT_RUN_OUTPUT_DIR/source_err"
397400
# shellcheck source=/dev/null
398401
source "$test_file" 2>"$source_err_file"
399402
source_status=$?
400403
source_err=""
401404
if [ -s "$source_err_file" ]; then
402405
source_err="$(cat "$source_err_file")"
403406
fi
404-
rm -f "$source_err_file"
405407
# A non-zero source status, or a syntax-error line on stderr, means the file
406408
# failed to load. Match the captured stderr with `case` (no grep fork).
407409
local source_failed=false

tests/acceptance/bashunit_run_forks_test.sh

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,28 @@ function test_running_a_test_file_stays_within_the_awk_fork_budget() {
157157

158158
assert_less_or_equal_than 3 "$awk_forks"
159159
}
160+
161+
# Regression guard: a run must remove its run-output scratch directory on exit.
162+
# The directory (introduced in #801 under $TMPDIR/bashunit/run/) had no cleanup,
163+
# leaking one empty directory per bashunit invocation. Point the nested run at a
164+
# private TMPDIR so the check is deterministic and parallel-safe.
165+
function test_run_removes_its_run_output_dir() {
166+
local dir
167+
dir="$(bashunit::temp_dir)"
168+
local fixture="$dir/leak_probe_test.sh"
169+
printf 'function test_ok() { assert_true true; }\n' >"$fixture"
170+
171+
TMPDIR="$dir" ./bashunit --no-parallel "$fixture" >/dev/null 2>&1
172+
# Early-exit paths (no test run) must clean up via the EXIT trap.
173+
TMPDIR="$dir" ./bashunit --version >/dev/null 2>&1
174+
175+
local leftover=0
176+
if [ -d "$dir/bashunit/run" ]; then
177+
local entry
178+
for entry in "$dir/bashunit/run"/*/*; do
179+
[ -e "$entry" ] && leftover=$((leftover + 1))
180+
done
181+
fi
182+
183+
assert_equals 0 "$leftover"
184+
}

0 commit comments

Comments
 (0)