Extract .nu-env tests and more granularity (#3078)

The autoenv logic mutates environment variables in the running session as
it operates and decides what to do for trusted directories containing `.nu-env`
files. Few of the ways to interact with it were all in a single test function.

We separate out all the ways that were done in the single test function to document
 it better. This will greatly help once we start refactoring our way out from setting
 environment variables this way to just setting them to `Scope`.

This is part of an on-going effort to keep variables (`PATH` and `ENV`)
in our `Scope` and rely on it for everything related to variables.

We expect to move away from setting (`std::*`) envrironment variables in the current
running process. This is non-trivial since we need to handle cases from vars
coming in from the outside world, prioritize, and also compare to the ones
we have both stored in memory and in configuration files.

Also to send out our in-memory (in `Scope`) variables properly to external
programs once we no longer rely on `std::env` vars from the running process.

Cargo.lock

@@ -2850,6 +2850,7 @@ dependencies = [
  "nu_plugin_tree",
  "nu_plugin_xpath",
  "pretty_env_logger",
+ "serial_test",
 ]
 
 [[package]]
@@ -5012,6 +5013,28 @@ dependencies = [
  "yaml-rust",
 ]
 
+[[package]]
+name = "serial_test"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e0bccbcf40c8938196944a3da0e133e031a33f4d6b72db3bda3cc556e361905d"
+dependencies = [
+ "lazy_static 1.4.0",
+ "parking_lot 0.11.1",
+ "serial_test_derive",
+]
+
+[[package]]
+name = "serial_test_derive"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b2acd6defeddb41eb60bb468f8825d0cfd0c2a76bc03bfd235b6a1dc4f6a1ad5"
+dependencies = [
+ "proc-macro2",
+ "quote 1.0.8",
+ "syn 1.0.60",
+]
+
 [[package]]
 name = "servo_arc"
 version = "0.1.1"

Cargo.toml

@@ -57,8 +57,10 @@ log = "0.4.14"
 pretty_env_logger = "0.4.0"
 
 [dev-dependencies]
-dunce = "1.0.1"
 nu-test-support = { version = "0.27.1", path = "./crates/nu-test-support" }
+dunce = "1.0.1"
+serial_test = "0.5.1"
+
 
 [build-dependencies]
 

crates/nu-command/src/commands/autoenv.rs

@@ -25,6 +25,7 @@ pub fn file_is_trusted(nu_env_file: &Path, content: &[u8]) -> Result<bool, Shell
     let nufile = std::fs::canonicalize(nu_env_file)?;
 
     let trusted = read_trusted()?;
+
     Ok(trusted.files.get(&nufile.to_string_lossy().to_string()) == Some(&contentdigest))
 }
 

crates/nu-test-support/src/lib.rs

@@ -4,6 +4,17 @@ pub mod macros;
 pub mod playground;
 pub mod value;
 
+pub struct Outcome {
+    pub out: String,
+    pub err: String,
+}
+
+impl Outcome {
+    pub fn new(out: String, err: String) -> Outcome {
+        Outcome { out, err }
+    }
+}
+
 pub fn pipeline(commands: &str) -> String {
     commands
         .lines()

crates/nu-test-support/src/macros.rs

@@ -71,7 +71,7 @@ macro_rules! nu {
 
             println!("=== stderr\n{}", err);
 
-        $crate::macros::Outcome::new(out,err.into_owned())
+        $crate::Outcome::new(out,err.into_owned())
     }};
 }
 
@@ -147,21 +147,10 @@ macro_rules! nu_with_plugins {
 
             println!("=== stderr\n{}", err);
 
-        $crate::macros::Outcome::new(out,err.into_owned())
+        $crate::Outcome::new(out,err.into_owned())
     }};
 }
 
-pub struct Outcome {
-    pub out: String,
-    pub err: String,
-}
-
-impl Outcome {
-    pub fn new(out: String, err: String) -> Outcome {
-        Outcome { out, err }
-    }
-}
-
 pub fn read_std(std: &[u8]) -> String {
     let out = String::from_utf8_lossy(std);
     let out = out.lines().skip(1).collect::<Vec<_>>().join("\n");

tests/shell/environment/mod.rs

@@ -0,0 +1,19 @@
+mod nu_env;
+
+pub mod support {
+    use nu_test_support::{nu, playground::*, Outcome};
+
+    pub struct Trusted;
+
+    impl Trusted {
+        pub fn in_path(dirs: &Dirs, block: impl FnOnce() -> Outcome) -> Outcome {
+            let for_env_manifest = dirs.test().to_string_lossy();
+
+            nu!(cwd: dirs.root(), format!("autoenv trust \"{}\"", for_env_manifest.to_string()));
+            let out = block();
+            nu!(cwd: dirs.root(), format!("autoenv untrust \"{}\"", for_env_manifest.to_string()));
+
+            out
+        }
+    }
+}

tests/shell/environment/nu_env.rs

@@ -0,0 +1,342 @@
+use super::support::Trusted;
+
+use nu_test_support::fs::Stub::FileWithContent;
+use nu_test_support::playground::Playground;
+use nu_test_support::{nu, pipeline};
+
+use serial_test::serial;
+
+// Windows uses a different command to create an empty file
+// so we need to have different content on windows.
+const SCRIPTS: &str = if cfg!(target_os = "windows") {
+    r#"[scripts]
+        entryscripts = ["echo nul > hello.txt"]
+        exitscripts = ["echo nul > bye.txt"]"#
+} else {
+    r#"[scripts]
+        entryscripts = ["touch hello.txt"]
+        exitscripts = ["touch bye.txt"]"#
+};
+
+#[test]
+#[serial]
+fn picks_up_env_keys_when_entering_trusted_directory() {
+    Playground::setup("autoenv_test_1", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContent(
+            ".nu-env",
+            &format!(
+                "{}\n{}",
+                r#"[env]
+                testkey = "testvalue"
+        
+               [scriptvars]
+                myscript = "echo myval"
+            "#,
+                SCRIPTS
+            ),
+        )]);
+
+        let expected = "testvalue";
+
+        let actual = Trusted::in_path(&dirs, || nu!(cwd: dirs.test(), "echo $nu.env.testkey"));
+
+        assert_eq!(actual.out, expected);
+    })
+}
+
+#[test]
+#[serial]
+fn picks_up_script_vars_when_entering_trusted_directory() {
+    Playground::setup("autoenv_test_2", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContent(
+            ".nu-env",
+            &format!(
+                "{}\n{}",
+                r#"[env]
+                testkey = "testvalue"
+        
+               [scriptvars]
+                myscript = "echo myval"
+            "#,
+                SCRIPTS
+            ),
+        )]);
+
+        let expected = "myval";
+
+        let actual = Trusted::in_path(&dirs, || nu!(cwd: dirs.test(), "echo $nu.env.myscript"));
+
+        assert_eq!(actual.out, expected);
+    })
+}
+
+#[test]
+#[serial]
+fn picks_up_env_keys_when_entering_trusted_directory_indirectly() {
+    Playground::setup("autoenv_test_3", |dirs, sandbox| {
+        sandbox.mkdir("crates");
+        sandbox.with_files(vec![FileWithContent(
+            ".nu-env",
+            r#"[env]
+                nu-version = "0.27.1" "#,
+        )]);
+
+        let expected = "0.27.1";
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test().join("crates"), r#"
+                cd ../../autoenv_test_3
+                echo $nu.env.nu-version
+            "#)
+        });
+
+        assert_eq!(actual.out, expected);
+    })
+}
+
+#[test]
+#[serial]
+fn entering_a_trusted_directory_runs_entry_scripts() {
+    Playground::setup("autoenv_test_4", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContent(
+            ".nu-env",
+            &format!(
+                "{}\n{}",
+                r#"[env]
+                testkey = "testvalue"
+        
+               [scriptvars]
+                myscript = "echo myval"
+            "#,
+                SCRIPTS
+            ),
+        )]);
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test(), pipeline(r#"
+                ls
+                | where name == "hello.txt"
+                | get name
+            "#))
+        });
+
+        assert_eq!(actual.out, "hello.txt");
+    })
+}
+
+#[test]
+#[serial]
+fn leaving_a_trusted_directory_runs_exit_scripts() {
+    Playground::setup("autoenv_test_5", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContent(
+            ".nu-env",
+            &format!(
+                "{}\n{}",
+                r#"[env]
+                testkey = "testvalue"
+        
+               [scriptvars]
+                myscript = "echo myval"
+            "#,
+                SCRIPTS
+            ),
+        )]);
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test(), r#"
+              cd ..
+              ls autoenv_test_5 | get name | path basename | where $it == "bye.txt"
+            "#)
+        });
+
+        assert_eq!(actual.out, "bye.txt");
+    })
+}
+
+#[test]
+#[serial]
+fn entry_scripts_are_called_when_revisiting_a_trusted_directory() {
+    Playground::setup("autoenv_test_6", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContent(
+            ".nu-env",
+            &format!(
+                "{}\n{}",
+                r#"[env]
+                testkey = "testvalue"
+        
+               [scriptvars]
+                myscript = "echo myval"
+            "#,
+                SCRIPTS
+            ),
+        )]);
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test(), r#"
+                do { rm hello.txt ; = $nothing } ; # Silence file deletion message from output
+                cd ..
+                cd autoenv_test_6
+                ls | where name == "hello.txt" | get name
+            "#)
+        });
+
+        assert_eq!(actual.out, "hello.txt");
+    })
+}
+
+#[test]
+#[serial]
+fn given_a_trusted_directory_with_entry_scripts_when_entering_a_subdirectory_entry_scripts_are_not_called(
+) {
+    Playground::setup("autoenv_test_7", |dirs, sandbox| {
+        sandbox.mkdir("time_to_cook_arepas");
+        sandbox.with_files(vec![FileWithContent(
+            ".nu-env",
+            &format!(
+                "{}\n{}",
+                r#"[env]
+                testkey = "testvalue"
+        
+               [scriptvars]
+                myscript = "echo myval"
+            "#,
+                SCRIPTS
+            ),
+        )]);
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test(), r#"
+                cd time_to_cook_arepas
+                ls | where name == "hello.txt" | count
+            "#)
+        });
+
+        assert_eq!(actual.out, "0");
+    })
+}
+
+#[test]
+#[serial]
+fn given_a_trusted_directory_with_exit_scripts_when_entering_a_subdirectory_exit_scripts_are_not_called(
+) {
+    Playground::setup("autoenv_test_8", |dirs, sandbox| {
+        sandbox.mkdir("time_to_cook_arepas");
+        sandbox.with_files(vec![FileWithContent(
+            ".nu-env",
+            &format!(
+                "{}\n{}",
+                r#"[env]
+                testkey = "testvalue"
+        
+               [scriptvars]
+                myscript = "echo myval"
+            "#,
+                SCRIPTS
+            ),
+        )]);
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test(), r#"
+                cd time_to_cook_arepas
+                ls | where name == "bye.txt" | count
+            "#)
+        });
+
+        assert_eq!(actual.out, "0");
+    })
+}
+
+#[test]
+#[serial]
+fn given_a_hierachy_of_trusted_directories_when_entering_in_any_nested_ones_should_carry_over_variables_set_from_the_root(
+) {
+    Playground::setup("autoenv_test_9", |dirs, sandbox| {
+        sandbox.mkdir("nu_plugin_rb");
+        sandbox.with_files(vec![
+            FileWithContent(
+                ".nu-env",
+                r#"[env]
+                organization = "nushell""#,
+            ),
+            FileWithContent(
+                "nu_plugin_rb/.nu-env",
+                r#"[env]
+                language = "Ruby""#,
+            ),
+        ]);
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test().parent().unwrap(), r#"
+                do { autoenv trust autoenv_test_9/nu_plugin_rb ; = $nothing } # Silence autoenv trust message from output
+                cd autoenv_test_9/nu_plugin_rb
+                echo $nu.env.organization
+            "#)
+        });
+
+        assert_eq!(actual.out, "nushell");
+    })
+}
+
+#[test]
+#[serial]
+fn given_a_hierachy_of_trusted_directories_nested_ones_should_overwrite_variables_from_parent_directories(
+) {
+    Playground::setup("autoenv_test_10", |dirs, sandbox| {
+        sandbox.mkdir("nu_plugin_rb");
+        sandbox.with_files(vec![
+            FileWithContent(
+                ".nu-env",
+                r#"[env]
+                organization = "nushell""#,
+            ),
+            FileWithContent(
+                "nu_plugin_rb/.nu-env",
+                r#"[env]
+                organization = "Andrab""#,
+            ),
+        ]);
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test().parent().unwrap(), r#"
+                do { autoenv trust autoenv_test_10/nu_plugin_rb ; = $nothing } # Silence autoenv trust message from output
+                cd autoenv_test_10/nu_plugin_rb
+                echo $nu.env.organization
+            "#)
+        });
+
+        assert_eq!(actual.out, "Andrab");
+    })
+}
+
+#[test]
+#[serial]
+fn given_a_hierachy_of_trusted_directories_going_back_restores_overwritten_variables() {
+    Playground::setup("autoenv_test_11", |dirs, sandbox| {
+        sandbox.mkdir("nu_plugin_rb");
+        sandbox.with_files(vec![
+            FileWithContent(
+                ".nu-env",
+                r#"[env]
+                organization = "nushell""#,
+            ),
+            FileWithContent(
+                "nu_plugin_rb/.nu-env",
+                r#"[env]
+                organization = "Andrab""#,
+            ),
+        ]);
+
+        let actual = Trusted::in_path(&dirs, || {
+            nu!(cwd: dirs.test().parent().unwrap(), r#"
+                do { autoenv trust autoenv_test_11/nu_plugin_rb ; = $nothing } # Silence autoenv trust message from output
+                cd autoenv_test_11
+                cd nu_plugin_rb
+                do { rm ../.nu-env ; = $nothing } # By deleting the root nu-env we have guarantees that the variable gets restored (not by autoenv when re-entering)
+                cd ..
+                echo $nu.env.organization
+            "#)
+        });
+
+        assert_eq!(actual.out, "nushell");
+    })
+}

tests/shell/mod.rs

@@ -1,5 +1,9 @@
 use nu_test_support::{nu, pipeline};
 
+#[cfg(feature = "directories-support")]
+#[cfg(feature = "which-support")]
+mod environment;
+
 mod pipeline;
 
 #[test]

tests/shell/pipeline/commands/internal.rs

@@ -34,237 +34,6 @@ fn takes_rows_of_nu_value_strings_and_pipes_it_to_stdin_of_external() {
     })
 }
 
-#[cfg(feature = "directories-support")]
-#[cfg(feature = "which-support")]
-#[test]
-fn autoenv() {
-    use nu_test_support::fs::Stub::FileWithContent;
-    Playground::setup("autoenv_test", |dirs, sandbox| {
-        sandbox.mkdir("foo/bar");
-        sandbox.mkdir("bizz/buzz");
-        sandbox.mkdir("foob");
-
-        // Windows uses a different command to create an empty file so we need to have different content on windows.
-        let full_nu_env = if cfg!(target_os = "windows") {
-            r#"[env]
-                testkey = "testvalue"
-
-                [scriptvars]
-                myscript = "echo myval"
-
-                [scripts]
-                entryscripts = ["echo nul > hello.txt"]
-                exitscripts = ["echo nul > bye.txt"]"#
-        } else {
-            r#"[env]
-                testkey = "testvalue"
-
-                [scriptvars]
-                myscript = "echo myval"
-
-                [scripts]
-                entryscripts = ["touch hello.txt"]
-                exitscripts = ["touch bye.txt"]"#
-        };
-
-        sandbox.with_files(vec![
-            FileWithContent(".nu-env", full_nu_env),
-            FileWithContent(
-                "foo/.nu-env",
-                r#"[env]
-                    overwrite_me = "set_in_foo"
-                    fookey = "fooval" "#,
-            ),
-            FileWithContent(
-                "foo/bar/.nu-env",
-                r#"[env]
-                    overwrite_me = "set_in_bar""#,
-            ),
-            FileWithContent("bizz/.nu-env", full_nu_env),
-        ]);
-
-        //Make sure basic keys are set
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust .
-               echo $nu.env.testkey"#
-        );
-        assert!(actual.out.ends_with("testvalue"));
-
-        // Make sure exitscripts are run in the directory they were specified.
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust
-               cd ..
-               cd autoenv_test
-               ls
-               ls | where name == "bye.txt" | get name"#
-        );
-        assert!(actual.out.contains("bye.txt"));
-
-        // Make sure entry scripts are run
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"cd ..
-               autoenv trust autoenv_test
-               cd autoenv_test
-               ls | where name == "hello.txt" | get name"#
-        );
-        assert!(actual.out.contains("hello.txt"));
-
-        // If inside a directory with exitscripts, entering a subdirectory should not trigger the exitscripts.
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust
-               cd foob
-               ls | where name == "bye.txt" | get name"#
-        );
-        assert!(!actual.out.contains("bye.txt"));
-
-        // Make sure entryscripts are run when re-visiting a directory
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust bizz
-               cd bizz
-               rm hello.txt
-               cd ..
-               cd bizz
-               ls | where name == "hello.txt" | get name"#
-        );
-        assert!(actual.out.contains("hello.txt"));
-
-        // Entryscripts should not run after changing to a subdirectory.
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust bizz
-               cd bizz
-               cd buzz
-               ls | where name == hello.txt | get name"#
-        );
-        assert!(!actual.out.ends_with("hello.txt"));
-
-        //Backing out of the directory should unset the keys
-        // let actual = nu!(
-        //     cwd: dirs.test(),
-        //     r#"cd ..
-        //        echo $nu.env.testkey"#
-        // );
-        // assert!(!actual.out.ends_with("testvalue"));
-
-        // Make sure script keys are set
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"echo $nu.env.myscript"#
-        );
-        assert!(actual.out.ends_with("myval"));
-
-        //Going to sibling directory without passing parent should work.
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust foo
-               cd foob
-               cd ../foo
-               echo $nu.env.fookey
-               cd .."#
-        );
-        assert!(actual.out.ends_with("fooval"));
-
-        //Going to sibling directory should unset keys
-        // let actual = nu!(
-        //     cwd: dirs.test(),
-        //     r#"cd foo
-        //        cd ../foob
-        //        echo $nu.env.fookey
-        //        cd .."#
-        // );
-        // assert!(!actual.out.ends_with("fooval"));
-
-        // Make sure entry scripts are run
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"ls | where name == "hello.txt" | get name"#
-        );
-        assert!(actual.out.contains("hello.txt"));
-
-        //Variables set in parent directories should be set even if you directly cd to a subdir
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust foo
-                   cd foo/bar
-                   autoenv trust
-                   echo $nu.env.fookey"#
-        );
-        assert!(actual.out.ends_with("fooval"));
-
-        //Subdirectories should overwrite the values of parent directories.
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust foo
-                   cd foo/bar
-                   autoenv trust
-                   echo $nu.env.overwrite_me"#
-        );
-        assert!(actual.out.ends_with("set_in_bar"));
-
-        //Make sure that overwritten values are restored.
-        //By deleting foo/.nu-env, we make sure that the value is actually restored and not just set again by autoenv when we re-visit foo.
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"cd foo
-                   cd bar
-                   rm ../.nu-env
-                   cd ..
-                   echo $nu.env.overwrite_me"#
-        );
-        assert!(actual.out.ends_with("set_in_foo"))
-    })
-}
-
-#[cfg(feature = "which")]
-#[test]
-fn nu_let_env_overwrites() {
-    Playground::setup("syncs_env_test_1", |dirs, sandbox| {
-        sandbox.with_files(vec![FileWithContent(
-            "configuration.toml",
-            r#"
-            [env]
-            SHELL = "/usr/bin/you_already_made_the_nu_choice"
-        "#,
-        )]);
-
-        let mut file = dirs.test().clone();
-        file.push("configuration.toml");
-
-        let fake_config = FakeConfig::new(&file);
-        let mut actual = EnvironmentSyncer::new();
-        actual.set_config(Box::new(fake_config));
-
-        // Here, the environment variables from the current session
-        // are cleared since we will load and set them from the
-        // configuration file (if any)
-        actual.clear_env_vars(&mut ctx);
-
-        // Nu loads the environment variables from the configuration file (if any)
-        actual.load_environment();
-
-        // By this point, Nu has already loaded the environment variables
-        // stored in the configuration file. Before continuing we check
-        // if any new environment variables have been added from the ones loaded
-        // in the configuration file.
-        //
-        // Nu sees the missing "USER" variable and accounts for it.
-        actual.sync_env_vars(&mut ctx);
-
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"let-env SHELL = bob
-            echo $nu.env.SHELL
-            "#
-        );
-        assert!(actual.out.ends_with("set_in_foo"))
-    });
-}
-
 #[test]
 fn invocation_properly_redirects() {
     let actual = nu!(