From d735607ac844d944d2760c94c4efb82d7bd8d114 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Tue, 9 Apr 2024 15:27:46 -0700 Subject: [PATCH] Isolate tests from user config (#12437) # Description This is an attempt to isolate the unit tests from whatever might be in the user's config. If the user's config is broken in some way or incompatible with this version (for example, especially if there are plugins that aren't built for this version), tests can spuriously fail. This makes tests more reliably pass the same way they would on CI even if the user has config, and should also make them run faster. I think this is _good enough_, but I still think we should have a specific config dir env variable for nushell specifically (rather than having to use `XDG_CONFIG_HOME`, which would mess with other things) and then we can just have `nu-test-support` set that to a temporary dir containing the shipped default config files. # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` --- crates/nu-command/tests/commands/do_.rs | 2 +- crates/nu-command/tests/commands/exec.rs | 8 +- .../nu-command/tests/commands/interleave.rs | 4 +- crates/nu-command/tests/commands/return_.rs | 4 +- crates/nu-command/tests/commands/save.rs | 10 +- .../tests/logger_tests/test_basic_commands.nu | 4 +- .../tests/logger_tests/test_log_custom.nu | 6 +- .../logger_tests/test_log_format_flag.nu | 4 +- crates/nu-test-support/src/macros.rs | 19 +++- src/run.rs | 104 ++++++++++-------- tests/shell/mod.rs | 6 +- tests/shell/pipeline/commands/external.rs | 14 +-- toolkit.nu | 2 +- 13 files changed, 105 insertions(+), 82 deletions(-) diff --git a/crates/nu-command/tests/commands/do_.rs b/crates/nu-command/tests/commands/do_.rs index e7ffd3ae3d..6a71a0f025 100644 --- a/crates/nu-command/tests/commands/do_.rs +++ b/crates/nu-command/tests/commands/do_.rs @@ -47,7 +47,7 @@ fn ignore_shell_errors_works_for_external_with_semicolon() { #[test] fn ignore_program_errors_works_for_external_with_semicolon() { - let actual = nu!(r#"do -p { nu -c 'exit 1' }; "text""#); + let actual = nu!(r#"do -p { nu -n -c 'exit 1' }; "text""#); assert_eq!(actual.err, ""); assert_eq!(actual.out, "text"); diff --git a/crates/nu-command/tests/commands/exec.rs b/crates/nu-command/tests/commands/exec.rs index deb2686593..9972df6a50 100644 --- a/crates/nu-command/tests/commands/exec.rs +++ b/crates/nu-command/tests/commands/exec.rs @@ -7,7 +7,7 @@ fn basic_exec() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - nu -c 'exec nu --testbin cococo a b c' + nu -n -c 'exec nu --testbin cococo a b c' "# )); @@ -21,7 +21,7 @@ fn exec_complex_args() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - nu -c 'exec nu --testbin cococo b --bar=2 -sab --arwr - -DTEEE=aasd-290 -90 --' + nu -n -c 'exec nu --testbin cococo b --bar=2 -sab --arwr - -DTEEE=aasd-290 -90 --' "# )); @@ -35,7 +35,7 @@ fn exec_fail_batched_short_args() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - nu -c 'exec nu --testbin cococo -ab 10' + nu -n -c 'exec nu --testbin cococo -ab 10' "# )); @@ -49,7 +49,7 @@ fn exec_misc_values() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - nu -c 'let x = "abc"; exec nu --testbin cococo $x ...[ a b c ]' + nu -n -c 'let x = "abc"; exec nu --testbin cococo $x ...[ a b c ]' "# )); diff --git a/crates/nu-command/tests/commands/interleave.rs b/crates/nu-command/tests/commands/interleave.rs index 398be6b283..e7cf8cfe4c 100644 --- a/crates/nu-command/tests/commands/interleave.rs +++ b/crates/nu-command/tests/commands/interleave.rs @@ -3,8 +3,8 @@ use nu_test_support::nu; #[test] fn interleave_external_commands() { let result = nu!("interleave \ - { nu -c 'print hello; print world' | lines | each { 'greeter: ' ++ $in } } \ - { nu -c 'print nushell; print rocks' | lines | each { 'evangelist: ' ++ $in } } | \ + { nu -n -c 'print hello; print world' | lines | each { 'greeter: ' ++ $in } } \ + { nu -n -c 'print nushell; print rocks' | lines | each { 'evangelist: ' ++ $in } } | \ each { print }; null"); assert!(result.out.contains("greeter: hello"), "{}", result.out); assert!(result.out.contains("greeter: world"), "{}", result.out); diff --git a/crates/nu-command/tests/commands/return_.rs b/crates/nu-command/tests/commands/return_.rs index f8fefeb8b6..708103d184 100644 --- a/crates/nu-command/tests/commands/return_.rs +++ b/crates/nu-command/tests/commands/return_.rs @@ -18,7 +18,7 @@ fn early_return_if_false() { fn return_works_in_script_without_def_main() { let actual = nu!( cwd: "tests/fixtures/formats", pipeline( - "nu early_return.nu" + "nu -n early_return.nu" )); assert!(actual.err.is_empty()); @@ -28,7 +28,7 @@ fn return_works_in_script_without_def_main() { fn return_works_in_script_with_def_main() { let actual = nu!( cwd: "tests/fixtures/formats", - pipeline("nu early_return_outside_main.nu") + pipeline("nu -n early_return_outside_main.nu") ); assert!(actual.err.is_empty()); } diff --git a/crates/nu-command/tests/commands/save.rs b/crates/nu-command/tests/commands/save.rs index db30faecd9..9febfdc2e7 100644 --- a/crates/nu-command/tests/commands/save.rs +++ b/crates/nu-command/tests/commands/save.rs @@ -93,7 +93,7 @@ fn save_stderr_and_stdout_to_afame_file() { r#" $env.FOO = "bar"; $env.BAZ = "ZZZ"; - do -c {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_5/new-file.txt --stderr save_test_5/new-file.txt + do -c {nu -n -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_5/new-file.txt --stderr save_test_5/new-file.txt "#, ); assert!(actual @@ -115,7 +115,7 @@ fn save_stderr_and_stdout_to_diff_file() { r#" $env.FOO = "bar"; $env.BAZ = "ZZZ"; - do -c {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_6/log.txt --stderr save_test_6/err.txt + do -c {nu -n -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_6/log.txt --stderr save_test_6/err.txt "#, ); @@ -208,7 +208,7 @@ fn save_append_works_on_stderr() { r#" $env.FOO = " New"; $env.BAZ = " New Err"; - do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -a -r save_test_11/log.txt --stderr save_test_11/err.txt"#, + do -i {nu -n -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -a -r save_test_11/log.txt --stderr save_test_11/err.txt"#, ); let actual = file_contents(expected_file); @@ -229,7 +229,7 @@ fn save_not_overrides_err_by_default() { r#" $env.FOO = " New"; $env.BAZ = " New Err"; - do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_12/log.txt --stderr save_test_12/err.txt"#, + do -i {nu -n -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_12/log.txt --stderr save_test_12/err.txt"#, ); assert!(actual.err.contains("Destination file already exists")); @@ -252,7 +252,7 @@ fn save_override_works_stderr() { r#" $env.FOO = "New"; $env.BAZ = "New Err"; - do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -f -r save_test_13/log.txt --stderr save_test_13/err.txt"#, + do -i {nu -n -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -f -r save_test_13/log.txt --stderr save_test_13/err.txt"#, ); let actual = file_contents(expected_file); diff --git a/crates/nu-std/tests/logger_tests/test_basic_commands.nu b/crates/nu-std/tests/logger_tests/test_basic_commands.nu index c2dcba5a71..7956d5fbd6 100644 --- a/crates/nu-std/tests/logger_tests/test_basic_commands.nu +++ b/crates/nu-std/tests/logger_tests/test_basic_commands.nu @@ -6,9 +6,9 @@ def run [ --short ] { if $short { - ^$nu.current-exe --commands $'use std; NU_log-level=($system_level) std log ($message_level) --short "test message"' + ^$nu.current-exe --no-config-file --commands $'use std; NU_log-level=($system_level) std log ($message_level) --short "test message"' } else { - ^$nu.current-exe --commands $'use std; NU_log-level=($system_level) std log ($message_level) "test message"' + ^$nu.current-exe --no-config-file --commands $'use std; NU_log-level=($system_level) std log ($message_level) "test message"' } | complete | get --ignore-errors stderr } diff --git a/crates/nu-std/tests/logger_tests/test_log_custom.nu b/crates/nu-std/tests/logger_tests/test_log_custom.nu index 626e1f50b0..57aef1f547 100644 --- a/crates/nu-std/tests/logger_tests/test_log_custom.nu +++ b/crates/nu-std/tests/logger_tests/test_log_custom.nu @@ -12,12 +12,12 @@ def run-command [ ] { if ($level_prefix | is-empty) { if ($ansi | is-empty) { - ^$nu.current-exe --commands $'use std; NU_log-level=($system_level) std log custom "($message)" "($format)" ($log_level)' + ^$nu.current-exe --no-config-file --commands $'use std; NU_log-level=($system_level) std log custom "($message)" "($format)" ($log_level)' } else { - ^$nu.current-exe --commands $'use std; NU_log-level=($system_level) std log custom "($message)" "($format)" ($log_level) --ansi "($ansi)"' + ^$nu.current-exe --no-config-file --commands $'use std; NU_log-level=($system_level) std log custom "($message)" "($format)" ($log_level) --ansi "($ansi)"' } } else { - ^$nu.current-exe --commands $'use std; NU_log-level=($system_level) std log custom "($message)" "($format)" ($log_level) --level-prefix "($level_prefix)" --ansi "($ansi)"' + ^$nu.current-exe --no-config-file --commands $'use std; NU_log-level=($system_level) std log custom "($message)" "($format)" ($log_level) --level-prefix "($level_prefix)" --ansi "($ansi)"' } | complete | get --ignore-errors stderr } diff --git a/crates/nu-std/tests/logger_tests/test_log_format_flag.nu b/crates/nu-std/tests/logger_tests/test_log_format_flag.nu index 93dec10acc..f2a6f6e27f 100644 --- a/crates/nu-std/tests/logger_tests/test_log_format_flag.nu +++ b/crates/nu-std/tests/logger_tests/test_log_format_flag.nu @@ -10,9 +10,9 @@ def run-command [ --short ] { if $short { - ^$nu.current-exe --commands $'use std; NU_log-level=($system_level) std log ($message_level) --format "($format)" --short "($message)"' + ^$nu.current-exe --no-config-file --commands $'use std; NU_log-level=($system_level) std log ($message_level) --format "($format)" --short "($message)"' } else { - ^$nu.current-exe --commands $'use std; NU_log-level=($system_level) std log ($message_level) --format "($format)" "($message)"' + ^$nu.current-exe --no-config-file --commands $'use std; NU_log-level=($system_level) std log ($message_level) --format "($format)" "($message)"' } | complete | get --ignore-errors stderr } diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index d0957355f1..386127f0b3 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -255,8 +255,8 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef, with_std: bool) -> O command .env(nu_utils::locale::LOCALE_OVERRIDE_ENV_VAR, locale) .env(NATIVE_PATH_ENV_VAR, paths_joined); - // TODO: consider adding custom plugin path for tests to - // not interfere with user local environment + // Ensure that the user's config doesn't interfere with the tests + command.arg("--no-config-file"); if !with_std { command.arg("--no-std-lib"); } @@ -265,6 +265,9 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef, with_std: bool) -> O .stdout(Stdio::piped()) .stderr(Stdio::piped()); + // Uncomment to debug the command being run: + // println!("=== command\n{command:?}\n"); + let process = match command.spawn() { Ok(child) => child, Err(why) => panic!("Can't run test {:?} {}", crate::fs::executable_path(), why), @@ -293,8 +296,12 @@ pub fn nu_with_plugin_run_test(cwd: impl AsRef, plugins: &[&str], command: }); let temp = tempdir().expect("couldn't create a temporary directory"); - let temp_plugin_file = temp.path().join("plugin.nu"); - std::fs::File::create(&temp_plugin_file).expect("couldn't create temporary plugin file"); + let [temp_config_file, temp_env_config_file, temp_plugin_file] = + ["config.nu", "env.nu", "plugin.nu"].map(|name| { + let temp_file = temp.path().join(name); + std::fs::File::create(&temp_file).expect("couldn't create temporary config file"); + temp_file + }); crate::commands::ensure_plugins_built(); @@ -320,6 +327,10 @@ pub fn nu_with_plugin_run_test(cwd: impl AsRef, plugins: &[&str], command: let process = match setup_command(&executable_path, &target_cwd) .arg("--commands") .arg(commands) + .arg("--config") + .arg(temp_config_file) + .arg("--env-config") + .arg(temp_env_config_file) .arg("--plugin-config") .arg(temp_plugin_file) .stdout(Stdio::piped()) diff --git a/src/run.rs b/src/run.rs index 2dc3afa383..eb6af3f0ab 100644 --- a/src/run.rs +++ b/src/run.rs @@ -146,57 +146,69 @@ pub(crate) fn run_file( ) -> Result<(), miette::ErrReport> { trace!("run_file"); let mut stack = nu_protocol::engine::Stack::new(); - let start_time = std::time::Instant::now(); - #[cfg(feature = "plugin")] - read_plugin_file( - engine_state, - &mut stack, - parsed_nu_cli_args.plugin_file, - NUSHELL_FOLDER, - ); - perf( - "read plugins", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - - let start_time = std::time::Instant::now(); - // only want to load config and env if relative argument is provided. - if parsed_nu_cli_args.env_file.is_some() { - config_files::read_config_file(engine_state, &mut stack, parsed_nu_cli_args.env_file, true); - } else { - config_files::read_default_env_file(engine_state, &mut stack) - } - perf( - "read env.nu", - start_time, - file!(), - line!(), - column!(), - use_color, - ); - - let start_time = std::time::Instant::now(); - if parsed_nu_cli_args.config_file.is_some() { - config_files::read_config_file( + // if the --no-config-file(-n) option is NOT passed, load the plugin file, + // load the default env file or custom (depending on parsed_nu_cli_args.env_file), + // and maybe a custom config file (depending on parsed_nu_cli_args.config_file) + // + // if the --no-config-file(-n) flag is passed, do not load plugin, env, or config files + if parsed_nu_cli_args.no_config_file.is_none() { + let start_time = std::time::Instant::now(); + #[cfg(feature = "plugin")] + read_plugin_file( engine_state, &mut stack, - parsed_nu_cli_args.config_file, - false, + parsed_nu_cli_args.plugin_file, + NUSHELL_FOLDER, + ); + perf( + "read plugins", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + + let start_time = std::time::Instant::now(); + // only want to load config and env if relative argument is provided. + if parsed_nu_cli_args.env_file.is_some() { + config_files::read_config_file( + engine_state, + &mut stack, + parsed_nu_cli_args.env_file, + true, + ); + } else { + config_files::read_default_env_file(engine_state, &mut stack) + } + perf( + "read env.nu", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + + let start_time = std::time::Instant::now(); + if parsed_nu_cli_args.config_file.is_some() { + config_files::read_config_file( + engine_state, + &mut stack, + parsed_nu_cli_args.config_file, + false, + ); + } + perf( + "read config.nu", + start_time, + file!(), + line!(), + column!(), + use_color, ); } - perf( - "read config.nu", - start_time, - file!(), - line!(), - column!(), - use_color, - ); // Regenerate the $nu constant to contain the startup time and any other potential updates let nu_const = create_nu_constant(engine_state, input.span().unwrap_or_else(Span::unknown))?; diff --git a/tests/shell/mod.rs b/tests/shell/mod.rs index 433e193423..7f15d3027c 100644 --- a/tests/shell/mod.rs +++ b/tests/shell/mod.rs @@ -243,7 +243,7 @@ fn run_in_login_mode() { #[test] fn run_in_not_login_mode() { let child_output = std::process::Command::new(nu_test_support::fs::executable_path()) - .args(["-c", "echo $nu.is-login"]) + .args(["-n", "-c", "echo $nu.is-login"]) .output() .expect("failed to run nu"); @@ -254,7 +254,7 @@ fn run_in_not_login_mode() { #[test] fn run_in_interactive_mode() { let child_output = std::process::Command::new(nu_test_support::fs::executable_path()) - .args(["-i", "-c", "echo $nu.is-interactive"]) + .args(["-n", "-i", "-c", "echo $nu.is-interactive"]) .output() .expect("failed to run nu"); @@ -265,7 +265,7 @@ fn run_in_interactive_mode() { #[test] fn run_in_noninteractive_mode() { let child_output = std::process::Command::new(nu_test_support::fs::executable_path()) - .args(["-c", "echo $nu.is-interactive"]) + .args(["-n", "-c", "echo $nu.is-interactive"]) .output() .expect("failed to run nu"); diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index ea644710ff..6cd11fd5e9 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -375,7 +375,7 @@ mod nu_commands { #[test] fn echo_internally_externally() { let actual = nu!(r#" - nu -c "echo 'foo'" + nu -n -c "echo 'foo'" "#); assert_eq!(actual.out, "foo"); @@ -385,7 +385,7 @@ mod nu_commands { fn failed_with_proper_exit_code() { Playground::setup("external failed", |dirs, _sandbox| { let actual = nu!(cwd: dirs.test(), r#" - nu -c "cargo build | complete | get exit_code" + nu -n -c "cargo build | complete | get exit_code" "#); // cargo for non rust project's exit code is 101. @@ -396,7 +396,7 @@ mod nu_commands { #[test] fn better_arg_quoting() { let actual = nu!(r#" - nu -c "\# '" + nu -n -c "\# '" "#); assert_eq!(actual.out, ""); @@ -405,7 +405,7 @@ mod nu_commands { #[test] fn command_list_arg_test() { let actual = nu!(" - nu ...['-c' 'version'] + nu ...['-n' '-c' 'version'] "); assert!(actual.out.contains("version")); @@ -416,7 +416,7 @@ mod nu_commands { #[test] fn command_cell_path_arg_test() { let actual = nu!(" - nu ...([ '-c' 'version' ]) + nu ...([ '-n' '-c' 'version' ]) "); assert!(actual.out.contains("version")); @@ -431,7 +431,7 @@ mod nu_script { #[test] fn run_nu_script() { let actual = nu!(cwd: "tests/fixtures/formats", " - nu script.nu + nu -n script.nu "); assert_eq!(actual.out, "done"); @@ -440,7 +440,7 @@ mod nu_script { #[test] fn run_nu_script_multiline() { let actual = nu!(cwd: "tests/fixtures/formats", " - nu script_multiline.nu + nu -n script_multiline.nu "); assert_eq!(actual.out, "23"); diff --git a/toolkit.nu b/toolkit.nu index bfd1d01a5b..a8007b4237 100644 --- a/toolkit.nu +++ b/toolkit.nu @@ -107,7 +107,7 @@ export def test [ export def "test stdlib" [ --extra-args: string = '' ] { - cargo run -- -c $" + cargo run -- --no-config-file -c $" use crates/nu-std/testing.nu testing run-tests --path crates/nu-std ($extra_args) "