Merge pull request #1560 from EliahKagan/run-ci/env-lifo

Let `gix_testtools::Env` undo multiple changes to the same var

Author: Byron

Cargo.lock

@@ -49,6 +49,9 @@ xz2 = { version = "0.1.6", optional = true }
 
 document-features = { version = "0.2.1", optional = true }
 
+[dev-dependencies]
+serial_test = { version = "3.1.0", default-features = false }
+
 [package.metadata.docs.rs]
 all-features = true
 features = ["document-features"]

tests/tools/Cargo.toml

@@ -823,7 +823,7 @@ fn family_name() -> &'static str {
     }
 }
 
-/// A utility to set environment variables, while unsetting them (or resetting them to their previous value) on drop.
+/// A utility to set and unset environment variables, while restoring or removing them on drop.
 #[derive(Default)]
 pub struct Env<'a> {
     altered_vars: Vec<(&'a str, Option<OsString>)>,
@@ -845,7 +845,7 @@ impl<'a> Env<'a> {
         self
     }
 
-    /// Set `var` to `value`.
+    /// Unset `var`.
     pub fn unset(mut self, var: &'a str) -> Self {
         let prev = std::env::var_os(var);
         std::env::remove_var(var);
@@ -856,7 +856,7 @@ impl<'a> Env<'a> {
 
 impl<'a> Drop for Env<'a> {
     fn drop(&mut self) {
-        for (var, prev_value) in &self.altered_vars {
+        for (var, prev_value) in self.altered_vars.iter().rev() {
             match prev_value {
                 Some(value) => std::env::set_var(var, value),
                 None => std::env::remove_var(var),

tests/tools/src/lib.rs

@@ -0,0 +1,93 @@
+use std::env;
+
+use gix_testtools::Env;
+use serial_test::serial;
+
+// We rely on these not already existing, to test `Env` without using or rewriting it.
+static VAR1: &str = "VAR_03FC4045_6043_4A61_9D15_852236CB632B";
+static VAR2: &str = "VAR_8C135840_05DB_4F3A_BFDD_FC755EC35B89";
+static VAR3: &str = "VAR_9B23A2BE_E20B_4670_93E2_3A6A8D47F274";
+
+struct TestEnv;
+
+impl TestEnv {
+    fn new() -> Self {
+        assert_eq!(env::var_os(VAR1), None);
+        assert_eq!(env::var_os(VAR2), None);
+        assert_eq!(env::var_os(VAR3), None);
+        Self
+    }
+}
+
+impl Drop for TestEnv {
+    fn drop(&mut self) {
+        env::remove_var(VAR1);
+        env::remove_var(VAR2);
+        env::remove_var(VAR3);
+    }
+}
+
+#[test]
+#[serial]
+fn nonoverlapping() {
+    let _meta = TestEnv::new();
+    env::set_var(VAR1, "old1");
+    env::set_var(VAR2, "old2");
+    {
+        let _env = Env::new().set(VAR1, "new1").unset(VAR2).set(VAR3, "new3");
+        assert_eq!(env::var_os(VAR1), Some("new1".into()));
+        assert_eq!(env::var_os(VAR2), None);
+        assert_eq!(env::var_os(VAR3), Some("new3".into()));
+    }
+    assert_eq!(env::var_os(VAR1), Some("old1".into()));
+    assert_eq!(env::var_os(VAR2), Some("old2".into()));
+    assert_eq!(env::var_os(VAR3), None);
+}
+
+#[test]
+#[serial]
+fn overlapping_reset() {
+    let _meta = TestEnv::new();
+    {
+        let _env = Env::new().set(VAR1, "new1A").set(VAR1, "new1B");
+        assert_eq!(env::var_os(VAR1), Some("new1B".into()));
+    }
+    assert_eq!(env::var_os(VAR1), None);
+}
+
+#[test]
+#[serial]
+fn overlapping_unset() {
+    let _meta = TestEnv::new();
+    env::set_var(VAR1, "old1");
+    {
+        let _env = Env::new().unset(VAR1).unset(VAR1);
+        assert_eq!(env::var_os(VAR1), None);
+    }
+    assert_eq!(env::var_os(VAR1), Some("old1".into()));
+}
+
+#[test]
+#[serial]
+fn overlapping_combo() {
+    let _meta = TestEnv::new();
+    env::set_var(VAR1, "old1");
+    env::set_var(VAR2, "old2");
+    {
+        let _env = Env::new()
+            .set(VAR1, "new1A")
+            .unset(VAR2)
+            .set(VAR1, "new1B")
+            .unset(VAR3)
+            .set(VAR2, "new2")
+            .set(VAR3, "new3")
+            .unset(VAR1)
+            .unset(VAR3);
+        assert_eq!(env::var_os(VAR1), None);
+        assert_eq!(env::var_os(VAR2), Some("new2".into()));
+        assert_eq!(env::var_os(VAR3), None);
+    }
+    assert_eq!(env::var_os(VAR1), Some("old1".into()));
+    assert_eq!(env::var_os(VAR2), Some("old2".into()));
+    assert_eq!(env::var_os(VAR3), None);
+}