Skip to content
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

Shorter output for `rustc --test` binaries. #31887

Merged
merged 2 commits into from Mar 15, 2016

Conversation

Projects
None yet
3 participants
@SimonSapin
Copy link
Contributor

SimonSapin commented Feb 25, 2016

Until now, a program created with rustc --test prints at least one line per test. This can be very verbose, especially with data-driven tests when hundreds or thousands of tests is not rare.

This changes the default output to one character per test (except metrics and benchmarks results which have additional data to show):

     Running target/debug/wpt-75c594dc1e6e6187

running 314 tests
..............................................................................
..............................................................................
..............................................................................
..............................................................................
..
test result: ok. 314 passed; 0 failed; 0 ignored; 0 measured

The previous behavior is available by passing --verbose to the test program. Maybe cargo test --verbose could be changed to do that? Edit: the default is now unchanged, -q or --quiet enables the new output.

SimonSapin added a commit to servo/html5ever that referenced this pull request Feb 25, 2016

@SimonSapin SimonSapin force-pushed the SimonSapin:quiet-test branch from 4a32a10 to 7c288b0 Feb 25, 2016

SimonSapin added a commit to servo/html5ever that referenced this pull request Feb 25, 2016

@SimonSapin SimonSapin force-pushed the SimonSapin:quiet-test branch 2 times, most recently from 1598260 to 0412918 Feb 26, 2016

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 26, 2016

I personally think that the ship has unfortunately sailed on changing libtest too much right now. We've talked a bit about custom test frameworks/runners recently and this would certainly be easily fixed by a custom test runner, and until that time we may just not be able to change the format in terms of stability

In terms of personal opinion, I can see the value in having a more terse output but I prefer to see the longer output. It allows easily diffing two runs to see which test deadlocked, in single threaded mode it shows which test is running (to see what deadlocked), etc.

@SimonSapin

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Feb 26, 2016

Regarding stability: would it be better if this new output was not the default? Maybe with a .cargo/config pref.

Regarding preference: that’s why the old output is still available with --verbose. (Single-threaded mode is not the default either, so you have to opt into something anyway.) Sure, when things go wrong more output can help diagnose what’s going on. But most of the time flooding the terminal with hundreds of lines of test foo... ok is just noise that hides relevant bits of outputs like build warnings.

@SimonSapin SimonSapin force-pushed the SimonSapin:quiet-test branch 2 times, most recently from 6124641 to f4d71e2 Feb 26, 2016

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 26, 2016

Oh last time I meant to cc @rust-lang/tools

I don't think .cargo/config has much to do here unless we want to add a TOML parser to libtest, and I'd also be more ok with adding a flag to get output different than what's there today

@SimonSapin

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Feb 26, 2016

Re .cargo/config you’re right, I was confused with cargo test.

For what it’s worth, I’ve applied this to https://crates.io/crates/rustc-test. You can see it in action in https://travis-ci.org/servo/html5ever/jobs/111813368#L281

@alexcrichton alexcrichton self-assigned this Mar 6, 2016

@alexcrichton alexcrichton added the T-tools label Mar 6, 2016

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 9, 2016

Ok, we discussed this in the tools triage meeting today, and the conclusion was that we probably want to leave the output the same as-is today, but we're fine with a flag being added to enable this output. Would you be ok with that @SimonSapin?

@alexcrichton alexcrichton added relnotes and removed T-tools labels Mar 9, 2016

@SimonSapin SimonSapin force-pushed the SimonSapin:quiet-test branch from f4d71e2 to a5cafc2 Mar 9, 2016

@SimonSapin

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Mar 9, 2016

Alright. I’ve amended the commit to flip the default.

$(call RUN,test-ignore-cfg) | grep 'shouldnotignore ... ok'
$(call RUN,test-ignore-cfg) | grep 'shouldignore ... ignored'
$(call RUN,test-ignore-cfg) --verbose | grep 'shouldnotignore ... ok'
$(call RUN,test-ignore-cfg) --verbose | grep 'shouldignore ... ignored'

This comment has been minimized.

@alexcrichton

alexcrichton Mar 10, 2016

Member

This and the previous test can be reverted to what they once were, right?

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 10, 2016

Thanks! Can you also add a test for this switch + output?

@SimonSapin SimonSapin force-pushed the SimonSapin:quiet-test branch from a5cafc2 to 5b372cf Mar 10, 2016

SimonSapin added some commits Feb 25, 2016

Shorter output for `rustc --test` binaries.
A program created with `rustc --test` prints at least one line per test.
This can be very verbose, especially with [data-driven tests](
https://internals.rust-lang.org/t/test-and-external-test-harnesses/3145)
when hundreds or thousands of tests is not rare.

This adds a `-q` or `--quiet` option that changes the output
to one character instead of one line per test
(except metrics and benchmarks results which have additional data to
show):

```
     Running target/debug/wpt-75c594dc1e6e6187

running 314 tests
..............................................................................
..............................................................................
..............................................................................
..............................................................................
..
test result: ok. 314 passed; 0 failed; 0 ignored; 0 measured
```

This is a breaking change since the `test::TestOpts` struct
now has one more field.

@SimonSapin SimonSapin force-pushed the SimonSapin:quiet-test branch from a92b1a4 to d23fd71 Mar 15, 2016

@SimonSapin

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Mar 15, 2016

Done.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 15, 2016

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 15, 2016

⌛️ Testing commit d23fd71 with merge 74dfc1d...

bors added a commit that referenced this pull request Mar 15, 2016

Auto merge of #31887 - SimonSapin:quiet-test, r=alexcrichton
Shorter output for `rustc --test` binaries.

Until now, a program created with `rustc --test` prints at least one line per test. This can be very verbose, especially with [data-driven tests](https://internals.rust-lang.org/t/test-and-external-test-harnesses/3145) when hundreds or thousands of tests is not rare.

This changes the default output to one character per test (except metrics and benchmarks results which have additional data to show):

```
     Running target/debug/wpt-75c594dc1e6e6187

running 314 tests
..............................................................................
..............................................................................
..............................................................................
..............................................................................
..
test result: ok. 314 passed; 0 failed; 0 ignored; 0 measured
```

<s>The previous behavior is available by passing `--verbose` to the test program. Maybe `cargo test --verbose` could be changed to do that?</s> **Edit:** the default is now unchanged, `-q` or `--quiet` enables the new output.

@bors bors merged commit d23fd71 into rust-lang:master Mar 15, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@SimonSapin SimonSapin deleted the SimonSapin:quiet-test branch Mar 16, 2016

@mcarton mcarton referenced this pull request Mar 16, 2016

Merged

Rustup, support `quiet` #34

SimonSapin added a commit to servo/html5ever that referenced this pull request Mar 24, 2016

bors-servo added a commit to servo/html5ever that referenced this pull request Mar 24, 2016

Auto merge of #200 - servo:shrink, r=nox
Use shrinked output built into the test harness.

servo/rustc-test@e3a9adc

Also proposed upstream at rust-lang/rust#31887

r? @nox

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/html5ever/200)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.