Skip to content

Commit 6a331bd

Browse files
committed
fix: ensure script-level temp files are always cleaned up
This commit resolves an issue where temporary files created in `set_up_before_script` were not properly cleaned up due to test ID contamination between test files. Unset BASHUNIT_CURRENT_TEST_ID and set BASHUNIT_CURRENT_SCRIPT_ID at the start of a script in order to make sure `temp_file()` and `temp_dir()` use the correct prefix for their path names. Remove `cleanup_temp_files()` in favor of more explicit `cleanup_testcase_temp_files()` and `cleanup_script_temp_files()` functions. Call `cleanup_testcase_temp_files()` in the testcase exit trap handler to make sure that testcase temp files are always cleaned. Call `cleanup_script_temp_files()` explicitly at the end of test file runs, as well as in cleanup handlers. This makes the general cleaning at the end of `main::exec_tests()` and `main::exec_benchmarks` obsolete. The fallback branch in the previous implementation (`rm -rf "${TMPDIR:-/tmp}/bashunit/tmp"/*`) was not a good idea anyway, as it potentially broke parallel running instances of bashunit using the same TMPDIR.
1 parent 21891f8 commit 6a331bd

4 files changed

Lines changed: 37 additions & 11 deletions

File tree

src/globals.sh

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ function temp_file() {
4343
mkdir -p "$base_dir" && chmod -R 777 "$base_dir"
4444
local test_prefix=""
4545
if [[ -n "${BASHUNIT_CURRENT_TEST_ID:-}" ]]; then
46+
# We're inside a test function - use test ID
4647
test_prefix="${BASHUNIT_CURRENT_TEST_ID}_"
48+
elif [[ -n "${BASHUNIT_CURRENT_SCRIPT_ID:-}" ]]; then
49+
# We're at script level (e.g., in set_up_before_script) - use script ID
50+
test_prefix="${BASHUNIT_CURRENT_SCRIPT_ID}_"
4751
fi
4852
mktemp "$base_dir/${test_prefix}${prefix}.XXXXXXX"
4953
}
@@ -54,17 +58,26 @@ function temp_dir() {
5458
mkdir -p "$base_dir" && chmod -R 777 "$base_dir"
5559
local test_prefix=""
5660
if [[ -n "${BASHUNIT_CURRENT_TEST_ID:-}" ]]; then
61+
# We're inside a test function - use test ID
5762
test_prefix="${BASHUNIT_CURRENT_TEST_ID}_"
63+
elif [[ -n "${BASHUNIT_CURRENT_SCRIPT_ID:-}" ]]; then
64+
# We're at script level (e.g., in set_up_before_script) - use script ID
65+
test_prefix="${BASHUNIT_CURRENT_SCRIPT_ID}_"
5866
fi
5967
mktemp -d "$base_dir/${test_prefix}${prefix}.XXXXXXX"
6068
}
6169

62-
function cleanup_temp_files() {
63-
internal_log "cleanup_temp_files"
70+
function cleanup_testcase_temp_files() {
71+
internal_log "cleanup_testcase_temp_files"
6472
if [[ -n "${BASHUNIT_CURRENT_TEST_ID:-}" ]]; then
6573
rm -rf "${TMPDIR:-/tmp}/bashunit/tmp/${BASHUNIT_CURRENT_TEST_ID}"_*
66-
else
67-
rm -rf "${TMPDIR:-/tmp}/bashunit/tmp"/*
74+
fi
75+
}
76+
77+
function cleanup_script_temp_files() {
78+
internal_log "cleanup_script_temp_files"
79+
if [[ -n "${BASHUNIT_CURRENT_SCRIPT_ID:-}" ]]; then
80+
rm -rf "${TMPDIR:-/tmp}/bashunit/tmp/${BASHUNIT_CURRENT_SCRIPT_ID}"_*
6881
fi
6982
}
7083

src/main.sh

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ function main::exec_tests() {
7070
reports::generate_report_html "$BASHUNIT_REPORT_HTML"
7171
fi
7272

73-
cleanup_temp_files
7473
internal_log "Finished tests" "exit_code:$exit_code"
7574
exit $exit_code
7675
}
@@ -98,23 +97,22 @@ function main::exec_benchmarks() {
9897

9998
benchmark::print_results
10099

101-
cleanup_temp_files
102100
internal_log "Finished benchmarks"
103101
}
104102

105103
function main::cleanup() {
106104
printf "%sCaught Ctrl-C, killing all child processes...%s\n" "${_COLOR_SKIPPED}" "${_COLOR_DEFAULT}"
107105
# Kill all child processes of this script
108106
pkill -P $$
109-
cleanup_temp_files
107+
cleanup_script_temp_files
110108
exit 1
111109
}
112110

113111
function main::handle_stop_on_failure_sync() {
114112
printf "\n%sStop on failure enabled...%s\n" "${_COLOR_SKIPPED}" "${_COLOR_DEFAULT}"
115113
console_results::print_failing_tests_and_reset
116114
console_results::render_result
117-
cleanup_temp_files
115+
cleanup_script_temp_files
118116
exit 1
119117
}
120118

src/runner.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@ function runner::load_test_files() {
55
local filter=$1
66
shift
77
local files=("${@}")
8+
local scripts_ids=()
89

910
for test_file in "${files[@]}"; do
1011
if [[ ! -f $test_file ]]; then
1112
continue
1213
fi
14+
unset BASHUNIT_CURRENT_TEST_ID
15+
export BASHUNIT_CURRENT_SCRIPT_ID="$(helper::generate_id "${test_file}")"
16+
scripts_ids+=("${BASHUNIT_CURRENT_SCRIPT_ID}")
1317
internal_log "Loading file" "$test_file"
1418
# shellcheck source=/dev/null
1519
source "$test_file"
@@ -21,6 +25,9 @@ function runner::load_test_files() {
2125
fi
2226
runner::run_tear_down_after_script
2327
runner::clean_set_up_and_tear_down_after_script
28+
if ! parallel::is_enabled; then
29+
cleanup_script_temp_files
30+
fi
2431
internal_log "Finished file" "$test_file"
2532
done
2633

@@ -32,6 +39,10 @@ function runner::load_test_files() {
3239
# Kill the spinner once the aggregation finishes
3340
disown "$spinner_pid" && kill "$spinner_pid" &>/dev/null
3441
printf "\r " # Clear the spinner output
42+
for script_id in "${scripts_ids[@]}"; do
43+
export BASHUNIT_CURRENT_SCRIPT_ID="${script_id}"
44+
cleanup_script_temp_files
45+
done
3546
fi
3647
}
3748

@@ -42,12 +53,15 @@ function runner::load_bench_files() {
4253

4354
for bench_file in "${files[@]}"; do
4455
[[ -f $bench_file ]] || continue
56+
unset BASHUNIT_CURRENT_TEST_ID
57+
export BASHUNIT_CURRENT_SCRIPT_ID="$(helper::generate_id "${test_file}")"
4558
# shellcheck source=/dev/null
4659
source "$bench_file"
4760
runner::run_set_up_before_script
4861
runner::call_bench_functions "$bench_file" "$filter"
4962
runner::run_tear_down_after_script
5063
runner::clean_set_up_and_tear_down_after_script
64+
cleanup_script_temp_files
5165
done
5266
}
5367

@@ -259,6 +273,7 @@ function runner::run_test() {
259273
state::set_test_exit_code "$exit_code"
260274
runner::run_tear_down
261275
runner::clear_mocks
276+
cleanup_testcase_temp_files
262277
state::export_subshell_context
263278
' EXIT
264279
state::initialize_assertions_count

tests/unit/globals_test.sh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,22 +67,22 @@ function test_globals_temp_file_in_test_function() {
6767
# shellcheck disable=SC2155
6868
local temp_file=$(temp_file "custom-prefix")
6969
assert_file_exists "$temp_file"
70-
cleanup_temp_files
70+
cleanup_testcase_temp_files
7171
assert_file_not_exists "$temp_file"
7272
}
7373

7474
function test_globals_temp_dir_in_test_function() {
7575
# shellcheck disable=SC2155
7676
local temp_dir=$(temp_dir "custom-prefix")
7777
assert_directory_exists "$temp_dir"
78-
cleanup_temp_files
78+
cleanup_testcase_temp_files
7979
assert_directory_not_exists "$temp_dir"
8080
}
8181

8282
function test_globals_temp_dir_and_file_in_script() {
8383
assert_directory_exists "$SCRIPT_TEMP_DIR"
8484
assert_file_exists "$SCRIPT_TEMP_FILE"
85-
cleanup_temp_files
85+
cleanup_script_temp_files
8686
assert_directory_not_exists "$SCRIPT_TEMP_DIR"
8787
assert_file_not_exists "$SCRIPT_TEMP_FILE"
8888
}

0 commit comments

Comments
 (0)