From 6eac9bfd0fa15e0b323d4a8413894d22684edcb5 Mon Sep 17 00:00:00 2001 From: Kiryl Mialeshka <8974488+meskill@users.noreply.github.com> Date: Mon, 14 Aug 2023 12:49:55 +0200 Subject: [PATCH] test: clear parent envs to prevent leakage to tests (#9976) Running tests locally from nushell with customizations (i.e. $env.PROMPT_COMMAND etc) may lead to failing tests as that customization leaks to the sandboxed nu itself. Remove `FILE_PWD` from env # Tests + Formatting Tests are now passing locally without issue in my case --- crates/nu-protocol/src/shell_error.rs | 2 +- crates/nu-test-support/src/macros.rs | 31 +++++++++++++++++---------- tests/shell/environment/env.rs | 4 ++-- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index 00273d14cc..9974a43c6f 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -431,7 +431,7 @@ pub enum ShellError { #[diagnostic( code(nu::shell::automatic_env_var_set_manually), help( - r#"The environment variable '{envvar_name}' is set automatically by Nushell and cannot not be set manually."# + r#"The environment variable '{envvar_name}' is set automatically by Nushell and cannot be set manually."# ) )] AutomaticEnvVarSetManually { diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index 39cc988025..d109cf4e70 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -158,12 +158,11 @@ macro_rules! nu { let target_cwd = $opts.cwd.unwrap_or(".".to_string()); let locale = $opts.locale.unwrap_or("en_US.UTF-8".to_string()); + let executable_path = $crate::fs::executable_path(); - let mut command = Command::new($crate::fs::executable_path()); + let mut command = $crate::macros::run_command(&executable_path, &target_cwd); command - .env("PWD", &target_cwd) .env(nu_utils::locale::LOCALE_OVERRIDE_ENV_VAR, locale) - .current_dir(target_cwd) .env(NATIVE_PATH_ENV_VAR, paths_joined) // .arg("--skip-plugins") // .arg("--no-history") @@ -278,7 +277,7 @@ macro_rules! nu_with_std { (@main $opts:expr, $path:expr) => {{ pub use std::error::Error; pub use std::io::prelude::*; - pub use std::process::{Command, Stdio}; + pub use std::process::Stdio; pub use $crate::NATIVE_PATH_ENV_VAR; pub fn escape_quote_string(input: String) -> String { @@ -319,12 +318,11 @@ macro_rules! nu_with_std { let target_cwd = $opts.cwd.unwrap_or(".".to_string()); let locale = $opts.locale.unwrap_or("en_US.UTF-8".to_string()); + let executable_path = $crate::fs::executable_path(); - let mut command = Command::new($crate::fs::executable_path()); + let mut command = $crate::macros::run_command(&executable_path, &target_cwd); command - .env("PWD", &target_cwd) .env(nu_utils::locale::LOCALE_OVERRIDE_ENV_VAR, locale) - .current_dir(target_cwd) .env(NATIVE_PATH_ENV_VAR, paths_joined) // .arg("--skip-plugins") // .arg("--no-history") @@ -396,7 +394,7 @@ macro_rules! nu_with_plugins { ($cwd:expr, [$(($format:expr, $plugin_name:expr)),+$(,)?], $command:expr) => {{ pub use std::error::Error; pub use std::io::prelude::*; - pub use std::process::{Command, Stdio}; + pub use std::process::Stdio; pub use tempfile::tempdir; pub use $crate::{NATIVE_PATH_ENV_VAR, with_exe}; @@ -436,9 +434,7 @@ macro_rules! nu_with_plugins { if !executable_path.exists() { executable_path = $crate::fs::installed_nu_path(); } - let mut process = match Command::new(executable_path) - .current_dir(&target_cwd) - .env("PWD", &target_cwd) // setting PWD is enough to set cwd + let mut process = match $crate::macros::run_command(&executable_path, &target_cwd) .arg("--commands") .arg(commands) .arg("--plugin-config") @@ -470,3 +466,16 @@ pub fn read_std(std: &[u8]) -> String { let out = out.replace("\r\n", ""); out.replace('\n', "") } + +use std::{path::PathBuf, process::Command}; + +pub fn run_command(executable_path: &PathBuf, target_cwd: &str) -> Command { + let mut command = Command::new(executable_path); + + command + .current_dir(target_cwd) + .env_remove("FILE_PWD") + .env("PWD", target_cwd); // setting PWD is enough to set cwd; + + command +} diff --git a/tests/shell/environment/env.rs b/tests/shell/environment/env.rs index f46b828f55..f9b9bfce6d 100644 --- a/tests/shell/environment/env.rs +++ b/tests/shell/environment/env.rs @@ -190,7 +190,7 @@ fn hides_env_in_block() { #[test] fn env_var_not_var() { let actual = nu!(" - echo $CARGO + echo $PWD "); - assert!(actual.err.contains("use $env.CARGO instead of $CARGO")); + assert!(actual.err.contains("use $env.PWD instead of $PWD")); }