h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-05-26 07:41:15 -03:00
Code Issues Packages Projects Releases Wiki Activity

Reimplement should_suppress_stderr_for_tests() for some tests

Browse Source 
"cargo test" captures stdout by default but not stderr.

So it's probably still useful to suppress test output like

	in function 'recursive1'
	in function 'recursive2'
	[repeats many times]

This was done by should_suppress_stderr_for_tests() which has been
broken. Fix that, but only for the relevant cases instead of setting
a global.
This commit is contained in:
Johannes Altmanninger
2025-10-25 16:46:47 +02:00
parent def9230ad6
commit 708703b9ec
7 changed files with 74 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/bin/fish.rs
View File

@@ -228,7 +228,7 @@ fn run_command_list(parser: &Parser, cmds: &[OsString]) -> Result<(), libc::c_in
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);
let _ = parser.eval_parsed_source(&ps, &IoChain::new(), None, BlockType::top, false);
retval = Ok(());
} else {
let backtrace = parser.get_backtrace(&cmd_wcs, &errors);

8
src/builtins/eval.rs
View File

@@ -52,7 +52,13 @@ pub fn eval(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Bui
}
}
let res = parser.eval_with(&new_cmd, &ios, streams.job_group.as_ref(), BlockType::top);
let res = parser.eval_with(
&new_cmd,
&ios,
streams.job_group.as_ref(),
BlockType::top,
false,
);
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.

12
src/builtins/test.rs
View File

@@ -1072,14 +1072,12 @@ pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Bui
let mut eval_errors = Vec::new();
let result = expr.evaluate(streams, &mut eval_errors);
if !eval_errors.is_empty() {
if !common::should_suppress_stderr_for_tests() {
for eval_error in eval_errors {
streams.err.appendln(&eval_error);
}
// Add a backtrace but not the "see help" message
// because this isn't about passing the wrong options.
streams.err.append(parser.current_line());
for eval_error in eval_errors {
streams.err.appendln(&eval_error);
}
// Add a backtrace but not the "see help" message
// because this isn't about passing the wrong options.
streams.err.append(parser.current_line());
return Err(STATUS_INVALID_ARGS);
}

13
src/common.rs
View File

@@ -1219,19 +1219,6 @@ pub fn wcs2bytes_appending(output: &mut Vec<u8>, input: &wstr) {
});
}
// Check if we are running in the test mode, where we should suppress error output
pub const TESTS_PROGRAM_NAME: &wstr = L!("(ignore)");
/// Hack to not print error messages in the tests. Do not call this from functions in this module
/// like `debug()`. It is only intended to suppress diagnostic noise from testing things like the
/// fish parser where we expect a lot of diagnostic messages due to testing error conditions.
pub fn should_suppress_stderr_for_tests() -> bool {
PROGRAM_NAME
.get()
.map(|p| p == TESTS_PROGRAM_NAME)
.unwrap_or_default()
}
/// Stored in blocks to reference the file which created the block.
pub type FilenameRef = Arc<WString>;

12
src/exec.rs
View File

@@ -997,7 +997,7 @@ fn get_performer_for_block_node(p: &Process, job: &Job, io_chain: &IoChain) -> B
let node = node.clone();
Box::new(move |parser: &Parser, _out, _err| {
parser
.eval_node(&node, &io_chain, job_group.as_ref(), BlockType::top)
.eval_node(&node, &io_chain, job_group.as_ref(), BlockType::top, false)
.status
})
}
@@ -1033,7 +1033,13 @@ 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);
let mut res = parser.eval_node(
&body_node,
&io_chain,
job_group.as_ref(),
BlockType::top,
false,
);
function_restore_environment(parser, fb);
// If the function did not execute anything, treat it as success.
@@ -1488,7 +1494,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);
let eval_res = parser.eval_with(cmd, &io_chain, job_group, BlockType::subst, false);
let buffer = IoBufferfill::finish(bufferfill);
if buffer.discarded() {
*break_expand = true;

10
src/parse_execution.rs
View File

@@ -10,8 +10,7 @@
builtin_exists,
};
use crate::common::{
ScopeGuard, ScopeGuarding, ScopedRefCell, escape, should_suppress_stderr_for_tests,
truncate_at_nul, valid_var_name,
ScopeGuard, ScopeGuarding, ScopedRefCell, escape, truncate_at_nul, valid_var_name,
};
use crate::complete::CompletionList;
use crate::env::{EnvMode, EnvStackSetResult, EnvVar, EnvVarFlags, Environment, Statuses};
@@ -90,6 +89,9 @@ pub struct ExecutionContext<'a> {
/// 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
@@ -119,12 +121,14 @@ pub fn new(
pstree: ParsedSourceRef,
block_io: IoChain,
line_counter: &'a ScopedRefCell<LineCounter<ast::JobPipeline>>,
test_only_suppress_stderr: bool,
) -> Self {
Self {
pstree,
cancel_signal: None,
line_counter,
block_io,
test_only_suppress_stderr,
}
}
@@ -242,7 +246,7 @@ fn report_errors(
let backtrace_and_desc = ctx.parser().get_backtrace(&self.pstree().src, error_list);
// Print it.
if !should_suppress_stderr_for_tests() {
if !self.test_only_suppress_stderr {
eprintf!("%s", backtrace_and_desc);
}

55
src/parser.rs
View File

@@ -493,7 +493,7 @@ pub fn is_command_substitution(&self) -> bool {
}
pub fn eval(&self, cmd: &wstr, io: &IoChain) -> EvalRes {
self.eval_with(cmd, io, None, BlockType::top)
self.eval_with(cmd, io, None, BlockType::top, false)
}
/// Evaluate the expressions contained in cmd.
@@ -510,6 +510,7 @@ 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();
@@ -518,14 +519,22 @@ pub fn eval_with(
ParseTreeFlags::empty(),
Some(&mut error_list),
) {
return self.eval_parsed_source(&ps, io, job_group, block_type);
return self.eval_parsed_source(
&ps,
io,
job_group,
block_type,
test_only_suppress_stderr,
);
}
// Get a backtrace. This includes the message.
let backtrace_and_desc = self.get_backtrace(cmd, &error_list);
// Print it.
eprintf!("%s\n", backtrace_and_desc);
if !test_only_suppress_stderr {
// Print it.
eprintf!("%s\n", backtrace_and_desc);
}
// Set a valid status.
self.set_last_statuses(Statuses::just(STATUS_ILLEGAL_CMD));
@@ -545,12 +554,19 @@ 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)
self.eval_node(
&job_list,
io,
job_group,
block_type,
test_only_suppress_stderr,
)
} else {
let status = ProcStatus::from_exit_code(self.get_last_status());
EvalRes {
@@ -585,7 +601,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))
Ok(self.eval_parsed_source(&ps, io, job_group, block_type, false))
}
pub fn eval_file_wstr(
@@ -616,6 +632,7 @@ 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!(
@@ -674,7 +691,12 @@ pub fn eval_node<T: Node>(
let restore_line_counter = self.line_counter.scoped_replace(ps.line_counter());
// Create a new execution context.
let mut execution_context = ExecutionContext::new(ps, block_io.clone(), &self.line_counter);
let mut execution_context = ExecutionContext::new(
ps,
block_io.clone(),
&self.line_counter,
test_only_suppress_stderr,
);
// Check the exec count so we know if anything got executed.
let prev_exec_count = self.libdata().exec_count;
@@ -1401,6 +1423,7 @@ mod tests {
};
use crate::parse_tree::{LineCounter, parse_source};
use crate::parse_util::{parse_util_detect_errors, parse_util_detect_errors_in_argument};
use crate::parser::BlockType;
use crate::reader::{fake_scoped_reader, reader_reset_interrupted};
use crate::signal::{signal_clear_cancel, signal_reset_handlers, signal_set_handlers};
use crate::tests::prelude::*;
@@ -2033,12 +2056,15 @@ fn test_eval_recursion_detection() {
&IoChain::new(),
);
parser.eval(
parser.eval_with(
L!(concat!(
"function recursive1 ; recursive2 ; end ; ",
"function recursive2 ; recursive1 ; end ; recursive1; ",
)),
&IoChain::new(),
None,
BlockType::top,
/*test_only_suppress_stderr=*/ true,
);
}
@@ -2049,7 +2075,13 @@ fn test_eval_illegal_exit_code() {
let parser = TestParser::new();
macro_rules! validate {
($cmd:expr, $result:expr) => {
parser.eval($cmd, &IoChain::new());
parser.eval_with(
$cmd,
&IoChain::new(),
None,
BlockType::top,
/*test_only_suppress_stderr=*/ true,
);
let exit_status = parser.get_last_status();
assert_eq!(exit_status, parser.get_last_status());
};
@@ -2073,9 +2105,12 @@ macro_rules! validate {
fn test_eval_empty_function_name() {
let _cleanup = test_init();
let parser = TestParser::new();
parser.eval(
parser.eval_with(
L!("function '' ; echo fail; exit 42 ; end ; ''"),
&IoChain::new(),
None,
BlockType::top,
/*test_only_suppress_stderr=*/ true,
);
}