diff --git a/.github/workflows/tests-bash-3.0.yml b/.github/workflows/tests-bash-3.0.yml index 3a86efdc..e8097764 100644 --- a/.github/workflows/tests-bash-3.0.yml +++ b/.github/workflows/tests-bash-3.0.yml @@ -84,6 +84,8 @@ jobs: flags: "--simple" - name: "Simple Parallel" flags: "--simple --parallel" + - name: "Strict" + flags: "--simple --parallel --strict" steps: - name: Checkout uses: actions/checkout@v4 diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d92ae2d..a47ee9a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Killed tests report the cause: timeout, SIGINT, SIGKILL/OOM, SIGTERM (#683) ### Fixed +- `--strict` no longer enables Bash 3.0's broken `pipefail` (which wrongly reports failing pipelines as successful); `set -eu` is still applied, `pipefail` only on Bash >= 3.1 - `bashunit watch` forwards `--filter` and other flags correctly (#682) - `learn` and coverage use `mktemp -d` for temp directories - `parallel::cleanup` refuses to `rm -rf` outside `*/bashunit/parallel/*` diff --git a/src/console_results.sh b/src/console_results.sh index f835657b..2d4a8c7a 100644 --- a/src/console_results.sh +++ b/src/console_results.sh @@ -337,10 +337,12 @@ function bashunit::console_results::print_failed_snapshot_test() { local actual_file="${snapshot_file}.tmp" echo "$actual_content" >"$actual_file" + # `git diff` exits non-zero when the files differ; guard with `|| true` so + # the assignment does not trip `set -e`/`pipefail` under --strict. local git_diff_output git_diff_output="$(git diff --no-index --word-diff --color=always \ "$snapshot_file" "$actual_file" 2>/dev/null | - tail -n +6 | sed "s/^/ /")" + tail -n +6 | sed "s/^/ /")" || true line="$line$git_diff_output" rm "$actual_file" diff --git a/src/runner.sh b/src/runner.sh index 7bc201d5..fd71e0bf 100755 --- a/src/runner.sh +++ b/src/runner.sh @@ -12,6 +12,19 @@ function bashunit::runner::restore_workdir() { cd "$BASHUNIT_WORKING_DIR" 2>/dev/null || true } +## +# Whether the running Bash has a reliable `set -o pipefail`. Bash 3.0 shipped a +# broken pipefail (a failing pipeline can wrongly report success), which makes +# `--strict` unsound; on 3.0 we fall back to `set -eu` without pipefail. +# Returns: 0 when pipefail is reliable (Bash >= 3.1), 1 otherwise. +## +function bashunit::runner::_supports_reliable_pipefail() { + if [ "${BASH_VERSINFO[0]:-0}" -gt 3 ]; then + return 0 + fi + [ "${BASH_VERSINFO[0]:-0}" -eq 3 ] && [ "${BASH_VERSINFO[1]:-0}" -ge 1 ] +} + # Caches BASHUNIT_COVERAGE into _BASHUNIT_COVERAGE_ON ("1"|"0") so hot-path checks # avoid a function dispatch per call. Call once after arg parsing; tests that # toggle BASHUNIT_COVERAGE mid-run must call this again to refresh. @@ -887,7 +900,13 @@ function bashunit::runner::run_test() { # Apply shell mode setting for test execution if bashunit::env::is_strict_mode_enabled; then - set -euo pipefail + set -eu + # Bash 3.0 ships a broken pipefail; only enable it where it is reliable. + if bashunit::runner::_supports_reliable_pipefail; then + set -o pipefail + else + set +o pipefail + fi else set +euo pipefail fi diff --git a/tests/unit/runner_test.sh b/tests/unit/runner_test.sh index b9d7a58a..62734c26 100644 --- a/tests/unit/runner_test.sh +++ b/tests/unit/runner_test.sh @@ -232,3 +232,15 @@ function test_classify_kill_signal_generic_signal() { function test_classify_kill_signal_empty_for_normal_exit() { assert_empty "$(bashunit::runner::classify_kill_signal 1)" } + +function test_supports_reliable_pipefail_matches_bash_version() { + # Reliable on Bash >= 3.1; Bash 3.0 ships a broken pipefail. + local expected_rc=0 + if [ "${BASH_VERSINFO[0]}" -eq 3 ] && [ "${BASH_VERSINFO[1]}" -eq 0 ]; then + expected_rc=1 + fi + + local actual_rc=0 + bashunit::runner::_supports_reliable_pipefail || actual_rc=$? + assert_same "$expected_rc" "$actual_rc" +}