From 40da3b12def596d1c04e14590c625c239d75c9b6 Mon Sep 17 00:00:00 2001 From: Julian Howes Date: Thu, 30 Jul 2026 11:21:33 +0200 Subject: [PATCH 1/9] Emit JUnit XML from rust_test Until now a rust_test never wrote to $XML_OUTPUT_FILE, so Bazel fell back to synthesising a single generic for the whole binary. That makes every Rust test show up under the same name in tools that read the XML (test UIs, CI test reporting), which isn't much use once a suite has more than a handful of tests. Wrap the test binary in a small runner that runs it, echoes its output through unchanged, and turns libtest's textual results into a real per-test JUnit report. When $XML_OUTPUT_FILE isn't set (e.g. under `bazel run`) the runner just execs the test binary, so there's no overhead outside of `bazel test`. The wrapper is on by default and can be turned off per-target with `junit = False`. --- rust/private/rust.bzl | 42 ++ rust/runfiles/BUILD.bazel | 6 + test/junit/BUILD.bazel | 14 + test/junit/lib.rs | 11 + util/collect_coverage/collect_coverage.rs | 56 ++- util/junit_runner/BUILD.bazel | 20 + util/junit_runner/junit_runner.rs | 469 ++++++++++++++++++++++ 7 files changed, 606 insertions(+), 12 deletions(-) create mode 100644 test/junit/BUILD.bazel create mode 100644 test/junit/lib.rs create mode 100644 util/junit_runner/BUILD.bazel create mode 100644 util/junit_runner/junit_runner.rs diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 0697c6c2f8..3262750fe8 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -598,6 +598,32 @@ def _rust_test_impl(ctx): env["CC_CODE_COVERAGE_SCRIPT"] = ctx.executable._collect_cc_coverage.path components = "{}/{}".format(ctx.label.workspace_root, ctx.label.package).split("/") env["CARGO_MANIFEST_DIR"] = "/".join([c for c in components if c]) + + if ctx.attr.junit: + test_bin_short = output.short_path + if test_bin_short.startswith("../"): + rust_test_bin_rloc = test_bin_short[len("../"):] + else: + rust_test_bin_rloc = ctx.workspace_name + "/" + test_bin_short + env["RUST_TEST_BIN"] = rust_test_bin_rloc + + junit_runner = ctx.actions.declare_file(ctx.label.name + "_junit_runner" + toolchain.binary_ext) + ctx.actions.symlink( + output = junit_runner, + target_file = ctx.executable._junit_runner, + is_executable = True, + ) + + original_default_info = providers[0] + runner_runfiles = ctx.attr._junit_runner[DefaultInfo].default_runfiles + test_bin_runfiles = ctx.runfiles(files = [output]) + merged_runfiles = original_default_info.default_runfiles.merge(runner_runfiles).merge(test_bin_runfiles) + providers[0] = DefaultInfo( + files = original_default_info.files, + runfiles = merged_runfiles, + executable = junit_runner, + ) + providers.append(RunEnvironmentInfo( environment = env, inherited_environment = ctx.attr.env_inherit, @@ -996,6 +1022,22 @@ _RUST_TEST_ATTRS = { E.g. `bazel test //src:rust_test --test_arg=foo::test::test_fn`. """), ), + "junit": attr.bool( + default = True, + doc = dedent("""\ + If True (default), wrap the test binary with a JUnit XML runner that + parses libtest output and writes JUnit XML to `$XML_OUTPUT_FILE` when + run under `bazel test`. When `$XML_OUTPUT_FILE` is not set (e.g. + `bazel run`), the runner execs the test binary directly with zero + overhead. Set to False to bypass the wrapper entirely (useful for + debugging or attaching a debugger). + """), + ), + "_junit_runner": attr.label( + default = Label("//util/junit_runner"), + executable = True, + cfg = "target", + ), } | _COVERAGE_ATTRS | _EXPERIMENTAL_USE_CC_COMMON_LINK_ATTRS rust_library = rule( diff --git a/rust/runfiles/BUILD.bazel b/rust/runfiles/BUILD.bazel index 695fa918f9..98e1e64437 100644 --- a/rust/runfiles/BUILD.bazel +++ b/rust/runfiles/BUILD.bazel @@ -16,6 +16,12 @@ rust_test( name = "runfiles_test", crate = ":runfiles", data = ["data/sample.txt"], + # These tests inspect the process's own runfiles -- the manifest layout and + # the repo mapping that rlocation! resolves against. The junit wrapper runs + # the test as a child under the wrapper's runfiles, which is a different + # ambient environment than the one being asserted on, so the runfiles + # library's own tests need to run unwrapped. + junit = False, ) rust_doc( diff --git a/test/junit/BUILD.bazel b/test/junit/BUILD.bazel new file mode 100644 index 0000000000..1fed7c6ce8 --- /dev/null +++ b/test/junit/BUILD.bazel @@ -0,0 +1,14 @@ +load("//rust:defs.bzl", "rust_test") + +rust_test( + name = "junit_test", + srcs = ["lib.rs"], + edition = "2021", +) + +rust_test( + name = "no_junit_test", + srcs = ["lib.rs"], + edition = "2021", + junit = False, +) diff --git a/test/junit/lib.rs b/test/junit/lib.rs new file mode 100644 index 0000000000..38b2fa80fa --- /dev/null +++ b/test/junit/lib.rs @@ -0,0 +1,11 @@ +#[cfg(test)] +mod tests { + #[test] + fn test_passing() { + assert_eq!(2 + 2, 4); + } + + #[test] + #[ignore] + fn test_ignored() {} +} diff --git a/util/collect_coverage/collect_coverage.rs b/util/collect_coverage/collect_coverage.rs index 7fbe08a35b..b3bc0271e9 100644 --- a/util/collect_coverage/collect_coverage.rs +++ b/util/collect_coverage/collect_coverage.rs @@ -129,7 +129,9 @@ fn main() { None => debug_log!("RUNFILES_DIR: not set (split coverage postprocessing)"), } - let coverage_output_file = coverage_dir.join("coverage.dat"); + let coverage_output_file = env::var("COVERAGE_OUTPUT_FILE") + .map(PathBuf::from) + .unwrap_or_else(|_| coverage_dir.join("coverage.dat")); let profdata_file = coverage_dir.join("coverage.profdata"); let llvm_cov_path = env::var("RUST_LLVM_COV").unwrap(); let llvm_profdata_path = env::var("RUST_LLVM_PROFDATA").unwrap(); @@ -141,15 +143,43 @@ fn main() { Some(ref rd) => find_metadata_file(&execroot, rd, &llvm_profdata_path), None => execroot.join(&llvm_profdata_path), }; - let test_binary = match runfiles_dir { - Some(ref rd) => find_test_binary(&execroot, rd), - None => { - let bin_dir = config_bin_dir(&execroot, &coverage_dir); - let test_binary = execroot - .join(bin_dir) - .join(env::var("TEST_BINARY").unwrap()); - debug_log!("Resolved TEST_BINARY to: {}", test_binary.display()); - test_binary + // When the JUnit runner wraps the test, TEST_BINARY points to the runner + // and RUST_TEST_BIN holds the actual instrumented binary that llvm-cov needs. + let test_binary = if let Ok(rust_test_bin) = env::var("RUST_TEST_BIN") { + debug_log!("Using RUST_TEST_BIN: {}", rust_test_bin); + match runfiles_dir { + Some(ref rd) => { + let candidate = rd + .join(env::var("TEST_WORKSPACE").unwrap_or_default()) + .join(&rust_test_bin); + if candidate.exists() { + candidate + } else { + let candidate = rd.join(&rust_test_bin); + if candidate.exists() { + candidate + } else { + let bin_dir = config_bin_dir(&execroot, &coverage_dir); + execroot.join(bin_dir).join(&rust_test_bin) + } + } + } + None => { + let bin_dir = config_bin_dir(&execroot, &coverage_dir); + execroot.join(bin_dir).join(&rust_test_bin) + } + } + } else { + match runfiles_dir { + Some(ref rd) => find_test_binary(&execroot, rd), + None => { + let bin_dir = config_bin_dir(&execroot, &coverage_dir); + let test_binary = execroot + .join(bin_dir) + .join(env::var("TEST_BINARY").unwrap()); + debug_log!("Resolved TEST_BINARY to: {}", test_binary.display()); + test_binary + } } }; let profraw_files: Vec = fs::read_dir(coverage_dir) @@ -199,7 +229,8 @@ fn main() { .arg("-format=lcov") .arg("-instr-profile") .arg(&profdata_file) - .arg("-ignore-filename-regex=.*external/.+") + .arg(r"-ignore-filename-regex=.*external[/\\].+") + .arg(r"-ignore-filename-regex=.*rustc[/\\].+") .arg("-ignore-filename-regex=/tmp/.+") .arg(format!("-path-equivalence=.,{}", execroot.display())) .arg(test_binary) @@ -234,7 +265,8 @@ fn main() { coverage_output_file, report_str .replace("#/proc/self/cwd/", "") - .replace(&execroot.display().to_string(), ""), + .replace(&execroot.display().to_string(), "") + .replace('\\', "/"), ) .unwrap(); diff --git a/util/junit_runner/BUILD.bazel b/util/junit_runner/BUILD.bazel new file mode 100644 index 0000000000..799ae41656 --- /dev/null +++ b/util/junit_runner/BUILD.bazel @@ -0,0 +1,20 @@ +load("//rust:defs.bzl", "rust_binary", "rust_test") + +rust_binary( + name = "junit_runner", + srcs = ["junit_runner.rs"], + edition = "2021", + rustc_flags = select({ + "@platforms//os:linux": ["-Cstrip=debuginfo"], + "@platforms//os:macos": ["-Cstrip=symbols"], + "//conditions:default": [], + }), + visibility = ["//visibility:public"], +) + +rust_test( + name = "junit_runner_test", + srcs = ["junit_runner.rs"], + edition = "2021", + junit = False, +) diff --git a/util/junit_runner/junit_runner.rs b/util/junit_runner/junit_runner.rs new file mode 100644 index 0000000000..a32ba9c26a --- /dev/null +++ b/util/junit_runner/junit_runner.rs @@ -0,0 +1,469 @@ +use std::collections::HashMap; +use std::env; +use std::fs; +use std::io::{BufRead, BufReader, Write}; +use std::path::PathBuf; +use std::process::{Command, Stdio}; + +fn resolve_runfiles(rlocation_path: &str) -> PathBuf { + if let Ok(manifest) = env::var("RUNFILES_MANIFEST_FILE") { + if let Ok(contents) = fs::read_to_string(&manifest) { + let prefix = format!("{} ", rlocation_path); + for line in contents.lines() { + if let Some(abs_path) = line.strip_prefix(&prefix) { + let p = PathBuf::from(abs_path); + if p.exists() { + return p; + } + } + } + } + } + + if let Ok(dir) = env::var("RUNFILES_DIR") { + let candidate = PathBuf::from(&dir).join(rlocation_path); + if candidate.exists() { + return candidate; + } + } + + if let Ok(dir) = env::var("TEST_SRCDIR") { + let candidate = PathBuf::from(&dir).join(rlocation_path); + if candidate.exists() { + return candidate; + } + } + + eprintln!( + "ERROR: junit_runner: cannot resolve runfiles path: {}", + rlocation_path + ); + eprintln!(" RUNFILES_MANIFEST_FILE={:?}", env::var("RUNFILES_MANIFEST_FILE").ok()); + eprintln!(" RUNFILES_DIR={:?}", env::var("RUNFILES_DIR").ok()); + eprintln!(" TEST_SRCDIR={:?}", env::var("TEST_SRCDIR").ok()); + std::process::exit(1); +} + +fn exec_passthrough(test_bin: &PathBuf, args: &[String]) -> ! { + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + let err = Command::new(test_bin).args(args).exec(); + eprintln!("ERROR: junit_runner: exec failed: {}", err); + std::process::exit(1); + } + + #[cfg(not(unix))] + { + let status = Command::new(test_bin) + .args(args) + .status() + .unwrap_or_else(|e| { + eprintln!("ERROR: junit_runner: failed to spawn test binary: {}", e); + std::process::exit(1); + }); + std::process::exit(status.code().unwrap_or(1)); + } +} + +#[derive(Debug, PartialEq)] +struct TestResult { + name: String, + status: String, +} + +struct ParsedOutput { + results: Vec, + failures: HashMap, + suite_time: f64, +} + +fn parse_libtest_output(output: &str) -> ParsedOutput { + let mut results = Vec::new(); + let mut failures = HashMap::new(); + let mut current_failure: Option = None; + let mut failure_lines: Vec = Vec::new(); + let mut suite_time = 0.0; + + for line in output.lines() { + // Check for failure header: ---- stdout ---- + if line.starts_with("---- ") && line.ends_with(" stdout ----") { + if let Some(ref name) = current_failure { + failures.insert(name.clone(), failure_lines.join("\n")); + } + let name = &line["---- ".len()..line.len() - " stdout ----".len()]; + current_failure = Some(name.to_string()); + failure_lines.clear(); + continue; + } + + if current_failure.is_some() { + if line.trim().is_empty() && failure_lines.is_empty() { + continue; + } + if line.starts_with("----") { + if let Some(ref name) = current_failure { + failures.insert(name.clone(), failure_lines.join("\n")); + } + current_failure = None; + failure_lines.clear(); + } else { + failure_lines.push(line.to_string()); + } + continue; + } + + // Check for test result: test ... ok|FAILED|ignored|bench + if line.starts_with("test ") && line.contains(" ... ") { + if let Some(result) = parse_test_result_line(line) { + results.push(result); + continue; + } + } + + // Check for suite summary: test result: ... + if line.starts_with("test result: ") { + if let Some(time) = parse_suite_time(line) { + suite_time = time; + } + } + } + + if let Some(ref name) = current_failure { + failures.insert(name.clone(), failure_lines.join("\n")); + } + + ParsedOutput { + results, + failures, + suite_time, + } +} + +fn parse_test_result_line(line: &str) -> Option { + // Format: "test ... " + // The name can contain spaces in some edge cases, but typically doesn't. + // We split on " ... " to separate name from status. + let after_test = line.strip_prefix("test ")?; + let sep_pos = after_test.find(" ... ")?; + let name = &after_test[..sep_pos]; + let rest = &after_test[sep_pos + " ... ".len()..]; + + // Status is the first word of rest + let status = rest.split_whitespace().next()?; + match status { + "ok" | "FAILED" | "ignored" | "bench" => Some(TestResult { + name: name.to_string(), + status: status.to_string(), + }), + _ => None, + } +} + +fn parse_suite_time(line: &str) -> Option { + // Format: "test result: ok. N passed; M failed; K ignored; ... finished in X.XXXs" + let finished_marker = "finished in "; + let pos = line.find(finished_marker)?; + let after = &line[pos + finished_marker.len()..]; + let time_str = after.strip_suffix('s')?; + time_str.parse::().ok() +} + +fn xml_escape(s: &str) -> String { + let mut result = String::with_capacity(s.len()); + for c in s.chars() { + match c { + '&' => result.push_str("&"), + '<' => result.push_str("<"), + '>' => result.push_str(">"), + '"' => result.push_str("""), + '\'' => result.push_str("'"), + _ => result.push(c), + } + } + result +} + +fn build_junit_xml(binary_name: &str, parsed: &ParsedOutput) -> String { + let n_tests = parsed.results.len(); + let n_fail = parsed.results.iter().filter(|r| r.status == "FAILED").count(); + let n_skip = parsed.results.iter().filter(|r| r.status == "ignored").count(); + + let mut xml = String::new(); + xml.push_str("\n"); + xml.push_str("\n"); + xml.push_str(&format!( + "\n", + xml_escape(binary_name), + n_tests, + n_fail, + n_skip, + parsed.suite_time, + )); + + for r in &parsed.results { + let name = xml_escape(&r.name); + let classname = xml_escape(binary_name); + match r.status.as_str() { + "FAILED" => { + let msg = parsed.failures.get(&r.name).map(|s| s.as_str()).unwrap_or(""); + xml.push_str(&format!( + "\n", + name, classname, + )); + xml.push_str(&format!( + "{}\n", + xml_escape(msg), + )); + xml.push_str("\n"); + } + "ignored" => { + xml.push_str(&format!( + "\n", + name, classname, + )); + xml.push_str("\n"); + xml.push_str("\n"); + } + _ => { + xml.push_str(&format!( + "\n", + name, classname, + )); + } + } + } + + xml.push_str("\n"); + xml.push_str("\n"); + xml +} + +fn main() { + let rust_test_bin = env::var("RUST_TEST_BIN").unwrap_or_else(|_| { + eprintln!("ERROR: junit_runner: RUST_TEST_BIN environment variable not set"); + std::process::exit(1); + }); + + let test_bin = resolve_runfiles(&rust_test_bin); + let args: Vec = env::args().skip(1).collect(); + + let xml_path = env::var("XML_OUTPUT_FILE").ok(); + if xml_path.is_none() { + exec_passthrough(&test_bin, &args); + } + let xml_path = xml_path.unwrap(); + + let binary_name = test_bin + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("rust_test") + .to_string(); + + let mut child = Command::new(&test_bin) + .args(&args) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .unwrap_or_else(|e| { + eprintln!("ERROR: junit_runner: failed to spawn test binary: {}", e); + std::process::exit(1); + }); + + let stdout = child.stdout.take().expect("stdout was piped"); + let reader = BufReader::new(stdout); + let mut collected = Vec::new(); + + let out = std::io::stdout(); + let mut out = out.lock(); + for line in reader.lines() { + match line { + Ok(l) => { + let _ = writeln!(out, "{}", l); + collected.push(l); + } + Err(e) => { + eprintln!("WARNING: junit_runner: error reading stdout: {}", e); + break; + } + } + } + drop(out); + + let status = child.wait().unwrap_or_else(|e| { + eprintln!("ERROR: junit_runner: failed to wait for test binary: {}", e); + std::process::exit(1); + }); + + let output = collected.join("\n"); + let parsed = parse_libtest_output(&output); + let xml = build_junit_xml(&binary_name, &parsed); + + if let Err(e) = fs::write(&xml_path, xml.as_bytes()) { + eprintln!("WARNING: junit_runner: failed to write XML to {}: {}", xml_path, e); + } + + std::process::exit(status.code().unwrap_or(1)); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_result_line_ok() { + assert_eq!( + parse_test_result_line("test foo::bar ... ok"), + Some(TestResult { name: "foo::bar".into(), status: "ok".into() }), + ); + } + + #[test] + fn parse_result_line_failed() { + assert_eq!( + parse_test_result_line("test my_test ... FAILED"), + Some(TestResult { name: "my_test".into(), status: "FAILED".into() }), + ); + } + + #[test] + fn parse_result_line_ignored() { + assert_eq!( + parse_test_result_line("test skipped_test ... ignored"), + Some(TestResult { name: "skipped_test".into(), status: "ignored".into() }), + ); + } + + #[test] + fn parse_result_line_bench() { + assert_eq!( + parse_test_result_line("test bench_thing ... bench"), + Some(TestResult { name: "bench_thing".into(), status: "bench".into() }), + ); + } + + #[test] + fn parse_result_line_invalid() { + assert_eq!(parse_test_result_line("not a test line"), None); + assert_eq!(parse_test_result_line("test incomplete"), None); + assert_eq!(parse_test_result_line("test bad ... unknown_status"), None); + } + + #[test] + fn parse_suite_time_valid() { + let line = "test result: ok. 3 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 1.234s"; + assert_eq!(parse_suite_time(line), Some(1.234)); + } + + #[test] + fn parse_suite_time_no_match() { + assert_eq!(parse_suite_time("no time here"), None); + } + + #[test] + fn parse_libtest_output_mixed() { + let output = "\ +running 3 tests +test pass_test ... ok +test fail_test ... FAILED +test skip_test ... ignored + +failures: + +---- fail_test stdout ---- +assertion failed: false +---- + +failures: + fail_test + +test result: FAILED. 1 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.500s"; + + let parsed = parse_libtest_output(output); + assert_eq!(parsed.results.len(), 3); + assert_eq!(parsed.results[0].status, "ok"); + assert_eq!(parsed.results[1].status, "FAILED"); + assert_eq!(parsed.results[2].status, "ignored"); + assert!(parsed.failures.contains_key("fail_test")); + assert!(parsed.failures["fail_test"].contains("assertion failed")); + assert!((parsed.suite_time - 0.5).abs() < 0.001); + } + + #[test] + fn xml_escape_special_chars() { + assert_eq!(xml_escape("a&bd\"e'f"), "a&b<c>d"e'f"); + } + + #[test] + fn xml_escape_no_special() { + assert_eq!(xml_escape("hello world"), "hello world"); + } + + #[test] + fn build_xml_passing_tests() { + let parsed = ParsedOutput { + results: vec![ + TestResult { name: "test_a".into(), status: "ok".into() }, + TestResult { name: "test_b".into(), status: "ok".into() }, + ], + failures: HashMap::new(), + suite_time: 1.0, + }; + let xml = build_junit_xml("my_test", &parsed); + assert!(xml.contains("")); + } + + #[test] + fn build_xml_with_failure() { + let mut failures = HashMap::new(); + failures.insert("bad_test".to_string(), "something broke".to_string()); + let parsed = ParsedOutput { + results: vec![ + TestResult { name: "bad_test".into(), status: "FAILED".into() }, + ], + failures, + suite_time: 0.1, + }; + let xml = build_junit_xml("my_test", &parsed); + assert!(xml.contains("failures=\"1\"")); + assert!(xml.contains("something broke")); + } + + #[test] + fn build_xml_with_ignored() { + let parsed = ParsedOutput { + results: vec![ + TestResult { name: "skip_me".into(), status: "ignored".into() }, + ], + failures: HashMap::new(), + suite_time: 0.0, + }; + let xml = build_junit_xml("my_test", &parsed); + assert!(xml.contains("skipped=\"1\"")); + assert!(xml.contains("")); + } + + #[test] + fn build_xml_escapes_special_chars() { + let mut failures = HashMap::new(); + failures.insert("test".to_string(), "a & b".to_string()); + let parsed = ParsedOutput { + results: vec![ + TestResult { name: "test".into(), status: "FAILED".into() }, + ], + failures, + suite_time: 0.0, + }; + let xml = build_junit_xml("bin&name", &parsed); + assert!(xml.contains("name=\"bin&name\"")); + assert!(xml.contains("name=\"test<x>\"")); + assert!(xml.contains("a & b")); + } +} From c0ae7691c4b425d3172444b2f4ee63b751d7c95e Mon Sep 17 00:00:00 2001 From: Julian Howes Date: Thu, 30 Jul 2026 11:21:35 +0200 Subject: [PATCH 2/9] Fix coverage collection for junit-wrapped rust_test With the junit wrapper in place, RUST_TEST_BIN points at the real test binary via a runfiles path. collect_coverage resolves that against RUNFILES_DIR, but under --experimental_split_coverage_postprocessing Bazel doesn't set RUNFILES_DIR, so it fell back to rebuilding the path from the bin dir and the workspace-prefixed runfiles path. That fallback inserts a workspace segment that isn't in the output tree, so llvm-cov couldn't find the binary and every coverage run of a wrapped test failed with 'failed to load coverage: ... No such file or directory'. Pass the binary's execroot-relative path (output.path) straight from the rule and use it in the fallback instead of reconstructing one. --- rust/private/rust.bzl | 7 +++++ util/collect_coverage/collect_coverage.rs | 35 ++++++++++++++--------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 3262750fe8..fbdfe0c9f1 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -607,6 +607,13 @@ def _rust_test_impl(ctx): rust_test_bin_rloc = ctx.workspace_name + "/" + test_bin_short env["RUST_TEST_BIN"] = rust_test_bin_rloc + # RUST_TEST_BIN above is a runfiles path and only resolves against + # RUNFILES_DIR. Coverage postprocessing may run with RUNFILES_DIR + # unset (--experimental_split_coverage_postprocessing), so also hand + # over the binary's execroot-relative path directly instead of making + # collect_coverage reconstruct it. + env["RUST_TEST_BIN_EXECROOT_PATH"] = output.path + junit_runner = ctx.actions.declare_file(ctx.label.name + "_junit_runner" + toolchain.binary_ext) ctx.actions.symlink( output = junit_runner, diff --git a/util/collect_coverage/collect_coverage.rs b/util/collect_coverage/collect_coverage.rs index b3bc0271e9..f13ba09fde 100644 --- a/util/collect_coverage/collect_coverage.rs +++ b/util/collect_coverage/collect_coverage.rs @@ -145,29 +145,36 @@ fn main() { }; // When the JUnit runner wraps the test, TEST_BINARY points to the runner // and RUST_TEST_BIN holds the actual instrumented binary that llvm-cov needs. + // + // RUST_TEST_BIN is a runfiles path (workspace-prefixed) and only resolves + // against RUNFILES_DIR. When RUNFILES_DIR is gone -- Bazel drops it under + // --experimental_split_coverage_postprocessing -- we need the binary's + // location relative to the execroot instead. Rather than try to rebuild + // that here by stitching the workspace-prefixed runfiles path onto the + // bin dir (which mislays the workspace segment and points at a file that + // isn't there), read RUST_TEST_BIN_EXECROOT_PATH, which the test rule + // already knows exactly. let test_binary = if let Ok(rust_test_bin) = env::var("RUST_TEST_BIN") { debug_log!("Using RUST_TEST_BIN: {}", rust_test_bin); + let execroot_path = || { + env::var("RUST_TEST_BIN_EXECROOT_PATH") + .map(|p| execroot.join(p)) + .unwrap_or_else(|_| { + let bin_dir = config_bin_dir(&execroot, &coverage_dir); + execroot.join(bin_dir).join(&rust_test_bin) + }) + }; match runfiles_dir { Some(ref rd) => { - let candidate = rd - .join(env::var("TEST_WORKSPACE").unwrap_or_default()) - .join(&rust_test_bin); + let candidate = rd.join(&rust_test_bin); if candidate.exists() { candidate } else { - let candidate = rd.join(&rust_test_bin); - if candidate.exists() { - candidate - } else { - let bin_dir = config_bin_dir(&execroot, &coverage_dir); - execroot.join(bin_dir).join(&rust_test_bin) - } + debug_log!("RUST_TEST_BIN missing under RUNFILES_DIR; using execroot path"); + execroot_path() } } - None => { - let bin_dir = config_bin_dir(&execroot, &coverage_dir); - execroot.join(bin_dir).join(&rust_test_bin) - } + None => execroot_path(), } } else { match runfiles_dir { From 29d6abd38ba3da608aaa59236996d0c42b28798f Mon Sep 17 00:00:00 2001 From: Julian Howes Date: Thu, 30 Jul 2026 11:21:35 +0200 Subject: [PATCH 3/9] Improve junit failure reporting and crash handling The first cut of the runner emitted a bare with no message and dropped everything the test printed, so a failing test in CI told you which test failed but not why. Pull the panic/assertion line out of the failure block and use it as the failure message, and attach the captured stdout and stderr to the testcase so that detail survives into the XML. Also handle the case where the binary never prints a "test result:" summary -- a segfault or abort partway through the run. Before, a crash after some tests had already passed could look like a clean pass; now it surfaces as an error on the suite. While doing this I hit a parsing bug: modern libtest doesn't print a closing "----" after a failure's detail block, so the parser was swallowing the summary line and losing the run totals. Terminate the block at the trailing "failures:" list and the summary line instead. --- util/junit_runner/junit_runner.rs | 614 +++++++++++++++++++++++++----- 1 file changed, 529 insertions(+), 85 deletions(-) diff --git a/util/junit_runner/junit_runner.rs b/util/junit_runner/junit_runner.rs index a32ba9c26a..7e1cc5a41b 100644 --- a/util/junit_runner/junit_runner.rs +++ b/util/junit_runner/junit_runner.rs @@ -4,6 +4,7 @@ use std::fs; use std::io::{BufRead, BufReader, Write}; use std::path::PathBuf; use std::process::{Command, Stdio}; +use std::thread; fn resolve_runfiles(rlocation_path: &str) -> PathBuf { if let Ok(manifest) = env::var("RUNFILES_MANIFEST_FILE") { @@ -38,7 +39,10 @@ fn resolve_runfiles(rlocation_path: &str) -> PathBuf { "ERROR: junit_runner: cannot resolve runfiles path: {}", rlocation_path ); - eprintln!(" RUNFILES_MANIFEST_FILE={:?}", env::var("RUNFILES_MANIFEST_FILE").ok()); + eprintln!( + " RUNFILES_MANIFEST_FILE={:?}", + env::var("RUNFILES_MANIFEST_FILE").ok() + ); eprintln!(" RUNFILES_DIR={:?}", env::var("RUNFILES_DIR").ok()); eprintln!(" TEST_SRCDIR={:?}", env::var("TEST_SRCDIR").ok()); std::process::exit(1); @@ -72,10 +76,44 @@ struct TestResult { status: String, } +/// The counts libtest prints on its `test result:` summary line. +#[derive(Debug, PartialEq)] +struct Summary { + passed: usize, + failed: usize, + ignored: usize, +} + struct ParsedOutput { results: Vec, failures: HashMap, suite_time: f64, + /// `None` if libtest never printed its summary line, which is our main + /// signal that the binary didn't finish running as a test harness. + summary: Option, +} + +/// A `---- stdout ----` (or `stderr`) banner that opens a failure +/// detail block; returns the test name it belongs to. +fn failure_header(line: &str) -> Option<&str> { + let inner = line.strip_prefix("---- ")?; + inner + .strip_suffix(" stdout ----") + .or_else(|| inner.strip_suffix(" stderr ----")) +} + +fn record_failure(failures: &mut HashMap, name: &str, lines: &[String]) { + let body = lines.join("\n").trim_end().to_string(); + // A test can have both stdout and stderr blocks; keep both. + match failures.get_mut(name) { + Some(existing) => { + existing.push('\n'); + existing.push_str(&body); + } + None => { + failures.insert(name.to_string(), body); + } + } } fn parse_libtest_output(output: &str) -> ParsedOutput { @@ -84,36 +122,45 @@ fn parse_libtest_output(output: &str) -> ParsedOutput { let mut current_failure: Option = None; let mut failure_lines: Vec = Vec::new(); let mut suite_time = 0.0; + let mut summary = None; for line in output.lines() { - // Check for failure header: ---- stdout ---- - if line.starts_with("---- ") && line.ends_with(" stdout ----") { - if let Some(ref name) = current_failure { - failures.insert(name.clone(), failure_lines.join("\n")); + // A new detail banner both opens a block and closes the previous one. + if let Some(name) = failure_header(line) { + if let Some(prev) = current_failure.take() { + record_failure(&mut failures, &prev, &failure_lines); } - let name = &line["---- ".len()..line.len() - " stdout ----".len()]; current_failure = Some(name.to_string()); failure_lines.clear(); continue; } - if current_failure.is_some() { - if line.trim().is_empty() && failure_lines.is_empty() { - continue; - } - if line.starts_with("----") { - if let Some(ref name) = current_failure { - failures.insert(name.clone(), failure_lines.join("\n")); - } + if let Some(name) = ¤t_failure { + // A detail block runs until the trailing `failures:` name list, the + // `test result:` summary, or a bare `----` terminator (older + // libtest). Everything else is part of the captured output/panic. + // Note we must not stop the block until here, or the summary line + // gets swallowed and we lose the run totals entirely. + if line == "failures:" || line.starts_with("test result: ") { + record_failure(&mut failures, name, &failure_lines); + current_failure = None; + failure_lines.clear(); + // fall through so the summary line is still parsed below + } else if line.starts_with("----") { + record_failure(&mut failures, name, &failure_lines); current_failure = None; failure_lines.clear(); + continue; } else { - failure_lines.push(line.to_string()); + // Drop the blank line libtest prints right after the banner. + if !(line.trim().is_empty() && failure_lines.is_empty()) { + failure_lines.push(line.to_string()); + } + continue; } - continue; } - // Check for test result: test ... ok|FAILED|ignored|bench + // A per-test result: "test ... ok|FAILED|ignored|bench" if line.starts_with("test ") && line.contains(" ... ") { if let Some(result) = parse_test_result_line(line) { results.push(result); @@ -121,22 +168,26 @@ fn parse_libtest_output(output: &str) -> ParsedOutput { } } - // Check for suite summary: test result: ... + // The run summary: "test result: ok. N passed; M failed; ..." if line.starts_with("test result: ") { if let Some(time) = parse_suite_time(line) { suite_time = time; } + if let Some(counts) = parse_suite_counts(line) { + summary = Some(counts); + } } } - if let Some(ref name) = current_failure { - failures.insert(name.clone(), failure_lines.join("\n")); + if let Some(name) = ¤t_failure { + record_failure(&mut failures, name, &failure_lines); } ParsedOutput { results, failures, suite_time, + summary, } } @@ -169,6 +220,69 @@ fn parse_suite_time(line: &str) -> Option { time_str.parse::().ok() } +fn parse_suite_counts(line: &str) -> Option { + // "test result: ok. 3 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; ..." + // Each ';'-separated clause ends in "