test(ci): cover Windows and Git Bash so #112 cannot regress - #114
Merged
Conversation
The Git Bash bug in #112 shipped because nothing ran there: unit tests were Linux-only, and the Windows E2E legs only ran PowerShell and pwsh. Git Bash runs the same native wt.exe as PowerShell but through the bash integration — a combination no job covered. - Add a windows-latest leg to the unit test job (no -race there: it needs a cgo toolchain, and the point is running the suite on a Windows host). - Add a windows/bash E2E leg. run.go resolves bash from the Git for Windows install rather than PATH, so the WSL launcher in System32 is never used, and translates paths with cygpath: the interpreter only understands POSIX paths, while wt.exe only understands native ones. - Add E2E regression scenarios for the two symptoms of #112: init under Git Bash must configure bash rather than the PowerShell profile, and shellenv must emit the bash wrapper rather than PowerShell code. - Add unit tests that execute the generated bash wrapper against a fake wt, covering the missing-script(1) fallback (auto-cd still works, output is replayed, the command's exit status survives rather than tee's) and the cygpath translation of native paths. Both branches are driven from an ordinary bash by controlling what the wrapper's probes find, so they run on every OS rather than only on Windows. Refs #112
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #114 +/- ##
==========================================
+ Coverage 32.22% 36.91% +4.68%
==========================================
Files 27 27
Lines 3066 3102 +36
==========================================
+ Hits 988 1145 +157
+ Misses 1993 1871 -122
- Partials 85 86 +1
🚀 New features to boost your workflow:
|
Fallout from the new windows-latest unit test leg, which was the point of adding it — three of these failures predate this branch and had simply never run anywhere. - findCaseInsensitivePathCollision gave up on the whole path when a component was absent from the directory listing under that exact spelling. On Windows an 8.3 short name (RUNNER~1 for runneradmin) is exactly that: valid, but reported by ReadDir only under its long name. Keep walking when the component resolves through Stat, or a collision below such a component goes unwarned. - TestValidateShellEnvAcceptsValidXdgConfigHome built its "absolute" path as \abs\config, which on Windows is rooted but relative — it has no volume — so the test asserted the opposite of its name there. - The bash wrapper tests compared the shell's idea of the working directory with Go's. Under Git Bash those are different spellings of the same directory (/tmp/... vs C:\Users\...\Temp\...), which no amount of EvalSymlinks reconciles. Identify the directory by a marker file instead, and convert paths through cygpath inside the harness so a native path in PATH is not split on its drive-letter colon. Refs #112
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.
The Git Bash bug in #112 shipped because nothing ran there: unit tests were Linux-only, and the Windows E2E legs only ran PowerShell and pwsh. Git Bash runs the same native
wt.exeas PowerShell but through the bash integration — a combination no job covered.CI coverage
ubuntu-latest+windows-latest. No-raceon the Windows leg (it needs a cgo toolchain, and the point is running the suite on a Windows host); coverage is still uploaded from Linux only.windows-latest×bash.e2e/run.goresolves bash from the Git for Windows install rather thanPATH, so the WSL launcher inSystem32is never picked up; exportsMSYSTEM(abash -cdoes not read/etc/profile); and translates paths withcygpath, since the interpreter only understands POSIX paths whilewt.exeonly understands native ones.Regression scenarios
Windows-only (
skip_os: [linux, darwin]), one per symptom of #112:wt init --dry-rununder Git Bash must emiteval "$(wt shellenv bash)"and notInvoke-Expression.wt shellenvwith no args must emit the bash wrapper, not PowerShell code.The existing
checkout_auto_cd_verifiedscenario now also runs on Git Bash, which is the literal symptom reported in the issue.Unit tests
cmd/shellenv_wrapper_test.goexecutes the generated bash wrapper against a fakewtonPATH, covering:script(1)fallback — auto-cd still works, redirected output is replayed, and the command's exit status survives rather thantee's;cygpathtranslation of native Windows paths.Both branches are driven from an ordinary bash by controlling what the wrapper's
command -vprobes find, so they run on every OS rather than only on Windows. Reverting either half of the #112 fix makes them fail.Refs #112