diff --git a/CHANGELOG.md b/CHANGELOG.md index b11b9af3..574762cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,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) ## [0.50.0](https://github.com/TypedDevs/bashunit/compare/0.49.0...0.50.0) - 2026-08-18 diff --git a/docs/ai-agents.md b/docs/ai-agents.md index ea9c9fa7..e339a93a 100644 --- a/docs/ai-agents.md +++ b/docs/ai-agents.md @@ -141,8 +141,6 @@ actually make against this API: tell `cmd "a b"` from `cmd a b` — a quoting bug passes it. Use `assert_have_been_called_with_args ...` (no `call_index`) to compare argument by argument whenever an argument may contain a space. -- Do not delete a shared fixture in `tear_down_after_script`: under `--parallel` the - file's tests may still be running, and they will vanish from the totals silently. - Re-record snapshots with `--snapshot-update`, scoped by `--filter`; do not delete snapshot files. A missing snapshot is written silently and never fails, so a wrong `rm` turns a real assertion into a rubber stamp. Read `git diff` afterwards either way. diff --git a/src/runner/discovery.sh b/src/runner/discovery.sh index 1b232702..9d980ab2 100644 --- a/src/runner/discovery.sh +++ b/src/runner/discovery.sh @@ -228,11 +228,22 @@ function bashunit::runner::load_test_files() { worker_stderr_paths[worker_stderr_count]="$_worker_stderr" worker_stderr_owners[worker_stderr_count]="$test_file" worker_stderr_count=$((worker_stderr_count + 1)) - bashunit::runner::call_test_functions "$test_file" "$_cached_fns" 2>"$_worker_stderr" & + # The file's teardown belongs inside the worker. Run from this shell it + # released the fixture while the worker's tests were still reading it, so + # the same file passed sequentially and failed under --parallel (#1320). + # call_test_functions waits for its own per-test workers before it + # returns, which is what makes this ordering hold. + { + bashunit::runner::call_test_functions "$test_file" "$_cached_fns" + bashunit::runner::run_tear_down_after_script "$test_file" + # A hook failure recorded in here dies with the subshell (#1147), so + # publish it the way a test publishes its result. + bashunit::runner::publish_file_hook_failure "$?" "$test_file" + } 2>"$_worker_stderr" & else bashunit::runner::call_test_functions "$test_file" "$_cached_fns" + bashunit::runner::run_tear_down_after_script "$test_file" fi - bashunit::runner::run_tear_down_after_script "$test_file" 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/result.sh b/src/runner/result.sh index 2474adf2..900c7d40 100644 --- a/src/runner/result.sh +++ b/src/runner/result.sh @@ -62,6 +62,48 @@ function bashunit::runner::parse_result_parallel() { echo "$execution_result" >"$unique_test_result_file" } +## +# Publishes a file-scoped hook failure from a --parallel worker, so the parent's +# summary counts it. +# +# tear_down_after_script runs inside the file's worker (#1320) and +# record_file_hook_failure's counter lives in that subshell, so without this the +# hook failed, said so on the terminal, and left the run green. The aggregator +# reads one `.result` per test out of the file's suite dir, so the hook publishes +# its own. A fixed basename: the per-test ordinals are scoped inside +# call_test_functions, and the aggregator globs `*.result`. +# +# Written only on failure. A zero-assertion, exit-0 payload is what the +# aggregator reads as a risky test, which would add a phantom risky test to every +# file that defines the hook. +# +# Arguments: $1 - the hook's exit status, $2 - the test file +## +function bashunit::runner::publish_file_hook_failure() { + local status=$1 + local test_file=$2 + + [ "$status" -ne 0 ] || return 0 + bashunit::parallel::is_enabled || return 0 + + bashunit::runner::parallel_suite_dir_to_slot "$test_file" + local test_suite_dir=$_BASHUNIT_RUNNER_SUITE_DIR_OUT + [ -d "$test_suite_dir" ] || mkdir -p "$test_suite_dir" + + # Hand-built rather than exported: export_subshell_context would carry the + # assertion totals this file's own tests already published, and the aggregator + # would count every one of them twice. The hook contributes a failed test, no + # assertions. + local payload="\ +##ASSERTIONS_FAILED=0\ +##ASSERTIONS_PASSED=0\ +##ASSERTIONS_SKIPPED=0\ +##ASSERTIONS_INCOMPLETE=0\ +##ASSERTIONS_SNAPSHOT=0\ +##TEST_EXIT_CODE=$status##" + printf '%s\n' "$payload" >"$test_suite_dir/tear_down_after_script.result" +} + function bashunit::runner::parse_result_sync() { local fn_name=$1 local execution_result=$2 diff --git a/tests/acceptance/bashunit_parallel_consistency_test.sh b/tests/acceptance/bashunit_parallel_consistency_test.sh index eb4e7f4e..9a03b3ac 100644 --- a/tests/acceptance/bashunit_parallel_consistency_test.sh +++ b/tests/acceptance/bashunit_parallel_consistency_test.sh @@ -104,3 +104,39 @@ Assertions: 5 passed, 5 total" assert_same "3 passed 3 total 5 passed 5 total " "$(summary_counts "$capture")" } + +# A file-scoped teardown has to run after the file's tests, not alongside them. +# Under --parallel the runner dispatched the worker and ran the hook on the very +# next line, so a resource acquired in set_up_before_script vanished mid-test and +# the file failed in parallel while passing sequentially (#1320). +function test_parallel_runs_tear_down_after_script_after_the_files_tests() { + local dir fixture + dir="$(bashunit::temp_dir parallel_file_teardown)" + fixture="$dir/shared_fixture_test.sh" + { + printf 'function set_up_before_script() {\n' + printf ' : >"$SHARED_FIXTURE"\n' + printf '}\n' + printf 'function tear_down_after_script() {\n' + printf ' rm -f "$SHARED_FIXTURE"\n' + printf '}\n' + printf 'function test_reads_the_shared_fixture() {\n' + printf ' sleep 0.5\n' + printf ' assert_file_exists "$SHARED_FIXTURE"\n' + printf '}\n' + } >"$fixture" + + local sequential_output parallel_output + sequential_output=$(SHARED_FIXTURE="$dir/sequential.fixture" NO_COLOR=1 \ + ./bashunit --no-parallel --env "$TEST_ENV_FILE" "$fixture" 2>&1) || true + parallel_output=$(SHARED_FIXTURE="$dir/parallel.fixture" NO_COLOR=1 \ + ./bashunit --parallel --env "$TEST_ENV_FILE" "$fixture" 2>&1) || true + + local sequential_counts parallel_counts + sequential_counts=$(summary_counts "$sequential_output") + parallel_counts=$(summary_counts "$parallel_output") + + assert_same "$sequential_counts" "$parallel_counts" + # Guard against both sides failing the same way and matching vacuously. + assert_contains "1 passed 1 total" "$sequential_counts" +} diff --git a/tests/acceptance/bashunit_teardown_after_script_error_test.sh b/tests/acceptance/bashunit_teardown_after_script_error_test.sh index 30dfb2e6..15a51dc2 100644 --- a/tests/acceptance/bashunit_teardown_after_script_error_test.sh +++ b/tests/acceptance/bashunit_teardown_after_script_error_test.sh @@ -84,3 +84,30 @@ test_bashunit_when_teardown_after_script_with_intermediate_failing_command.sh assert_contains "$assertions_summary" "$actual" assert_general_error "$(./bashunit --no-parallel --env "$TEST_ENV_FILE" "$test_file")" } + +# Under --parallel the hook runs inside the file's worker (#1320), where its own +# counter dies with the subshell the way #1147 describes. The count reaches the +# parent through publish_file_hook_failure. Drop that and the hook error still +# prints while the run reports "All tests passed", so pin the count here. +function test_bashunit_when_tear_down_after_script_errors_in_parallel() { + local test_file=./tests/acceptance/fixtures/test_bashunit_when_teardown_after_script_errors.sh + + local error_line="✗ Error: Tear down after script" + local message_line="missing_cleanup_command" + local tests_summary="Tests: 1 passed, 1 failed, 2 total" + local assertions_summary="Assertions: 1 passed, 0 failed, 1 total" + + local actual_raw + set +e + actual_raw="$(./bashunit --parallel --detailed --env "$TEST_ENV_FILE" "$test_file")" + set -e + + local actual + actual="$(printf "%s" "$actual_raw" | strip_ansi)" + + assert_contains "$error_line" "$actual" + assert_contains "$message_line" "$actual" + assert_contains "$tests_summary" "$actual" + assert_contains "$assertions_summary" "$actual" + assert_general_error "$(./bashunit --parallel --env "$TEST_ENV_FILE" "$test_file")" +}