From 166b17701a463a00d57c1d42264fc46a92ba2021 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Fri, 26 Sep 2025 11:33:03 +0200 Subject: [PATCH] Remove spurious terminal interaction from unit tests Some of our integration tests require a reader for code execution and testing cancellation etc., but they never actually read from the terminal. So they don't need to call reader_interactive_init(), and doing so is a bit weird. Let's stop that; this allows us to assert that reader_push() is always called with an input file descriptor that refers to a TTY. --- src/reader.rs | 26 ++++++++++++++++++++++---- src/tests/debounce.rs | 7 ++----- src/tests/env_universal_common.rs | 6 ++---- src/tests/parser.rs | 6 ++---- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/reader.rs b/src/reader.rs index 2ab2b1e5e..91313f1ed 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -52,6 +52,7 @@ use crate::builtins::shared::ErrorCode; use crate::builtins::shared::STATUS_CMD_ERROR; use crate::builtins::shared::STATUS_CMD_OK; +use crate::common::ScopeGuarding; use crate::common::{ escape, escape_string, exit_without_destructors, get_ellipsis_char, get_obfuscation_read_char, restore_term_foreground_process_group_for_exit, shell_modes, str2wcstring, write_loop, @@ -261,15 +262,15 @@ fn redirect_tty_after_sighup() { } } -fn querying_allowed(in_fd: RawFd) -> bool { +fn querying_allowed() -> bool { future_feature_flags::test(FeatureFlag::query_term) && !is_dumb() && std::env::var_os("MC_TMPDIR").is_none() // Could use /dev/tty in future. - && isatty(in_fd) && isatty(STDOUT_FILENO) } pub fn terminal_init(vars: &dyn Environment, inputfd: RawFd) -> InputEventQueue { + assert!(isatty(inputfd)); reader_interactive_init(); const INITIAL_QUERY_TIMEOUT_SECONDS: u64 = 2; @@ -282,7 +283,7 @@ pub fn terminal_init(vars: &dyn Environment, inputfd: RawFd) -> InputEventQueue initialize_tty_metadata(); }); - if !querying_allowed(inputfd) { + if !querying_allowed() { return input_queue; } @@ -428,6 +429,23 @@ pub fn reader_pop() { } } +pub fn fake_scoped_reader<'a>(parser: &'a Parser) -> impl ScopeGuarding> + 'a { + let inputfd = -1; + let conf = ReaderConfig { + inputfd, + ..Default::default() + }; + let hist = History::with_name(L!("")); + let input_data = InputData::new(inputfd, None); + let data = ReaderData::new(input_data, hist, conf, reader_data_stack().is_empty()); + reader_data_stack().push(data); + let data = current_data().unwrap(); + let reader = Reader { data, parser }; + ScopeGuard::new(reader, |_reader| { + reader_data_stack().pop().unwrap(); + }) +} + /// Configuration that we provide to a reader. #[derive(Default)] pub struct ReaderConfig { @@ -1633,7 +1651,7 @@ pub fn combine_command_and_autosuggestion( impl<'a> Reader<'a> { pub fn request_cursor_position(&mut self, out: &mut Outputter, q: CursorPositionQuery) { - if !querying_allowed(self.get_in_fd()) { + if !querying_allowed() { return; } let mut query = self.blocking_query(); diff --git a/src/tests/debounce.rs b/src/tests/debounce.rs index 8a07e7c6b..f36d478c6 100644 --- a/src/tests/debounce.rs +++ b/src/tests/debounce.rs @@ -4,12 +4,10 @@ }; use std::time::Duration; -use crate::common::ScopeGuard; use crate::global_safety::RelaxedAtomicBool; -use crate::reader::{reader_pop, reader_push, Reader, ReaderConfig}; +use crate::reader::{fake_scoped_reader, Reader}; use crate::tests::prelude::*; use crate::threads::{iothread_drain_all, iothread_service_main, Debounce}; -use crate::wchar::prelude::*; #[test] #[serial] @@ -61,8 +59,7 @@ struct Context { ctx.cv.notify_all(); // Wait until the last completion is done. - let mut reader = reader_push(&parser, L!(""), ReaderConfig::default()); - let _pop = ScopeGuard::new((), |()| reader_pop()); + let mut reader = fake_scoped_reader(&parser); while !ctx.completion_ran.last().unwrap().load() { iothread_service_main(&mut reader); } diff --git a/src/tests/env_universal_common.rs b/src/tests/env_universal_common.rs index e01acd275..087f169be 100644 --- a/src/tests/env_universal_common.rs +++ b/src/tests/env_universal_common.rs @@ -1,10 +1,9 @@ use crate::common::char_offset; use crate::common::wcs2osstring; -use crate::common::ScopeGuard; use crate::common::ENCODE_DIRECT_BASE; use crate::env::{EnvVar, EnvVarFlags, VarTable}; use crate::env_universal_common::{EnvUniversal, UvarFormat}; -use crate::reader::{reader_pop, reader_push, ReaderConfig}; +use crate::reader::fake_scoped_reader; use crate::tests::prelude::*; use crate::threads::{iothread_drain_all, iothread_perform}; use crate::wchar::prelude::*; @@ -43,8 +42,7 @@ fn test_universal() { std::fs::create_dir_all("test/fish_uvars_test/").unwrap(); let parser = TestParser::new(); - let mut reader = reader_push(&parser, L!(""), ReaderConfig::default()); - let _pop = ScopeGuard::new((), |()| reader_pop()); + let mut reader = fake_scoped_reader(&parser); let threads = 1; for i in 0..threads { diff --git a/src/tests/parser.rs b/src/tests/parser.rs index 3dc508b11..cc08d11bd 100644 --- a/src/tests/parser.rs +++ b/src/tests/parser.rs @@ -1,5 +1,4 @@ use crate::ast::{self, is_same_node, Ast, Castable, JobList, JobPipeline, Kind, Node, Traversal}; -use crate::common::ScopeGuard; use crate::env::EnvStack; use crate::expand::ExpandFlags; use crate::io::{IoBufferfill, IoChain}; @@ -9,7 +8,7 @@ use crate::parse_tree::{parse_source, LineCounter}; use crate::parse_util::{parse_util_detect_errors, parse_util_detect_errors_in_argument}; use crate::parser::{CancelBehavior, Parser}; -use crate::reader::{reader_pop, reader_push, reader_reset_interrupted, ReaderConfig}; +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::*; use crate::threads::iothread_perform; @@ -730,8 +729,7 @@ fn test_1_cancellation(parser: &Parser, src: &wstr) { fn test_cancellation() { let _cleanup = test_init(); let parser = Parser::new(EnvStack::new(), CancelBehavior::Clear); - reader_push(&parser, L!(""), ReaderConfig::default()); - let _pop = ScopeGuard::new((), |()| reader_pop()); + let _pop = fake_scoped_reader(&parser); printf!("Testing Ctrl-C cancellation. If this hangs, that's a bug!\n");