terminal: Fix `test_basic_terminal` test (#38059)

Lukas Wirth created 2 months ago

`echo` isn't a program on windows, so we need to spawn a shell that
executes it for the test

Release Notes:

- N/A

Change summary

Cargo.lock                      |  1 
crates/terminal/Cargo.toml      |  1 
crates/terminal/src/terminal.rs | 67 +++++++++++++++++-----------------
3 files changed, 35 insertions(+), 34 deletions(-)

Detailed changes

Cargo.lock

@@ -16441,6 +16441,7 @@ dependencies = [
  "collections",
  "futures 0.3.31",
  "gpui",
+ "itertools 0.14.0",
  "libc",
  "rand 0.9.1",
  "regex",

crates/terminal/Cargo.toml

@@ -40,6 +40,7 @@ util.workspace = true
 regex.workspace = true
 urlencoding.workspace = true
 workspace-hack.workspace = true
+itertools.workspace = true
 
 [target.'cfg(windows)'.dependencies]
 windows.workspace = true

crates/terminal/src/terminal.rs

@@ -32,6 +32,7 @@ use futures::{
     channel::mpsc::{UnboundedReceiver, UnboundedSender, unbounded},
 };
 
+use itertools::Itertools as _;
 use mappings::mouse::{
     alt_scroll, grid_point, grid_point_and_side, mouse_button_report, mouse_moved_report,
     scroll_report,
@@ -48,10 +49,7 @@ use terminal_hyperlinks::RegexSearches;
 use terminal_settings::{AlternateScroll, CursorShape, TerminalSettings};
 use theme::{ActiveTheme, Theme};
 use urlencoding;
-use util::{
-    paths::{self, home_dir},
-    truncate_and_trailoff,
-};
+use util::truncate_and_trailoff;
 
 use std::{
     borrow::Cow,
@@ -276,7 +274,9 @@ impl Dimensions for TerminalBounds {
 #[derive(Error, Debug)]
 pub struct TerminalError {
     pub directory: Option<PathBuf>,
-    pub shell: Shell,
+    pub program: Option<String>,
+    pub args: Option<Vec<String>>,
+    pub title_override: Option<SharedString>,
     pub source: std::io::Error,
 }
 
@@ -294,29 +294,23 @@ impl TerminalError {
                     Err(s) => s,
                 }
             })
-            .unwrap_or_else(|| {
-                format!(
-                    "<none specified, using home directory> {}",
-                    paths::home_dir().to_string_lossy()
-                )
-            })
+            .unwrap_or_else(|| "<none specified>".to_string())
     }
 
     pub fn fmt_shell(&self) -> String {
-        match &self.shell {
-            Shell::System => "<system defined shell>".to_string(),
-            Shell::Program(s) => s.to_string(),
-            Shell::WithArguments {
-                program,
-                args,
-                title_override,
-            } => {
-                if let Some(title_override) = title_override {
-                    format!("{} {} ({})", program, args.join(" "), title_override)
-                } else {
-                    format!("{} {}", program, args.join(" "))
-                }
-            }
+        if let Some(title_override) = &self.title_override {
+            format!(
+                "{} {} ({})",
+                self.program.as_deref().unwrap_or("<system defined shell>"),
+                self.args.as_ref().into_iter().flatten().format(" "),
+                title_override
+            )
+        } else {
+            format!(
+                "{} {}",
+                self.program.as_deref().unwrap_or("<system defined shell>"),
+                self.args.as_ref().into_iter().flatten().format(" ")
+            )
         }
     }
 }
@@ -420,15 +414,16 @@ impl TerminalBuilder {
         });
 
         let pty_options = {
-            let alac_shell = shell_params.map(|params| {
-                alacritty_terminal::tty::Shell::new(params.program, params.args.unwrap_or_default())
+            let alac_shell = shell_params.as_ref().map(|params| {
+                alacritty_terminal::tty::Shell::new(
+                    params.program.clone(),
+                    params.args.clone().unwrap_or_default(),
+                )
             });
 
             alacritty_terminal::tty::Options {
                 shell: alac_shell,
-                working_directory: working_directory
-                    .clone()
-                    .or_else(|| Some(home_dir().to_path_buf())),
+                working_directory: working_directory.clone(),
                 drain_on_exit: true,
                 env: env.clone().into_iter().collect(),
             }
@@ -474,7 +469,9 @@ impl TerminalBuilder {
             Err(error) => {
                 bail!(TerminalError {
                     directory: working_directory,
-                    shell,
+                    program: shell_params.as_ref().map(|params| params.program.clone()),
+                    args: shell_params.as_ref().and_then(|params| params.args.clone()),
+                    title_override: terminal_title_override,
                     source: error,
                 });
             }
@@ -2205,20 +2202,22 @@ mod tests {
     use collections::HashMap;
     use gpui::{Pixels, Point, TestAppContext, bounds, point, size};
     use rand::{Rng, distr, rngs::ThreadRng};
+    use task::ShellBuilder;
 
-    #[ignore = "Test is flaky on macOS, and doesn't run on Windows"]
     #[gpui::test]
     async fn test_basic_terminal(cx: &mut TestAppContext) {
         cx.executor().allow_parking();
 
         let (completion_tx, completion_rx) = smol::channel::unbounded();
+        let (program, args) = ShellBuilder::new(None, &Shell::System)
+            .build(Some("echo".to_owned()), &["hello".to_owned()]);
         let terminal = cx.new(|cx| {
             TerminalBuilder::new(
                 None,
                 None,
                 task::Shell::WithArguments {
-                    program: "echo".into(),
-                    args: vec!["hello".into()],
+                    program,
+                    args,
                     title_override: None,
                 },
                 HashMap::default(),