diff --git a/nmrs/src/api/builders/openvpn_builder.rs b/nmrs/src/api/builders/openvpn_builder.rs index e54a71ca..b0d179d8 100644 --- a/nmrs/src/api/builders/openvpn_builder.rs +++ b/nmrs/src/api/builders/openvpn_builder.rs @@ -906,20 +906,7 @@ mod tests { // --- from_ovpn_str tests --- - use std::sync::Mutex; - - static ENV_LOCK: Mutex<()> = Mutex::new(()); - - fn with_fake_xdg(f: impl FnOnce() -> R) -> R { - let _g = ENV_LOCK.lock().unwrap(); - let base = std::env::temp_dir().join(format!("nmrs-ovpn-test-{}", uuid::Uuid::new_v4())); - std::fs::create_dir_all(&base).unwrap(); - unsafe { std::env::set_var("XDG_DATA_HOME", &base) }; - let out = f(); - unsafe { std::env::remove_var("XDG_DATA_HOME") }; - let _ = std::fs::remove_dir_all(&base); - out - } + use crate::util::test_utils::with_fake_xdg; #[test] fn from_ovpn_str_basic_tls_file_certs() { diff --git a/nmrs/src/util/cert_store.rs b/nmrs/src/util/cert_store.rs index 4f07bb2a..9c37ebae 100644 --- a/nmrs/src/util/cert_store.rs +++ b/nmrs/src/util/cert_store.rs @@ -161,25 +161,7 @@ fn filename_for(cert_type: &str) -> Result<&'static str, ConnectionError> { #[cfg(test)] mod tests { use super::*; - use std::sync::Mutex; - - static ENV_LOCK: Mutex<()> = Mutex::new(()); - - fn with_fake_xdg(f: impl FnOnce() -> R) -> R { - let _g = ENV_LOCK.lock().unwrap(); - let base = std::env::temp_dir().join(format!("nmrs-cert-{}", uuid::Uuid::new_v4())); - std::fs::create_dir_all(&base).unwrap(); - // SAFETY: tests are serialized on this mutex; no other thread reads env concurrently. - unsafe { - std::env::set_var("XDG_DATA_HOME", &base); - } - let out = f(); - unsafe { - std::env::remove_var("XDG_DATA_HOME"); - } - let _ = std::fs::remove_dir_all(&base); - out - } + use crate::util::test_utils::with_fake_xdg; #[test] fn write_read_cleanup_cycle() { diff --git a/nmrs/src/util/mod.rs b/nmrs/src/util/mod.rs index 5750dba4..d87bd12e 100644 --- a/nmrs/src/util/mod.rs +++ b/nmrs/src/util/mod.rs @@ -5,3 +5,6 @@ pub(crate) mod cert_store; pub(crate) mod utils; pub(crate) mod validation; + +#[cfg(test)] +pub(crate) mod test_utils; diff --git a/nmrs/src/util/test_utils.rs b/nmrs/src/util/test_utils.rs new file mode 100644 index 00000000..7270ba15 --- /dev/null +++ b/nmrs/src/util/test_utils.rs @@ -0,0 +1,56 @@ +//! Shared test utilities. +//! +//! This module provides common test helpers that need to be shared across +//! multiple test modules to avoid race conditions. + +use std::sync::Mutex; + +/// Global mutex for tests that manipulate environment variables. +/// +/// Any test that sets `XDG_DATA_HOME` or other env vars must hold this lock +/// to avoid race conditions with other tests running in parallel. +pub static ENV_LOCK: Mutex<()> = Mutex::new(()); + +/// Runs a test closure with a fake `XDG_DATA_HOME` pointing to a unique temp directory. +/// +/// This function: +/// 1. Acquires the global `ENV_LOCK` to serialize env var access +/// 2. Creates a unique temp directory +/// 3. Sets `XDG_DATA_HOME` to that directory +/// 4. Runs the provided closure +/// 5. Cleans up the env var and temp directory +/// +/// If the closure panics, cleanup still happens (via Drop) but the mutex +/// will be poisoned. Use `ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner())` +/// if you need to recover from poisoned state. +pub fn with_fake_xdg(f: impl FnOnce() -> R) -> R { + let _guard = ENV_LOCK.lock().unwrap_or_else(|poisoned| { + // Recover from poisoned mutex (previous test panicked) + poisoned.into_inner() + }); + + let base = std::env::temp_dir().join(format!("nmrs-test-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&base).expect("failed to create temp directory for test"); + + // SAFETY: tests are serialized on ENV_LOCK; no other thread modifies env concurrently. + unsafe { + std::env::set_var("XDG_DATA_HOME", &base); + } + + // Use a guard struct to ensure cleanup happens even on panic + struct Cleanup { + base: std::path::PathBuf, + } + + impl Drop for Cleanup { + fn drop(&mut self) { + unsafe { + std::env::remove_var("XDG_DATA_HOME"); + } + let _ = std::fs::remove_dir_all(&self.base); + } + } + + let _cleanup = Cleanup { base }; + f() +}