From 077f439283af984088b48770b4e3023cb1bccc04 Mon Sep 17 00:00:00 2001 From: Peter Ammon Date: Wed, 19 Jun 2024 14:05:27 -0700 Subject: [PATCH] Remove uses of EnvStack::principal() in the tests --- src/builtins/tests/string_tests.rs | 5 ++- src/tests/abbrs.rs | 36 +++++++++---------- src/tests/complete.rs | 30 +++++++--------- src/tests/env.rs | 7 ++-- src/tests/expand.rs | 5 +-- src/tests/highlight.rs | 10 +++--- src/tests/mod.rs | 58 ++++++++++++++++++++---------- src/tests/parser.rs | 6 ++-- 8 files changed, 87 insertions(+), 70 deletions(-) diff --git a/src/builtins/tests/string_tests.rs b/src/builtins/tests/string_tests.rs index 47a7423eb..d44357d64 100644 --- a/src/builtins/tests/string_tests.rs +++ b/src/builtins/tests/string_tests.rs @@ -11,7 +11,6 @@ fn test_string() { use crate::common::escape; use crate::future_feature_flags::{scoped_test, FeatureFlag}; use crate::io::{IoStreams, OutputStream, StringOutputStream}; - use crate::parser::Parser; use crate::tests::prelude::*; use crate::wchar::prelude::*; @@ -25,14 +24,14 @@ fn test_string() { // TODO: these should be individual tests, not all in one, port when we can run these with `cargo test` fn string_test(mut args: Vec<&wstr>, expected_rc: Option, expected_out: &wstr) { - let parser: &Parser = Parser::principal_parser(); + let parser = TestParser::new(); let mut outs = OutputStream::String(StringOutputStream::new()); let mut errs = OutputStream::Null; let io_chain = IoChain::new(); let mut streams = IoStreams::new(&mut outs, &mut errs, &io_chain); streams.stdin_is_directly_redirected = false; // read from argv instead of stdin - let rc = string(parser, &mut streams, args.as_mut_slice()).expect("string failed"); + let rc = string(&parser, &mut streams, args.as_mut_slice()).expect("string failed"); let actual = escape(outs.contents()); let expected = escape(expected_out); diff --git a/src/tests/abbrs.rs b/src/tests/abbrs.rs index caf8c7dbb..99936745b 100644 --- a/src/tests/abbrs.rs +++ b/src/tests/abbrs.rs @@ -1,7 +1,6 @@ use crate::abbrs::{self, abbrs_get_set, abbrs_match, Abbreviation}; use crate::editable_line::{apply_edit, Edit}; use crate::highlight::HighlightSpec; -use crate::parser::Parser; use crate::reader::reader_expand_abbreviation_at_cursor; use crate::tests::prelude::*; use crate::wchar::prelude::*; @@ -10,6 +9,7 @@ use crate::wchar::prelude::*; #[serial] fn test_abbreviations() { let _cleanup = test_init(); + let parser = TestParser::new(); { let mut abbrs = abbrs_get_set(); abbrs.add(Abbreviation::new( @@ -67,24 +67,22 @@ fn test_abbreviations() { abbr_expand_1!("gc", cmd, "git checkout"); abbr_expand_1!("foo", cmd, "bar"); - fn expand_abbreviation_in_command( - cmdline: &wstr, - cursor_pos: Option, - ) -> Option { - let replacement = reader_expand_abbreviation_at_cursor( - cmdline, - cursor_pos.unwrap_or(cmdline.len()), - Parser::principal_parser(), - )?; - let mut cmdline_expanded = cmdline.to_owned(); - let mut colors = vec![HighlightSpec::new(); cmdline.len()]; - apply_edit( - &mut cmdline_expanded, - &mut colors, - &Edit::new(replacement.range.into(), replacement.text), - ); - Some(cmdline_expanded) - } + let expand_abbreviation_in_command = + |cmdline: &wstr, cursor_pos: Option| -> Option { + let replacement = reader_expand_abbreviation_at_cursor( + cmdline, + cursor_pos.unwrap_or(cmdline.len()), + &parser, + )?; + let mut cmdline_expanded = cmdline.to_owned(); + let mut colors = vec![HighlightSpec::new(); cmdline.len()]; + apply_edit( + &mut cmdline_expanded, + &mut colors, + &Edit::new(replacement.range.into(), replacement.text), + ); + Some(cmdline_expanded) + }; macro_rules! validate { ($cmdline:expr, $cursor:expr) => {{ diff --git a/src/tests/complete.rs b/src/tests/complete.rs index 360a1eaae..1835390bc 100644 --- a/src/tests/complete.rs +++ b/src/tests/complete.rs @@ -9,7 +9,6 @@ use crate::io::IoChain; use crate::operation_context::{ no_cancel, OperationContext, EXPANSION_LIMIT_BACKGROUND, EXPANSION_LIMIT_DEFAULT, }; -use crate::parser::Parser; use crate::reader::completion_apply_to_command_line; use crate::tests::prelude::*; use crate::wchar::prelude::*; @@ -43,8 +42,8 @@ fn test_complete() { }, }; - let parser = Parser::principal_parser(); - let ctx = OperationContext::test_only_foreground(parser, &vars, Box::new(no_cancel)); + let parser = TestParser::new(); + let ctx = OperationContext::test_only_foreground(&parser, &vars, Box::new(no_cancel)); let do_complete = |cmd: &wstr, flags: CompletionRequestOptions| complete(cmd, flags, &ctx).0; @@ -288,7 +287,7 @@ fn test_complete() { assert_eq!(completions.len(), 1); assert_eq!(completions[0].completion, L!("stfile")); - pushd("test/complete_test"); + parser.pushd("test/complete_test"); let completions = do_complete(L!("cat te"), CompletionRequestOptions::default()); assert_eq!(completions.len(), 1); assert_eq!(completions[0].completion, L!("stfile")); @@ -342,7 +341,7 @@ fn test_complete() { let mut completions = do_complete(L!("cat te\\0"), CompletionRequestOptions::default()); assert_eq!(&completions, &[]); - popd(); + parser.popd(); completions.clear(); // Test abbreviations. @@ -417,7 +416,7 @@ fn test_complete() { ); // Test cd wrapping chain - pushd("test/complete_test"); + parser.pushd("test/complete_test"); complete_add_wrapper(L!("cdwrap1").into(), L!("cd").into()); complete_add_wrapper(L!("cdwrap2").into(), L!("cdwrap1").into()); @@ -445,7 +444,7 @@ fn test_complete() { complete_remove_wrapper(L!("cdwrap1").into(), L!("cd")); complete_remove_wrapper(L!("cdwrap2").into(), L!("cdwrap1")); - popd(); + parser.popd(); } // Testing test_autosuggest_suggest_special, in particular for properly handling quotes and @@ -454,6 +453,7 @@ fn test_complete() { #[serial] fn test_autosuggest_suggest_special() { let _cleanup = test_init(); + let parser = TestParser::new(); macro_rules! perform_one_autosuggestion_cd_test { ($command:literal, $expected:literal, $vars:expr) => { let command = L!($command); @@ -489,7 +489,7 @@ fn test_autosuggest_suggest_special() { L!($command), CompletionRequestOptions::default(), &OperationContext::foreground( - Parser::principal_parser(), + &parser, Box::new(no_cancel), EXPANSION_LIMIT_DEFAULT, ), @@ -520,7 +520,7 @@ fn test_autosuggest_suggest_special() { // This is to ensure tilde expansion is handled. See the `cd ~/test_autosuggest_suggest_specia` // test below. // Fake out the home directory - Parser::principal_parser().vars().set_one( + parser.vars().set_one( L!("HOME"), EnvMode::LOCAL | EnvMode::EXPORT, L!("test/test-home").to_owned(), @@ -540,11 +540,7 @@ fn test_autosuggest_suggest_special() { let mut vars = PwdEnvironment::default(); vars.parent.vars.insert( L!("HOME").into(), - Parser::principal_parser() - .vars() - .get(L!("HOME")) - .unwrap() - .as_string(), + parser.vars().get(L!("HOME")).unwrap().as_string(), ); perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/0", "foobar/", &vars); @@ -583,7 +579,7 @@ fn test_autosuggest_suggest_special() { perform_one_autosuggestion_cd_test!("cd test/autosuggest_test/has_loop/", "loopy/loop/", &vars); - pushd(wd); + parser.pushd(wd); perform_one_autosuggestion_cd_test!("cd 0", "foobar/", &vars); perform_one_autosuggestion_cd_test!("cd \"0", "foobar/", &vars); perform_one_autosuggestion_cd_test!("cd '0", "foobar/", &vars); @@ -616,10 +612,10 @@ fn test_autosuggest_suggest_special() { perform_one_completion_cd_test!("cd ~absolutelynosuchus", "er/"); perform_one_completion_cd_test!("cd ~absolutelynosuchuser/", "path1/"); - Parser::principal_parser() + parser .vars() .remove(L!("HOME"), EnvMode::LOCAL | EnvMode::EXPORT); - popd(); + parser.popd(); } #[test] diff --git a/src/tests/env.rs b/src/tests/env.rs index a90de33d2..6ba875603 100644 --- a/src/tests/env.rs +++ b/src/tests/env.rs @@ -112,8 +112,9 @@ fn test_env_vars() { fn test_env_snapshot() { let _cleanup = test_init(); std::fs::create_dir_all("test/fish_env_snapshot_test/").unwrap(); - pushd("test/fish_env_snapshot_test/"); - let vars = Parser::principal_parser().vars(); + let parser = TestParser::new(); + let vars = parser.vars(); + parser.pushd("test/fish_env_snapshot_test/"); vars.push(true); let before_pwd = vars.get(L!("PWD")).unwrap().as_string(); vars.set_one( @@ -175,7 +176,7 @@ fn test_env_snapshot() { ); vars.pop(); - popd(); + parser.popd(); } // Can't push/pop from globals. diff --git a/src/tests/expand.rs b/src/tests/expand.rs index ee351579a..a456e0300 100644 --- a/src/tests/expand.rs +++ b/src/tests/expand.rs @@ -66,6 +66,7 @@ fn expand_test_impl( #[serial] fn test_expand() { let _cleanup = test_init(); + let parser = TestParser::new(); /// Perform parameter expansion and test if the output equals the zero-terminated parameter list /// supplied. /// /// \param in the string to expand @@ -331,7 +332,7 @@ fn test_expand() { "" ); - pushd("test/fish_expand_test"); + parser.pushd("test/fish_expand_test"); expand_test!( "b/xx", @@ -343,7 +344,7 @@ fn test_expand() { // multiple slashes with fuzzy matching - #3185 expand_test!("l///n", fuzzy_comp, "lol///nub/", "Wrong fuzzy matching 6"); - popd(); + parser.popd(); } #[test] diff --git a/src/tests/highlight.rs b/src/tests/highlight.rs index 710e04ee3..d6bb6294f 100644 --- a/src/tests/highlight.rs +++ b/src/tests/highlight.rs @@ -1,7 +1,6 @@ use crate::common::ScopeGuard; use crate::env::EnvMode; use crate::future_feature_flags::{self, FeatureFlag}; -use crate::parser::Parser; use crate::tests::prelude::*; use crate::wchar::prelude::*; use crate::{ @@ -161,9 +160,10 @@ fn test_is_potential_path() { #[serial] fn test_highlighting() { let _cleanup = test_init(); + let parser = TestParser::new(); // Testing syntax highlighting - pushd("test/fish_highlight_test/"); - let _popd = ScopeGuard::new((), |_| popd()); + parser.pushd("test/fish_highlight_test/"); + let _popd = ScopeGuard::new((), |_| parser.popd()); std::fs::create_dir_all("dir").unwrap(); std::fs::create_dir_all("cdpath-entry/dir-in-cdpath").unwrap(); std::fs::write("foo", []).unwrap(); @@ -201,7 +201,7 @@ fn test_highlighting() { component!($comp), )* ]; - let vars = Parser::principal_parser().vars(); + let vars = parser.vars(); // Generate the text. let mut text = WString::new(); let mut expected_colors = vec![]; @@ -248,7 +248,7 @@ fn test_highlighting() { let fg = HighlightSpec::with_fg; // Verify variables and wildcards in commands using /bin/cat. - let vars = Parser::principal_parser().vars(); + let vars = parser.vars(); vars.set_one( L!("CDPATH"), EnvMode::LOCAL, diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 72932abbf..2d43507a0 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -30,36 +30,58 @@ mod wgetopt; pub mod prelude { use crate::common::ScopeGuarding; use crate::env::{env_init, misc_init}; + use crate::parser::{Parser, ParserRef}; use crate::reader::reader_init; use crate::signal::signal_reset_handlers; pub use crate::tests::env::{PwdEnvironment, TestEnvironment}; use crate::topic_monitor::topic_monitor_init; use crate::wutil::wgetcwd; use crate::{env::EnvStack, proc::proc_init}; - use once_cell::sync::Lazy; use once_cell::sync::OnceCell; + use std::cell::RefCell; use std::env::set_current_dir; use std::ffi::CString; - use std::sync::Mutex; + use std::rc::Rc; - static PUSHED_DIRS: Lazy>> = Lazy::new(Mutex::default); - /// Helper to chdir and then update $PWD. - pub fn pushd(path: &str) { - let cwd = wgetcwd(); - PUSHED_DIRS.lock().unwrap().push(cwd.to_string()); - - // We might need to create the directory. We don't care if this fails due to the directory - // already being present. - std::fs::create_dir_all(path).unwrap(); - - std::env::set_current_dir(path).unwrap(); - EnvStack::principal().set_pwd_from_getcwd(); + /// A wrapper around a Parser with some test helpers. + pub struct TestParser { + parser: ParserRef, + pushed_dirs: RefCell>, } - pub fn popd() { - let old_cwd = PUSHED_DIRS.lock().unwrap().pop().unwrap(); - std::env::set_current_dir(old_cwd).unwrap(); - EnvStack::principal().set_pwd_from_getcwd(); + impl TestParser { + pub fn new() -> TestParser { + TestParser { + parser: Parser::new(Rc::new(EnvStack::new()), false), + pushed_dirs: RefCell::new(Vec::new()), + } + } + + /// Helper to chdir and then update $PWD. + pub fn pushd(&self, path: &str) { + let cwd = wgetcwd(); + self.pushed_dirs.borrow_mut().push(cwd.to_string()); + + // We might need to create the directory. We don't care if this fails due to the directory + // already being present. + std::fs::create_dir_all(path).unwrap(); + + std::env::set_current_dir(path).unwrap(); + self.parser.vars().set_pwd_from_getcwd(); + } + + pub fn popd(&self) { + let old_cwd = self.pushed_dirs.borrow_mut().pop().unwrap(); + std::env::set_current_dir(old_cwd).unwrap(); + self.parser.vars().set_pwd_from_getcwd(); + } + } + + impl std::ops::Deref for TestParser { + type Target = Parser; + fn deref(&self) -> &Self::Target { + &self.parser + } } pub fn test_init() -> impl ScopeGuarding { diff --git a/src/tests/parser.rs b/src/tests/parser.rs index fad36a5ed..a838d8011 100644 --- a/src/tests/parser.rs +++ b/src/tests/parser.rs @@ -631,9 +631,9 @@ fn test_eval_recursion_detection() { #[serial] fn test_eval_illegal_exit_code() { let _cleanup = test_init(); + let parser = TestParser::new(); macro_rules! validate { ($cmd:expr, $result:expr) => { - let parser = Parser::principal_parser(); parser.eval($cmd, &IoChain::new()); let exit_status = parser.get_last_status(); assert_eq!(exit_status, parser.get_last_status()); @@ -643,7 +643,7 @@ fn test_eval_illegal_exit_code() { // We need to be in an empty directory so that none of the wildcards match a file that might be // in the fish source tree. In particular we need to ensure that "?" doesn't match a file // named by a single character. See issue #3852. - pushd("test/temp"); + parser.pushd("test/temp"); validate!(L!("echo -n"), STATUS_CMD_OK.unwrap()); validate!(L!("pwd"), STATUS_CMD_OK.unwrap()); validate!( @@ -656,7 +656,7 @@ fn test_eval_illegal_exit_code() { ); validate!(L!("?"), STATUS_UNMATCHED_WILDCARD.unwrap()); validate!(L!("abc?def"), STATUS_UNMATCHED_WILDCARD.unwrap()); - popd(); + parser.popd(); } #[test]