Repository navigation
Keep Go unit tests off the host's Go env - #2057
Conversation
`utils::go_env` reads GOWORK, GOFLAGS, GOPATH and friends from the process and from the `go env -w` file under HOME. The Go unit tests call production code that goes through it, so a developer's ambient Go config changes what every fixture resolves. On origin/main `cargo test -p socket-patch-core --lib` fails 15 tests with GOWORK=off (a common `go env -w` setting), 103 with GOWORK pointing at any absolute file, and 99 with GOFLAGS=-modfile=alt.mod. In unit-test builds `go_env` now reads only the vars a test holds with an EnvGuard (`test_env::guarded_var`); everything else, the go env file's HOME/XDG_CONFIG_HOME/AppData included, is invisible. Production builds read the process env exactly as before. The three registry_fetch tests that hand-rolled set_var/restore now hold their keys with EnvGuard, so their values stay visible. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011f9aeSwyBVuHJgG8G9SKZ1
|
bugbot run Generated by Claude Code |
PR SummaryLow Risk Overview
A new Reviewed by Cursor Bugbot for commit 740eb68. Configure here. |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 740eb68. Configure here.
|
Ready for review at
Generated by Claude Code |
Final review briefWhat it does: In Risk: low. The production Look here
Verified: I read the full diff (4 files, +71/−44). Locally, Changes I made: none. Open questions: none. One minor point that isn't blocking: Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
Problem
CI is healthy this run. The merge queue is empty, every merge_group run since 11-10 passed, main's latest push run (42975739856) and today's nightly (42967443938) are green, nothing is queued, and no run in the last two days needed a re-run. Every PR failure in that window traces to the PR's own change. So this run takes a priority-4 robustness target in the same class as #1919/#2054/#2055/#2056: unit tests that pass only because CI's environment is clean. Those PRs cover npm/yarn, python and ruby. This one covers Go, and touches none of their files.
utils::go_envresolves Go settings from the process env, then thego env -wfile underHOME. The Go unit tests run production code that reads it. Reproduced onorigin/main(c09cb0e) withcargo test -p socket-patch-core --lib:GOWORK=off(a commongo env -wsetting)GOWORK=<any absolute file>golang_local,vendor::golang,go_consumer_sync,go_mod_edit,go_sum_backup,vex::discover::golang)GOFLAGS=-modfile=alt.modHOMEwith.config/go/env→GOWORK=off,GOFLAGS=-modfile=alt.mod* The same 4 tests fail in every row, before and after:
copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, and the twopypi_*wire-failure tests. They rely on permission errors, and my sandbox runs as root. CI doesn't run as root.A Go developer with
GOWORK=offorGOFLAGSset gets red tests on a clean checkout. The CI image could get the same result if it ever exports one of them.Root cause
go_envis the single place that reads process env for Go settings (GOWORK,GOFLAGS,GOPATH,GOMODCACHE,GOBIN, plusGOENV/HOME/XDG_CONFIG_HOME/AppDatato find the go env file). In unit tests it reads the developer's real environment.Fix
go_envreads throughprocess_env. In production builds that'sstd::env::var, unchanged. In#[cfg(test)]builds it'stest_env::guarded_var, which returns a var only while anEnvGuardholds it. Host values, including the go env file reached throughHOME, are invisible.EnvGuardrecords the keys it holds in a mutex-protected multiset and releases them on drop. Nested guards work, and so does a panic partway throughmany.registry_fetchgoproxy tests hand-rolledset_var/restore. They now hold their keys withEnvGuard, which also drops about 35 lines of manual save/restore. Assertions are unchanged.guarded_var_sees_only_vars_a_live_guard_holds.ctor) on purpose: that would add two new crates to the lockfile, and it would mutate env before main.No test is removed and no assertion is changed or weakened.
Proof
--libsuite (7,692 tests).GOWORK=offloop over the Go,test_envand goproxy tests: 30 runs, 0 failures.cargo clippy --locked -p socket-patch-core --all-features -- -D warnings(CI's command) is clean.--all-targetsreports no lint in any touched file. The pre-existing--all-targetslints are elsewhere.test_env.rswith#[path]pass:crawler_npm_e2e128,crawler_deno_e2e13,upstream_restore_golden62.https://claude.ai/code/session_011f9aeSwyBVuHJgG8G9SKZ1
Generated by Claude Code