From f45c5ff7a3eb3d083235a9cc1374fd47a6846241 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Tue, 28 Jul 2026 20:51:33 +0200 Subject: [PATCH] fix(compat): ignore an external git differ, drop locale env prefixes Three failures reported in #912, none of them visible to CI: - render_diff ran `git diff` without `--no-ext-diff`, so a user's `diff.external` (difftastic) replaced the word-diff with its own output and the multiline/snapshot failure diffs came out empty. - `bashunit::clock::shell_time` and the JUnit report prefixed a command with `LC_ALL=C`, reported to segfault inside a command substitution on Bash 5.3.9 macOS. `env LC_ALL=C cmd` keeps awk's radix a dot without bash touching its own locale; shell_time never needed it at all, since the value is expanded before the temporary environment applies. - The syntax-error acceptance test used the same prefix form. A source-level gate in bash_compatibility_test.sh keeps the prefix out of src/, since no CI job runs Bash 5.3 on macOS. Closes #912 --- CHANGELOG.md | 2 ++ src/clock.sh | 5 +++- src/console_results.sh | 5 ++-- src/reports.sh | 7 ++++-- .../acceptance/bashunit_syntax_error_test.sh | 6 +++-- tests/unit/bash_compatibility_test.sh | 11 +++++++++ tests/unit/console_results_diff_test.sh | 23 +++++++++++++++++++ 7 files changed, 52 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33fffd7e..486e81e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ - The `--parallel` unsupported-OS warning no longer claims Alpine is excluded ### Fixed +- A configured external git differ (`diff.external` / `GIT_EXTERNAL_DIFF`, e.g. difftastic) no longer blanks the multiline and snapshot failure diffs, which now render with `--no-ext-diff` (#912) +- Time reads and the JUnit report no longer set the locale with a temporary-environment prefix (`LC_ALL=C cmd`), reported to segfault inside a command substitution on Bash 5.3.9 macOS (#912) - Call assertions (`assert_not_called`, `assert_have_been_called*`) fail with `was never registered as a spy` instead of reporting zero calls when the name was never spied — a typo used to pass silently (#895) - The per-argument form a spy records was written with a literal `$'\x1f'` separator instead of the byte, so it could not be compared against (#894) - `--parallel` no longer discards worker stderr written outside a test body; it renders as a `Stderr from ` block (#864) diff --git a/src/clock.sh b/src/clock.sh index 035cb66e..9c5737f2 100644 --- a/src/clock.sh +++ b/src/clock.sh @@ -182,7 +182,10 @@ function bashunit::clock::now() { function bashunit::clock::shell_time() { # Get time directly from the shell variable EPOCHREALTIME (Bash 5+) - [ -n "${EPOCHREALTIME+x}" ] && [ -n "$EPOCHREALTIME" ] && LC_ALL=C echo "$EPOCHREALTIME" + # No LC_ALL=C prefix: it cannot normalize the decimal separator (bash expands the + # value before the temp env applies, and callers accept '.' and ','), and that form + # was reported to segfault inside a command substitution on Bash 5.3.9 macOS (#912). + [ -n "${EPOCHREALTIME+x}" ] && [ -n "$EPOCHREALTIME" ] && echo "$EPOCHREALTIME" } function bashunit::clock::total_runtime_in_milliseconds() { diff --git a/src/console_results.sh b/src/console_results.sh index 882baf09..50579133 100644 --- a/src/console_results.sh +++ b/src/console_results.sh @@ -312,8 +312,9 @@ function bashunit::console_results::render_diff() { # `git diff` exits non-zero when the files differ; the `|| true` keeps that # from tripping `set -e`/`pipefail` under --strict. `tail -n +6` drops git's - # header lines; `sed` indents the body. - git diff --no-index --word-diff "$color_flag" \ + # header lines; `sed` indents the body. `--no-ext-diff` ignores a user's + # `diff.external`/`GIT_EXTERNAL_DIFF`, which would replace this word-diff. + git diff --no-ext-diff --no-index --word-diff "$color_flag" \ "$expected_file" "$actual_file" 2>/dev/null | tail -n +6 | sed "s/^/ /" || true } diff --git a/src/reports.sh b/src/reports.sh index dcbddc40..7aba10fd 100755 --- a/src/reports.sh +++ b/src/reports.sh @@ -102,7 +102,10 @@ function bashunit::reports::generate_junit_xml() { local tests_failed=$(bashunit::state::get_tests_failed) local time_ms=$(bashunit::clock::total_runtime_in_milliseconds) local time - time=$(LC_ALL=C awk -v ms="$time_ms" 'BEGIN {printf "%.3f", ms/1000}') + # `env` rather than a bare LC_ALL=C prefix (which keeps awk's radix a dot + # without bash changing its own locale, reported to segfault on Bash 5.3.9 + # macOS inside a command substitution, #912). + time=$(env LC_ALL=C awk -v ms="$time_ms" 'BEGIN {printf "%.3f", ms/1000}') { echo "" @@ -120,7 +123,7 @@ function bashunit::reports::generate_junit_xml() { local test_time_ms="${_BASHUNIT_REPORTS_TEST_DURATIONS[$i]:-}" local failure_message="${_BASHUNIT_REPORTS_TEST_FAILURES[$i]:-}" local test_time - test_time=$(LC_ALL=C awk -v ms="$test_time_ms" 'BEGIN {printf "%.3f", ms/1000}') + test_time=$(env LC_ALL=C awk -v ms="$test_time_ms" 'BEGIN {printf "%.3f", ms/1000}') echo " &1)" set -e @@ -19,6 +21,6 @@ function test_bashunit_when_test_file_has_syntax_error() { assert_contains "failed" "$actual" assert_contains "Error" "$actual" - assert_general_error "$(LC_ALL=C LANG=C ./bashunit \ + assert_general_error "$(env LC_ALL=C LANG=C ./bashunit \ --no-parallel --env "$TEST_ENV_FILE" "$test_file" 2>&1)" } diff --git a/tests/unit/bash_compatibility_test.sh b/tests/unit/bash_compatibility_test.sh index 1faeee99..5f05a9fd 100644 --- a/tests/unit/bash_compatibility_test.sh +++ b/tests/unit/bash_compatibility_test.sh @@ -102,3 +102,14 @@ function test_src_has_no_coproc() { function test_src_has_no_parameter_transformations() { assert_empty "$(bashunit::compat::offenders '\$\{[A-Za-z_][A-Za-z0-9_]*@[QEPAKa]\}')" } + +# A temporary-environment locale prefix (`LC_ALL=C cmd`) makes bash change its +# own locale for that command. Bash 5.3.9 on macOS was reported to SIGSEGV on +# that form inside a command substitution -- `x=$(LC_ALL=C echo hi)` exits 139 +# (#912) -- and no CI job runs that build. Use `env LC_ALL=C cmd` instead, which +# passes the locale straight to the child and never touches bash's own. +function test_src_has_no_temporary_locale_assignment_prefix() { + local pattern='(^|[;&|(])[[:space:]]*((LC_[A-Z_]+|LANG)=[^[:space:]]*[[:space:]]+)+[^[:space:]=]' + + assert_empty "$(bashunit::compat::offenders "$pattern")" +} diff --git a/tests/unit/console_results_diff_test.sh b/tests/unit/console_results_diff_test.sh index b8500cd4..e154daf2 100644 --- a/tests/unit/console_results_diff_test.sh +++ b/tests/unit/console_results_diff_test.sh @@ -43,6 +43,29 @@ function test_render_diff_is_empty_for_identical_files() { rm -f "$a" "$b" } +function test_render_diff_ignores_a_configured_external_diff() { + if ! bashunit::dependencies::has_git; then + bashunit::skip "git not available" && return + fi + export BASHUNIT_NO_COLOR=true + # An external differ (difftastic in #912) replaces git's own output, so + # without --no-ext-diff the rendered diff is whatever it prints — here nothing. + export GIT_EXTERNAL_DIFF=true + local a b + a=$(bashunit::temp_file diff_a) + b=$(bashunit::temp_file diff_b) + printf 'alpha\nbeta\ngamma\n' >"$a" + printf 'alpha\nDELTA\ngamma\n' >"$b" + + local output + output=$(bashunit::console_results::render_diff "$a" "$b") + + assert_contains "beta" "$output" + assert_contains "DELTA" "$output" + rm -f "$a" "$b" + unset BASHUNIT_NO_COLOR GIT_EXTERNAL_DIFF +} + function test_render_diff_shows_changed_tokens_without_color() { if ! bashunit::dependencies::has_git; then bashunit::skip "git not available" && return