Skip to content

Introduce test results file #140805

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

patskovn
Copy link

@patskovn patskovn commented May 8, 2025

Separating test results from other output avoids contamination. If libtest only writes output to stdout, any non-test output (log messages, debug prints, etc.) may corrupt the stream and break parsers. Rust’s println’s are wrapped by libtest, but anything can (and does, in real world) use libc, or have C code using libc that corrupts stdout. Also, in practice, projects often resort to external post-processing to filter test output. As one tracking discussion notes, “due to limitations of Rust libtest formatters, Rust developers often use a separate tool to postprocess the test results output”. By writing test results directly to a file, we can guarantee the test results are isolated and parseable, without third-party noise.

Here's the full proposal: rust-lang/testing-devex-team#11

patskovn added 2 commits May 8, 2025 15:03
Separating test results from other output **avoids contamination**. If `libtest` only writes output to stdout, any non-test output (log messages, debug prints, etc.) may corrupt the stream and break parsers. Rust’s `println`’s are wrapped by libtest, but anything can (and does, in real world) use `libc`, or have C code using `libc` that corrupts stdout. Also, in practice, projects often resort to external post-processing to filter test output. As one [tracking discussion notes](rust-lang#85563), *“due to limitations of Rust libtest formatters, Rust developers often use a separate tool to postprocess the test results output”*. By writing test results directly to a file, we can guarantee the test results are isolated and parseable, without third-party noise.
I had to introduce `test_helepr.rs` basically re-copying it yet another time from `std` as it was done in other places. It is better than keeping new functionality untested.
@rustbot
Copy link
Collaborator

rustbot commented May 8, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 8, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 8, 2025

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@jieyouxu
Copy link
Member

jieyouxu commented May 8, 2025

r? testing-devex

@rustbot rustbot added the T-testing-devex Relevant to the testing devex team (testing DX), which will review and decide on the PR/issue. label May 8, 2025
@rustbot rustbot assigned weihanglo and unassigned tgross35 May 8, 2025
@jieyouxu
Copy link
Member

jieyouxu commented May 8, 2025

Triage note: I can't speak for T-testing-devex, but AFAIK libtest is currently in a soft feature freeze, and the proposed options here AFAIK are insta-stable CLI and behavior?

@epage
Copy link
Contributor

epage commented May 8, 2025

They also raised this in rust-lang/testing-devex-team#11.

Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not looking over the code changes until rust-lang/testing-devex-team#11 is resolved

@@ -59,6 +60,7 @@ fn optgroups() -> getopts::Options {
.optflag("", "list", "List all tests and benchmarks")
.optflag("h", "help", "Display this message")
.optopt("", "logfile", "Write logs to the specified file (deprecated)", "PATH")
.optopt("", "test-results-file", "Write test results to the specified file", "PATH")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jieyouxu
Copy link
Member

jieyouxu commented May 8, 2025

@rustbot blocked (rust-lang/testing-devex-team#11)

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 8, 2025
@rust-lang rust-lang deleted a comment from rustbot May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-testing-devex Relevant to the testing devex team (testing DX), which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants