-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support test output postprocessing by a child process. #122145
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
This comment has been minimized.
This comment has been minimized.
cc @rust-lang/testing-devex |
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
Thank you for documenting the rationale. However, could you describe what you mean by "support postprocessing" without relying on us having to dig into the code and infer it from the implementation? |
I've just updated the PR and commit description. Please let me know if I missed anything. |
Add the following two optional flags to `libtest` (rustc's built-in unit-test framework), in order to support postprocessing of the test results using a separate executable: * `--output_postprocess_executable [PATH]` * `--output_postprocess_args [ARGUMENT]` (can be repeated.) If you don't pass `--output_postprocess_executable [PATH]`, the behavior stays the same as before this commit. That is, the test results are sent to stdout. If you pass `--output_postprocess_executable [PATH]`, `libtest` will 1. Spawn a child process from the executable binary (aka *postprocessor*) at the given path. 2. Pass the arguments from the `--output_postprocess_args [ARGUMENT]` flags (if any) to the child process. If `--output_postprocess_args` was used multiple times, all listed arguments will be passed in the original order. 3. Propagate the environment variables to the child process. The *postprocessor* executable is expected to wait for the end of input (EOF) and then terminate. Usage example #1: Filter lines of the test results ```shell $ LD_LIBRARY_PATH=$(pwd) ./test-05daf44cb501aee6 --output_postprocess_executable=/usr/bin/grep --output_postprocess_args="test result" test result: ok. 59 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.31s ``` Usage example rust-lang#2: Save test results into a file ```shell $ LD_LIBRARY_PATH=$(pwd) ./test-05daf44cb501aee6 --output_postprocess_executable=/usr/bin/sh --output_postprocess_args=-c --output_postprocess_args="cat > /tmp/output.txt" ``` Usage example rust-lang#3: Save test results into a file while keeping the command line output ```shell $ LD_LIBRARY_PATH=$(pwd) ./test-05daf44cb501aee6 --output_postprocess_executable=/usr/bin/tee --output_postprocess_args="/tmp/output.txt" running 60 tests ... ``` Usage example rust-lang#4: Prepend every line of test results with the value of an environment variable (to demonstrate environment variable propagation) ```shell $ LOG_PREFIX=">>>" LD_LIBRARY_PATH=$(pwd) ./test-05daf44cb501aee6 --output_postprocess_executable=/usr/bin/sh --output_postprocess_args=-c --output_postprocess_args="sed s/^/\$LOG_PREFIX/" >>> >>>running 60 tests ... ``` Usage example rust-lang#5: Change format of JSON output (using https://jqlang.github.io/jq/) ```shell $ LD_LIBRARY_PATH=$(pwd) ./test-05daf44cb501aee6 -Zunstable-options --format=json --output_postprocess_executable=/usr/bin/jq ``` Usage example rust-lang#6: Print test execution time in machine-readable format ```shell $ LD_LIBRARY_PATH=$(pwd) ./test-05daf44cb501aee6 -Zunstable-options --format=json --output_postprocess_executable=/usr/bin/jq --output_postprocess_args=".exec_time | numbers" 0.234317996 ``` 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 rust-lang#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 results 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 rust-lang#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 results 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`.
Personally, I think this is the wrong direction for libtest to be going. This should be the responsibility of the calling tool to deal with but is being shifted here due to limitations in the calling tool. In doing this, we are increasing what we need to maintain compatibility on and what other test harness frameworks need to support for interoperability. T-testing-devex is working on shifting responsibilities from test harnesses to test runners (bazel, cargo). The hope is that libtest can remove, deprecate, or simplify as many features as possible. From there, we expect to work to make custom test harnesses first class which will need to maintain the same interface as libtest. In addition to that... If the post-processing is expected to be responsible for output, then this would interfere with test runners programmatically processing the output. If the post-processing is not meant to output to stdout, then this is a logging process sync and the conversation just returns to logfile. Either way, I feel like this would unintentionally encourage people to be programmatically reacting to non-programmatic output which can change at any time (even one of the included examples does this). |
r? epage |
☔ The latest upstream changes (presumably #122036) made this pull request unmergeable. Please resolve the merge conflicts. |
Sorry for the super-late reply.
@epage Thanks for letting me know! This seems to be tracked in rust-lang/testing-devex-team#2 (correct me if I'm wrong), although I'm yet to evaluate how well the new approach may support running tests with Bazel. In the meantime, I think fixing file output in XML/JSON format(s) would help unblock the specific Rust+Bazel use case that I'm working on. Looking forward to see progress on #123365 or on alternative solutions for |
That issue is for shifting focus from libtest to custom test harnesses. That is another potential reason to limit what we add to libtest: libtest could be considered a minimum bar of support and we'd be increasing the burden on what custom test harnesses should be supporting. The work I'm referring to is part of rust-lang/testing-devex-team#1. |
@aspotashev it is unclear from your message. Are you seeing this PR as no longer relevant, preferring #123365 or other work instead? Should this be closed? |
Thanks for the update! Let me close this one, since it may not fit well in the future plans for libtest as you described. |
Support test results output postprocessing by a child process.
Add the following two optional flags to
libtest
(rustc's built-in unit-testframework), in order to support postprocessing of the test results using a
separate executable:
--output_postprocess_executable [PATH]
--output_postprocess_args [ARGUMENT]
(can be repeated.)If you don't pass
--output_postprocess_executable [PATH]
, the behavior staysthe same as before this commit. That is, the test results are sent to stdout.
If you pass
--output_postprocess_executable [PATH]
,libtest
willthe given path.
--output_postprocess_args [ARGUMENT]
flags (ifany) to the child process. If
--output_postprocess_args
was used multipletimes, all listed arguments will be passed in the original order.
The postprocessor executable is expected to wait for the end of input (EOF)
and then terminate.
Usage example #1: Filter lines of the test results
Usage example #2: Save test results into a file
Usage example #3: Save test results into a file while keeping the command line
output
Usage example #4: Prepend every line of test results with the value of an
environment variable (to demonstrate environment variable propagation)
Usage example #5: Change format of JSON output (using
https://jqlang.github.io/jq/)
$ LD_LIBRARY_PATH=$(pwd) ./test-05daf44cb501aee6 -Zunstable-options --format=json --output_postprocess_executable=/usr/bin/jq
Usage example #6: Print test execution time in machine-readable format
Rationale for adding this functionality:
(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.
and then modify Rust Bazel rules to pass
--logfile
on the command lineto provisionally unblock
Supporting junit XML report in rust_test bazelbuild/rules_rust#1303, that solution
still wouldn't allow advanced test results postprocessing such as
changing JSON/XML schema and injecting extra JUnit properties.
separate tool to postprocess the test results output (see comments to
Tracking Issue for libtest's JUnit reporter #85563).
--output_postprocess_executable
,--output_postprocess_args
insteadof piping the test results explicitly, e.g. to more reliably separate
test results from other output.
Rationale for implementation details:
possible to enable unstable feature
bindeps
(https://rust-lang.github.io/rfcs/3028-cargo-binary-dependencies.html) in
"x.py" by default.
library/std/src/process/tests.rs
.