Fix parallel env-sensitive tests

Author: TopiCsarno

src/app/refresh.rs

@@ -284,16 +284,11 @@ mod tests {
 
     #[test]
     fn refresh_tasks_reconcile_claude_host_active_account_before_fetching() {
-        let _guard = test_support::env_lock();
+        let mut env = test_support::test_env();
         let state_root = temp_state_root("claude-active");
-        let prev_home = std::env::var_os("HOME");
-        let prev_state = std::env::var_os("XDG_STATE_HOME");
-        let prev_flatpak = std::env::var_os("FLATPAK_ID");
-        unsafe {
-            std::env::set_var("HOME", &state_root);
-            std::env::set_var("XDG_STATE_HOME", &state_root);
-            std::env::remove_var("FLATPAK_ID");
-        }
+        env.set("HOME", state_root.as_os_str());
+        env.set("XDG_STATE_HOME", state_root.as_os_str());
+        env.remove("FLATPAK_ID");
         let storage = ProviderAccountStorage::new(paths().claude_accounts_dir.clone());
         let account_a = stored_claude_account(&storage, "a@example.com", "acct-a");
         let account_b = stored_claude_account(&storage, "b@example.com", "acct-b");
@@ -312,23 +307,6 @@ mod tests {
 
         let _tasks = refresh_provider_tasks(&config, &mut state);
 
-        unsafe {
-            if let Some(value) = prev_home {
-                std::env::set_var("HOME", value);
-            } else {
-                std::env::remove_var("HOME");
-            }
-            if let Some(value) = prev_state {
-                std::env::set_var("XDG_STATE_HOME", value);
-            } else {
-                std::env::remove_var("XDG_STATE_HOME");
-            }
-            if let Some(value) = prev_flatpak {
-                std::env::set_var("FLATPAK_ID", value);
-            } else {
-                std::env::remove_var("FLATPAK_ID");
-            }
-        }
         assert_eq!(
             state
                 .provider(ProviderId::Claude)

src/config.rs

@@ -455,23 +455,9 @@ mod tests {
 
     #[test]
     fn flatpak_paths_use_dot_var_layout() {
-        use std::sync::Mutex;
-
-        static ENV_LOCK: Mutex<()> = Mutex::new(());
-
-        let _guard = ENV_LOCK.lock().expect("flatpak path test env lock");
-
-        let prev_id = std::env::var_os("FLATPAK_ID");
-
-        let p = unsafe {
-            std::env::set_var("FLATPAK_ID", "com.example.YapCapTest");
-            let p = paths();
-            std::env::remove_var("FLATPAK_ID");
-            if let Some(ref v) = prev_id {
-                std::env::set_var("FLATPAK_ID", v);
-            }
-            p
-        };
+        let mut env = crate::test_support::test_env();
+        env.set("FLATPAK_ID", "com.example.YapCapTest");
+        let p = paths();
 
         use std::path::Path;
         assert!(
@@ -497,16 +483,8 @@ mod tests {
 
     #[test]
     fn host_user_home_dir_matches_dirs_home_without_flatpak() {
-        let _guard = crate::test_support::env_lock();
-        let prev = std::env::var_os("FLATPAK_ID");
-        unsafe {
-            std::env::remove_var("FLATPAK_ID");
-        }
+        let mut env = crate::test_support::test_env();
+        env.remove("FLATPAK_ID");
         assert_eq!(host_user_home_dir(), dirs::home_dir());
-        if let Some(v) = prev {
-            unsafe {
-                std::env::set_var("FLATPAK_ID", v);
-            }
-        }
     }
 }

src/test_support.rs

@@ -1,8 +1,71 @@
 // SPDX-License-Identifier: MPL-2.0
 
+use std::ffi::{OsStr, OsString};
 use std::sync::{Mutex, MutexGuard, OnceLock};
 
 pub fn env_lock() -> MutexGuard<'static, ()> {
+    lock_env()
+}
+
+pub struct TestEnv {
+    _guard: MutexGuard<'static, ()>,
+    saved: Vec<(OsString, Option<OsString>)>,
+}
+
+pub fn test_env() -> TestEnv {
+    TestEnv {
+        _guard: lock_env(),
+        saved: Vec::new(),
+    }
+}
+
+impl TestEnv {
+    pub fn set<K, V>(&mut self, key: K, value: V)
+    where
+        K: AsRef<OsStr>,
+        V: AsRef<OsStr>,
+    {
+        self.save_once(key.as_ref());
+        unsafe {
+            std::env::set_var(key, value);
+        }
+    }
+
+    pub fn remove<K>(&mut self, key: K)
+    where
+        K: AsRef<OsStr>,
+    {
+        self.save_once(key.as_ref());
+        unsafe {
+            std::env::remove_var(key);
+        }
+    }
+
+    fn save_once(&mut self, key: &OsStr) {
+        if self.saved.iter().any(|(saved, _)| saved == key) {
+            return;
+        }
+        self.saved.push((key.to_os_string(), std::env::var_os(key)));
+    }
+}
+
+impl Drop for TestEnv {
+    fn drop(&mut self) {
+        for (key, value) in self.saved.iter().rev() {
+            unsafe {
+                if let Some(value) = value {
+                    std::env::set_var(key, value);
+                } else {
+                    std::env::remove_var(key);
+                }
+            }
+        }
+    }
+}
+
+fn lock_env() -> MutexGuard<'static, ()> {
     static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
-    LOCK.get_or_init(|| Mutex::new(())).lock().unwrap()
+    LOCK.get_or_init(|| Mutex::new(()))
+        .lock()
+        .unwrap_or_else(|poisoned| poisoned.into_inner())
 }