fix(#354): consolidate env var mutex to fix flaky cert store tests

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<R>(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() {

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<R>(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() {

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;

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<R>(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()
+}