diff --git a/Cargo.lock b/Cargo.lock index 23d15e832..d2e519a43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4598,6 +4598,7 @@ dependencies = [ "encoding_rs", "ide-build", "ide-core", + "ide-language", "libc", "parking_lot", "portable-pty", diff --git a/crates/ide-host/src/ops/debug.rs b/crates/ide-host/src/ops/debug.rs index 737db0e2f..a13284b4e 100644 --- a/crates/ide-host/src/ops/debug.rs +++ b/crates/ide-host/src/ops/debug.rs @@ -31,6 +31,8 @@ pub async fn debug_start(host: &Host, window: &str, config: LaunchConfig) -> Ide ide_sdk::go::configure_environment(&root, &settings, &mut config.env); ide_sdk::go::apply_to_debug( &mut config.env, + &mut config.program, + &mut config.args, &mut config.extra_json, &settings.languages.go.build_tags, )?; diff --git a/crates/ide-run/Cargo.toml b/crates/ide-run/Cargo.toml index 1e362ef3a..e2328b041 100644 --- a/crates/ide-run/Cargo.toml +++ b/crates/ide-run/Cargo.toml @@ -7,6 +7,7 @@ license.workspace = true [dependencies] ide-core.workspace = true +ide-language.workspace = true dunce.workspace = true encoding_rs.workspace = true serde.workspace = true diff --git a/crates/ide-run/src/go.rs b/crates/ide-run/src/go.rs index 96169a682..18e8225bb 100644 --- a/crates/ide-run/src/go.rs +++ b/crates/ide-run/src/go.rs @@ -1,9 +1,11 @@ //! Typed Go configurations use the process service for both build and execution. use crate::RunConfig; use ide_core::{IdeError, IdeResult}; +use std::collections::HashSet; use std::hash::{Hash, Hasher}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::sync::OnceLock; use std::time::{Duration, Instant}; pub struct Plan { @@ -85,7 +87,17 @@ pub fn plan(root: &Path, config: &RunConfig) -> IdeResult> { /// Uses Go's own build-file selection, including platform suffixes, cgo and build tags. /// `-find` avoids dependency traversal and package downloads for this local directory. -pub fn package_sources(mut command: Command, dir: &Path, flags: &[String]) -> IdeResult> { +pub fn package_sources(command: Command, dir: &Path, flags: &[String]) -> IdeResult> { + let package = package_info(command, dir, flags)?; + Ok(["GoFiles", "CgoFiles"] + .iter() + .flat_map(|key| package[key].as_array().into_iter().flatten()) + .filter_map(|p| p.as_str()) + .map(|p| dir.join(p)) + .collect()) +} + +fn package_info(mut command: Command, dir: &Path, flags: &[String]) -> IdeResult { command.args(["list", "-find", "-e", "-json"]); let mut it = flags.iter(); while let Some(arg) = it.next() { @@ -138,13 +150,101 @@ pub fn package_sources(mut command: Command, dir: &Path, flags: &[String]) -> Id if !status.success() { return Err(IdeError::invalid(format!("Go package discovery failed: {errors}"))); } - let package: serde_json::Value = serde_json::from_str(&text).map_err(|e| IdeError::invalid(e.to_string()))?; - Ok(["GoFiles", "CgoFiles"] + serde_json::from_str(&text).map_err(|e| IdeError::invalid(e.to_string())) +} + +/// Keep the real package and its helpers, selecting tests by their declaration file. +pub fn file_test_exclusions(command: Command, file: &Path, flags: &[String]) -> IdeResult { + let dir = file.parent().ok_or_else(|| IdeError::invalid("Go test file has no directory"))?; + let package = package_info(command, dir, flags)?; + let files: Vec<_> = ["TestGoFiles", "XTestGoFiles"] .iter() .flat_map(|key| package[key].as_array().into_iter().flatten()) - .filter_map(|p| p.as_str()) - .map(|p| dir.join(p)) - .collect()) + .filter_map(|path| path.as_str()) + .collect(); + if !files.iter().any(|name| Some(std::ffi::OsStr::new(name)) == file.file_name()) { + return Err(IdeError::invalid("The selected Go test file is excluded by the current build constraints")); + } + let selected: HashSet<_> = test_names(&std::fs::read_to_string(file)?).into_iter().collect(); + let mut excluded = Vec::new(); + for name in files { + if Some(std::ffi::OsStr::new(name)) != file.file_name() { + for test in test_names(&std::fs::read_to_string(dir.join(name))?) { + if selected.contains(&test) { + return Err(IdeError::invalid(format!( + "Cannot select this file alone: {test} is also declared in {name}. Run the package instead." + ))); + } + if !selected.contains(&test) { + excluded.push(test); + } + } + } + } + excluded.sort(); + excluded.dedup(); + Ok(if excluded.is_empty() { String::new() } else { format!("^({})$", excluded.join("|")) }) +} + +fn test_names(text: &str) -> Vec { + static SYNTAX: OnceLock = OnceLock::new(); + SYNTAX + .get_or_init(Default::default) + .outline(&ide_core::LanguageId("go".into()), text) + .into_iter() + .filter(|item| item.kind == ide_core::SymbolKind::Function) + .map(|item| item.name) + .filter(|name| { + name != "TestMain" + && ["Test", "Benchmark", "Example", "Fuzz"].iter().any(|prefix| { + name.strip_prefix(prefix) + .is_some_and(|suffix| suffix.chars().next().is_none_or(|c| !c.is_lowercase())) + }) + }) + .collect() +} + +pub fn exclude_tests(args: &mut Vec, exclusions: &str, binary: bool) { + if exclusions.is_empty() { + return; + } + let original = std::mem::take(args); + let mut existing = None; + let mut program = binary; + let mut insert_at = binary.then_some(0); + let mut i = 0; + while i < original.len() { + let arg = &original[i]; + if arg == "--" { + insert_at.get_or_insert(args.len()); + args.extend_from_slice(&original[i..]); + break; + } + if arg == "-test.skip" || (!program && arg == "-skip") { + existing = original.get(i + 1).cloned(); + i += 2; + continue; + } + if let Some(value) = + arg.strip_prefix("-test.skip=").or_else(|| if program { None } else { arg.strip_prefix("-skip=") }) + { + existing = Some(value.to_owned()); + i += 1; + continue; + } + args.push(arg.clone()); + if arg == "-args" { + program = true; + insert_at = Some(args.len()); + } + i += 1; + } + let pattern = existing + .filter(|old| !old.is_empty()) + .map_or_else(|| exclusions.to_owned(), |old| format!("{old}|{exclusions}")); + let flag = if program { "-test.skip" } else { "-skip" }; + let at = insert_at.unwrap_or(args.len()); + args.splice(at..at, [flag.into(), pattern]); } pub fn prepare_test_file(root: &Path, config: &mut RunConfig) -> IdeResult<()> { @@ -158,14 +258,14 @@ pub fn prepare_test_file(root: &Path, config: &mut RunConfig) -> IdeResult<()> { let dir = file.parent().ok_or_else(|| IdeError::invalid("Go test file has no directory"))?; let mut command = Command::new(&config.command); crate::process::configure_environment(&mut command, root, config)?; - let mut files = package_sources(command, dir, &config.args)?; - files.push(file); + let exclusions = file_test_exclusions(command, &file, &config.args)?; let at = config .args .iter() .position(|a| a == target) .ok_or_else(|| IdeError::invalid("Go test file target is missing"))?; - config.args.splice(at..=at, files.iter().map(|p| p.to_string_lossy().into_owned())); + config.args[at] = dunce::canonicalize(dir)?.to_string_lossy().into_owned(); + exclude_tests(&mut config.args, &exclusions, false); Ok(()) } @@ -290,3 +390,52 @@ fn quoted_work_path(chars: &mut std::iter::Peekable>, quote: } None } + +#[cfg(test)] +mod test_scope_tests { + use super::*; + + #[test] + fn finds_declarations_without_requiring_a_testing_import_spelling() { + assert_eq!( + test_names( + "package p\nimport t \"testing\"\n// func TestComment(t *t.T) {}\nvar s = `func TestString(t *t.T) {}`\nfunc TestAlias(\n t *t.T,\n) {}\nfunc Benchmark速度(b *t.B) {}\nfunc (s Suite) TestMethod(t *t.T) {}\n" + ), + ["TestAlias", "Benchmark速度"] + ); + } + + #[test] + fn rejects_names_shared_by_internal_and_external_test_packages() { + if Command::new("go").arg("version").output().is_err() { + return; + } + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("go.mod"), "module example.com/ambiguous\ngo 1.22\n").unwrap(); + let file = dir.path().join("internal_test.go"); + std::fs::write(&file, "package p\nimport \"testing\"\nfunc TestSame(t *testing.T) {}\n").unwrap(); + std::fs::write( + dir.path().join("external_test.go"), + "package p_test\nimport \"testing\"\nfunc TestSame(t *testing.T) {}\n", + ) + .unwrap(); + let error = file_test_exclusions(Command::new("go"), &file, &[]).unwrap_err(); + assert!(error.to_string().contains("Run the package instead"), "{error}"); + } + + #[test] + fn exclusions_preserve_user_filters_and_precede_binary_argument_terminators() { + for (input, expected) in [ + (vec!["test", ".", "-run", "TestChosen"], vec!["test", ".", "-run", "TestChosen", "-skip", "^TestOther$"]), + (vec!["test", ".", "-skip="], vec!["test", ".", "-skip", "^TestOther$"]), + ( + vec!["test", ".", "-skip", "Old", "-args", "-test.skip=Newest", "--", "data"], + vec!["test", ".", "-args", "-test.skip", "Newest|^TestOther$", "--", "data"], + ), + ] { + let mut args: Vec = input.into_iter().map(str::to_owned).collect(); + exclude_tests(&mut args, "^TestOther$", false); + assert_eq!(args, expected); + } + } +} diff --git a/crates/ide-run/tests/process.rs b/crates/ide-run/tests/process.rs index 20619efe6..a382701ad 100644 --- a/crates/ide-run/tests/process.rs +++ b/crates/ide-run/tests/process.rs @@ -643,12 +643,12 @@ fn go_file_tests_use_build_selected_sources_and_only_the_selected_test_file() { std::fs::write(h.root.path().join("generate.go"), "//go:build ignore\n\npackage main\nfunc main() {}\n").unwrap(); std::fs::write( h.root.path().join("demo_test.go"), - "package filetests\nimport \"testing\"\nfunc TestChosen(t *testing.T) { if value()!=2 { t.Fatal(\"bad\") } }\n", + "package filetests\nimport \"testing\"\nfunc TestChosen(t *testing.T) { if helper()!=2 { t.Fatal(\"bad\") } }\n", ) .unwrap(); std::fs::write( h.root.path().join("other_test.go"), - "package filetests\nimport \"testing\"\nfunc TestOther(t *testing.T) { t.Fatal(\"must not run\") }\n", + "package filetests\nimport (\"testing\"; \"os\")\nvar initialized int\nfunc TestMain(m *testing.M) { initialized=value(); os.Exit(m.Run()) }\nfunc helper() int { return initialized }\nfunc TestOther(t *testing.T) { t.Fatal(\"must not run\") }\n", ) .unwrap(); let mut c = cfg("go-file", &go.to_string_lossy(), &["test", "-json", "demo_test.go"]); @@ -661,7 +661,30 @@ fn go_file_tests_use_build_selected_sources_and_only_the_selected_test_file() { ide_run::go::prepare_test_file(h.root.path(), &mut c).unwrap(); assert!(!c.args.iter().any(|a| a.ends_with("generate.go") || a.ends_with("other_test.go"))); let (id, out) = start(&h, &c); - assert_eq!(wait_exit(&h, id, Duration::from_secs(60)), Some(0), "{}", text(&out, stderr)); + assert_eq!( + wait_exit(&h, id, Duration::from_secs(60)), + Some(0), + "{} {} {:?}", + text(&out, stderr), + text(&out, stdout), + c.args + ); assert!(text(&out, stdout).contains("TestChosen")); assert!(!text(&out, stdout).contains("TestOther")); + std::fs::write( + h.root.path().join("external_test.go"), + "package filetests_test\nimport t \"testing\"\nfunc TestExternal(\n t *t.T,\n) { if externalHelper()!=3 {t.Fail()} }\n", + ).unwrap(); + std::fs::write( + h.root.path().join("external_helpers_test.go"), + "package filetests_test\nimport \"testing\"\nfunc externalHelper() int {return 3}\nfunc TestOutside(t *testing.T) {t.Fatal(\"must not run\")}\n", + ).unwrap(); + c.args = vec!["test".into(), "-json".into(), "external_test.go".into()]; + c.extras.params.insert("target".into(), "external_test.go".into()); + ide_run::go::prepare_test_file(h.root.path(), &mut c).unwrap(); + let (id, out) = start(&h, &c); + assert_eq!(wait_exit(&h, id, Duration::from_secs(60)), Some(0), "{} {}", text(&out, stderr), text(&out, stdout)); + assert!(text(&out, stdout).contains("TestExternal")); + assert!(!text(&out, stdout).contains("TestChosen")); + assert!(!text(&out, stdout).contains("TestOutside")); } diff --git a/crates/ide-sdk/src/go.rs b/crates/ide-sdk/src/go.rs index d6a815d3e..e29fce2a3 100644 --- a/crates/ide-sdk/src/go.rs +++ b/crates/ide-sdk/src/go.rs @@ -129,6 +129,8 @@ fn delve_build_flags(args: &[String], files: bool) -> Value { pub fn apply_to_debug( env: &mut std::collections::BTreeMap, + program: &mut Option, + program_args: &mut Vec, extra_json: &mut Option, tags: &GoBuildTags, ) -> ide_core::IdeResult<()> { @@ -166,13 +168,17 @@ pub fn apply_to_debug( } if let Some(files) = extra.get("__goFiles").and_then(Value::as_array) { if extra["mode"] == "test" { - if let Some(dir) = files.first().and_then(Value::as_str).and_then(|f| std::path::Path::new(f).parent()) - { + if let Some(file) = files.first().and_then(Value::as_str).map(std::path::Path::new) { let mut command = std::process::Command::new("go"); ide_core::process::host_env(&mut command); command.envs(env.iter()); - let sources = ide_run::go::package_sources(command, dir, &args)?; - args.extend(sources.iter().map(|p| p.to_string_lossy().into_owned())); + let exclusions = ide_run::go::file_test_exclusions(command, file, &args)?; + *program = file + .parent() + .map(dunce::canonicalize) + .transpose()? + .map(|dir| dir.to_string_lossy().into_owned()); + ide_run::go::exclude_tests(program_args, &exclusions, true); } } else { args.extend(files.iter().skip(1).filter_map(Value::as_str).map(str::to_owned)); @@ -180,7 +186,7 @@ pub fn apply_to_debug( } // Delve appends its -o after array build flags; positional source files // require the string form so all compiler flags precede the file list. - extra["buildFlags"] = delve_build_flags(&args, extra.get("__goFiles").is_some()); + extra["buildFlags"] = delve_build_flags(&args, extra.get("__goFiles").is_some() && extra["mode"] != "test"); } if let Some(output) = extra.get("output").and_then(Value::as_str).and_then(|p| std::path::Path::new(p).parent()) { std::fs::create_dir_all(output)?; @@ -199,6 +205,37 @@ mod command_tests { use super::*; use std::collections::BTreeMap; + #[test] + fn file_tests_debug_the_real_package_and_keep_sibling_test_helpers() { + if crate::exec::which("go").is_none() { + return; + } + let dir = tempfile::tempdir().unwrap(); + let root = dunce::canonicalize(dir.path()).unwrap(); + std::fs::write(root.join("go.mod"), "module example.com/scoped\ngo 1.22\n").unwrap(); + std::fs::write( + root.join("chosen_test.go"), + "package scoped\nimport t \"testing\"\nfunc TestChosen(\n t *t.T,\n) { helper() }\n", + ) + .unwrap(); + std::fs::write( + root.join("helpers_test.go"), + "package scoped\nimport \"testing\"\nfunc helper() {}\nfunc TestOther(t *testing.T) {}\n", + ) + .unwrap(); + let mut env = BTreeMap::from([("GOWORK".into(), "off".into())]); + let mut program = Some(root.join("chosen_test.go").to_string_lossy().into_owned()); + let mut args = vec!["-test.run".into(), "^TestChosen$".into(), "-test.skip=Old".into()]; + let mut extra = + Some(json!({"mode":"test","__goToolArgs":[],"__goFiles":[program.as_ref().unwrap()]}).to_string()); + apply_to_debug(&mut env, &mut program, &mut args, &mut extra, &GoBuildTags::default()).unwrap(); + assert_eq!(program.as_deref(), root.to_str()); + assert_eq!(args, ["-test.skip", "Old|^(TestOther)$", "-test.run", "^TestChosen$"]); + let extra: Value = serde_json::from_str(extra.as_deref().unwrap()).unwrap(); + assert_eq!(extra["buildFlags"], json!([])); + assert!(extra.get("__goFiles").is_none()); + } + #[test] fn merges_custom_tags_without_touching_program_arguments() { let mut flags = vec!["-tags".into(), "explicit".into(), ".".into(), "-args".into(), "-tags=program".into()]; @@ -219,12 +256,12 @@ mod command_tests { let mut env = BTreeMap::from([("GOOS".into(), "darwin".into())]); let mut extra = Some(json!({"mode":"test", "__goUseTags":true, "__goToolArgs":["-tags", "explicit"]}).to_string()); - apply_to_debug(&mut env, &mut extra, &tags).unwrap(); + apply_to_debug(&mut env, &mut None, &mut Vec::new(), &mut extra, &tags).unwrap(); let extra: Value = serde_json::from_str(extra.as_deref().unwrap()).unwrap(); assert_eq!(extra["buildFlags"], json!(["-tags=project,explicit"])); assert!(extra.get("__goUseTags").is_none()); assert_eq!(env["GOOS"], "darwin"); - assert!(apply_to_debug(&mut env, &mut Some("{".into()), &tags).is_err()); + assert!(apply_to_debug(&mut env, &mut None, &mut Vec::new(), &mut Some("{".into()), &tags).is_err()); } }