From 5278048abb7ac130e93568df19ce751d04471124 Mon Sep 17 00:00:00 2001 From: Tee8z Date: Thu, 22 Oct 2026 18:13:08 -0400 Subject: [PATCH] test(cli): kill the CLI when a process test fails, so the run cannot hang When a test that drives the CLI failed before it reaped the process, for example at a prompt that never appeared, the process kept running. So did the blocking task that read its terminal or waited for it, and the test runtime waits for such tasks at shutdown, so the whole run hung. Each of these tests now holds a KillOnDrop guard from spawn, and waits through exit_within, which kills the process on a timeout and disarms the guard once the process is reaped. A terminal step may now take 30 seconds instead of 10, because a device login waits out a 5-second poll interval before its first prompt. --- tests/cli_integration.rs | 210 ++++++++++++++++++++++++++------------- 1 file changed, 139 insertions(+), 71 deletions(-) diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 53c22e4..e5057ae 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -476,14 +476,15 @@ async fn result_output_treats_an_early_closing_pipe_as_success_without_resubmitt .stderr(Stdio::piped()) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); drop(child.stdout.take()); - let output = tokio::time::timeout( + let output = exit_within( + child, + running, Duration::from_secs(10), - tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()), + "the command did not exit after its stdout closed", ) - .await - .unwrap() - .unwrap(); + .await; assert!( output.status.success(), "{}", @@ -1987,6 +1988,51 @@ fn open_pty() -> (std::fs::File, std::fs::File) { } } +/// Kills a CLI process when dropped, until `exit_within` reaps it. A test that fails while +/// the process still runs, for example at a prompt that never appears, would otherwise leave +/// it running, and with it each blocking task that waits for it or reads its terminal. The +/// test runtime waits for those tasks when it shuts down, so one failure would hang the run. +#[cfg(unix)] +struct KillOnDrop(Option); + +#[cfg(unix)] +impl KillOnDrop { + fn new(child: &std::process::Child) -> Self { + Self(Some(child.id())) + } +} + +#[cfg(unix)] +impl Drop for KillOnDrop { + fn drop(&mut self) { + // Until the process is reaped, its ID stays its own, so a kill after it exited does + // nothing. + if let Some(pid) = self.0 { + let _ = std::process::Command::new("kill") + .args(["-KILL", &pid.to_string()]) + .status(); + } + } +} + +/// Wait up to `limit` for `child` to exit. If it does not, fail with `what`, and `running` +/// kills it as the test unwinds. A reaped child's ID can be reused, so `running` is disarmed +/// once the child is reaped. +#[cfg(unix)] +async fn exit_within( + child: std::process::Child, + mut running: KillOnDrop, + limit: Duration, + what: &str, +) -> std::process::Output { + let waited = tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()); + let Ok(output) = tokio::time::timeout(limit, waited).await else { + panic!("{what}"); + }; + running.0 = None; + output.unwrap() +} + /// Deliver SIGINT, as Ctrl-C does on a terminal. #[cfg(unix)] fn interrupt(child: &std::process::Child) { @@ -2026,18 +2072,19 @@ async fn interrupt_in_flight( .stderr(Stdio::piped()) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); // Generous: a device login waits out its first poll interval before any request. tokio::time::timeout(Duration::from_secs(15), arrived.notified()) .await .unwrap(); interrupt(&child); - let output = tokio::time::timeout( + let output = exit_within( + child, + running, Duration::from_secs(5), - tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()), + "Ctrl-C during a request did not end the process", ) - .await - .unwrap() - .unwrap(); + .await; assert_eq!( output.status.code(), Some(130), @@ -2079,15 +2126,20 @@ async fn progress_is_terminal_only_and_cleared_on_completion() { .stdout(Stdio::piped()) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); let reader = tokio::task::spawn_blocking(move || { let mut shown = Vec::new(); // Linux PTYs return EIO rather than EOF when the terminal end closes. let _ = controller.read_to_end(&mut shown); shown }); - let output = tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()) - .await - .unwrap(); + let output = exit_within( + child, + running, + Duration::from_secs(10), + "the listing did not exit", + ) + .await; assert!(output.status.success()); let shown = reader.await.unwrap(); let shown = String::from_utf8_lossy(&shown); @@ -2229,6 +2281,7 @@ async fn interrupt_the_hidden_key_prompt(ctrl_c: CtrlC) -> (Option, bool, S }); } let child = command.spawn().unwrap(); + let running = KillOnDrop::new(&child); // A session leader's exit waits for its terminal output to drain, so the controller is // read until the child closes the terminal, not only until the prompt appears. let (prompted, prompt) = tokio::sync::oneshot::channel(); @@ -2261,20 +2314,15 @@ async fn interrupt_the_hidden_key_prompt(ctrl_c: CtrlC) -> (Option, bool, S CtrlC::Signal => interrupt(&child), CtrlC::Typed => (&observer).write_all(b"\x03").unwrap(), } - let pid = child.id().to_string(); - let exited = tokio::time::timeout( + let code = exit_within( + child, + running, Duration::from_secs(5), - tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()), + "Ctrl-C at the hidden key prompt did not end the process", ) - .await; - let Ok(output) = exited else { - std::process::Command::new("kill") - .args(["-KILL", &pid]) - .status() - .unwrap(); - panic!("Ctrl-C at the hidden key prompt did not end the process"); - }; - let code = output.unwrap().status.code(); + .await + .status + .code(); let echo = local_modes(&observer) & libc::ECHO != 0; // The controller reports end of input once the last terminal descriptor closes. drop(command); @@ -2520,6 +2568,7 @@ async fn interrupt_while_a_request_body_is_still_being_read() { .stderr(Stdio::piped()) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); // Held open and never written, so the CLI's read blocks until the process ends. let _writer = tokio::time::timeout( Duration::from_secs(15), @@ -2531,20 +2580,13 @@ async fn interrupt_while_a_request_body_is_still_being_read() { .unwrap() .unwrap(); interrupt(&child); - let pid = child.id().to_string(); - let Ok(output) = tokio::time::timeout( + let output = exit_within( + child, + running, Duration::from_secs(5), - tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()), + "Ctrl-C during a blocked --data read did not end the process", ) - .await - else { - std::process::Command::new("kill") - .args(["-KILL", &pid]) - .status() - .unwrap(); - panic!("Ctrl-C during a blocked --data read did not end the process"); - }; - let output = output.unwrap(); + .await; assert_eq!( output.status.code(), Some(130), @@ -5084,6 +5126,7 @@ async fn interrupting_checkout_stream_returns_130_without_leaking_its_token() { .stderr(Stdio::piped()) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); let (mut socket, _) = tokio::task::spawn_blocking(move || listener.accept().unwrap()) .await .unwrap(); @@ -5107,13 +5150,13 @@ async fn interrupting_checkout_stream_returns_130_without_leaking_its_token() { .args(["-INT", &child.id().to_string()]) .status() .unwrap(); - let output = tokio::time::timeout( + let output = exit_within( + child, + running, Duration::from_secs(5), - tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()), + "Ctrl-C did not end the checkout event stream", ) - .await - .unwrap() - .unwrap(); + .await; assert_eq!( output.status.code(), Some(130), @@ -5150,6 +5193,12 @@ async fn on_a_terminal( on_a_terminal_with(dir, server, args, steps, |_| {}).await } +/// How long a terminal step may take: until its prompt is shown, and until the command exits. +/// A device login waits out its 5-second poll interval before its first prompt, and a loaded +/// machine is slower still. A passing step finishes well within this; it only bounds a failure. +#[cfg(unix)] +const TERMINAL_STEP: Duration = Duration::from_secs(30); + /// `on_a_terminal`, calling `meanwhile` with each step's index once its prompt is shown and /// before its keys are typed, as another command could run while a prompt is open. #[cfg(unix)] @@ -5190,6 +5239,7 @@ async fn on_a_terminal_with( }); } let child = command.spawn().unwrap(); + let running = KillOnDrop::new(&child); let mut prompts = steps .iter() .map(|&(prompt, _)| prompt.as_bytes()) @@ -5216,7 +5266,7 @@ async fn on_a_terminal_with( String::from_utf8_lossy(&shown).into_owned() }); for (index, &(_, keys)) in steps.iter().enumerate() { - tokio::time::timeout(Duration::from_secs(10), shown_prompt.recv()) + tokio::time::timeout(TERMINAL_STEP, shown_prompt.recv()) .await .unwrap() .unwrap(); @@ -5227,20 +5277,13 @@ async fn on_a_terminal_with( (&observer).write_all(keys).unwrap(); } } - let pid = child.id().to_string(); - let Ok(output) = tokio::time::timeout( - Duration::from_secs(10), - tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()), + let output = exit_within( + child, + running, + TERMINAL_STEP, + "the command on the terminal did not exit", ) - .await - else { - std::process::Command::new("kill") - .args(["-KILL", &pid]) - .status() - .unwrap(); - panic!("the command on the terminal did not exit"); - }; - let output = output.unwrap(); + .await; let after = local_modes(&observer); drop(command); let shown = tokio::time::timeout(Duration::from_secs(5), drain) @@ -5927,15 +5970,20 @@ async fn fully_on_a_terminal_ending( .stderr(Stdio::from(terminal)) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); let reader = tokio::task::spawn_blocking(move || { let mut shown = Vec::new(); // Linux PTYs return EIO rather than EOF when the terminal end closes. let _ = controller.read_to_end(&mut shown); shown }); - let output = tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()) - .await - .unwrap(); + let output = exit_within( + child, + running, + TERMINAL_STEP, + "the command on the terminal did not exit", + ) + .await; let shown = strip_styles(&String::from_utf8_lossy(&reader.await.unwrap())); (output.status.code(), shown) } @@ -6273,6 +6321,7 @@ async fn a_redirected_watch_writes_one_event_per_new_or_changed_payment() { .stderr(Stdio::piped()) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); let stdout = child.stdout.take().unwrap(); let events = tokio::task::spawn_blocking(move || { BufReader::new(stdout) @@ -6317,9 +6366,13 @@ async fn a_redirected_watch_writes_one_event_per_new_or_changed_payment() { ); assert!(events.iter().all(|event| event["outcome"] == "event")); interrupt(&child); - let output = tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()) - .await - .unwrap(); + let output = exit_within( + child, + running, + Duration::from_secs(5), + "Ctrl-C did not end the payment watch", + ) + .await; assert_eq!(output.status.code(), Some(130)); assert_eq!( String::from_utf8_lossy(&output.stderr), @@ -6478,6 +6531,7 @@ async fn a_watch_on_a_saved_login_exchanges_again_when_its_token_expires() { .stderr(std::process::Stdio::piped()) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); let stdout = child.stdout.take().unwrap(); let events = tokio::task::spawn_blocking(move || { use std::io::{BufRead, BufReader}; @@ -6500,9 +6554,13 @@ async fn a_watch_on_a_saved_login_exchanges_again_when_its_token_expires() { .unwrap(); assert_eq!(events, ["snapshot receiving", "updated completed"]); interrupt(&child); - let output = tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()) - .await - .unwrap(); + let output = exit_within( + child, + running, + Duration::from_secs(5), + "Ctrl-C did not end the payment watch", + ) + .await; assert_eq!(output.status.code(), Some(130)); server.verify().await; } @@ -6538,10 +6596,15 @@ async fn a_live_watch_that_ends_in_an_error_puts_the_screen_back_first() { .stderr(std::process::Stdio::from(terminal)) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); let shown = drain(controller); - let output = tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()) - .await - .unwrap(); + let output = exit_within( + child, + running, + TERMINAL_STEP, + "the watch did not end on its error", + ) + .await; assert_eq!(output.status.code(), Some(3)); let raw = shown.finish(); let left = raw.find("\u{1b}[?25h\u{1b}[?1049l").expect(&raw); @@ -6578,6 +6641,7 @@ async fn a_watch_on_a_terminal_is_a_live_table_that_marks_changes() { .stderr(Stdio::from(terminal)) .spawn() .unwrap(); + let running = KillOnDrop::new(&child); // Stop once three frames are drawn: the snapshot, B new and A changed, then a quiet one. let shown = drain(controller); let wait = shown.clone(); @@ -6589,9 +6653,13 @@ async fn a_watch_on_a_terminal_is_a_live_table_that_marks_changes() { .await .unwrap(); interrupt(&child); - let output = tokio::task::spawn_blocking(move || child.wait_with_output().unwrap()) - .await - .unwrap(); + let output = exit_within( + child, + running, + Duration::from_secs(5), + "Ctrl-C did not end the live payment watch", + ) + .await; assert_eq!(output.status.code(), Some(130)); let raw = shown.finish(); // The alternate screen is entered, then left with the cursor shown again.