diff --git a/crates/socket-patch-core/src/test_env.rs b/crates/socket-patch-core/src/test_env.rs index fd34e92f5..6fe5f64db 100644 --- a/crates/socket-patch-core/src/test_env.rs +++ b/crates/socket-patch-core/src/test_env.rs @@ -4,6 +4,22 @@ //! must also be `#[serial_test::serial]`. use std::ffi::{OsStr, OsString}; +use std::sync::{Mutex, MutexGuard, PoisonError}; + +/// The keys live guards hold, once per guard entry. +static GUARDED: Mutex> = Mutex::new(Vec::new()); + +fn guarded() -> MutexGuard<'static, Vec> { + GUARDED.lock().unwrap_or_else(PoisonError::into_inner) +} + +/// `key`'s value while an [`EnvGuard`] holds it, else `None` whatever the +/// host set. For lookups that must not see the developer's environment +/// (`utils::go_env` in the unit tests). +pub(crate) fn guarded_var(key: &str) -> Option { + let held = guarded().iter().any(|k| k == key); + held.then(|| std::env::var(key).ok()).flatten() +} /// Sets or removes env vars and puts the previous values back on drop, a /// panicking assertion included. Previous values are kept as `OsString`, @@ -33,6 +49,7 @@ impl EnvGuard { let mut guard = Self { saved: Vec::new() }; for (key, value) in pairs { let key = key.as_ref(); + guarded().push(key.to_os_string()); guard .saved .push((key.to_os_string(), std::env::var_os(key))); @@ -53,6 +70,10 @@ impl Drop for EnvGuard { Some(v) => std::env::set_var(&key, v), None => std::env::remove_var(&key), } + let mut held = guarded(); + if let Some(i) = held.iter().rposition(|k| *k == key) { + held.swap_remove(i); + } } } } diff --git a/crates/socket-patch-core/src/test_env_tests.rs b/crates/socket-patch-core/src/test_env_tests.rs index 4b4bcc80b..76e8b0817 100644 --- a/crates/socket-patch-core/src/test_env_tests.rs +++ b/crates/socket-patch-core/src/test_env_tests.rs @@ -47,3 +47,22 @@ fn keeps_a_non_utf8_previous_value() { assert_eq!(std::env::var_os(KEY).as_deref(), Some(raw)); std::env::remove_var(KEY); } + +#[test] +#[serial_test::serial] +fn guarded_var_sees_only_vars_a_live_guard_holds() { + std::env::set_var(KEY, "host"); + assert_eq!(crate::test_env::guarded_var(KEY), None, "host value hidden"); + { + let _outer = EnvGuard::set(KEY, "a"); + assert_eq!(crate::test_env::guarded_var(KEY).as_deref(), Some("a")); + { + let _inner = EnvGuard::unset(KEY); + assert_eq!(crate::test_env::guarded_var(KEY), None); + } + // The outer guard still holds it once the inner one drops. + assert_eq!(crate::test_env::guarded_var(KEY).as_deref(), Some("a")); + } + assert_eq!(crate::test_env::guarded_var(KEY), None, "released on drop"); + std::env::remove_var(KEY); +} diff --git a/crates/socket-patch-core/src/utils/go_env.rs b/crates/socket-patch-core/src/utils/go_env.rs index 0c319f251..705fbec74 100644 --- a/crates/socket-patch-core/src/utils/go_env.rs +++ b/crates/socket-patch-core/src/utils/go_env.rs @@ -24,7 +24,22 @@ use std::path::{Path, PathBuf}; /// caller applies go's default). An empty environment variable falls /// through to the file, as go's own `cfg.Getenv` does. pub(crate) fn go_env(key: &str) -> Option { - go_env_with(key, |k| std::env::var(k).ok()) + go_env_with(key, process_env) +} + +/// The process environment [`go_env`] reads. +#[cfg(not(test))] +fn process_env(key: &str) -> Option { + std::env::var(key).ok() +} + +/// In the unit tests, only the vars a test holds with an +/// [`EnvGuard`](crate::test_env::EnvGuard). A developer's `GOWORK=off`, +/// `GOFLAGS=-modfile=...` or `go env -w` file (found through `HOME`) would +/// otherwise change what every Go fixture resolves. +#[cfg(test)] +fn process_env(key: &str) -> Option { + crate::test_env::guarded_var(key) } /// [`go_env`] over an injected environment lookup. diff --git a/crates/socket-patch-core/src/vendor/registry_fetch.rs b/crates/socket-patch-core/src/vendor/registry_fetch.rs index 0f7e534bc..cf119c9ac 100644 --- a/crates/socket-patch-core/src/vendor/registry_fetch.rs +++ b/crates/socket-patch-core/src/vendor/registry_fetch.rs @@ -1546,24 +1546,17 @@ mod tests { // GOPROXY is a comma- OR pipe-separated list (go help goproxy); a // pipe-separated value must yield the first usable proxy, not a // `https://a|b`-shaped base that builds an unparseable URL. - let saved_socket = std::env::var("SOCKET_GOPROXY").ok(); - let saved = std::env::var("GOPROXY").ok(); - std::env::remove_var("SOCKET_GOPROXY"); - std::env::set_var( - "GOPROXY", - "https://athens.example|https://proxy.golang.org|direct", - ); + let _env = crate::test_env::EnvGuard::many([ + ("SOCKET_GOPROXY", None), + ( + "GOPROXY", + Some("https://athens.example|https://proxy.golang.org|direct"), + ), + ]); let piped = goproxy_base(MODULE); - std::env::set_var("GOPROXY", "https://mirror.example/,direct"); + let _mixed_env = + crate::test_env::EnvGuard::set("GOPROXY", "https://mirror.example/,direct"); let mixed = goproxy_base(MODULE); - match saved { - Some(v) => std::env::set_var("GOPROXY", v), - None => std::env::remove_var("GOPROXY"), - } - match saved_socket { - Some(v) => std::env::set_var("SOCKET_GOPROXY", v), - None => std::env::remove_var("SOCKET_GOPROXY"), - } assert_eq!(piped.as_deref(), Ok("https://athens.example")); assert_eq!(mixed.as_deref(), Ok("https://mirror.example")); } @@ -1671,7 +1664,7 @@ mod tests { "GONOPROXY", "SOCKET_GOPROXY", ]; - let saved: Vec<_> = keys.iter().map(|k| (*k, std::env::var(k).ok())).collect(); + let _env = crate::test_env::EnvGuard::many(keys.map(|k| (k, None::<&str>))); let dir = tempfile::tempdir().unwrap(); let file = dir.path().join("env"); std::fs::write( @@ -1679,9 +1672,6 @@ mod tests { "GOPROXY=http://127.0.0.1:18702\nGOPRIVATE=example.com\n", ) .unwrap(); - for k in &keys[1..] { - std::env::remove_var(k); - } std::env::set_var("GOENV", &file); let private = goproxy_base("example.com/upstream"); let public = goproxy_base("golang.org/x/text"); @@ -1690,12 +1680,6 @@ mod tests { std::env::set_var("GOENV", "off"); std::env::remove_var("GOPROXY"); let off = goproxy_base("example.com/upstream"); - for (k, v) in saved { - match v { - Some(v) => std::env::set_var(k, v), - None => std::env::remove_var(k), - } - } let err = private.unwrap_err(); assert!(err.contains("matches GOPRIVATE"), "{err}"); assert_eq!(public.as_deref(), Ok("http://127.0.0.1:18702")); @@ -1707,11 +1691,12 @@ mod tests { #[serial_test::serial] fn goproxy_base_env_precedence() { const MODULE: &str = "example.com/m"; - let saved_socket = std::env::var("SOCKET_GOPROXY").ok(); - let saved = std::env::var("GOPROXY").ok(); // No `go env -w` file: the developer's own must not leak in. - let saved_goenv = std::env::var("GOENV").ok(); - std::env::set_var("GOENV", "off"); + let _env = crate::test_env::EnvGuard::many([ + ("SOCKET_GOPROXY", None), + ("GOPROXY", None), + ("GOENV", Some("off")), + ]); // SOCKET_GOPROXY wins over GOPROXY (trailing slash trimmed). std::env::set_var("SOCKET_GOPROXY", "https://socket.example/"); @@ -1729,19 +1714,6 @@ mod tests { // default proxy. std::env::set_var("GOPROXY", "direct,off"); let no_proxy = goproxy_base(MODULE); - - match saved_socket { - Some(v) => std::env::set_var("SOCKET_GOPROXY", v), - None => std::env::remove_var("SOCKET_GOPROXY"), - } - match saved { - Some(v) => std::env::set_var("GOPROXY", v), - None => std::env::remove_var("GOPROXY"), - } - match saved_goenv { - Some(v) => std::env::set_var("GOENV", v), - None => std::env::remove_var("GOENV"), - } assert_eq!(socket_wins.as_deref(), Ok("https://socket.example")); assert_eq!( empty_falls_through.as_deref(),