fix(tests): resolve Git Bash explicitly so WSL cannot claim bash - #294
Open
ppcvote wants to merge 1 commit into
Open
fix(tests): resolve Git Bash explicitly so WSL cannot claim bash#294ppcvote wants to merge 1 commit into
bash#294ppcvote wants to merge 1 commit into
Conversation
Fixes openai#206. The release-automation suite spawns an unqualified `bash` in 20 places. Windows resolves that against PATH, and on a machine with WSL enabled C:\Windows\System32\bash.exe is a real executable that frequently precedes Git for Windows. The extracted workflow fragments then run inside WSL, where the Windows node, gh and jq the suite depends on are absent and its Windows path assumptions do not hold, so tests fail for reasons unrelated to the code under test. `resolveBash()` walks PATH and skips the two launcher locations before taking the first bash.exe it finds: %SystemRoot%\System32 and Sysnative (the WSL launcher) and any WindowsApps directory (the Store execution alias, also WSL). Non-Windows platforms keep the bare `bash` they use today. Measured on Windows 10 with Git for Windows 2.52.0, WSL2 enabled and bun 1.3.11, by putting System32 ahead of Git on PATH to match the report: before after System32 first (as reported) 116 pass 69 fail 180 pass 5 fail Git Bash first (default order) 180 pass 5 fail 180 pass 5 fail 64 tests recovered, and the WSL-first environment now produces exactly the result the Git-Bash-first environment does. The 5 remaining failures are `jq: command not found` on this host, present identically before and after and in both PATH orders, so they are an unrelated environment gap rather than something this change leaves behind. PATH is split on `delimiter` from node:path rather than a literal ";" — bun on Windows reports a Windows-style PATH even when launched from Git Bash, so the platform separator is the honest thing to split on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #206.
The release-automation suite spawns an unqualified
bashin 20 places. On Windows that resolves against PATH, and with WSL enabledC:\Windows\System32\bash.exeis a real executable that frequently precedes Git for Windows. The extracted workflow fragments then run inside WSL, where the Windowsnode,ghandjqthis suite depends on are absent and its Windows path assumptions do not hold.Measurement
I could not reproduce this by accident: on this host
where.exe bashhappens to return Git Bash first, so the suite was already green. I reproduced it deliberately by putting System32 ahead of Git on PATH, which is the reporter's configuration.Windows 10, Git for Windows 2.52.0, WSL2 enabled, bun 1.3.11:
64 tests recovered, and the WSL-first environment now produces exactly what the Git-Bash-first environment does.
The 5 remaining failures are
jq: command not foundon this host. They are identical before and after and in both PATH orders, so they are an unrelated environment gap on my machine rather than residue from this change.The change
resolveBash()walks PATH and skips the launcher locations before taking the firstbash.exeit finds:%SystemRoot%\System32and%SystemRoot%\Sysnative— the WSL launcherWindowsAppsdirectory — the Store execution alias, also WSLNon-Windows platforms keep the bare
bashthey use today, so nothing changes on CI's Linux runners.Two details worth flagging for review:
delimiterfromnode:pathrather than a literal";". I checked what bun actually reports on Windows when launched from Git Bash, and it is a Windows-style semicolon PATH, but splitting on the platform separator is the honest expression of the intent.bash.exein bothGit\binandGit\usr\bin, and only the first sits next togit.exe, so a positive test would have to encode more layout assumptions than an exclusion does.Happy to switch to a positive Git-Bash lookup, or to hoist the helper somewhere shared, if either fits the codebase better.