fix: preserve PTY wait errors and remove test serialization

Author: branchseer

crates/pty_terminal/src/terminal.rs

@@ -32,7 +32,7 @@ pub struct PtyWriter {
 /// A cloneable handle to a child process spawned in a PTY.
 pub struct ChildHandle {
     child_killer: Box<dyn ChildKiller + Send + Sync>,
-    exit_status: Arc<OnceLock<ExitStatus>>,
+    exit_status: Arc<OnceLock<std::io::Result<ExitStatus>>>,
 }
 
 impl Clone for ChildHandle {
@@ -221,9 +221,16 @@ impl PtyWriter {
 
 impl ChildHandle {
     /// Blocks until the child process has exited and returns its exit status.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if waiting for the child process exit status fails.
     #[must_use]
-    pub fn wait(&self) -> ExitStatus {
-        self.exit_status.wait().clone()
+    pub fn wait(&self) -> std::io::Result<ExitStatus> {
+        match self.exit_status.wait() {
+            Ok(status) => Ok(status.clone()),
+            Err(error) => Err(std::io::Error::new(error.kind(), error.to_string())),
+        }
     }
 
     /// Kills the child process.
@@ -238,11 +245,10 @@ impl ChildHandle {
 }
 
 fn set_exit_status_from_wait_result(
-    exit_status: &OnceLock<ExitStatus>,
+    exit_status: &OnceLock<std::io::Result<ExitStatus>>,
     wait_result: std::io::Result<ExitStatus>,
 ) {
-    let status = wait_result.unwrap_or_else(|_| ExitStatus::with_exit_code(1));
-    let _ = exit_status.set(status);
+    let _ = exit_status.set(wait_result);
 }
 
 impl Terminal {
@@ -271,7 +277,7 @@ impl Terminal {
         let child_killer = child.clone_killer();
         drop(pty_pair.slave); // Critical: drop slave so EOF is signaled when child exits
         let master = pty_pair.master;
-        let exit_status: Arc<OnceLock<ExitStatus>> = Arc::new(OnceLock::new());
+        let exit_status: Arc<OnceLock<std::io::Result<ExitStatus>>> = Arc::new(OnceLock::new());
 
         // Background thread: wait for child to exit, set exit status, then close writer to trigger EOF
         thread::spawn({
@@ -307,16 +313,17 @@ mod tests {
     use super::*;
 
     #[test]
-    fn set_exit_status_from_wait_result_sets_error_fallback() {
+    fn set_exit_status_from_wait_result_preserves_error() {
         let exit_status = OnceLock::new();
         set_exit_status_from_wait_result(
             &exit_status,
             Err(std::io::Error::other("forced wait error for test")),
         );
 
         let status = exit_status.wait();
-        assert!(!status.success());
-        assert_eq!(status.exit_code(), 1);
+        assert!(status.is_err());
+        assert_eq!(status.as_ref().unwrap_err().kind(), std::io::ErrorKind::Other);
+        assert_eq!(status.as_ref().unwrap_err().to_string(), "forced wait error for test");
     }
 
     #[test]
@@ -325,7 +332,7 @@ mod tests {
         set_exit_status_from_wait_result(&exit_status, Ok(ExitStatus::with_exit_code(42)));
 
         let status = exit_status.wait();
-        assert!(!status.success());
-        assert_eq!(status.exit_code(), 42);
+        assert!(!status.as_ref().unwrap().success());
+        assert_eq!(status.as_ref().unwrap().exit_code(), 42);
     }
 }

crates/pty_terminal/tests/terminal.rs

@@ -1,5 +1,3 @@
-#[cfg(windows)]
-use std::sync::{LazyLock, Mutex, MutexGuard};
 use std::{
     io::{BufRead, BufReader, IsTerminal, Read, Write, stderr, stdin, stdout},
     time::{Duration, Instant},
@@ -10,19 +8,10 @@ use portable_pty::CommandBuilder;
 use pty_terminal::{geo::ScreenSize, terminal::Terminal};
 use subprocess_test::command_for_fn;
 
-#[cfg(windows)]
-fn windows_test_serial_guard() -> MutexGuard<'static, ()> {
-    static WINDOWS_TEST_MUTEX: LazyLock<Mutex<()>> = LazyLock::new(|| Mutex::new(()));
-    WINDOWS_TEST_MUTEX.lock().unwrap()
-}
-
 #[test]
 #[timeout(5000)]
 #[expect(clippy::print_stdout, reason = "subprocess test output")]
 fn is_terminal() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         println!("{} {} {}", stdin().is_terminal(), stdout().is_terminal(), stderr().is_terminal());
     }));
@@ -31,7 +20,7 @@ fn is_terminal() {
         Terminal::spawn(ScreenSize { rows: 80, cols: 80 }, cmd).unwrap();
     let mut discard = Vec::new();
     pty_reader.read_to_end(&mut discard).unwrap();
-    let _ = child_handle.wait();
+    let _ = child_handle.wait().unwrap();
     let output = pty_reader.screen_contents();
     assert_eq!(output.trim(), "true true true");
 }
@@ -40,9 +29,6 @@ fn is_terminal() {
 #[timeout(5000)]
 #[expect(clippy::print_stdout, reason = "subprocess test output")]
 fn write_basic_echo() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         use std::io::{BufRead, Write, stdin, stdout};
         let stdin = stdin();
@@ -61,7 +47,7 @@ fn write_basic_echo() {
 
     let mut discard = Vec::new();
     pty_reader.read_to_end(&mut discard).unwrap();
-    let _ = child_handle.wait();
+    let _ = child_handle.wait().unwrap();
 
     let output = pty_reader.screen_contents();
     // PTY echoes the input, so we see "hello world\nhello world"
@@ -72,9 +58,6 @@ fn write_basic_echo() {
 #[timeout(5000)]
 #[expect(clippy::print_stdout, reason = "subprocess test output")]
 fn write_multiple_lines() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         use std::io::{BufRead, Write, stdin, stdout};
         let stdin = stdin();
@@ -115,7 +98,7 @@ fn write_multiple_lines() {
 
     let mut discard = Vec::new();
     pty_reader.read_to_end(&mut discard).unwrap();
-    let _ = child_handle.wait();
+    let _ = child_handle.wait().unwrap();
 
     let output = pty_reader.screen_contents();
     // PTY echoes input, then child prints "Echo: {line}\n" for each
@@ -126,9 +109,6 @@ fn write_multiple_lines() {
 #[timeout(5000)]
 #[expect(clippy::print_stdout, reason = "subprocess test output")]
 fn write_after_exit() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         print!("exiting");
     }));
@@ -139,7 +119,7 @@ fn write_after_exit() {
     // Read all output - this blocks until child exits and EOF is reached
     let mut discard = Vec::new();
     pty_reader.read_to_end(&mut discard).unwrap();
-    let _ = child_handle.wait();
+    let _ = child_handle.wait().unwrap();
 
     // Writer shutdown is done by a background thread after child wait returns.
     // Poll briefly for the writer state to flip to closed before asserting write failure.
@@ -157,9 +137,6 @@ fn write_after_exit() {
 #[timeout(5000)]
 #[expect(clippy::print_stdout, reason = "subprocess test output")]
 fn write_interactive_prompt() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         use std::io::{Write, stdin, stdout};
         let mut stdout = stdout();
@@ -188,7 +165,7 @@ fn write_interactive_prompt() {
 
     let mut discard = Vec::new();
     pty_reader.read_to_end(&mut discard).unwrap();
-    let _ = child_handle.wait();
+    let _ = child_handle.wait().unwrap();
 
     let output = pty_reader.screen_contents();
     assert_eq!(output.trim(), "Name: Alice\nHello, Alice");
@@ -198,9 +175,6 @@ fn write_interactive_prompt() {
 #[timeout(5000)]
 #[expect(clippy::print_stdout, reason = "subprocess test output")]
 fn resize_terminal() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         use std::io::{Write, stdin, stdout};
         #[cfg(unix)]
@@ -298,9 +272,6 @@ fn resize_terminal() {
 #[timeout(5000)]
 #[expect(clippy::print_stdout, reason = "subprocess test output")]
 fn send_ctrl_c_interrupts_process() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         use std::io::{Write, stdout};
 
@@ -367,9 +338,6 @@ fn send_ctrl_c_interrupts_process() {
 #[timeout(5000)]
 #[expect(clippy::print_stdout, reason = "subprocess test output")]
 fn read_to_end_returns_exit_status_success() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         println!("success");
     }));
@@ -378,17 +346,14 @@ fn read_to_end_returns_exit_status_success() {
         Terminal::spawn(ScreenSize { rows: 80, cols: 80 }, cmd).unwrap();
     let mut discard = Vec::new();
     pty_reader.read_to_end(&mut discard).unwrap();
-    let status = child_handle.wait();
+    let status = child_handle.wait().unwrap();
     assert!(status.success());
     assert_eq!(status.exit_code(), 0);
 }
 
 #[test]
 #[timeout(5000)]
 fn read_to_end_returns_exit_status_nonzero() {
-    #[cfg(windows)]
-    let _guard = windows_test_serial_guard();
-
     let cmd = CommandBuilder::from(command_for_fn!((), |(): ()| {
         std::process::exit(42);
     }));
@@ -397,7 +362,7 @@ fn read_to_end_returns_exit_status_nonzero() {
         Terminal::spawn(ScreenSize { rows: 80, cols: 80 }, cmd).unwrap();
     let mut discard = Vec::new();
     pty_reader.read_to_end(&mut discard).unwrap();
-    let status = child_handle.wait();
+    let status = child_handle.wait().unwrap();
     assert!(!status.success());
     assert_eq!(status.exit_code(), 42);
 }

crates/pty_terminal_test/src/lib.rs

@@ -91,10 +91,14 @@ impl Reader {
 
     /// Reads all remaining PTY output until the child exits, then returns the exit status.
     ///
+    /// # Errors
+    ///
+    /// Returns an error if waiting for the child process exit status fails.
+    ///
     /// # Panics
     ///
     /// Panics if reading from the PTY fails.
-    pub fn wait_for_exit(&mut self) -> ExitStatus {
+    pub fn wait_for_exit(&mut self) -> std::io::Result<ExitStatus> {
         let mut discard = Vec::new();
         self.pty.read_to_end(&mut discard).expect("PTY read_to_end failed");
         self.child_handle.wait()

crates/pty_terminal_test/tests/milestone.rs

@@ -67,7 +67,7 @@ fn milestone_raw_mode_keystrokes() {
     // Write 'q' to quit and wait for the child to exit
     writer.write_all(b"q").unwrap();
     writer.flush().unwrap();
-    let status = reader.wait_for_exit();
+    let status = reader.wait_for_exit().unwrap();
     assert!(status.success());
 }
 
@@ -122,6 +122,6 @@ fn milestone_does_not_pollute_screen() {
 
     writer.write_all(b"q").unwrap();
     writer.flush().unwrap();
-    let status = reader.wait_for_exit();
+    let status = reader.wait_for_exit().unwrap();
     assert!(status.success());
 }

crates/vite_task_bin/tests/e2e_snapshots/main.rs

@@ -391,7 +391,7 @@ fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, fixture
                     }
                 }
 
-                let status = terminal.reader.wait_for_exit();
+                let status = terminal.reader.wait_for_exit().unwrap();
                 let screen = terminal.reader.screen_contents();
 
                 {