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.

Sign up for GitHub

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

Jump to bottom

rustdoctest: suppress the default allow(unused) under --display-warnings #49064

Merged
merged 1 commit into from
Apr 6, 2018
Merged

rustdoctest: suppress the default allow(unused) under --display-warnings #49064

merged 1 commit into from
Apr 6, 2018

Conversation

QuietMisdreavus
Copy link
Member

If you're passing rustdoc --display-warnings, you probably want to see the default ones too. This change modifies test::make_test to suppress the default #![allow(unused)] if the --display-warnings CLI flag was provided to rustdoc.

cc #41574

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 15, 2018
@QuietMisdreavus
Copy link
Member Author

r? @GuillaumeGomez

GuillaumeGomez
GuillaumeGomez reviewed Mar 16, 2018
View reviewed changes
src/librustdoc/test.rs
let mut opts = TestOptions::default();
opts.display_warnings = true;
let input =
"assert_eq!(2+2, 4);";
Copy link
Member

Choose a reason for hiding this comment

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

Why this strange indent?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since make_test doesn't add indentation to any of the code it parses, i moved everything to the far left margin so i wouldn't have to deal with other ways of keeping the indentation right. For these one-line examples it's not necessary but i wanted both the "before" and "after" to be on the same indent level to visually compare them more easily.

@GuillaumeGomez
Copy link
Member

With last explanation, everything seems good to me. Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 19, 2018

📌 Commit 261efb6 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 19, 2018
@kennytm kennytm mentioned this pull request Mar 21, 2018
Closed
@kennytm
Copy link
Member

kennytm commented Mar 21, 2018

Note: This PR is going to have conflict with #49188, since that #[macro_use] extern crate <crate> is going to add one more line.

[01:26:52] ---- test::tests::make_test_display_warnings stdout ----
[01:26:52] 	thread 'test::tests::make_test_display_warnings' panicked at 'assertion failed: `(left == right)`
[01:26:52]   left: `("fn main() {\nassert_eq!(2+2, 4);\n}", 1)`,
[01:26:52]  right: `("fn main() {\nassert_eq!(2+2, 4);\n}", 2)`', librustdoc/test.rs:969:9
[01:26:52] note: Run with `RUST_BACKTRACE=1` for a backtrace.

@QuietMisdreavus
Copy link
Member Author

@kennytm Oh right, that will cause some issues. Go ahead and land the other one first, then i'll update this one once that lands.

@kennytm
Copy link
Member

kennytm commented Mar 21, 2018

@bors p=1

(Do not roll this up together with #49188)

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 23, 2018
@frewsxcv
Rollup merge of rust-lang#49064 - QuietMisdreavus:piercing-the-veil, …
db65b0c 
…r=GuillaumeGomez

rustdoctest: suppress the default allow(unused) under --display-warnings

If you're passing rustdoc `--display-warnings`, you probably want to see the default ones too. This change modifies `test::make_test` to suppress the default `#![allow(unused)]` if the `--display-warnings` CLI flag was provided to rustdoc.

cc rust-lang#41574
@frewsxcv frewsxcv mentioned this pull request Mar 23, 2018
Closed
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 23, 2018
@frewsxcv
Rollup merge of rust-lang#49064 - QuietMisdreavus:piercing-the-veil, …
79224ae 
…r=GuillaumeGomez

rustdoctest: suppress the default allow(unused) under --display-warnings

If you're passing rustdoc `--display-warnings`, you probably want to see the default ones too. This change modifies `test::make_test` to suppress the default `#![allow(unused)]` if the `--display-warnings` CLI flag was provided to rustdoc.

cc rust-lang#41574
@frewsxcv frewsxcv mentioned this pull request Mar 23, 2018
Closed
@bors
Copy link
Contributor

bors commented Mar 23, 2018

⌛ Testing commit 261efb6 with merge d412d704baae8d3b0e3aad547a7326a34f45b488...

@alexcrichton
Copy link
Member

@bors: retry

prioritizing the rollup and I think this only has p=1 to not be in a rollup

@bors
Copy link
Contributor

bors commented Mar 23, 2018

⌛ Testing commit 261efb6 with merge de3aeb349ed842bcfd23d351986f73bdb6e60670...

@bors bors mentioned this pull request Mar 23, 2018
Merged
@alexcrichton
Copy link
Member

@bors: retry

same reason as before

@bors
Copy link
Contributor

bors commented Mar 23, 2018

⌛ Testing commit 261efb6 with merge 2a9aab802d2eb10933938c961334661b9075349b...

@bors
Copy link
Contributor

bors commented Mar 23, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 23, 2018
@kennytm
Copy link
Member

kennytm commented Mar 23, 2018 •
edited
Loading

Failure is expected. #49064 (comment)

@bors p=0

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 23, 2018
@shepmaster
Copy link
Member

Ping from triage @QuietMisdreavus ! It's been a week since we last heard from you, but I'm not sure exactly where the problem is here... Maybe @kennytm or @alexcrichton can chime in with the suggested next step...

@QuietMisdreavus
Copy link
Member Author

@shepmaster The failure was expected, because a different PR changed the return value for one of these tests. I need to rebase and fix the test. Due to the all-hands last week, i also let my github notifications slide a little... >_>

@QuietMisdreavus
Copy link
Member Author

Rebased and fixed the test. It passed locally, but i'll wait for Travis to be sure.

@QuietMisdreavus
Copy link
Member Author

(3 days later... >_>)

Travis passed, gonna poke this one back out.

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Apr 5, 2018

📌 Commit 8145a77 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 5, 2018
@GuillaumeGomez
Copy link
Member

You did well!

@bors
Copy link
Contributor

bors commented Apr 5, 2018

⌛ Testing commit 8145a77 with merge 486757e28916a381c08697dd514d87dcdc0d2615...

@bors
Copy link
Contributor

bors commented Apr 6, 2018

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 6, 2018
@alexcrichton
Copy link
Member

@bors: retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 6, 2018
@bors
Copy link
Contributor

bors commented Apr 6, 2018

⌛ Testing commit 8145a77 with merge eeea94c...

bors added a commit that referenced this pull request Apr 6, 2018
@bors
Auto merge of #49064 - QuietMisdreavus:piercing-the-veil, r=Guillaume…
eeea94c 
…Gomez

rustdoctest: suppress the default allow(unused) under --display-warnings

If you're passing rustdoc `--display-warnings`, you probably want to see the default ones too. This change modifies `test::make_test` to suppress the default `#![allow(unused)]` if the `--display-warnings` CLI flag was provided to rustdoc.

cc #41574
@bors
Copy link
Contributor

bors commented Apr 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: GuillaumeGomez
Pushing eeea94c to master...

@bors bors merged commit 8145a77 into rust-lang:master Apr 6, 2018
@QuietMisdreavus QuietMisdreavus deleted the piercing-the-veil branch May 9, 2018 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez left review comments

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@QuietMisdreavus @rust-highfive @GuillaumeGomez @bors @kennytm @alexcrichton @shepmaster @steveklabnik