-
Notifications
You must be signed in to change notification settings - Fork 13.8k
compiletest: Implement an experimental --new-output-capture
mode
#146119
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
Conversation
Some changes occurred in src/tools/compiletest cc @jieyouxu This PR modifies If appropriate, please update This PR modifies If appropriate, please update |
This comment was marked as outdated.
This comment was marked as outdated.
I think that if this is an experimental feature designed for testing with a handful of people, I wouldn't add it to bootstrap.toml (nor add a change tracker entry), and just use some magic environment variable for it. |
Fair enough; I'll switch over to an environment variable instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! Other changes look good to me, just a tiny nit regarding "please don't support so many ways to spell a true/false" of the flag lol
@rustbot author |
Reminder, once the PR becomes ready for a review, use |
@bors r+ rollup |
Added a short comment to @bors r=jieyouxu |
compiletest: Implement an experimental `--new-output-capture` mode Thanks to the efforts on rust-lang#140192, compiletest no longer has an unstable dependency on libtest, but it still has an unstable dependency on `#![feature(internal_output_capture)]`. That makes building compiletest more complicated than for most other bootstrap tools. This PR therefore adds opt-in support for an experimental compiletest mode that avoids the use of `internal_output_capture` APIs, and instead uses more mundane means to capture the output of individual test runners. Each `TestCx` now contains `&dyn ConsoleOut` references for stdout and stderr. All print statements in `compiletests::runtest` have been replaced with `write!` or `writeln!` calls that explicitly write to one of those trait objects. The underlying implementation then forwards to `print!` or `eprint!` (for `--no-capture` or old-output-capture mode), or writes to a separate buffer (in new-output-capture mode). --- Currently, new-output-capture is disabled by default. It can be explicitly enabled in one of two ways: - When running `x test`, pass `--new-output-capture=on` as a *compiletest* argument (after `--`). - E.g. `x test ui -- --new-output-capture=on`. - The short form is `-Non` or `-Ny`. - Set environment variable `COMPILETEST_NEW_OUTPUT_CAPTURE=on`. After some amount of opt-in testing, new-output-capture will become the default (with a temporary opt-out). Eventually, old-output-capture and `#![feature(internal_output_capture)]` will be completely removed from compiletest. r? jieyouxu
Rollup of 5 pull requests Successful merges: - #145682 (Promote aarch64-pc-windows-msvc to Tier 1) - #145690 (Implement Integer funnel shifts) - #146119 (compiletest: Implement an experimental `--new-output-capture` mode) - #146168 (Update bootstrap's dependencies to remove winapi and old windows-sys) - #146182 (Don't require next-solver `ProbeRef` to be `Copy`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #146119 - Zalathar:capture, r=jieyouxu compiletest: Implement an experimental `--new-output-capture` mode Thanks to the efforts on #140192, compiletest no longer has an unstable dependency on libtest, but it still has an unstable dependency on `#![feature(internal_output_capture)]`. That makes building compiletest more complicated than for most other bootstrap tools. This PR therefore adds opt-in support for an experimental compiletest mode that avoids the use of `internal_output_capture` APIs, and instead uses more mundane means to capture the output of individual test runners. Each `TestCx` now contains `&dyn ConsoleOut` references for stdout and stderr. All print statements in `compiletests::runtest` have been replaced with `write!` or `writeln!` calls that explicitly write to one of those trait objects. The underlying implementation then forwards to `print!` or `eprint!` (for `--no-capture` or old-output-capture mode), or writes to a separate buffer (in new-output-capture mode). --- Currently, new-output-capture is disabled by default. It can be explicitly enabled in one of two ways: - When running `x test`, pass `--new-output-capture=on` as a *compiletest* argument (after `--`). - E.g. `x test ui -- --new-output-capture=on`. - The short form is `-Non` or `-Ny`. - Set environment variable `COMPILETEST_NEW_OUTPUT_CAPTURE=on`. After some amount of opt-in testing, new-output-capture will become the default (with a temporary opt-out). Eventually, old-output-capture and `#![feature(internal_output_capture)]` will be completely removed from compiletest. r? jieyouxu
Rollup of 5 pull requests Successful merges: - rust-lang/rust#145682 (Promote aarch64-pc-windows-msvc to Tier 1) - rust-lang/rust#145690 (Implement Integer funnel shifts) - rust-lang/rust#146119 (compiletest: Implement an experimental `--new-output-capture` mode) - rust-lang/rust#146168 (Update bootstrap's dependencies to remove winapi and old windows-sys) - rust-lang/rust#146182 (Don't require next-solver `ProbeRef` to be `Copy`) r? `@ghost` `@rustbot` modify labels: rollup
I was thinking of making this the default after a week had passed, but maybe I should wait another week to dodge the release window and nightly-to-beta uplift. (Arguably I'm being overly cautious, but also we're in no great hurry.) |
compiletest: Enable new-output-capture by default The new output-capture implementation was added in rust-lang#146119, but was disabled by default and required opt-in. Since then, I haven't encountered any problems in my own testing/usage, and I haven't heard any problem reports from other contributors who might have opted in. It's unlikely that more opt-in testing will help, so the next step is to enable new-output-capture by default and see if anyone complains. (Hopefully nobody!) If needed, the new default can be overridden (for now) by setting environment variable `COMPILETEST_NEW_OUTPUT_CAPTURE=off`. Please file an issue (or let me know) if anyone finds a reason to do this. r? jieyouxu
Rollup merge of #146574 - Zalathar:capture, r=jieyouxu compiletest: Enable new-output-capture by default The new output-capture implementation was added in #146119, but was disabled by default and required opt-in. Since then, I haven't encountered any problems in my own testing/usage, and I haven't heard any problem reports from other contributors who might have opted in. It's unlikely that more opt-in testing will help, so the next step is to enable new-output-capture by default and see if anyone complains. (Hopefully nobody!) If needed, the new default can be overridden (for now) by setting environment variable `COMPILETEST_NEW_OUTPUT_CAPTURE=off`. Please file an issue (or let me know) if anyone finds a reason to do this. r? jieyouxu
compiletest: Enable new-output-capture by default The new output-capture implementation was added in rust-lang/rust#146119, but was disabled by default and required opt-in. Since then, I haven't encountered any problems in my own testing/usage, and I haven't heard any problem reports from other contributors who might have opted in. It's unlikely that more opt-in testing will help, so the next step is to enable new-output-capture by default and see if anyone complains. (Hopefully nobody!) If needed, the new default can be overridden (for now) by setting environment variable `COMPILETEST_NEW_OUTPUT_CAPTURE=off`. Please file an issue (or let me know) if anyone finds a reason to do this. r? jieyouxu
Thanks to the efforts on #140192, compiletest no longer has an unstable dependency on libtest, but it still has an unstable dependency on
#![feature(internal_output_capture)]
. That makes building compiletest more complicated than for most other bootstrap tools.This PR therefore adds opt-in support for an experimental compiletest mode that avoids the use of
internal_output_capture
APIs, and instead uses more mundane means to capture the output of individual test runners.Each
TestCx
now contains&dyn ConsoleOut
references for stdout and stderr. All print statements incompiletests::runtest
have been replaced withwrite!
orwriteln!
calls that explicitly write to one of those trait objects. The underlying implementation then forwards toprint!
oreprint!
(for--no-capture
or old-output-capture mode), or writes to a separate buffer (in new-output-capture mode).Currently, new-output-capture is disabled by default. It can be explicitly enabled in one of two ways:
x test
, pass--new-output-capture=on
as a compiletest argument (after--
).x test ui -- --new-output-capture=on
.-Non
or-Ny
.COMPILETEST_NEW_OUTPUT_CAPTURE=on
.After some amount of opt-in testing, new-output-capture will become the default (with a temporary opt-out). Eventually, old-output-capture and
#![feature(internal_output_capture)]
will be completely removed from compiletest.r? jieyouxu