diff --git a/CHANGELOG.md b/CHANGELOG.md index 574762cd..91d978a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - `tear_down_after_script` runs when `set_up_before_script` fails, so it can release file-scoped resources acquired before the failure (#1318) - Under `--parallel`, `tear_down_after_script` runs after the file's own tests instead of alongside them, so a fixture it releases stays alive for the tests that read it. The same file no longer passed sequentially and failed in parallel (#1320) +- `--stop-on-failure` runs `tear_down_after_script` for the file it halts in, so a sequential run releases what `set_up_before_script` acquired before the halt (#1321) ## [0.50.0](https://github.com/TypedDevs/bashunit/compare/0.49.0...0.50.0) - 2026-08-18 diff --git a/src/main/run.sh b/src/main/run.sh index b770c16e..d3eedbfb 100644 --- a/src/main/run.sh +++ b/src/main/run.sh @@ -393,6 +393,11 @@ function bashunit::main::cleanup() { } function bashunit::main::handle_stop_on_failure_sync() { + # The exit that lands here came from inside the test loop, so the file's + # teardown has not run. First, so the hook line follows the last test line as + # it does in a normal run, and so a hook reading a bashunit::temp_file still + # finds it (#1321). + bashunit::runner::run_pending_file_teardown || true printf "\n%sStop on failure enabled...%s\n" "${_BASHUNIT_COLOR_SKIPPED}" "${_BASHUNIT_COLOR_DEFAULT}" bashunit::console_results::print_failing_tests_and_reset bashunit::console_results::print_risky_tests_and_reset diff --git a/src/runner/discovery.sh b/src/runner/discovery.sh index 9d980ab2..83cbe10e 100644 --- a/src/runner/discovery.sh +++ b/src/runner/discovery.sh @@ -212,6 +212,10 @@ function bashunit::runner::load_test_files() { bashunit::runner::restore_workdir continue fi + # From here this shell owes the file's teardown until the loop runs it below. + # --stop-on-failure exits from inside call_test_functions and never reaches + # that line, so the exit path settles the debt instead (#1321). + bashunit::runner::mark_file_teardown_pending "$test_file" local _cached_fns="$functions_for_script" # In the parent, before dispatch: under --parallel call_test_functions is a # background subshell, so a check run inside it sets state that dies with @@ -244,6 +248,9 @@ function bashunit::runner::load_test_files() { bashunit::runner::call_test_functions "$test_file" "$_cached_fns" bashunit::runner::run_tear_down_after_script "$test_file" fi + # Sequential ran the hook just above; under --parallel the worker owns it. + # Either way this shell owes it no longer. + _BASHUNIT_FILE_TEARDOWN_PENDING="" bashunit::runner::clean_script_test_functions "$_script_fns_to_clean" bashunit::runner::clean_set_up_and_tear_down_after_script if ! bashunit::parallel::is_enabled; then diff --git a/src/runner/hooks.sh b/src/runner/hooks.sh index 22c41b42..30062341 100644 --- a/src/runner/hooks.sh +++ b/src/runner/hooks.sh @@ -303,6 +303,42 @@ function bashunit::runner::clear_mocks() { done } +# The file whose tear_down_after_script this shell still owes, empty when it owes +# none. --stop-on-failure exits from inside the test loop, so the loop's own +# teardown call is never reached and the hook has to run from the exit path +# instead (#1321). +_BASHUNIT_FILE_TEARDOWN_PENDING="" + +## +# Records that $1's file teardown is still owed by this shell. +# +# Gated on the file declaring the hook, not on setup having run: with no hook +# declared run_tear_down_after_script prints a blank line, which is console +# decoration belonging to the normal path. Emitting it from the halt path would +# change the output of every halted run that defines no teardown. +# Arguments: $1 - the test file +## +function bashunit::runner::mark_file_teardown_pending() { + if declare -F tear_down_after_script >/dev/null 2>&1; then + _BASHUNIT_FILE_TEARDOWN_PENDING="$1" + fi +} + +## +# Runs the file teardown this shell still owes, if any, and clears the debt. +# +# For the paths that leave the test loop without reaching its own teardown call. +# The debt is cleared before the hook runs, so a hook that re-enters this path +# cannot run twice. +## +function bashunit::runner::run_pending_file_teardown() { + local test_file="$_BASHUNIT_FILE_TEARDOWN_PENDING" + [ -n "$test_file" ] || return 0 + + _BASHUNIT_FILE_TEARDOWN_PENDING="" + bashunit::runner::run_tear_down_after_script "$test_file" +} + function bashunit::runner::run_tear_down_after_script() { local test_file="$1" bashunit::internal_log "run_tear_down_after_script" diff --git a/tests/acceptance/bashunit_stop_on_failure_test.sh b/tests/acceptance/bashunit_stop_on_failure_test.sh index 23818583..5afb1914 100644 --- a/tests/acceptance/bashunit_stop_on_failure_test.sh +++ b/tests/acceptance/bashunit_stop_on_failure_test.sh @@ -47,3 +47,53 @@ function test_bashunit_stop_on_failure_with_runtime_error() { assert_contains "A runtime error" "$output" assert_not_contains "B not executed" "$output" } + +# --stop-on-failure ends a sequential run with `exit` from inside the test loop, +# so the file's tear_down_after_script never ran and whatever +# set_up_before_script acquired was leaked (#1321). +function test_bashunit_stop_on_failure_runs_tear_down_after_script() { + local dir fixture marker + dir="$(bashunit::temp_dir stop_on_failure_teardown)" + fixture="$dir/halted_test.sh" + marker="$dir/resource" + { + printf 'RESOURCE=""\n' + printf 'function set_up_before_script() {\n' + printf ' RESOURCE="$HALT_MARKER"\n' + printf ' : >"$RESOURCE"\n' + printf '}\n' + printf 'function tear_down_after_script() {\n' + printf ' rm -f "$RESOURCE"\n' + printf '}\n' + printf 'function test_a_halts_the_run() { assert_same 1 2; }\n' + printf 'function test_b_not_executed() { assert_same 1 1; }\n' + } >"$fixture" + + local output="" exit_code=0 + output="$(HALT_MARKER="$marker" ./bashunit --no-parallel --env "$TEST_ENV_FILE" \ + --stop-on-failure "$fixture")" || exit_code=$? + + assert_same 1 "$exit_code" + assert_not_contains "B not executed" "$output" + assert_file_not_exists "$marker" +} + +# A hook that fails while the run is halting still has to report itself, and must +# not turn the halt into a different exit code (#1321). +function test_bashunit_stop_on_failure_reports_a_failing_tear_down_after_script() { + local dir fixture + dir="$(bashunit::temp_dir stop_on_failure_teardown_error)" + fixture="$dir/halted_bad_teardown_test.sh" + { + printf 'function set_up_before_script() { :; }\n' + printf 'function tear_down_after_script() { missing_cleanup_command; }\n' + printf 'function test_a_halts_the_run() { assert_same 1 2; }\n' + } >"$fixture" + + local output="" exit_code=0 + output="$(./bashunit --no-parallel --env "$TEST_ENV_FILE" \ + --stop-on-failure "$fixture" 2>&1)" || exit_code=$? + + assert_same 1 "$exit_code" + assert_contains "Tear down after script" "$(printf "%s" "$output" | strip_ansi)" +}