Repository navigation
fix(sandbox): Make WSL backend tests deterministic - #948
Conversation
Co-Authored-By: cairn-code <cairn-code@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughBackend selection now accepts injectable WSL detection and preserves real detection by default. Tests cover Linux helper-missing fallback behavior for both non-WSL and WSL2 environments. ChangesSandbox backend WSL detection
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change makes WSL backend selection tests deterministic while preserving production host detection defaults; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Approving. Right fix, and it is complete, which I checked rather than assumed.
The interesting question with a change like this is not whether the one named test is now pinned, but whether any other test still reaches the real host detector. Eyeballing the call sites is not proof, so I made detectWSL panic and ran the whole package on both heads:
main: --- FAIL: TestSelectBackendChoosesPlatformAdapterWithFallback/linux_helper_missing_falls_back_explicitly
panic: PRODUCTION detectWSL REACHED FROM A TEST
this PR: ok github.com/Gitlawb/zero/internal/sandbox
Exactly one subtest was host-dependent, and after this nothing in the package touches the production detector. The other linux cases all supply a lookup that finds the helper, so they return before the WSL branch and never needed injecting.
Production behaviour is unchanged: selectPlatformBackend falls back to detectWSL when the field is nil, and both entry points forward it. gofmt clean, go vet clean for linux, darwin and windows, go build ./... clean.
Adding DetectWSL to the exported options structs is a test seam in a public API, but LookupExecutable is already exactly that in the same two structs, so it fits rather than sets a precedent.
The added WSL2 case is a real gain on its own: the positive branch had no coverage at all before, so nobody would have noticed if the fallback stopped reporting WSL2.
Summary
Make Linux backend selection tests deterministic on WSL hosts by allowing WSL detection to be injected. Production still defaults to detecting the real host environment.
Changes
DetectWSLtoBackendOptionsandSandboxManagerOptions.detectWSL()implementation when no detector is supplied.Test plan
go test ./internal/sandbox -run TestSelectBackendChoosesPlatformAdapterWithFallback/linux_helper_missing_falls_back_explicitly -count=1go test ./...go vet ./...Fixes #947
Summary by CodeRabbit
Bug Fixes
Tests