fix: process manager tests tty (#7156)

### Description

I realized that the process manager tests were broken if they were run
from a TTY on non-windows. This PR does a few things:

- Switches `use_pty` calculation to happen once during process manager
construction. This saves a `isatty` syscall from each process getting
spawned.
- Fixes how we constructed `portable_pty::CommandBuilder` as it differs
from the stdlib where if `cwd` isn't specified it uses the user home dir
instead of the current process cwd
[source](https://docs.rs/portable-pty/latest/src/portable_pty/cmdbuilder.rs.html#458).
This came up because process manager tests don't use absolute paths to
specify script paths where the child tests use absolute paths so cwd
doesn't matter.

We could update the process manager tests to cover both codepaths, but
the spawn method shouldn't impact the process manager behavior and
should be captured by the child tests.

### Testing Instructions

Pull onto your machine and verify that `cargo tr-test` passes now.


Closes TURBO-2185

Author: chris-olszewski

crates/turborepo-lib/src/process/command.rs

@@ -152,6 +152,11 @@ impl From<Command> for portable_pty::CommandBuilder {
         cmd.args(args);
         if let Some(cwd) = cwd {
             cmd.cwd(cwd.as_std_path());
+        } else if let Ok(cwd) = std::env::current_dir() {
+            // portably_pty defaults to a users home directory instead of cwd if one isn't
+            // configured on the command builder.
+            // We explicitly set the cwd if one exists to avoid this behavior
+            cmd.cwd(&cwd);
         }
         for (key, value) in env {
             cmd.env(key, value);

crates/turborepo-lib/src/process/mod.rs

@@ -30,7 +30,10 @@ pub use self::child::{Child, ChildExit};
 /// using `spawn`. When the manager is Closed, all currently-running children
 /// will be closed, and no new children can be spawned.
 #[derive(Debug, Clone)]
-pub struct ProcessManager(Arc<Mutex<ProcessManagerInner>>);
+pub struct ProcessManager {
+    state: Arc<Mutex<ProcessManagerInner>>,
+    use_pty: bool,
+}
 
 #[derive(Debug)]
 struct ProcessManagerInner {
@@ -39,11 +42,22 @@ struct ProcessManagerInner {
 }
 
 impl ProcessManager {
-    pub fn new() -> Self {
-        Self(Arc::new(Mutex::new(ProcessManagerInner {
-            is_closing: false,
-            children: Vec::new(),
-        })))
+    pub fn new(use_pty: bool) -> Self {
+        Self {
+            state: Arc::new(Mutex::new(ProcessManagerInner {
+                is_closing: false,
+                children: Vec::new(),
+            })),
+            use_pty,
+        }
+    }
+
+    /// Construct a process manager and infer if pty should be used
+    pub fn infer() -> Self {
+        // Only use PTY if we're not on windows and we're currently hooked up to a
+        // in a TTY
+        let use_pty = !cfg!(windows) && atty::is(atty::Stream::Stdout);
+        Self::new(use_pty)
     }
 }
 
@@ -61,17 +75,14 @@ impl ProcessManager {
         command: Command,
         stop_timeout: Duration,
     ) -> Option<io::Result<child::Child>> {
-        let mut lock = self.0.lock().unwrap();
+        let mut lock = self.state.lock().unwrap();
         if lock.is_closing {
             return None;
         }
-        // Only use PTY if we're not on windows and we're currently hooked up to a
-        // in a TTY
-        let use_pty = !cfg!(windows) && atty::is(atty::Stream::Stdout);
         let child = child::Child::spawn(
             command,
             child::ShutdownStyle::Graceful(stop_timeout),
-            use_pty,
+            self.use_pty,
         );
         if let Ok(child) = &child {
             lock.children.push(child.clone());
@@ -108,7 +119,7 @@ impl ProcessManager {
         let mut set = JoinSet::new();
 
         {
-            let mut lock = self.0.lock().expect("not poisoned");
+            let mut lock = self.state.lock().expect("not poisoned");
             lock.is_closing = true;
             for child in lock.children.iter() {
                 let child = child.clone();
@@ -123,7 +134,7 @@ impl ProcessManager {
         }
 
         {
-            let mut lock = self.0.lock().expect("not poisoned");
+            let mut lock = self.state.lock().expect("not poisoned");
 
             // just allocate a new vec rather than clearing the old one
             lock.children = vec![];
@@ -155,7 +166,7 @@ mod test {
 
     #[tokio::test]
     async fn test_basic() {
-        let manager = ProcessManager::new();
+        let manager = ProcessManager::new(false);
         let mut child = manager
             .spawn(get_script_command("hello_world.js"), Duration::from_secs(2))
             .unwrap()
@@ -168,7 +179,7 @@ mod test {
 
     #[tokio::test]
     async fn test_multiple() {
-        let manager = ProcessManager::new();
+        let manager = ProcessManager::new(false);
 
         let children = (0..2)
             .map(|_| {
@@ -191,7 +202,7 @@ mod test {
 
     #[tokio::test]
     async fn test_closed() {
-        let manager = ProcessManager::new();
+        let manager = ProcessManager::new(false);
         let mut child = manager
             .spawn(get_command(), Duration::from_secs(2))
             .unwrap()
@@ -218,7 +229,7 @@ mod test {
 
     #[tokio::test]
     async fn test_exit_code() {
-        let manager = ProcessManager::new();
+        let manager = ProcessManager::new(false);
         let mut child = manager
             .spawn(get_script_command("hello_world.js"), Duration::from_secs(2))
             .unwrap()
@@ -235,7 +246,7 @@ mod test {
     #[tokio::test]
     #[traced_test]
     async fn test_message_after_stop() {
-        let manager = ProcessManager::new();
+        let manager = ProcessManager::new(false);
         let mut child = manager
             .spawn(get_script_command("hello_world.js"), Duration::from_secs(2))
             .unwrap()
@@ -258,14 +269,14 @@ mod test {
 
     #[tokio::test]
     async fn test_reuse_manager() {
-        let manager = ProcessManager::new();
+        let manager = ProcessManager::new(false);
         manager.spawn(get_command(), Duration::from_secs(2));
 
         sleep(Duration::from_millis(100)).await;
 
         manager.stop().await;
 
-        assert!(manager.0.lock().unwrap().children.is_empty());
+        assert!(manager.state.lock().unwrap().children.is_empty());
 
         // TODO: actually do some check that this is idempotent
         // idempotent
@@ -280,7 +291,7 @@ mod test {
         script: &str,
         expected: Option<ChildExit>,
     ) {
-        let manager = ProcessManager::new();
+        let manager = ProcessManager::new(false);
         let tasks = FuturesUnordered::new();
 
         for _ in 0..10 {
@@ -315,7 +326,7 @@ mod test {
 
     #[tokio::test]
     async fn test_wait_multiple_tasks() {
-        let manager = ProcessManager::new();
+        let manager = ProcessManager::new(false);
 
         let mut out = Vec::new();
         let mut child = manager

crates/turborepo-lib/src/run/mod.rs

@@ -69,7 +69,7 @@ pub struct Run {
 
 impl Run {
     pub fn new(base: CommandBase, api_auth: Option<APIAuth>) -> Result<Self, Error> {
-        let processes = ProcessManager::new();
+        let processes = ProcessManager::infer();
         let mut opts: Opts = base.args().try_into()?;
         let config = base.config()?;
         let is_linked = turborepo_api_client::is_linked(&api_auth);