From ced8e4d88e125f29440fb1cbd94e6a18221f9633 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Thu, 22 May 2025 12:59:59 -0400 Subject: [PATCH] debugger beta: Move path resolution to resolve scenario instead of just in new session modal (#31185) This move was done so debug configs could use path resolution, and saving a configuration from the new session modal wouldn't resolve paths beforehand. I also added an integration test to make sure path resolution happens from an arbitrary config. The test was placed under the new session modal directory because it has to do with starting a session, and that's what the new session modal typically does, even if it's implicitly used in the test. In the future, I plan to add more tests to the new session modal too. Release Notes: - debugger beta: Allow configs from debug.json to resolve paths --- crates/debugger_ui/src/new_session_modal.rs | 30 ++-- crates/debugger_ui/src/session/running.rs | 32 +++- crates/debugger_ui/src/tests.rs | 3 + .../src/tests/new_session_modal.rs | 157 ++++++++++++++++++ 4 files changed, 210 insertions(+), 12 deletions(-) create mode 100644 crates/debugger_ui/src/tests/new_session_modal.rs diff --git a/crates/debugger_ui/src/new_session_modal.rs b/crates/debugger_ui/src/new_session_modal.rs index 9ea8399509..4c9b0c2067 100644 --- a/crates/debugger_ui/src/new_session_modal.rs +++ b/crates/debugger_ui/src/new_session_modal.rs @@ -779,7 +779,7 @@ impl CustomMode { } pub(super) fn debug_request(&self, cx: &App) -> task::LaunchRequest { - let path = resolve_path(&self.cwd.read(cx).text(cx)); + let path = self.cwd.read(cx).text(cx); if cfg!(windows) { return task::LaunchRequest { program: self.program.read(cx).text(cx), @@ -797,12 +797,12 @@ impl CustomMode { env.insert(lhs.to_string(), rhs.to_string()); } - let program = resolve_path(&if let Some(program) = args.next() { + let program = if let Some(program) = args.next() { program } else { env = FxHashMap::default(); command - }); + }; let args = args.collect::>(); @@ -1145,26 +1145,34 @@ impl PickerDelegate for DebugScenarioDelegate { } } -fn resolve_path(path: &str) -> String { +pub(crate) fn resolve_path(path: &mut String) { if path.starts_with('~') { let home = paths::home_dir().to_string_lossy().to_string(); - let path = path.trim().to_owned(); - path.replace('~', &home) - } else { - path.to_owned() - } + let trimmed_path = path.trim().to_owned(); + *path = trimmed_path.replacen('~', &home, 1); + } else if let Some(strip_path) = path.strip_prefix(&format!(".{}", std::path::MAIN_SEPARATOR)) { + *path = format!( + "$ZED_WORKTREE_ROOT{}{}", + std::path::MAIN_SEPARATOR, + &strip_path + ); + }; } #[cfg(test)] mod tests { use paths::home_dir; - use super::*; - #[test] fn test_normalize_paths() { let sep = std::path::MAIN_SEPARATOR; let home = home_dir().to_string_lossy().to_string(); + let resolve_path = |path: &str| -> String { + let mut path = path.to_string(); + super::resolve_path(&mut path); + path + }; + assert_eq!(resolve_path("bin"), format!("bin")); assert_eq!(resolve_path(&format!("{sep}foo")), format!("{sep}foo")); assert_eq!(resolve_path(""), format!("")); diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index d1bcd23174..5c6d9e904d 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -7,7 +7,10 @@ pub mod variable_list; use std::{any::Any, ops::ControlFlow, path::PathBuf, sync::Arc, time::Duration}; -use crate::persistence::{self, DebuggerPaneItem, SerializedLayout}; +use crate::{ + new_session_modal::resolve_path, + persistence::{self, DebuggerPaneItem, SerializedLayout}, +}; use super::DebugPanelItemEvent; use anyhow::{Context as _, Result, anyhow}; @@ -543,6 +546,32 @@ impl RunningState { } } + pub(crate) fn relativlize_paths( + key: Option<&str>, + config: &mut serde_json::Value, + context: &TaskContext, + ) { + match config { + serde_json::Value::Object(obj) => { + obj.iter_mut() + .for_each(|(key, value)| Self::relativlize_paths(Some(key), value, context)); + } + serde_json::Value::Array(array) => { + array + .iter_mut() + .for_each(|value| Self::relativlize_paths(None, value, context)); + } + serde_json::Value::String(s) if key == Some("program") || key == Some("cwd") => { + resolve_path(s); + + if let Some(substituted) = substitute_variables_in_str(&s, context) { + *s = substituted; + } + } + _ => {} + } + } + pub(crate) fn new( session: Entity, project: Entity, @@ -752,6 +781,7 @@ impl RunningState { mut config, tcp_connection, } = scenario; + Self::relativlize_paths(None, &mut config, &task_context); Self::substitute_variables_in_config(&mut config, &task_context); let request_type = dap_registry diff --git a/crates/debugger_ui/src/tests.rs b/crates/debugger_ui/src/tests.rs index 869a1cfced..ccb74a1bb4 100644 --- a/crates/debugger_ui/src/tests.rs +++ b/crates/debugger_ui/src/tests.rs @@ -25,6 +25,9 @@ mod inline_values; #[cfg(test)] mod module_list; #[cfg(test)] +#[cfg(not(windows))] +mod new_session_modal; +#[cfg(test)] mod persistence; #[cfg(test)] mod stack_frame_list; diff --git a/crates/debugger_ui/src/tests/new_session_modal.rs b/crates/debugger_ui/src/tests/new_session_modal.rs new file mode 100644 index 0000000000..ebef918a7f --- /dev/null +++ b/crates/debugger_ui/src/tests/new_session_modal.rs @@ -0,0 +1,157 @@ +use gpui::{BackgroundExecutor, TestAppContext, VisualTestContext}; +use project::{FakeFs, Project}; +use serde_json::json; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use task::{DebugScenario, TaskContext, VariableName}; +use util::path; + +use crate::tests::{init_test, init_test_workspace}; + +// todo(tasks) figure out why task replacement is broken on windows +#[gpui::test] +async fn test_debug_session_substitutes_variables_and_relativizes_paths( + executor: BackgroundExecutor, + cx: &mut TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(executor.clone()); + fs.insert_tree( + path!("/project"), + json!({ + "main.rs": "fn main() {}" + }), + ) + .await; + + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + let workspace = init_test_workspace(&project, cx).await; + let cx = &mut VisualTestContext::from_window(*workspace, cx); + + // Set up task variables to simulate a real environment + let test_variables = vec![( + VariableName::WorktreeRoot, + "/test/worktree/path".to_string(), + )] + .into_iter() + .collect(); + + let task_context = TaskContext { + cwd: None, + task_variables: test_variables, + project_env: Default::default(), + }; + + let home_dir = paths::home_dir(); + + let sep = std::path::MAIN_SEPARATOR; + + // Test cases for different path formats + let test_cases: Vec<(Arc, Arc)> = vec![ + // Absolute path - should not be relativized + ( + Arc::from(format!("{0}absolute{0}path{0}to{0}program", sep)), + Arc::from(format!("{0}absolute{0}path{0}to{0}program", sep)), + ), + // Relative path - should be prefixed with worktree root + ( + Arc::from(format!(".{0}src{0}program", sep)), + Arc::from(format!("{0}test{0}worktree{0}path{0}src{0}program", sep)), + ), + // Home directory path - should be prefixed with worktree root + ( + Arc::from(format!("~{0}src{0}program", sep)), + Arc::from(format!( + "{1}{0}src{0}program", + sep, + home_dir.to_string_lossy() + )), + ), + // Path with $ZED_WORKTREE_ROOT - should be substituted without double appending + ( + Arc::from(format!("$ZED_WORKTREE_ROOT{0}src{0}program", sep)), + Arc::from(format!("{0}test{0}worktree{0}path{0}src{0}program", sep)), + ), + ]; + + let called_launch = Arc::new(AtomicBool::new(false)); + + for (input_path, expected_path) in test_cases { + let _subscription = project::debugger::test::intercept_debug_sessions(cx, { + let called_launch = called_launch.clone(); + let input_path = input_path.clone(); + let expected_path = expected_path.clone(); + move |client| { + client.on_request::({ + let called_launch = called_launch.clone(); + let input_path = input_path.clone(); + let expected_path = expected_path.clone(); + + move |_, args| { + let config = args.raw.as_object().unwrap(); + + // Verify the program path was substituted correctly + assert_eq!( + config["program"].as_str().unwrap(), + expected_path.as_str(), + "Program path was not correctly substituted for input: {}", + input_path.as_str() + ); + + // Verify the cwd path was substituted correctly + assert_eq!( + config["cwd"].as_str().unwrap(), + expected_path.as_str(), + "CWD path was not correctly substituted for input: {}", + input_path.as_str() + ); + + // Verify that otherField was substituted but not relativized + // It should still have $ZED_WORKTREE_ROOT substituted if present + let expected_other_field = if input_path.contains("$ZED_WORKTREE_ROOT") { + input_path.replace("$ZED_WORKTREE_ROOT", "/test/worktree/path") + } else { + input_path.to_string() + }; + + assert_eq!( + config["otherField"].as_str().unwrap(), + expected_other_field, + "Other field was incorrectly modified for input: {}", + input_path + ); + + called_launch.store(true, Ordering::SeqCst); + + Ok(()) + } + }); + } + }); + + let scenario = DebugScenario { + adapter: "fake-adapter".into(), + label: "test-debug-session".into(), + build: None, + config: json!({ + "request": "launch", + "program": input_path, + "cwd": input_path, + "otherField": input_path + }), + tcp_connection: None, + }; + + workspace + .update(cx, |workspace, window, cx| { + workspace.start_debug_session(scenario, task_context.clone(), None, window, cx) + }) + .unwrap(); + + cx.run_until_parked(); + + assert!(called_launch.load(Ordering::SeqCst)); + called_launch.store(false, Ordering::SeqCst); + } +}