Skip to content

Commit 740eb68

Browse files
committed
Keep Go unit tests off the host's Go env
`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
1 parent c09cb0e commit 740eb68

4 files changed

Lines changed: 71 additions & 44 deletions

File tree

‎crates/socket-patch-core/src/test_env.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,22 @@
44
//! must also be `#[serial_test::serial]`.
55
66
use std::ffi::{OsStr, OsString};
7+
use std::sync::{Mutex, MutexGuard, PoisonError};
8+
9+
/// The keys live guards hold, once per guard entry.
10+
static GUARDED: Mutex<Vec<OsString>> = Mutex::new(Vec::new());
11+
12+
fn guarded() -> MutexGuard<'static, Vec<OsString>> {
13+
GUARDED.lock().unwrap_or_else(PoisonError::into_inner)
14+
}
15+
16+
/// `key`'s value while an [`EnvGuard`] holds it, else `None` whatever the
17+
/// host set. For lookups that must not see the developer's environment
18+
/// (`utils::go_env` in the unit tests).
19+
pub(crate) fn guarded_var(key: &str) -> Option<String> {
20+
let held = guarded().iter().any(|k| k == key);
21+
held.then(|| std::env::var(key).ok()).flatten()
22+
}
723

824
/// Sets or removes env vars and puts the previous values back on drop, a
925
/// panicking assertion included. Previous values are kept as `OsString`,
@@ -33,6 +49,7 @@ impl EnvGuard {
3349
let mut guard = Self { saved: Vec::new() };
3450
for (key, value) in pairs {
3551
let key = key.as_ref();
52+
guarded().push(key.to_os_string());
3653
guard
3754
.saved
3855
.push((key.to_os_string(), std::env::var_os(key)));
@@ -53,6 +70,10 @@ impl Drop for EnvGuard {
5370
Some(v) => std::env::set_var(&key, v),
5471
None => std::env::remove_var(&key),
5572
}
73+
let mut held = guarded();
74+
if let Some(i) = held.iter().rposition(|k| *k == key) {
75+
held.swap_remove(i);
76+
}
5677
}
5778
}
5879
}

‎crates/socket-patch-core/src/test_env_tests.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,22 @@ fn keeps_a_non_utf8_previous_value() {
4747
assert_eq!(std::env::var_os(KEY).as_deref(), Some(raw));
4848
std::env::remove_var(KEY);
4949
}
50+
51+
#[test]
52+
#[serial_test::serial]
53+
fn guarded_var_sees_only_vars_a_live_guard_holds() {
54+
std::env::set_var(KEY, "host");
55+
assert_eq!(crate::test_env::guarded_var(KEY), None, "host value hidden");
56+
{
57+
let _outer = EnvGuard::set(KEY, "a");
58+
assert_eq!(crate::test_env::guarded_var(KEY).as_deref(), Some("a"));
59+
{
60+
let _inner = EnvGuard::unset(KEY);
61+
assert_eq!(crate::test_env::guarded_var(KEY), None);
62+
}
63+
// The outer guard still holds it once the inner one drops.
64+
assert_eq!(crate::test_env::guarded_var(KEY).as_deref(), Some("a"));
65+
}
66+
assert_eq!(crate::test_env::guarded_var(KEY), None, "released on drop");
67+
std::env::remove_var(KEY);
68+
}

‎crates/socket-patch-core/src/utils/go_env.rs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,22 @@ use std::path::{Path, PathBuf};
2424
/// caller applies go's default). An empty environment variable falls
2525
/// through to the file, as go's own `cfg.Getenv` does.
2626
pub(crate) fn go_env(key: &str) -> Option<String> {
27-
go_env_with(key, |k| std::env::var(k).ok())
27+
go_env_with(key, process_env)
28+
}
29+
30+
/// The process environment [`go_env`] reads.
31+
#[cfg(not(test))]
32+
fn process_env(key: &str) -> Option<String> {
33+
std::env::var(key).ok()
34+
}
35+
36+
/// In the unit tests, only the vars a test holds with an
37+
/// [`EnvGuard`](crate::test_env::EnvGuard). A developer's `GOWORK=off`,
38+
/// `GOFLAGS=-modfile=...` or `go env -w` file (found through `HOME`) would
39+
/// otherwise change what every Go fixture resolves.
40+
#[cfg(test)]
41+
fn process_env(key: &str) -> Option<String> {
42+
crate::test_env::guarded_var(key)
2843
}
2944

3045
/// [`go_env`] over an injected environment lookup.

‎crates/socket-patch-core/src/vendor/registry_fetch.rs‎

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,24 +1546,17 @@ mod tests {
15461546
// GOPROXY is a comma- OR pipe-separated list (go help goproxy); a
15471547
// pipe-separated value must yield the first usable proxy, not a
15481548
// `https://a|b`-shaped base that builds an unparseable URL.
1549-
let saved_socket = std::env::var("SOCKET_GOPROXY").ok();
1550-
let saved = std::env::var("GOPROXY").ok();
1551-
std::env::remove_var("SOCKET_GOPROXY");
1552-
std::env::set_var(
1553-
"GOPROXY",
1554-
"https://athens.example|https://proxy.golang.org|direct",
1555-
);
1549+
let _env = crate::test_env::EnvGuard::many([
1550+
("SOCKET_GOPROXY", None),
1551+
(
1552+
"GOPROXY",
1553+
Some("https://athens.example|https://proxy.golang.org|direct"),
1554+
),
1555+
]);
15561556
let piped = goproxy_base(MODULE);
1557-
std::env::set_var("GOPROXY", "https://mirror.example/,direct");
1557+
let _mixed_env =
1558+
crate::test_env::EnvGuard::set("GOPROXY", "https://mirror.example/,direct");
15581559
let mixed = goproxy_base(MODULE);
1559-
match saved {
1560-
Some(v) => std::env::set_var("GOPROXY", v),
1561-
None => std::env::remove_var("GOPROXY"),
1562-
}
1563-
match saved_socket {
1564-
Some(v) => std::env::set_var("SOCKET_GOPROXY", v),
1565-
None => std::env::remove_var("SOCKET_GOPROXY"),
1566-
}
15671560
assert_eq!(piped.as_deref(), Ok("https://athens.example"));
15681561
assert_eq!(mixed.as_deref(), Ok("https://mirror.example"));
15691562
}
@@ -1671,17 +1664,14 @@ mod tests {
16711664
"GONOPROXY",
16721665
"SOCKET_GOPROXY",
16731666
];
1674-
let saved: Vec<_> = keys.iter().map(|k| (*k, std::env::var(k).ok())).collect();
1667+
let _env = crate::test_env::EnvGuard::many(keys.map(|k| (k, None::<&str>)));
16751668
let dir = tempfile::tempdir().unwrap();
16761669
let file = dir.path().join("env");
16771670
std::fs::write(
16781671
&file,
16791672
"GOPROXY=http://127.0.0.1:18702\nGOPRIVATE=example.com\n",
16801673
)
16811674
.unwrap();
1682-
for k in &keys[1..] {
1683-
std::env::remove_var(k);
1684-
}
16851675
std::env::set_var("GOENV", &file);
16861676
let private = goproxy_base("example.com/upstream");
16871677
let public = goproxy_base("golang.org/x/text");
@@ -1690,12 +1680,6 @@ mod tests {
16901680
std::env::set_var("GOENV", "off");
16911681
std::env::remove_var("GOPROXY");
16921682
let off = goproxy_base("example.com/upstream");
1693-
for (k, v) in saved {
1694-
match v {
1695-
Some(v) => std::env::set_var(k, v),
1696-
None => std::env::remove_var(k),
1697-
}
1698-
}
16991683
let err = private.unwrap_err();
17001684
assert!(err.contains("matches GOPRIVATE"), "{err}");
17011685
assert_eq!(public.as_deref(), Ok("http://127.0.0.1:18702"));
@@ -1707,11 +1691,12 @@ mod tests {
17071691
#[serial_test::serial]
17081692
fn goproxy_base_env_precedence() {
17091693
const MODULE: &str = "example.com/m";
1710-
let saved_socket = std::env::var("SOCKET_GOPROXY").ok();
1711-
let saved = std::env::var("GOPROXY").ok();
17121694
// No `go env -w` file: the developer's own must not leak in.
1713-
let saved_goenv = std::env::var("GOENV").ok();
1714-
std::env::set_var("GOENV", "off");
1695+
let _env = crate::test_env::EnvGuard::many([
1696+
("SOCKET_GOPROXY", None),
1697+
("GOPROXY", None),
1698+
("GOENV", Some("off")),
1699+
]);
17151700

17161701
// SOCKET_GOPROXY wins over GOPROXY (trailing slash trimmed).
17171702
std::env::set_var("SOCKET_GOPROXY", "https://socket.example/");
@@ -1729,19 +1714,6 @@ mod tests {
17291714
// default proxy.
17301715
std::env::set_var("GOPROXY", "direct,off");
17311716
let no_proxy = goproxy_base(MODULE);
1732-
1733-
match saved_socket {
1734-
Some(v) => std::env::set_var("SOCKET_GOPROXY", v),
1735-
None => std::env::remove_var("SOCKET_GOPROXY"),
1736-
}
1737-
match saved {
1738-
Some(v) => std::env::set_var("GOPROXY", v),
1739-
None => std::env::remove_var("GOPROXY"),
1740-
}
1741-
match saved_goenv {
1742-
Some(v) => std::env::set_var("GOENV", v),
1743-
None => std::env::remove_var("GOENV"),
1744-
}
17451717
assert_eq!(socket_wins.as_deref(), Ok("https://socket.example"));
17461718
assert_eq!(
17471719
empty_falls_through.as_deref(),

0 commit comments

Comments
 (0)