Skip to content

Commit 596404a

Browse files
committed
enahnce the junit file attribute script to handle nextest setup scripts
1 parent f35bf6d commit 596404a

File tree

3 files changed

+230
-57
lines changed

3 files changed

+230
-57
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tools/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ clap = { version = "4.0", features = ["derive"] }
2121
colored = "2"
2222
quick-xml = "0.37"
2323
regex = "1"
24+
toml = "0.8"
2425
wait-timeout = "0.2"
2526

2627
[[bin]]

tools/src/junit_file_attributes.rs

Lines changed: 228 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use cargo_metadata::{CargoOpt, MetadataCommand};
1111
use quick_xml::events::{BytesStart, Event};
1212
use quick_xml::{Reader, Writer};
1313
use std::collections::HashMap;
14+
use std::fs;
1415
use std::io::Cursor;
1516
use std::path::{Path, PathBuf};
1617

@@ -67,12 +68,39 @@ impl TargetLookup {
6768
self.targets.insert((package_name, target_name), src_path);
6869
}
6970

71+
/// Insert a binary target.
72+
pub fn insert_bin(&mut self, package_name: &str, target_name: &str, src_path: PathBuf) {
73+
let package_name = Self::normalize(package_name);
74+
let target_name = Self::normalize(target_name);
75+
self.targets.insert((package_name, target_name), src_path);
76+
}
77+
78+
/// Insert a setup script entry.
79+
/// Uses "@setup-script" as the package name so it can be looked up from
80+
/// classnames like "@setup-script:prebuild-bin-tests".
81+
fn insert_setup_script(&mut self, script_name: &str, src_path: PathBuf) {
82+
// Don't normalize - script names can have hyphens and we want exact match
83+
self.targets.insert(
84+
("@setup-script".to_string(), script_name.to_string()),
85+
src_path,
86+
);
87+
}
88+
7089
/// Look up a target's source path.
7190
///
7291
/// - If `target_name` is `Some`, looks up directly by `(package, target)`
7392
/// - If `target_name` is `None`, tries `(package, package)` first, then checks the alias map
7493
/// for packages where lib name differs from package name
7594
pub fn get(&self, package_name: &str, target_name: Option<&str>) -> Option<&PathBuf> {
95+
// Setup scripts are a bit of a special case. They use "@setup-script" as package name and
96+
// shouldn't be normalized
97+
if package_name == "@setup-script" {
98+
let target_name = target_name?;
99+
return self
100+
.targets
101+
.get(&(package_name.to_string(), target_name.to_string()));
102+
}
103+
76104
let package_name = Self::normalize(package_name);
77105
match target_name {
78106
Some(name) => {
@@ -118,25 +146,106 @@ pub fn build_target_lookup(manifest_path: Option<&Path>) -> Result<(TargetLookup
118146
for target in package.targets {
119147
let kind = target.kind.first().map(|s| s.as_str()).unwrap_or("unknown");
120148

121-
// Only interested in lib-like (lib, proc-macro) and test targets
122-
let is_lib_like = kind == "lib" || kind == "proc-macro";
123-
if !is_lib_like && kind != "test" {
124-
continue;
125-
}
126-
127149
let src_path = target.src_path.into_std_path_buf();
128150

129-
if is_lib_like {
130-
lookup.insert_lib(&package.name, &target.name, src_path);
131-
} else {
132-
lookup.insert_test(&package.name, &target.name, src_path);
151+
match kind {
152+
"lib" | "proc-macro" => {
153+
lookup.insert_lib(&package.name, &target.name, src_path);
154+
}
155+
"test" => {
156+
lookup.insert_test(&package.name, &target.name, src_path);
157+
}
158+
"bin" => {
159+
lookup.insert_bin(&package.name, &target.name, src_path);
160+
}
161+
_ => {}
133162
}
134163
}
135164
}
136165

166+
// Load setup scripts from nextest.toml if it exists
167+
load_setup_scripts(&workspace_root, &mut lookup);
168+
137169
Ok((lookup, workspace_root))
138170
}
139171

172+
/// Load setup scripts from nextest.toml and add them to the lookup.
173+
fn load_setup_scripts(workspace_root: &Path, lookup: &mut TargetLookup) {
174+
let nextest_path = workspace_root.join(".config/nextest.toml");
175+
let content = match fs::read_to_string(&nextest_path) {
176+
Ok(c) => c,
177+
Err(_) => return, // No nextest.toml, nothing to do
178+
};
179+
180+
let table: toml::Table = match content.parse() {
181+
Ok(t) => t,
182+
Err(_) => return, // Invalid TOML, skip
183+
};
184+
185+
// Look for [script.X] sections
186+
let script_section = match table.get("script").and_then(|v| v.as_table()) {
187+
Some(s) => s,
188+
None => return,
189+
};
190+
191+
for (script_name, script_value) in script_section {
192+
let command = match script_value.get("command").and_then(|v| v.as_str()) {
193+
Some(c) => c,
194+
None => continue,
195+
};
196+
197+
// Parse "cargo run -p <package> --bin <binary>" to get package and binary
198+
if let Some((package, binary)) = parse_cargo_run_command(command) {
199+
// Look up the binary's source path and insert as setup script
200+
if let Some(src_path) = lookup.get(&package, Some(&binary)).cloned() {
201+
lookup.insert_setup_script(script_name, src_path);
202+
}
203+
}
204+
}
205+
}
206+
207+
/// Parse a cargo run command to extract package and binary names.
208+
/// Expected format: "cargo run -p <package> --bin <binary>" (flags can be in any order)
209+
fn parse_cargo_run_command(command: &str) -> Option<(String, String)> {
210+
let parts: Vec<&str> = command.split_whitespace().collect();
211+
212+
// Must start with "cargo run"
213+
if parts.len() < 2 || parts[0] != "cargo" || parts[1] != "run" {
214+
return None;
215+
}
216+
217+
let mut package = None;
218+
let mut binary = None;
219+
let mut i = 2;
220+
221+
while i < parts.len() {
222+
match parts[i] {
223+
"-p" | "--package" => {
224+
if i + 1 < parts.len() {
225+
package = Some(parts[i + 1].to_string());
226+
i += 2;
227+
} else {
228+
i += 1;
229+
}
230+
}
231+
"--bin" => {
232+
if i + 1 < parts.len() {
233+
binary = Some(parts[i + 1].to_string());
234+
i += 2;
235+
} else {
236+
i += 1;
237+
}
238+
}
239+
_ => i += 1,
240+
}
241+
}
242+
243+
match (package, binary) {
244+
(Some(p), Some(b)) => Some((p, b)),
245+
_ => None,
246+
}
247+
}
248+
140249
/// Process the JUnit XML and add file attributes to testcase elements.
141250
pub fn process_junit_xml(
142251
xml: &str,
@@ -209,9 +318,41 @@ fn add_file_to_testcase(
209318
new_elem
210319
}
211320

212-
/// Normalize path separators to forward slashes for cross-platform CODEOWNERS compatibility
213-
fn normalize_path_separators(path: &str) -> String {
214-
path.replace('\\', "/")
321+
/// Convert an absolute path to a workspace-relative path with normalized separators
322+
fn to_relative_path(path: &Path, workspace_root: &Path) -> String {
323+
let relative = path.strip_prefix(workspace_root).unwrap_or(path);
324+
// Normalize path separators to forward slashes for cross-platform CODEOWNERS compatibility
325+
relative.to_string_lossy().replace('\\', "/")
326+
}
327+
328+
/// Resolve file path for a setup script
329+
fn resolve_setup_script_path(
330+
script_name: &str,
331+
targets: &TargetLookup,
332+
workspace_root: &Path,
333+
) -> Option<String> {
334+
let src_path = targets.get("@setup-script", Some(script_name))?;
335+
Some(to_relative_path(src_path, workspace_root))
336+
}
337+
338+
/// Resolve file path for a unit test, trying to find the specific module file
339+
fn resolve_unit_test_path(
340+
src_path: &Path,
341+
testcase_name: Option<&str>,
342+
workspace_root: &Path,
343+
) -> String {
344+
// Try to find the module file from the test name
345+
// Test name is like "trace_utils::tests::test_compute_top_level"
346+
// We want to find src/trace_utils.rs or src/trace_utils/mod.rs
347+
if let Some(test_name) = testcase_name {
348+
if let Some(src_dir) = src_path.parent() {
349+
if let Some(file_path) = resolve_module_file(test_name, src_dir, workspace_root) {
350+
return file_path;
351+
}
352+
}
353+
}
354+
// Fallback to lib.rs
355+
to_relative_path(src_path, workspace_root)
215356
}
216357

217358
/// Resolve a file path from classname using cargo metadata
@@ -223,45 +364,29 @@ fn resolve_file_path(
223364
) -> Option<String> {
224365
let classname = classname?;
225366

226-
// Split classname into parts: e.g., "libdd-trace-utils::test_send_data" ->
227-
// ["libdd-trace-utils", "test_send_data"]
228-
let parts: Vec<&str> = classname.split("::").collect();
229-
if parts.is_empty() {
230-
return None;
367+
// Handle setup scripts: classname is "@setup-script:script_name"
368+
if let Some(script_name) = classname.strip_prefix("@setup-script:") {
369+
return resolve_setup_script_path(script_name, targets, workspace_root);
231370
}
232371

233-
// For integration tests, classname is "package::target_name"
234-
// For unit tests, classname is just "package"
235-
let package_name = parts[0];
372+
// Parse classname: "package::target" for integration tests, "package" for unit tests
373+
let parts: Vec<&str> = classname.split("::").collect();
374+
let package_name = parts.first()?;
236375
let target_name = parts.get(1).copied();
237376

238-
if let Some(src_path) = targets.get(package_name, target_name) {
239-
// For integration tests, return the test file directly
240-
if target_name.is_some() {
241-
let relative = src_path.strip_prefix(workspace_root).unwrap_or(src_path);
242-
return Some(normalize_path_separators(
243-
relative.to_string_lossy().as_ref(),
244-
));
245-
}
246-
247-
// For unit tests, try to find the module file from the test name
248-
// Test name is like "trace_utils::tests::test_compute_top_level"
249-
// We want to find src/trace_utils.rs or src/trace_utils/mod.rs
250-
let src_dir = src_path.parent()?;
251-
if let Some(test_name) = testcase_name {
252-
if let Some(file_path) = resolve_module_file(test_name, src_dir, workspace_root) {
253-
return Some(file_path);
254-
}
255-
}
256-
257-
// Fallback to lib.rs
258-
let relative = src_path.strip_prefix(workspace_root).unwrap_or(src_path);
259-
return Some(normalize_path_separators(
260-
relative.to_string_lossy().as_ref(),
261-
));
377+
let src_path = targets.get(package_name, target_name)?;
378+
379+
if target_name.is_some() {
380+
// Integration test - return the test file directly
381+
Some(to_relative_path(src_path, workspace_root))
382+
} else {
383+
// Unit test - try to find module file, fallback to lib.rs
384+
Some(resolve_unit_test_path(
385+
src_path,
386+
testcase_name,
387+
workspace_root,
388+
))
262389
}
263-
264-
None
265390
}
266391

267392
/// Try to find the source file for a unit test based on its module path
@@ -284,21 +409,13 @@ fn resolve_module_file(test_name: &str, src_dir: &Path, workspace_root: &Path) -
284409
// Try module_path.rs (e.g., src/trace_utils.rs or src/span/trace_utils.rs)
285410
let module_file = src_dir.join(format!("{}.rs", module_parts.join("/")));
286411
if module_file.exists() {
287-
let relative = module_file
288-
.strip_prefix(workspace_root)
289-
.unwrap_or(&module_file);
290-
return Some(normalize_path_separators(
291-
relative.to_string_lossy().as_ref(),
292-
));
412+
return Some(to_relative_path(&module_file, workspace_root));
293413
}
294414

295415
// Try module_path/mod.rs (e.g., src/trace_utils/mod.rs)
296416
let mod_file = src_dir.join(format!("{}/mod.rs", module_parts.join("/")));
297417
if mod_file.exists() {
298-
let relative = mod_file.strip_prefix(workspace_root).unwrap_or(&mod_file);
299-
return Some(normalize_path_separators(
300-
relative.to_string_lossy().as_ref(),
301-
));
418+
return Some(to_relative_path(&mod_file, workspace_root));
302419
}
303420

304421
None
@@ -392,4 +509,58 @@ mod tests {
392509

393510
assert!(result.contains(r#"file="my-crate/tests/integration_test.rs""#));
394511
}
512+
513+
#[test]
514+
fn test_process_junit_xml_setup_script() {
515+
let mut lookup = TargetLookup::new();
516+
// Simulate what load_setup_scripts does - insert with @setup-script as package
517+
lookup.insert_setup_script(
518+
"prebuild-bin-tests",
519+
PathBuf::from("/workspace/bin_tests/src/bin/prebuild.rs"),
520+
);
521+
522+
let input = r#"<?xml version="1.0" encoding="UTF-8"?>
523+
<testsuites>
524+
<testsuite name="@setup-script:prebuild-bin-tests">
525+
<testcase classname="@setup-script:prebuild-bin-tests" name="prebuild-bin-tests" />
526+
</testsuite>
527+
</testsuites>"#;
528+
529+
let workspace_root = PathBuf::from("/workspace");
530+
let result = process_junit_xml(input, &lookup, &workspace_root).unwrap();
531+
532+
assert!(result.contains(r#"file="bin_tests/src/bin/prebuild.rs""#));
533+
}
534+
535+
#[test]
536+
fn test_parse_cargo_run_command() {
537+
// Standard format
538+
let result = parse_cargo_run_command("cargo run -p bin_tests --bin prebuild");
539+
assert_eq!(
540+
result,
541+
Some(("bin_tests".to_string(), "prebuild".to_string()))
542+
);
543+
544+
// Different flag order
545+
let result = parse_cargo_run_command("cargo run --bin prebuild -p bin_tests");
546+
assert_eq!(
547+
result,
548+
Some(("bin_tests".to_string(), "prebuild".to_string()))
549+
);
550+
551+
// With --package instead of -p
552+
let result = parse_cargo_run_command("cargo run --package bin_tests --bin prebuild");
553+
assert_eq!(
554+
result,
555+
Some(("bin_tests".to_string(), "prebuild".to_string()))
556+
);
557+
558+
// Missing binary
559+
let result = parse_cargo_run_command("cargo run -p bin_tests");
560+
assert_eq!(result, None);
561+
562+
// Not a cargo run command
563+
let result = parse_cargo_run_command("cargo build -p bin_tests");
564+
assert_eq!(result, None);
565+
}
395566
}

0 commit comments

Comments
 (0)