From b7d91e70c8ecb1a27881826d4fe1b823a06f66f6 Mon Sep 17 00:00:00 2001 From: Alexander Potashev Date: Tue, 20 Feb 2024 16:51:11 +0100 Subject: [PATCH] Support test output postprocessing by a child process. Rationale for adding this functionality: * Bazel (build system) doesn't provide a way to process output from a binary (in this case, Rust test binary's output) other using a wrapper binary. However, using a wrapper binary potentially breaks debugging, because Bazel would suggest to debug the wrapper binary rather than the Rust test itself. * See https://github.com/bazelbuild/rules_rust/issues/1303. * Cargo is not used in Rust Bazel rules. * Although we could wait for https://github.com/rust-lang/rust/pull/96290 and then modify Rust Bazel rules to pass `--logfile` on the command line to provisionally unblock https://github.com/bazelbuild/rules_rust/issues/1303, that solution still wouldn't allow advanced test output postprocessing such as changing JSON/XML schema and injecting extra JUnit properties. * Due to limitations of Rust libtest formatters, Rust developers often use a separate tool to postprocess the test results output (see comments to https://github.com/rust-lang/rust/issues/85563). * Examples of existing postprocessing tools: * https://crates.io/crates/cargo2junit * https://crates.io/crates/gitlab-report * https://crates.io/crates/cargo-suity * For these use cases, it might be helpful to use the new flags `--output_postprocess_executable`, `--output_postprocess_args` instead of piping the test output explicitly, e.g. to more reliably separate test results from other output. Rationale for implementation details: * Use platform-dependent scripts (.sh, .cmd) because it doesn't seem to be possible to enable unstable feature `bindeps` (https://rust-lang.github.io/rfcs/3028-cargo-binary-dependencies.html) in "x.py" by default. * Here's a preexisting test that also uses per-platform specialization: `library/std/src/process/tests.rs`. How to use: ``` ./testbinary --format=junit -Zunstable-options --output_postprocess_executable=/usr/bin/echo --output_postprocess_args=abc123 ``` --- Cargo.lock | 1 + library/test/Cargo.toml | 3 + library/test/src/cli.rs | 30 ++++++ library/test/src/console.rs | 110 +++++++++++++------ library/test/src/testdata/postprocess.cmd | 19 ++++ library/test/src/testdata/postprocess.sh | 18 ++++ library/test/src/tests.rs | 125 ++++++++++++++++++++++ 7 files changed, 271 insertions(+), 35 deletions(-) create mode 100644 library/test/src/testdata/postprocess.cmd create mode 100755 library/test/src/testdata/postprocess.sh diff --git a/Cargo.lock b/Cargo.lock index 635146492b042..f6f61e9cd86a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5445,6 +5445,7 @@ dependencies = [ "libc", "panic_abort", "panic_unwind", + "rand", "std", ] diff --git a/library/test/Cargo.toml b/library/test/Cargo.toml index 4c42e3bae563e..afbd882559fba 100644 --- a/library/test/Cargo.toml +++ b/library/test/Cargo.toml @@ -10,3 +10,6 @@ core = { path = "../core" } panic_unwind = { path = "../panic_unwind" } panic_abort = { path = "../panic_abort" } libc = { version = "0.2.150", default-features = false } + +[dev-dependencies] +rand = { version = "0.8.5" } diff --git a/library/test/src/cli.rs b/library/test/src/cli.rs index 6ac3b3eaa797b..475533a16d35a 100644 --- a/library/test/src/cli.rs +++ b/library/test/src/cli.rs @@ -31,6 +31,13 @@ pub struct TestOpts { /// abort as soon as possible. pub fail_fast: bool, pub options: Options, + /// The test results text output may optionally be piped into the given executable running as + /// a child process. The child process inherits the environment variables passed to the test + /// binary. + /// + /// If this is unset, the test results will be written to stdout. + pub output_postprocess_executable: Option, + pub output_postprocess_args: Vec, } impl TestOpts { @@ -148,6 +155,18 @@ fn optgroups() -> getopts::Options { "shuffle-seed", "Run tests in random order; seed the random number generator with SEED", "SEED", + ) + .optopt( + "", + "output_postprocess_executable", + "Postprocess test results using the given executable", + "PATH", + ) + .optmulti( + "", + "output_postprocess_args", + "When --output_postprocess_executable is set, pass the given command line arguments to the executable", + "ARGUMENT", ); opts } @@ -281,6 +300,9 @@ fn parse_opts_impl(matches: getopts::Matches) -> OptRes { let options = Options::new().display_output(matches.opt_present("show-output")); + let output_postprocess_executable = get_output_postprocess_executable(&matches)?; + let output_postprocess_args = matches.opt_strs("output_postprocess_args"); + let test_opts = TestOpts { list, filters, @@ -301,6 +323,8 @@ fn parse_opts_impl(matches: getopts::Matches) -> OptRes { time_options, options, fail_fast: false, + output_postprocess_executable, + output_postprocess_args, }; Ok(test_opts) @@ -493,3 +517,9 @@ fn get_log_file(matches: &getopts::Matches) -> OptPartRes> { Ok(logfile) } + +fn get_output_postprocess_executable(matches: &getopts::Matches) -> OptPartRes> { + let executable = matches.opt_str("output_postprocess_executable").map(|s| PathBuf::from(&s)); + + Ok(executable) +} diff --git a/library/test/src/console.rs b/library/test/src/console.rs index 09aa3bfb6aac3..e71759e2e0be0 100644 --- a/library/test/src/console.rs +++ b/library/test/src/console.rs @@ -3,6 +3,7 @@ use std::fs::File; use std::io; use std::io::prelude::Write; +use std::process::{Command, Stdio}; use std::time::Instant; use super::{ @@ -289,50 +290,89 @@ fn on_test_event( } /// A simple console test runner. -/// Runs provided tests reporting process and results to the stdout. +/// Runs provided tests reporting process and results. +/// +/// The results may optionally be piped to the specified postprocessor binary, otherwise written to stdout. pub fn run_tests_console(opts: &TestOpts, tests: Vec) -> io::Result { - let output = match term::stdout() { - None => OutputLocation::Raw(io::stdout()), - Some(t) => OutputLocation::Pretty(t), + let mut postprocessor = match &opts.output_postprocess_executable { + None => None, + Some(postprocess_executable) => Some( + Command::new(&postprocess_executable) + .args(&opts.output_postprocess_args) + .stdin(Stdio::piped()) + .spawn() + .expect("failed to execute test output postprocessor"), + ), }; - let max_name_len = tests - .iter() - .max_by_key(|t| len_if_padded(t)) - .map(|t| t.desc.name.as_slice().len()) - .unwrap_or(0); + let write_result; + { + // This inner block is to make sure `child_stdin` is dropped, so that the pipe + // and the postprocessor's stdin is closed to notify it of EOF. + // + // Otherwise, the postprocessor may be stuck waiting for more input. + + let mut child_stdin; + let mut host_stdout; + let output = match (&mut postprocessor, term::stdout()) { + (Some(child), _) => OutputLocation::Raw({ + child_stdin = child.stdin.take().unwrap(); + &mut child_stdin as &mut dyn Write + }), + (None, None) => OutputLocation::Raw({ + host_stdout = io::stdout(); + &mut host_stdout as &mut dyn Write + }), + (None, Some(t)) => OutputLocation::Pretty(t), + }; - let is_multithreaded = opts.test_threads.unwrap_or_else(get_concurrency) > 1; + let max_name_len = tests + .iter() + .max_by_key(|t| len_if_padded(t)) + .map(|t| t.desc.name.as_slice().len()) + .unwrap_or(0); + + let is_multithreaded = opts.test_threads.unwrap_or_else(get_concurrency) > 1; + + let mut out: Box = match opts.format { + OutputFormat::Pretty => Box::new(PrettyFormatter::new( + output, + opts.use_color(), + max_name_len, + is_multithreaded, + opts.time_options, + )), + OutputFormat::Terse => Box::new(TerseFormatter::new( + output, + opts.use_color(), + max_name_len, + is_multithreaded, + )), + OutputFormat::Json => Box::new(JsonFormatter::new(output)), + OutputFormat::Junit => Box::new(JunitFormatter::new(output)), + }; + let mut st = ConsoleTestState::new(opts)?; - let mut out: Box = match opts.format { - OutputFormat::Pretty => Box::new(PrettyFormatter::new( - output, - opts.use_color(), - max_name_len, - is_multithreaded, - opts.time_options, - )), - OutputFormat::Terse => { - Box::new(TerseFormatter::new(output, opts.use_color(), max_name_len, is_multithreaded)) - } - OutputFormat::Json => Box::new(JsonFormatter::new(output)), - OutputFormat::Junit => Box::new(JunitFormatter::new(output)), - }; - let mut st = ConsoleTestState::new(opts)?; + // Prevent the usage of `Instant` in some cases: + // - It's currently not supported for wasm targets. + // - We disable it for miri because it's not available when isolation is enabled. + let is_instant_supported = !cfg!(target_family = "wasm") && !cfg!(miri); - // Prevent the usage of `Instant` in some cases: - // - It's currently not supported for wasm targets. - // - We disable it for miri because it's not available when isolation is enabled. - let is_instant_supported = - !cfg!(target_family = "wasm") && !cfg!(target_os = "zkvm") && !cfg!(miri); + let start_time = is_instant_supported.then(Instant::now); + run_tests(opts, tests, |x| on_test_event(&x, &mut st, &mut *out))?; + st.exec_time = start_time.map(|t| TestSuiteExecTime(t.elapsed())); - let start_time = is_instant_supported.then(Instant::now); - run_tests(opts, tests, |x| on_test_event(&x, &mut st, &mut *out))?; - st.exec_time = start_time.map(|t| TestSuiteExecTime(t.elapsed())); + assert!(opts.fail_fast || st.current_test_count() == st.total); - assert!(opts.fail_fast || st.current_test_count() == st.total); + write_result = out.write_run_finish(&st); + } + + if let Some(mut child) = postprocessor { + let status = child.wait().expect("failed to get test output postprocessor status"); + assert!(status.success()); + } - out.write_run_finish(&st) + write_result } // Calculates padding for given test description. diff --git a/library/test/src/testdata/postprocess.cmd b/library/test/src/testdata/postprocess.cmd new file mode 100644 index 0000000000000..fc5736875ffc6 --- /dev/null +++ b/library/test/src/testdata/postprocess.cmd @@ -0,0 +1,19 @@ +@REM A very basic test output postprocessor. Used in `test_output_postprocessing()`. + +@echo off + +if [%TEST_POSTPROCESSOR_OUTPUT_FILE%] == [] ( + echo Required environment variable TEST_POSTPROCESSOR_OUTPUT_FILE is not set. + cmd /C exit /B 1 +) + +@REM Forward script's input into file. +find /v "" > %TEST_POSTPROCESSOR_OUTPUT_FILE% + +@REM Log every command line argument into the same file. +:start + if [%1] == [] goto done + echo %~1>> %TEST_POSTPROCESSOR_OUTPUT_FILE% + shift + goto start +:done diff --git a/library/test/src/testdata/postprocess.sh b/library/test/src/testdata/postprocess.sh new file mode 100755 index 0000000000000..b2f0fd39bc66e --- /dev/null +++ b/library/test/src/testdata/postprocess.sh @@ -0,0 +1,18 @@ +#!/bin/bash +# +# A very basic test output postprocessor. Used in `test_output_postprocessing()`. + +if [ -z "$TEST_POSTPROCESSOR_OUTPUT_FILE" ] +then + echo "Required environment variable TEST_POSTPROCESSOR_OUTPUT_FILE is not set." + exit 1 +fi + +# Forward script's input into file. +cat /dev/stdin > "$TEST_POSTPROCESSOR_OUTPUT_FILE" + +# Log every command line argument into the same file. +for i in "$@" +do + echo "$i" >> "$TEST_POSTPROCESSOR_OUTPUT_FILE" +done diff --git a/library/test/src/tests.rs b/library/test/src/tests.rs index 43a906ad298d1..bb35f1ae8f548 100644 --- a/library/test/src/tests.rs +++ b/library/test/src/tests.rs @@ -1,3 +1,7 @@ +use rand::RngCore; +use std::fs; +use std::path::PathBuf; + use super::*; use crate::{ @@ -35,10 +39,42 @@ impl TestOpts { time_options: None, options: Options::new(), fail_fast: false, + output_postprocess_executable: None, + output_postprocess_args: vec![], + } + } +} + +// These implementations of TempDir and tmpdir are forked from rust/library/std/src/sys_common/io.rs. +struct TempDir(PathBuf); + +impl TempDir { + fn join(&self, path: &str) -> PathBuf { + let TempDir(ref p) = *self; + p.join(path) + } +} + +impl Drop for TempDir { + fn drop(&mut self) { + let TempDir(ref p) = *self; + let result = fs::remove_dir_all(p); + // Avoid panicking while panicking as this causes the process to + // immediately abort, without displaying test results. + if !thread::panicking() { + result.unwrap(); } } } +fn tmpdir() -> TempDir { + let p = env::temp_dir(); + let mut r = rand::thread_rng(); + let ret = p.join(&format!("rust-{}", r.next_u32())); + fs::create_dir(&ret).unwrap(); + TempDir(ret) +} + fn one_ignored_one_unignored_test() -> Vec { vec![ TestDescAndFn { @@ -478,6 +514,25 @@ fn parse_include_ignored_flag() { assert_eq!(opts.run_ignored, RunIgnored::Yes); } +#[test] +fn parse_output_postprocess() { + let args = vec![ + "progname".to_string(), + "filter".to_string(), + "--output_postprocess_executable".to_string(), + "/tmp/postprocess.sh".to_string(), + "--output_postprocess_args".to_string(), + "--test1=a".to_string(), + "--output_postprocess_args=--test2=b".to_string(), + ]; + let opts = parse_opts(&args).unwrap().unwrap(); + assert_eq!(opts.output_postprocess_executable, Some(PathBuf::from("/tmp/postprocess.sh"))); + assert_eq!( + opts.output_postprocess_args, + vec!["--test1=a".to_string(), "--test2=b".to_string()] + ); +} + #[test] pub fn filter_for_ignored_option() { // When we run ignored tests the test filter should filter out all the @@ -922,3 +977,73 @@ fn test_dyn_bench_returning_err_fails_when_run_as_test() { let result = rx.recv().unwrap().result; assert_eq!(result, TrFailed); } + +#[test] +fn test_output_postprocessing() { + let desc = TestDescAndFn { + desc: TestDesc { + name: StaticTestName("whatever"), + ignore: false, + ignore_message: None, + source_file: "", + start_line: 0, + start_col: 0, + end_line: 0, + end_col: 0, + should_panic: ShouldPanic::No, + compile_fail: false, + no_run: false, + test_type: TestType::Unknown, + }, + testfn: DynTestFn(Box::new(move || Ok(()))), + }; + + let mut test_postprocessor: PathBuf = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + if cfg!(target_os = "windows") { + test_postprocessor.push("src/testdata/postprocess.cmd"); + } else { + test_postprocessor.push("src/testdata/postprocess.sh"); + } + + let tmpdir = tmpdir(); + let output_path = &tmpdir.join("output.txt"); + + std::env::set_var("TEST_POSTPROCESSOR_OUTPUT_FILE", output_path); + + let opts = TestOpts { + run_tests: true, + output_postprocess_executable: Some(test_postprocessor), + output_postprocess_args: vec!["--test1=a".to_string(), "--test2=b".to_string()], + format: OutputFormat::Json, + ..TestOpts::new() + }; + run_tests_console(&opts, vec![desc]).unwrap(); + + // Read output and replace the decimal value at `"exec_time": 0.000084974` to make the text deterministic. + // This replacement could be done easier with a regex, but `std` has no regex. + let mut contents = + fs::read_to_string(output_path).expect("Test postprocessor did not create file"); + let replace_trigger = r#""exec_time": "#; + let replace_start = + contents.find(replace_trigger).expect("exec_time not found in the output JSON") + + replace_trigger.len(); + let replace_end = replace_start + + contents[replace_start..] + .find(' ') + .expect("No space found after the decimal value for exec_time"); + contents.replace_range(replace_start..replace_end, "AAA.BBB"); + + // Split output at line breaks to make the comparison platform-agnostic regarding newline style. + let contents_lines = contents.as_str().lines().collect::>(); + + let expected_lines = vec![ + r#"{ "type": "suite", "event": "started", "test_count": 1 }"#, + r#"{ "type": "test", "event": "started", "name": "whatever" }"#, + r#"{ "type": "test", "name": "whatever", "event": "ok" }"#, + r#"{ "type": "suite", "event": "ok", "passed": 1, "failed": 0, "ignored": 0, "measured": 0, "filtered_out": 0, "exec_time": AAA.BBB }"#, + r#"--test1=a"#, + r#"--test2=b"#, + ]; + + assert_eq!(contents_lines, expected_lines); +}