From d8048f46ee9ed0ce3aa6ad1b9e080009817503ec Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 Sep 2025 14:12:39 +0300 Subject: [PATCH] Test task shell commands (#38706) 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 --- 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(-) diff --git a/Cargo.lock b/Cargo.lock index 1819b62c3434ad8bdea6dd526b7f68122378e290..39146342f2973691e00a9c5ab9a8d29e50539a47 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17156,6 +17156,7 @@ dependencies = [ "itertools 0.14.0", "language", "log", + "pretty_assertions", "project", "rand 0.9.1", "regex", diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index a18a186469a0aaaf5f3d061830446f5ba27dec72..1bef21ed8b19f01fc2371a785544a45cc88bfe9c 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/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); diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index 94e9999e1344efbc391476e22d107f10052d7694..1143bdebfd46d6c4f44b06cd62002557c0d46936 100644 --- a/crates/project/src/terminals.rs +++ b/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(); diff --git a/crates/task/src/shell_builder.rs b/crates/task/src/shell_builder.rs index c3f0646c02cc427a07505c2ff30157e84d2ca0fe..5ebf483bc3925542212ff125586016dd477c37e2 100644 --- a/crates/task/src/shell_builder.rs +++ b/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 + ) + } } } } diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 987e3272602763f93d350c07b10246707b0ea2ec..cb43d548211b1d154baacf6c1c07db6892cd6dc4 100644 --- a/crates/terminal/src/terminal.rs +++ b/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>, - 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) -> String { const TASK_DELIMITER: &str = "⏵ "; fn task_summary(task: &TaskState, error_code: Option) -> (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) -> (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) } diff --git a/crates/terminal_view/Cargo.toml b/crates/terminal_view/Cargo.toml index e424d89917bc02b3398f1457a676ab03cdd1b179..85ee506d69444fa7d58b536acac3a00088e3f047 100644 --- a/crates/terminal_view/Cargo.toml +++ b/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 diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index faea73849ba83b3e2899f3efd995d4557c6b1e3c..25b3671ccad69d02df383108e9d853ce9231ed70 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/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::(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::::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); + }); + } +} diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index d7adf74acb37e848ccb2d8670f970054d46ea0ae..1f1e48562fc35575813cd81921aace9188f8a0c0 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/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 { - 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)