Refactor env var guard

crates/comenqd/src/config.rs

@@ -118,46 +118,18 @@ mod tests {
     use std::fs;
     use tempfile::tempdir;
 
-    struct EnvVarGuard {
-        key: String,
-        original: Option<String>,
+    mod env_guard {
+        include!(concat!(
+            env!("CARGO_MANIFEST_DIR"),
+            "/../../tests/support/env_guard.rs"
+        ));
     }
 
-    impl EnvVarGuard {
-        fn set(key: &str, val: &str) -> Self {
-            let original = std::env::var(key).ok();
-            set_env_var(key, val);
-            Self {
-                key: key.to_string(),
-                original,
-            }
-        }
-    }
-
-    impl Drop for EnvVarGuard {
-        fn drop(&mut self) {
-            match &self.original {
-                Some(v) => set_env_var(&self.key, v),
-                None => remove_env_var(&self.key),
-            }
-        }
+    pub mod support {
+        pub use super::env_guard::{EnvVarGuard, remove_env_var, set_env_var};
     }
 
-    fn remove_env(key: &str) {
-        remove_env_var(key);
-    }
-
-    /// Safely set an environment variable for tests.
-    fn set_env_var(key: &str, val: &str) {
-        // Safety: tests using `serial_test::serial` run single-threaded.
-        unsafe { std::env::set_var(key, val) };
-    }
-
-    /// Safely remove an environment variable for tests.
-    fn remove_env_var(key: &str) {
-        // Safety: tests using `serial_test::serial` run single-threaded.
-        unsafe { std::env::remove_var(key) };
-    }
+    use support::{EnvVarGuard, remove_env_var};
 
     #[rstest]
     #[serial_test::serial]
@@ -169,7 +141,7 @@ mod tests {
             "github_token='abc'\nsocket_path='/tmp/s.sock'\nqueue_path='/tmp/q'",
         )
         .unwrap();
-        remove_env("COMENQD_SOCKET_PATH");
+        remove_env_var("COMENQD_SOCKET_PATH");
         let cfg = Config::from_file(&path).unwrap();
         assert_eq!(cfg.github_token, "abc");
         assert_eq!(cfg.socket_path, PathBuf::from("/tmp/s.sock"));

tests/cucumber.rs

@@ -1,4 +1,5 @@
 mod steps;
+mod support;
 use cucumber::World as _;
 use steps::{CliWorld, ClientWorld, CommentWorld, ConfigWorld, ListenerWorld, WorkerWorld};
 

tests/steps/config_steps.rs

@@ -5,49 +5,9 @@ use std::fs;
 use std::path::PathBuf;
 use tempfile::TempDir;
 
+use crate::support::env_guard::{EnvVarGuard, remove_env_var};
 use comenqd::config::Config;
 
-/// RAII guard for temporarily setting an environment variable.
-#[derive(Debug)]
-struct EnvVarGuard {
-    key: String,
-    original: Option<String>,
-}
-
-impl EnvVarGuard {
-    fn set(key: &str, value: &str) -> Self {
-        let original = std::env::var(key).ok();
-        set_env_var_safe(key, value);
-        Self {
-            key: key.to_string(),
-            original,
-        }
-    }
-}
-
-impl Drop for EnvVarGuard {
-    fn drop(&mut self) {
-        match &self.original {
-            Some(val) => set_env_var_safe(&self.key, val),
-            None => remove_env_var_safe(&self.key),
-        }
-    }
-}
-
-fn remove_env(key: &str) {
-    remove_env_var_safe(key);
-}
-
-fn set_env_var_safe(key: &str, value: &str) {
-    // Safety: each scenario runs under serial_test, so no concurrent access.
-    unsafe { std::env::set_var(key, value) };
-}
-
-fn remove_env_var_safe(key: &str) {
-    // Safety: each scenario runs under serial_test, so no concurrent access.
-    unsafe { std::env::remove_var(key) };
-}
-
 #[derive(Debug, Default, World)]
 pub struct ConfigWorld {
     dir: Option<TempDir>,
@@ -68,7 +28,7 @@ fn config_file_with_token(world: &mut ConfigWorld, token: String) {
     fs::write(&path, format!("github_token='{token}'")).expect("write file");
     world.dir = Some(dir);
     world.path = Some(path);
-    remove_env("COMENQD_SOCKET_PATH");
+    remove_env_var("COMENQD_SOCKET_PATH");
 }
 
 #[expect(clippy::expect_used, reason = "test setup uses expect")]
@@ -107,7 +67,7 @@ fn config_without_socket(world: &mut ConfigWorld, token: String) {
     .expect("write file");
     world.dir = Some(dir);
     world.path = Some(path);
-    remove_env("COMENQD_SOCKET_PATH");
+    remove_env_var("COMENQD_SOCKET_PATH");
 }
 
 #[given("a missing configuration file")]

tests/support/env_guard.rs

@@ -0,0 +1,106 @@
+//! Test helpers for managing environment variables.
+//!
+//! `EnvVarGuard` temporarily sets an environment variable and restores the
+//! previous value on drop.
+
+#[derive(Debug)]
+pub struct EnvVarGuard {
+    key: String,
+    original: Option<String>,
+}
+
+impl EnvVarGuard {
+    /// Set an environment variable for the lifetime of the returned guard.
+    pub fn set(key: &str, value: &str) -> Self {
+        let original = std::env::var(key).ok();
+        set_env_var(key, value);
+        Self {
+            key: key.to_string(),
+            original,
+        }
+    }
+}
+
+impl Drop for EnvVarGuard {
+    fn drop(&mut self) {
+        match &self.original {
+            Some(v) => set_env_var(&self.key, v),
+            None => remove_env_var(&self.key),
+        }
+    }
+}
+
+/// Set an environment variable for tests.
+///
+/// The nightly compiler marks `std::env::set_var` as `unsafe`.
+/// Tests run serially so using it is acceptable here.
+pub fn set_env_var(key: &str, value: &str) {
+    unsafe { std::env::set_var(key, value) };
+}
+
+/// Remove an environment variable for tests.
+///
+/// `std::env::remove_var` is also `unsafe` on nightly.
+pub fn remove_env_var(key: &str) {
+    unsafe { std::env::remove_var(key) };
+}
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    #[serial_test::serial]
+    fn set_env_var_sets_variable() {
+        let key = "ENV_GUARD_SET";
+        super::remove_env_var(key);
+        assert!(std::env::var(key).is_err());
+
+        super::set_env_var(key, "value");
+        assert_eq!(std::env::var(key).unwrap(), "value");
+
+        super::remove_env_var(key);
+    }
+
+    #[test]
+    #[serial_test::serial]
+    fn remove_env_var_removes_variable() {
+        let key = "ENV_GUARD_REMOVE";
+        super::set_env_var(key, "to_remove");
+        assert_eq!(std::env::var(key).unwrap(), "to_remove");
+
+        super::remove_env_var(key);
+        assert!(std::env::var(key).is_err());
+    }
+
+    #[test]
+    #[serial_test::serial]
+    fn remove_env_var_when_unset_is_noop() {
+        let key = "ENV_GUARD_REMOVE_UNSET";
+        super::remove_env_var(key);
+        assert!(std::env::var(key).is_err());
+    }
+
+    #[test]
+    #[serial_test::serial]
+    fn nested_env_var_guard_restores_previous_value() {
+        let key = "ENV_GUARD_TEST_NESTED";
+        super::remove_env_var(key);
+
+        super::set_env_var(key, "initial");
+        assert_eq!(std::env::var(key).unwrap(), "initial");
+
+        let guard1 = super::EnvVarGuard::set(key, "first");
+        assert_eq!(std::env::var(key).unwrap(), "first");
+
+        {
+            let _guard2 = super::EnvVarGuard::set(key, "second");
+            assert_eq!(std::env::var(key).unwrap(), "second");
+        }
+
+        assert_eq!(std::env::var(key).unwrap(), "first");
+
+        drop(guard1);
+        assert_eq!(std::env::var(key).unwrap(), "initial");
+
+        super::remove_env_var(key);
+    }
+}

tests/support/mod.rs

@@ -0,0 +1,3 @@
+//! Support utilities shared by tests.
+
+pub mod env_guard;