Skip to content

fix(compat): ignore an external git differ, drop locale env prefixes - #914

Closed
Chemaclass wants to merge 1 commit into
mainfrom
fix/912-test-failures-bash-5-3-macos
Closed

fix(compat): ignore an external git differ, drop locale env prefixes#914
Chemaclass wants to merge 1 commit into
mainfrom
fix/912-test-failures-bash-5-3-macos

Conversation

@Chemaclass

Copy link
Copy Markdown
Member

🤔 Background

Related #912

Seven tests failed on macOS with Bash 5.3 under nix-shell --pure, and two more for anyone with an external git differ configured. CI never sees either: macOS runners ship Apple's Bash 3.2, and no runner has diff.external set.

💡 Changes

  • The failure/snapshot diff renderer now passes --no-ext-diff, so a configured diff.external (difftastic) no longer replaces the word-diff with empty output.
  • 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. env LC_ALL=C cmd keeps the C radix; the clock never needed the prefix, since the value is expanded before the temporary environment applies.
  • The syntax-error acceptance test drops the same prefix form.
  • A source-level gate keeps the prefix out of src/, since no CI job runs that bash.

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
@Chemaclass Chemaclass added the bug Something isn't working label Jul 28, 2026
@Chemaclass Chemaclass self-assigned this Jul 28, 2026
@Chemaclass

Copy link
Copy Markdown
Member Author

Closing in favour of #913, which reported and fixed the same three bugs first.

Everything this PR had that #913 was missing is now on that branch: the env LC_ALL=C awk fix in src/reports.sh, the external-differ regression test, and the bash_compatibility_test.sh gate against the crashing locale prefix. Nothing is lost here.

#913 also carries two fixes this PR did not have: the init acceptance test writes its log under $TMP_DIR instead of /tmp, and the osx-with-perl clock test pins _BASHUNIT_CLOCK_NOW_IMPL so it keeps testing the perl path on Bash 5.

@Chemaclass Chemaclass closed this Jul 28, 2026
@Chemaclass
Chemaclass deleted the fix/912-test-failures-bash-5-3-macos branch July 28, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant