Skip to content

Commit

Permalink
Support test output postprocessing by a child process.
Browse files Browse the repository at this point in the history
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 bazelbuild/rules_rust#1303.
    * Cargo is not used in Rust Bazel rules.
    * Although we could wait for
      #96290 and then modify Rust
      Bazel rules to pass `--logfile` on the command line to
      provisionally unblock
      bazelbuild/rules_rust#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 #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
```
  • Loading branch information
aspotashev committed Mar 7, 2024
1 parent 52f8aec commit 9519fe2
Show file tree
Hide file tree
Showing 8 changed files with 273 additions and 35 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5445,6 +5445,7 @@ dependencies = [
"libc",
"panic_abort",
"panic_unwind",
"rand",
"std",
]

Expand Down
3 changes: 3 additions & 0 deletions library/test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
30 changes: 30 additions & 0 deletions library/test/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf>,
pub output_postprocess_args: Vec<String>,
}

impl TestOpts {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -493,3 +517,9 @@ fn get_log_file(matches: &getopts::Matches) -> OptPartRes<Option<PathBuf>> {

Ok(logfile)
}

fn get_output_postprocess_executable(matches: &getopts::Matches) -> OptPartRes<Option<PathBuf>> {
let executable = matches.opt_str("output_postprocess_executable").map(|s| PathBuf::from(&s));

Ok(executable)
}
110 changes: 75 additions & 35 deletions library/test/src/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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<TestDescAndFn>) -> io::Result<bool> {
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<dyn OutputFormatter> = 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<dyn OutputFormatter> = 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.
Expand Down
19 changes: 19 additions & 0 deletions library/test/src/testdata/postprocess.cmd
Original file line number Diff line number Diff line change
@@ -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
18 changes: 18 additions & 0 deletions library/test/src/testdata/postprocess.sh
Original file line number Diff line number Diff line change
@@ -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
125 changes: 125 additions & 0 deletions library/test/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use rand::RngCore;
use std::fs;
use std::path::PathBuf;

use super::*;

use crate::{
Expand Down Expand Up @@ -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<TestDescAndFn> {
vec![
TestDescAndFn {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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::<Vec<&str>>();

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);
}
Loading

0 comments on commit 9519fe2

Please sign in to comment.