Reimplement test_only_suppress_stderr

This parameter is used to suppress certain verbose errors that are
expected during tests. It was awkwardly threaded through multiple call
sites. Just set it (test only) on Parser.

src/bin/fish.rs

@@ -204,7 +204,7 @@ fn run_command_list(parser: &mut Parser, cmds: &[OsString]) -> Result<(), libc::
         if !errored {
             // Construct a parsed source ref.
             let ps = Arc::new(ParsedSource::new(cmd_wcs, ast));
-            let _ = parser.eval_parsed_source(&ps, &IoChain::new(), None, BlockType::Top, false);
+            let _ = parser.eval_parsed_source(&ps, &IoChain::new(), None, BlockType::Top);
             retval = Ok(());
         } else {
             let backtrace = parser.get_backtrace(&cmd_wcs, &errors);

src/builtins/eval.rs

@@ -52,13 +52,7 @@ pub fn eval(parser: &mut Parser, streams: &mut IoStreams, args: &mut [&wstr]) ->
         }
     }
 
-    let res = parser.eval_with(
-        &new_cmd,
-        &ios,
-        streams.job_group.as_ref(),
-        BlockType::Top,
-        false,
-    );
+    let res = parser.eval_with(&new_cmd, &ios, streams.job_group.as_ref(), BlockType::Top);
     let status = if res.was_empty {
         // Issue #5692, in particular, to catch `eval ""`, `eval "begin; end;"`, etc.
         // where we have an argument but nothing is executed.

src/builtins/shared/misc.rs

@@ -558,7 +558,6 @@ pub fn builtin_print_help(parser: &mut Parser, streams: &mut IoStreams, cmd: &ws
         streams.io_chain,
         streams.job_group.as_ref(),
         BlockType::Top,
-        false,
     );
     if res.status.normal_exited() && res.status.exit_code() == 2 {
         err_fmt!(Error::MISSING_HELP, name_esc)

src/exec.rs

@@ -1025,7 +1025,7 @@ fn get_performer_for_block_node(p: &Process, job: &Job, io_chain: &IoChain) -> B
     let node = node.clone();
     Box::new(move |parser: &mut Parser, _out, _err| {
         parser
-            .eval_node(&node, &io_chain, job_group.as_ref(), BlockType::Top, false)
+            .eval_node(&node, &io_chain, job_group.as_ref(), BlockType::Top)
             .status
     })
 }
@@ -1061,13 +1061,7 @@ fn get_performer_for_function(
         // Pull out the job list from the function.
         let fb = function_prepare_environment(parser, argv, &props);
         let body_node = props.func_node.child_ref(|n| &n.jobs);
-        let mut res = parser.eval_node(
-            &body_node,
-            &io_chain,
-            job_group.as_ref(),
-            BlockType::Top,
-            false,
-        );
+        let mut res = parser.eval_node(&body_node, &io_chain, job_group.as_ref(), BlockType::Top);
         function_restore_environment(parser, fb);
 
         // If the function did not execute anything, treat it as success.
@@ -1526,7 +1520,7 @@ fn exec_subshell_internal(
 
     let mut io_chain = IoChain::new();
     io_chain.push(bufferfill.clone());
-    let eval_res = parser.eval_with(cmd, &io_chain, job_group, BlockType::Subst, false);
+    let eval_res = parser.eval_with(cmd, &io_chain, job_group, BlockType::Subst);
     let buffer = IoBufferfill::finish(bufferfill);
     if buffer.discarded() {
         *break_expand = true;

src/parse_execution.rs

@@ -90,9 +90,6 @@ pub struct ExecutionContext {
     /// The block IO chain.
     /// For example, in `begin; foo ; end < file.txt` this would have the 'file.txt' IO.
     block_io: IoChain,
-
-    /// Hack to supress non-redirectable stderr in some unit tests.
-    test_only_suppress_stderr: bool,
 }
 
 // Report an error, setting $status to `status`. Always returns
@@ -127,16 +124,11 @@ pub fn varname_error<'a>(command: &'a wstr, bad_name: &'a wstr) -> Error<'a> {
 impl ExecutionContext {
     /// Construct a context in preparation for evaluating a node in a tree, with the given block_io.
     /// The execution context may access the parser and parent job group (if any) through ctx.
-    pub fn new(
-        pstree: ParsedSourceRef,
-        block_io: IoChain,
-        test_only_suppress_stderr: bool,
-    ) -> Self {
+    pub fn new(pstree: ParsedSourceRef, block_io: IoChain) -> Self {
         Self {
             pstree,
             cancel_signal: None,
             block_io,
-            test_only_suppress_stderr,
         }
     }
 
@@ -254,7 +246,7 @@ fn report_errors(
             let backtrace_and_desc = ctx.parser().get_backtrace(&self.pstree().src, error_list);
 
             // Print it.
-            if !self.test_only_suppress_stderr {
+            if !ctx.parser().test_only_suppress_stderr() {
                 eprintf!("%s", backtrace_and_desc);
             }
 

src/parser.rs

@@ -419,6 +419,10 @@ pub struct Parser {
 
     // Timeout for blocking terminal queries.
     pub blocking_query_timeout: Option<Duration>,
+
+    // If set, do not print certain error messages to stderr, to keep the tests clean.
+    #[cfg(test)]
+    pub test_only_suppress_stderr: bool,
 }
 
 #[derive(Copy, Clone, Default)]
@@ -454,6 +458,8 @@ pub fn new(variables: EnvStack, cancel_behavior: CancelBehavior) -> Parser {
             profile_items: Default::default(),
             global_event_blocks: 0,
             blocking_query_timeout: None,
+            #[cfg(test)]
+            test_only_suppress_stderr: false,
         };
 
         result
@@ -482,7 +488,7 @@ pub fn is_command_substitution(&self) -> bool {
     }
 
     pub fn eval(&mut self, cmd: &wstr, io: &IoChain) -> EvalRes {
-        self.eval_with(cmd, io, None, BlockType::Top, false)
+        self.eval_with(cmd, io, None, BlockType::Top)
     }
 
     /// Evaluate the expressions contained in cmd.
@@ -499,7 +505,6 @@ pub fn eval_with(
         io: &IoChain,
         job_group: Option<&JobGroupRef>,
         block_type: BlockType,
-        test_only_suppress_stderr: bool,
     ) -> EvalRes {
         // Parse the source into a tree, if we can.
         let mut error_list = ParseErrorList::new();
@@ -508,19 +513,13 @@ pub fn eval_with(
             ParseTreeFlags::default(),
             Some(&mut error_list),
         ) {
-            return self.eval_parsed_source(
-                &ps,
-                io,
-                job_group,
-                block_type,
-                test_only_suppress_stderr,
-            );
+            return self.eval_parsed_source(&ps, io, job_group, block_type);
         }
 
         // Get a backtrace. This includes the message.
         let backtrace_and_desc = self.get_backtrace(cmd, &error_list);
 
-        if !test_only_suppress_stderr {
+        if !self.test_only_suppress_stderr() {
             // Print it.
             eprintf!("%s\n", backtrace_and_desc);
         }
@@ -543,19 +542,12 @@ pub fn eval_parsed_source(
         io: &IoChain,
         job_group: Option<&JobGroupRef>,
         block_type: BlockType,
-        test_only_suppress_stderr: bool,
     ) -> EvalRes {
         assert_matches!(block_type, BlockType::Top | BlockType::Subst);
         let job_list = ps.top_job_list();
         if !job_list.is_empty() {
             // Execute the top job list.
-            self.eval_node(
-                &job_list,
-                io,
-                job_group,
-                block_type,
-                test_only_suppress_stderr,
-            )
+            self.eval_node(&job_list, io, job_group, block_type)
         } else {
             let status = ProcStatus::from_exit_code(self.last_status());
             EvalRes {
@@ -590,7 +582,7 @@ pub fn eval_wstr(
         // Construct a parsed source ref.
         // Be careful to transfer ownership, this could be a very large string.
         let ps = Arc::new(ParsedSource::new(src, ast));
-        Ok(self.eval_parsed_source(&ps, io, job_group, block_type, false))
+        Ok(self.eval_parsed_source(&ps, io, job_group, block_type))
     }
 
     pub fn eval_file_wstr(
@@ -619,7 +611,6 @@ pub fn eval_node<T: Node>(
         block_io: &IoChain,
         job_group: Option<&JobGroupRef>,
         block_type: BlockType,
-        test_only_suppress_stderr: bool,
     ) -> EvalRes {
         // Only certain blocks are allowed.
         assert_matches!(
@@ -676,11 +667,8 @@ pub fn eval_node<T: Node>(
         op_ctx.cancel_checker = cancel_checker;
 
         // Create a new execution context.
-        let mut execution_context = ExecutionContext::new(
-            node.parsed_source_ref(),
-            block_io.clone(),
-            test_only_suppress_stderr,
-        );
+        let mut execution_context =
+            ExecutionContext::new(node.parsed_source_ref(), block_io.clone());
 
         // Check the exec count so we know if anything got executed.
         let exec_counts = |ctx: &mut OperationContext<'_>| {
@@ -1244,6 +1232,17 @@ pub fn is_eval_depth_exceeded(&self) -> bool {
         self.scope().eval_level >= FISH_MAX_EVAL_DEPTH
     }
 
+    /// Return whether we should suppress certain error printing during tests.
+    #[cfg(test)]
+    pub fn test_only_suppress_stderr(&self) -> bool {
+        self.test_only_suppress_stderr
+    }
+
+    #[cfg(not(test))]
+    pub fn test_only_suppress_stderr(&self) -> bool {
+        false
+    }
+
     pub fn set_color_theme(&mut self, background_color: Option<&xterm_color::Color>) {
         let color_theme = match background_color.map(|c| c.perceived_lightness()) {
             Some(x) if x < 0.5 => L!("dark"),