Test task shell commands (#38706)

Kirill Bulatov and Lukas Wirth created

Add tests on task commands, to ensure things like
https://github.com/zed-industries/zed/issues/38343 do not come so easily
unnoticed and to provide a base to create more tests in the future, if
needed.

Release Notes:

- N/A

---------

Co-authored-by: Lukas Wirth <lukas@zed.dev>

Change summary

Cargo.lock                                 |   1 
crates/debugger_ui/src/session/running.rs  |   2 
crates/project/src/terminals.rs            |   9 -
crates/task/src/shell_builder.rs           |  32 +++--
crates/terminal/src/terminal.rs            |  34 +++--
crates/terminal_view/Cargo.toml            |   1 
crates/terminal_view/src/terminal_panel.rs | 140 +++++++++++++++++++++++
crates/terminal_view/src/terminal_view.rs  |   6 
8 files changed, 181 insertions(+), 44 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -17156,6 +17156,7 @@ dependencies = [
  "itertools 0.14.0",
  "language",
  "log",
+ "pretty_assertions",
  "project",
  "rand 0.9.1",
  "regex",

crates/debugger_ui/src/session/running.rs 🔗

@@ -1026,7 +1026,7 @@ impl RunningState {
                 };
 
                 let builder = ShellBuilder::new(remote_shell.as_deref(), &task.resolved.shell);
-                let command_label = builder.command_label(&task.resolved.command_label);
+                let command_label = builder.command_label(task.resolved.command.as_deref().unwrap_or(""));
                 let (command, args) =
                     builder.build(task.resolved.command.clone(), &task.resolved.args);
 

crates/project/src/terminals.rs 🔗

@@ -88,15 +88,8 @@ impl Project {
 
         let local_path = if is_via_remote { None } else { path.clone() };
         let task_state = Some(TaskState {
-            id: spawn_task.id,
-            full_label: spawn_task.full_label,
-            label: spawn_task.label,
-            command_label: spawn_task.command_label,
-            hide: spawn_task.hide,
+            spawned_task: spawn_task.clone(),
             status: TaskStatus::Running,
-            show_summary: spawn_task.show_summary,
-            show_command: spawn_task.show_command,
-            show_rerun: spawn_task.show_rerun,
             completion_rx,
         });
         let remote_client = self.remote_client.clone();

crates/task/src/shell_builder.rs 🔗

@@ -241,20 +241,24 @@ impl ShellBuilder {
     }
 
     /// Returns the label to show in the terminal tab
-    pub fn command_label(&self, command_label: &str) -> String {
-        match self.kind {
-            ShellKind::PowerShell => {
-                format!("{} -C '{}'", self.program, command_label)
-            }
-            ShellKind::Cmd => {
-                format!("{} /C '{}'", self.program, command_label)
-            }
-            ShellKind::Posix | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh => {
-                let interactivity = self.interactive.then_some("-i ").unwrap_or_default();
-                format!(
-                    "{} {interactivity}-c '$\"{}\"'",
-                    self.program, command_label
-                )
+    pub fn command_label(&self, command_to_use_in_label: &str) -> String {
+        if command_to_use_in_label.trim().is_empty() {
+            self.program.clone()
+        } else {
+            match self.kind {
+                ShellKind::PowerShell => {
+                    format!("{} -C '{}'", self.program, command_to_use_in_label)
+                }
+                ShellKind::Cmd => {
+                    format!("{} /C '{}'", self.program, command_to_use_in_label)
+                }
+                ShellKind::Posix | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh => {
+                    let interactivity = self.interactive.then_some("-i ").unwrap_or_default();
+                    format!(
+                        "{PROGRAM} {interactivity}-c '{command_to_use_in_label}'",
+                        PROGRAM = self.program
+                    )
+                }
             }
         }
     }

crates/terminal/src/terminal.rs 🔗

@@ -44,7 +44,7 @@ use pty_info::PtyProcessInfo;
 use serde::{Deserialize, Serialize};
 use settings::Settings;
 use smol::channel::{Receiver, Sender};
-use task::{HideStrategy, Shell, TaskId};
+use task::{HideStrategy, Shell, SpawnInTerminal};
 use terminal_hyperlinks::RegexSearches;
 use terminal_settings::{AlternateScroll, CursorShape, TerminalSettings};
 use theme::{ActiveTheme, Theme};
@@ -739,17 +739,11 @@ struct CopyTemplate {
     window_id: u64,
 }
 
+#[derive(Debug)]
 pub struct TaskState {
-    pub id: TaskId,
-    pub full_label: String,
-    pub label: String,
-    pub command_label: String,
     pub status: TaskStatus,
     pub completion_rx: Receiver<Option<ExitStatus>>,
-    pub hide: HideStrategy,
-    pub show_summary: bool,
-    pub show_command: bool,
-    pub show_rerun: bool,
+    pub spawned_task: SpawnInTerminal,
 }
 
 /// A status of the current terminal tab's task.
@@ -1839,9 +1833,9 @@ impl Terminal {
         match &self.task {
             Some(task_state) => {
                 if truncate {
-                    truncate_and_trailoff(&task_state.label, MAX_CHARS)
+                    truncate_and_trailoff(&task_state.spawned_task.label, MAX_CHARS)
                 } else {
-                    task_state.full_label.clone()
+                    task_state.spawned_task.full_label.clone()
                 }
             }
             None => self
@@ -1949,10 +1943,10 @@ impl Terminal {
 
         let (finished_successfully, task_line, command_line) = task_summary(task, error_code);
         let mut lines_to_show = Vec::new();
-        if task.show_summary {
+        if task.spawned_task.show_summary {
             lines_to_show.push(task_line.as_str());
         }
-        if task.show_command {
+        if task.spawned_task.show_command {
             lines_to_show.push(command_line.as_str());
         }
 
@@ -1964,7 +1958,7 @@ impl Terminal {
             unsafe { append_text_to_term(&mut self.term.lock(), &lines_to_show) };
         }
 
-        match task.hide {
+        match task.spawned_task.hide {
             HideStrategy::Never => {}
             HideStrategy::Always => {
                 cx.emit(Event::CloseTerminal);
@@ -2014,7 +2008,11 @@ pub fn row_to_string(row: &Row<Cell>) -> String {
 
 const TASK_DELIMITER: &str = "⏵ ";
 fn task_summary(task: &TaskState, error_code: Option<i32>) -> (bool, String, String) {
-    let escaped_full_label = task.full_label.replace("\r\n", "\r").replace('\n', "\r");
+    let escaped_full_label = task
+        .spawned_task
+        .full_label
+        .replace("\r\n", "\r")
+        .replace('\n', "\r");
     let (success, task_line) = match error_code {
         Some(0) => (
             true,
@@ -2031,7 +2029,11 @@ fn task_summary(task: &TaskState, error_code: Option<i32>) -> (bool, String, Str
             format!("{TASK_DELIMITER}Task `{escaped_full_label}` finished"),
         ),
     };
-    let escaped_command_label = task.command_label.replace("\r\n", "\r").replace('\n', "\r");
+    let escaped_command_label = task
+        .spawned_task
+        .command_label
+        .replace("\r\n", "\r")
+        .replace('\n', "\r");
     let command_line = format!("{TASK_DELIMITER}Command: {escaped_command_label}");
     (success, task_line, command_line)
 }

crates/terminal_view/Cargo.toml 🔗

@@ -29,6 +29,7 @@ gpui.workspace = true
 itertools.workspace = true
 language.workspace = true
 log.workspace = true
+pretty_assertions.workspace = true
 project.workspace = true
 regex.workspace = true
 task.workspace = true

crates/terminal_view/src/terminal_panel.rs 🔗

@@ -544,7 +544,7 @@ impl TerminalPanel {
             .and_then(|remote_client| remote_client.read(cx).shell());
 
         let builder = ShellBuilder::new(remote_shell.as_deref(), &task.shell);
-        let command_label = builder.command_label(&task.command_label);
+        let command_label = builder.command_label(task.command.as_deref().unwrap_or(""));
         let (command, args) = builder.build(task.command.clone(), &task.args);
 
         let task = SpawnInTerminal {
@@ -664,7 +664,7 @@ impl TerminalPanel {
                 .filter_map(|(index, item)| Some((index, item.act_as::<TerminalView>(cx)?)))
                 .filter_map(|(index, terminal_view)| {
                     let task_state = terminal_view.read(cx).terminal().read(cx).task()?;
-                    if &task_state.full_label == label {
+                    if &task_state.spawned_task.full_label == label {
                         Some((index, terminal_view))
                     } else {
                         None
@@ -1624,3 +1624,139 @@ impl Render for InlineAssistTabBarButton {
             })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use gpui::TestAppContext;
+    use pretty_assertions::assert_eq;
+    use project::FakeFs;
+    use settings::SettingsStore;
+
+    #[gpui::test]
+    async fn test_spawn_an_empty_task(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        let project = Project::test(fs, [], cx).await;
+        let workspace = cx.add_window(|window, cx| Workspace::test_new(project, window, cx));
+
+        let (window_handle, terminal_panel) = workspace
+            .update(cx, |workspace, window, cx| {
+                let window_handle = window.window_handle();
+                let terminal_panel = cx.new(|cx| TerminalPanel::new(workspace, window, cx));
+                (window_handle, terminal_panel)
+            })
+            .unwrap();
+
+        let task = window_handle
+            .update(cx, |_, window, cx| {
+                terminal_panel.update(cx, |terminal_panel, cx| {
+                    terminal_panel.spawn_task(&SpawnInTerminal::default(), window, cx)
+                })
+            })
+            .unwrap();
+
+        let terminal = task.await.unwrap();
+        let expected_shell = util::get_system_shell();
+        terminal
+            .update(cx, |terminal, _| {
+                let task_metadata = terminal
+                    .task()
+                    .expect("When spawning a task, should have the task metadata")
+                    .spawned_task
+                    .clone();
+                assert_eq!(task_metadata.env, HashMap::default());
+                assert_eq!(task_metadata.cwd, None);
+                assert_eq!(task_metadata.shell, task::Shell::System);
+                assert_eq!(
+                    task_metadata.command,
+                    Some(expected_shell.clone()),
+                    "Empty tasks should spawn a -i shell"
+                );
+                assert_eq!(task_metadata.args, Vec::<String>::new());
+                assert_eq!(
+                    task_metadata.command_label, expected_shell,
+                    "We show the shell launch for empty commands"
+                );
+            })
+            .unwrap();
+    }
+
+    // A complex Unix command won't be properly parsed by the Windows terminal hence omit the test there.
+    #[cfg(unix)]
+    #[gpui::test]
+    async fn test_spawn_script_like_task(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        let project = Project::test(fs, [], cx).await;
+        let workspace = cx.add_window(|window, cx| Workspace::test_new(project, window, cx));
+
+        let (window_handle, terminal_panel) = workspace
+            .update(cx, |workspace, window, cx| {
+                let window_handle = window.window_handle();
+                let terminal_panel = cx.new(|cx| TerminalPanel::new(workspace, window, cx));
+                (window_handle, terminal_panel)
+            })
+            .unwrap();
+
+        let user_command = r#"REPO_URL=$(git remote get-url origin | sed -e \"s/^git@\\(.*\\):\\(.*\\)\\.git$/https:\\/\\/\\1\\/\\2/\"); COMMIT_SHA=$(git log -1 --format=\"%H\" -- \"${ZED_RELATIVE_FILE}\"); echo \"${REPO_URL}/blob/${COMMIT_SHA}/${ZED_RELATIVE_FILE}#L${ZED_ROW}-$(echo $(($(wc -l <<< \"$ZED_SELECTED_TEXT\") + $ZED_ROW - 1)))\" | xclip -selection clipboard"#.to_string();
+
+        let expected_cwd = PathBuf::from("/some/work");
+        let task = window_handle
+            .update(cx, |_, window, cx| {
+                terminal_panel.update(cx, |terminal_panel, cx| {
+                    terminal_panel.spawn_task(
+                        &SpawnInTerminal {
+                            command: Some(user_command.clone()),
+                            cwd: Some(expected_cwd.clone()),
+                            ..SpawnInTerminal::default()
+                        },
+                        window,
+                        cx,
+                    )
+                })
+            })
+            .unwrap();
+
+        let terminal = task.await.unwrap();
+        let shell = util::get_system_shell();
+        terminal
+            .update(cx, |terminal, _| {
+                let task_metadata = terminal
+                    .task()
+                    .expect("When spawning a task, should have the task metadata")
+                    .spawned_task
+                    .clone();
+                assert_eq!(task_metadata.env, HashMap::default());
+                assert_eq!(task_metadata.cwd, Some(expected_cwd));
+                assert_eq!(task_metadata.shell, task::Shell::System);
+                assert_eq!(task_metadata.command, Some(shell.clone()));
+                assert_eq!(
+                    task_metadata.args,
+                    vec!["-i".to_string(), "-c".to_string(), user_command.clone(),],
+                    "Use command should have been moved into the arguments, as we're spawning a new -i shell",
+                );
+                assert_eq!(
+                    task_metadata.command_label,
+                    format!("{shell} {interactive}-c '{user_command}'", interactive = if cfg!(windows) {""} else {"-i "}),
+                    "We want to show to the user the entire command spawned");
+            })
+            .unwrap();
+    }
+
+    pub fn init_test(cx: &mut TestAppContext) {
+        cx.update(|cx| {
+            let store = SettingsStore::test(cx);
+            cx.set_global(store);
+            theme::init(theme::LoadThemes::JustBase, cx);
+            client::init_settings(cx);
+            language::init(cx);
+            Project::init_settings(cx);
+            workspace::init_settings(cx);
+            editor::init(cx);
+            crate::init(cx);
+        });
+    }
+}

crates/terminal_view/src/terminal_view.rs 🔗

@@ -476,7 +476,7 @@ impl TerminalView {
             .terminal
             .read(cx)
             .task()
-            .map(|task| terminal_rerun_override(&task.id))
+            .map(|task| terminal_rerun_override(&task.spawned_task.id))
             .unwrap_or_default();
         window.dispatch_action(Box::new(task), cx);
     }
@@ -831,11 +831,11 @@ impl TerminalView {
     }
 
     fn rerun_button(task: &TaskState) -> Option<IconButton> {
-        if !task.show_rerun {
+        if !task.spawned_task.show_rerun {
             return None;
         }
 
-        let task_id = task.id.clone();
+        let task_id = task.spawned_task.id.clone();
         Some(
             IconButton::new("rerun-icon", IconName::Rerun)
                 .icon_size(IconSize::Small)