From a46c04b1b6c4e31eacc122fe15572d19ee92f16f Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Thu, 13 Aug 2026 21:47:53 +0200 Subject: [PATCH] fix(runner): survive a missing run scratch dir when sourcing a test file load_test_files sources each file as `source "$file" 2>"$dir/source_err"`. When the run's scratch directory is missing the redirect fails, and bash reports that as the command failing -- exit 1 with nothing written to the capture file. The runner read that as the test file failing to source and printed 'Failed to source (exit 1, 52 bytes, no stderr)' against a file that was complete and valid, naming the one thing not at fault. Restore the directory before sourcing, falling back to /dev/null if that is refused: losing one file's stderr capture is worth far less than failing the file for a reason that is not its own. Not a fix for #1137. That flake did not reproduce in twelve isolated Bash 3.0 runs -- six with this change, six without -- so whether this mechanism is what hits CI there is unproven. Closes #1163 --- CHANGELOG.md | 1 + src/runner/discovery.sh | 10 ++++ .../bashunit_source_redirect_test.sh | 54 +++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 tests/acceptance/bashunit_source_redirect_test.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e685fd7..f9db3dcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - Performance: `--coverage` is roughly 5x faster and `--coverage-report-html` roughly 19x — a run over this repo went from 16.2s to 2.9s, and a 128-file HTML report from 58.7s to 3.1s. The report phase emits each format in one awk invocation per run instead of Bash loops and forks per file and per row, and the capture path writes records straight to disk, normalizes a path with one fork instead of four, and reads each cache once (#1092, #1096, #1098, #1099, #1102, #1104, #1110, #1117) ### Fixed +- A run survives its scratch directory going missing instead of reporting a valid test file as unsourceable: `source "$file" 2>"$dir/source_err"` fails on the *redirect* when the directory is gone, which bash reports as the command failing, so the runner printed `Failed to source '' (exit 1, ..., no stderr)` and named the one thing that was not at fault (#1163) - Duplicate test functions are detected under `--parallel` again: the check ran inside the per-file loop, which `--parallel` backgrounds, so the state it set died with the subshell and a file where one of two same-named tests never ran reported "All tests passed" and exited 0. The report also names the line of every definition now, instead of leaving the second one to be found by hand (#1147) - A `@data_provider` naming a function that is not defined no longer makes the test vanish: the run reported "No tests found", blaming a missing test rather than the missing provider and never printing the annotation's name. Both that and a provider that yields no data are now errors naming the provider, and under `--parallel` the failure reaches the aggregate instead of the run printing the error and exiting 0 (#1145) - `mock`, `mock_sequence` and `spy` report a usable-name error instead of a raw bash syntax error when the command name carries whitespace or shell syntax — `mock "ls -l" echo hi` used to print `syntax error near unexpected token '-l'` from inside bashunit (#1136) diff --git a/src/runner/discovery.sh b/src/runner/discovery.sh index 4d958000..1c8a8be2 100644 --- a/src/runner/discovery.sh +++ b/src/runner/discovery.sh @@ -67,6 +67,16 @@ function bashunit::runner::load_test_files() { # 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_RUN_OUTPUT_DIR/source_err" + # A missing scratch dir makes the redirect below fail, and bash reports that + # as the *command* failing: exit 1 with nothing written to the capture file, + # which read as "this test file failed to source" against a file that was + # complete and valid (#1137). Restore the directory, and fall back to + # /dev/null if even that is refused -- losing a file's stderr capture is + # worth far less than failing the file for a reason that is not its own. + if [ ! -d "$_BASHUNIT_RUN_OUTPUT_DIR" ] && + ! mkdir -p "$_BASHUNIT_RUN_OUTPUT_DIR" 2>/dev/null; then + source_err_file=/dev/null + fi # shellcheck source=/dev/null source "$test_file" 2>"$source_err_file" source_status=$? diff --git a/tests/acceptance/bashunit_source_redirect_test.sh b/tests/acceptance/bashunit_source_redirect_test.sh new file mode 100644 index 00000000..0ed9ea10 --- /dev/null +++ b/tests/acceptance/bashunit_source_redirect_test.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +set -euo pipefail + +# `source "$file" 2>"$_BASHUNIT_RUN_OUTPUT_DIR/source_err"` fails when the run's +# scratch directory is missing -- and it fails on the *redirect*, so bash +# returns 1 having written nothing to the capture file. The runner read that as +# the test file failing to source and reported +# +# Failed to source 'x_test.sh' (exit 1, 52 bytes, no stderr) +# +# against a file that was complete and valid. That message sent three separate +# investigations at the fixture (#1137). The directory going missing mid-run is +# a separate open question; a run should survive it either way. + +function set_up_before_script() { + TEST_ENV_FILE="tests/acceptance/fixtures/.env.default" +} + +# Two files: the first removes the run's scratch directory while it is being +# sourced, which is exactly the state the flake produces; the second must still +# be sourced and run. +function test_a_file_still_runs_when_the_run_scratch_dir_disappeared() { + local dir + dir="$(bashunit::temp_dir)" + printf '%s\n' 'rm -rf "$_BASHUNIT_RUN_OUTPUT_DIR" +function test_removes_the_scratch_dir() { assert_same 1 1; }' >"$dir/a_killer_test.sh" + printf '%s\n' 'function test_runs_after_the_scratch_dir_went_away() { assert_same 2 2; }' \ + >"$dir/b_victim_test.sh" + + local output code=0 + output="$(./bashunit --no-parallel --env "$TEST_ENV_FILE" \ + "$dir/a_killer_test.sh" "$dir/b_victim_test.sh" 2>&1)" || code=$? + output="$(printf '%s' "$output" | strip_ansi)" + + assert_same 0 "$code" + assert_contains "2 passed" "$output" + assert_not_contains "Failed to source" "$output" +} + +# The genuine case must keep reporting: a file whose top level returns non-zero +# is still a source failure, and the message still says so. +function test_a_real_source_failure_is_still_reported() { + local fixture + fixture="$(bashunit::temp_file real_source_failure).sh" + printf '%s\n' 'function test_never_runs() { assert_same 1 1; } +false' >"$fixture" + + local output code=0 + output="$(./bashunit --no-parallel --env "$TEST_ENV_FILE" "$fixture" 2>&1)" || code=$? + output="$(printf '%s' "$output" | strip_ansi)" + + assert_same 1 "$code" + assert_contains "Failed to source" "$output" +}