From 7dc1d6a350b7d2acf1b5786c439e39c142bc591e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20N=2E=20Robalino?= Date: Thu, 18 Feb 2021 20:24:27 -0500 Subject: [PATCH] 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 | 23 ++ Cargo.toml | 4 +- crates/nu-command/src/commands/autoenv.rs | 1 + crates/nu-test-support/src/lib.rs | 11 + crates/nu-test-support/src/macros.rs | 15 +- tests/shell/environment/mod.rs | 19 ++ tests/shell/environment/nu_env.rs | 342 ++++++++++++++++++++++ tests/shell/mod.rs | 4 + tests/shell/pipeline/commands/internal.rs | 231 --------------- 9 files changed, 405 insertions(+), 245 deletions(-) create mode 100644 tests/shell/environment/mod.rs create mode 100644 tests/shell/environment/nu_env.rs diff --git a/Cargo.lock b/Cargo.lock index c8ea0aa205..438d3381b1 100644 --- a/Cargo.lock +++ b/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" diff --git a/Cargo.toml b/Cargo.toml index 72efac421e..0bdc8336f0 100644 --- a/Cargo.toml +++ b/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] diff --git a/crates/nu-command/src/commands/autoenv.rs b/crates/nu-command/src/commands/autoenv.rs index e14260a3bf..c5eb7d26ba 100644 --- a/crates/nu-command/src/commands/autoenv.rs +++ b/crates/nu-command/src/commands/autoenv.rs @@ -25,6 +25,7 @@ pub fn file_is_trusted(nu_env_file: &Path, content: &[u8]) -> Result Outcome { + Outcome { out, err } + } +} + pub fn pipeline(commands: &str) -> String { commands .lines() diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index 08e2e091bd..0009da7935 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/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::>().join("\n"); diff --git a/tests/shell/environment/mod.rs b/tests/shell/environment/mod.rs new file mode 100644 index 0000000000..f1dce23e71 --- /dev/null +++ b/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 + } + } +} diff --git a/tests/shell/environment/nu_env.rs b/tests/shell/environment/nu_env.rs new file mode 100644 index 0000000000..67fb429b68 --- /dev/null +++ b/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"); + }) +} diff --git a/tests/shell/mod.rs b/tests/shell/mod.rs index 87fd0abede..25d3487a91 100644 --- a/tests/shell/mod.rs +++ b/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] diff --git a/tests/shell/pipeline/commands/internal.rs b/tests/shell/pipeline/commands/internal.rs index 8f4a87f370..d34eebfe3f 100644 --- a/tests/shell/pipeline/commands/internal.rs +++ b/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!(