Add option to display warnings in rustdoc #41678

GuillaumeGomez · 2017-05-01T19:25:58Z

Part of #41574.

The output for this file:

/// ```
/// fn foo(x: u32) {}
///
/// foo(2);
/// let x = 1;
/// panic!();
/// ```
fn foo() {}

/// ```
/// fn foo(x: u32) {}
///
/// foo(2);
/// let x = 1;
/// ```
fn foo2() {}

/// ```
/// fn foo(x: u32) {}
///
/// foo(2);
/// let x = 1;
/// panic!();
/// ```
fn foo3() {}

fn main() {
}

is the following:

> ./build/x86_64-apple-darwin/stage1/bin/rustdoc -Z unstable-options --display-warnings --test test.rs

running 3 tests
test test.rs - foo (line 1) ... FAILED
test test.rs - foo3 (line 18) ... FAILED
test test.rs - foo2 (line 10) ... ok

successes:

---- test.rs - foo2 (line 10) stdout ----
	warning: unused variable: `x`
 --> <anon>:2:8
  |
2 | fn foo(x: u32) {}
  |        ^
  |
  = note: #[warn(unused_variables)] on by default

warning: unused variable: `x`
 --> <anon>:5:5
  |
5 | let x = 1;
  |     ^
  |
  = note: #[warn(unused_variables)] on by default



successes:
    test.rs - foo2 (line 10)

failures:

---- test.rs - foo (line 1) stdout ----
	warning: unused variable: `x`
 --> <anon>:2:8
  |
2 | fn foo(x: u32) {}
  |        ^
  |
  = note: #[warn(unused_variables)] on by default

warning: unused variable: `x`
 --> <anon>:5:5
  |
5 | let x = 1;
  |     ^
  |
  = note: #[warn(unused_variables)] on by default

thread 'rustc' panicked at 'test executable failed:

thread 'main' panicked at 'explicit panic', <anon>:6
note: Run with `RUST_BACKTRACE=1` for a backtrace.

', src/librustdoc/test.rs:317
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- test.rs - foo3 (line 18) stdout ----
	warning: unused variable: `x`
 --> <anon>:2:8
  |
2 | fn foo(x: u32) {}
  |        ^
  |
  = note: #[warn(unused_variables)] on by default

warning: unused variable: `x`
 --> <anon>:5:5
  |
5 | let x = 1;
  |     ^
  |
  = note: #[warn(unused_variables)] on by default

thread 'rustc' panicked at 'test executable failed:

thread 'main' panicked at 'explicit panic', <anon>:6
note: Run with `RUST_BACKTRACE=1` for a backtrace.

', src/librustdoc/test.rs:317


failures:
    test.rs - foo (line 1)
    test.rs - foo3 (line 18)

test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured

frewsxcv · 2017-05-01T23:56:13Z

Travis found a failure

alexcrichton · 2017-05-02T14:51:17Z

src/librustdoc/markdown.rs

@@ -166,6 +166,9 @@ pub fn test(input: &str, cfgs: Vec<String>, libs: SearchPaths, externs: Externs,
    old_find_testable_code(&input_str, &mut collector, DUMMY_SP);
    find_testable_code(&input_str, &mut collector, DUMMY_SP);
    test_args.insert(0, "rustdoctest".to_string());
+    if display_warnings {
+        test_args.insert(1, "--display-stdout".to_string());


How come this was added? I thought rustc output went to stderr?

The name of the tag is up to debate. I'm using it to make the difference between warnings and errors, but it's clearly misleading. What name should we give it?

I think I may not even understand what this is doing. What is it doing?

So for now, when running rustdoc --test, warnings aren't displayed. Some people want them to be displayed anyway.

Then why not call it --display-warnings

@GuillaumeGomez that's not too helpful of a description, though? You're basically just restating the title and description of this PR.

This flag is implemented inside of libtest which means that it has nothing to do with doctest warnings, and is an instantly stable interface to all test binaries that we generate, hence my hesitation. If we can avoid it that would be great, but I do not understand what this flag is doing inside of the test harness.

Can you explain what's going on inside of libtest to why this is being passed?

Aaaaah. My bad. So basically, in libtest, if I have the display-stdout option enabled, for every non-failing test, I display what the test sent to the output.

Can this be done without exposing a new flag from libtest? Just by modifying internal data structures or using a custom api from libtest?

alexcrichton · 2017-05-02T14:51:34Z

src/librustdoc/lib.rs

@@ -172,6 +172,7 @@ pub fn opts() -> Vec<RustcOptGroup> {
                         or `#![doc(html_playground_url=...)]`",
                        "URL")),
        unstable(optflag("", "enable-commonmark", "to enable commonmark doc rendering/testing")),
+        unstable(optflag("", "display-warnings", "to print code warnings when testing doc")),


Can you open a tracking issue for this as well?

You mean besides the one already opened? (in the PR description)

Yes that can suffice, but then this PR should not close that issue.

GuillaumeGomez · 2017-05-04T21:55:16Z

Updated.

alexcrichton · 2017-05-05T04:42:21Z

Looks like a good strategy to me, but CI seems to be failing?

GuillaumeGomez · 2017-05-05T08:52:50Z

Yep, forgot to update some pieces of code.

alexcrichton · 2017-05-05T14:13:30Z

@bors: r+

bors · 2017-05-05T14:13:31Z

📌 Commit d5863e9 has been approved by alexcrichton

@alexcrichton

…s, r=alexcrichton Add option to display warnings in rustdoc Part of rust-lang#41574. r? @alexcrichton The output for this file: ```rust /// ``` /// fn foo(x: u32) {} /// /// foo(2); /// let x = 1; /// panic!(); /// ``` fn foo() {} /// ``` /// fn foo(x: u32) {} /// /// foo(2); /// let x = 1; /// ``` fn foo2() {} /// ``` /// fn foo(x: u32) {} /// /// foo(2); /// let x = 1; /// panic!(); /// ``` fn foo3() {} fn main() { } ``` is the following: ``` > ./build/x86_64-apple-darwin/stage1/bin/rustdoc -Z unstable-options --display-warnings --test test.rs running 3 tests test test.rs - foo (line 1) ... FAILED test test.rs - foo3 (line 18) ... FAILED test test.rs - foo2 (line 10) ... ok successes: ---- test.rs - foo2 (line 10) stdout ---- warning: unused variable: `x` --> <anon>:2:8 | 2 | fn foo(x: u32) {} | ^ | = note: #[warn(unused_variables)] on by default warning: unused variable: `x` --> <anon>:5:5 | 5 | let x = 1; | ^ | = note: #[warn(unused_variables)] on by default successes: test.rs - foo2 (line 10) failures: ---- test.rs - foo (line 1) stdout ---- warning: unused variable: `x` --> <anon>:2:8 | 2 | fn foo(x: u32) {} | ^ | = note: #[warn(unused_variables)] on by default warning: unused variable: `x` --> <anon>:5:5 | 5 | let x = 1; | ^ | = note: #[warn(unused_variables)] on by default thread 'rustc' panicked at 'test executable failed: thread 'main' panicked at 'explicit panic', <anon>:6 note: Run with `RUST_BACKTRACE=1` for a backtrace. ', src/librustdoc/test.rs:317 note: Run with `RUST_BACKTRACE=1` for a backtrace. ---- test.rs - foo3 (line 18) stdout ---- warning: unused variable: `x` --> <anon>:2:8 | 2 | fn foo(x: u32) {} | ^ | = note: #[warn(unused_variables)] on by default warning: unused variable: `x` --> <anon>:5:5 | 5 | let x = 1; | ^ | = note: #[warn(unused_variables)] on by default thread 'rustc' panicked at 'test executable failed: thread 'main' panicked at 'explicit panic', <anon>:6 note: Run with `RUST_BACKTRACE=1` for a backtrace. ', src/librustdoc/test.rs:317 failures: test.rs - foo (line 1) test.rs - foo3 (line 18) test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured ```

Rollup of 9 pull requests - Successful merges: #41064, #41307, #41512, #41582, #41678, #41722, #41734, #41761, #41763 - Failed merges:

rust-highfive assigned alexcrichton May 1, 2017

GuillaumeGomez force-pushed the rustdoc-test-warnings branch from a65bab9 to 85521a1 Compare May 1, 2017 20:57

GuillaumeGomez force-pushed the rustdoc-test-warnings branch from 85521a1 to 90344be Compare May 2, 2017 08:22

arielb1 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2017

Add option to display warnings in rustdoc

f30ed77

GuillaumeGomez force-pushed the rustdoc-test-warnings branch from 90344be to f30ed77 Compare May 2, 2017 11:57

alexcrichton reviewed May 2, 2017

View reviewed changes

Add Options type in libtest and remove argument

d5863e9

GuillaumeGomez force-pushed the rustdoc-test-warnings branch from bb91763 to d5863e9 Compare May 5, 2017 08:52

frewsxcv mentioned this pull request May 5, 2017

Rollup of 9 pull requests #41773

Merged

bors added a commit that referenced this pull request May 5, 2017

Auto merge of #41773 - frewsxcv:rollup, r=frewsxcv

42a4f37

Rollup of 9 pull requests - Successful merges: #41064, #41307, #41512, #41582, #41678, #41722, #41734, #41761, #41763 - Failed merges:

bors merged commit d5863e9 into rust-lang:master May 6, 2017

GuillaumeGomez deleted the rustdoc-test-warnings branch May 6, 2017 11:21

SimonSapin mentioned this pull request May 7, 2017

Upgrade to rustc 1.19.0-nightly (ced823e26 2017-05-07) Manishearth/compiletest-rs#63

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to display warnings in rustdoc #41678

Add option to display warnings in rustdoc #41678

GuillaumeGomez commented May 1, 2017 •

edited

Loading

frewsxcv commented May 1, 2017

alexcrichton May 2, 2017

GuillaumeGomez May 2, 2017

alexcrichton May 4, 2017

GuillaumeGomez May 4, 2017

frewsxcv May 4, 2017

alexcrichton May 4, 2017

GuillaumeGomez May 4, 2017

alexcrichton May 4, 2017

alexcrichton May 2, 2017

GuillaumeGomez May 2, 2017

alexcrichton May 4, 2017

GuillaumeGomez commented May 4, 2017

alexcrichton commented May 5, 2017

GuillaumeGomez commented May 5, 2017

alexcrichton commented May 5, 2017

bors commented May 5, 2017

Add option to display warnings in rustdoc #41678

Add option to display warnings in rustdoc #41678

Conversation

GuillaumeGomez commented May 1, 2017 • edited Loading

frewsxcv commented May 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented May 4, 2017

alexcrichton commented May 5, 2017

GuillaumeGomez commented May 5, 2017

alexcrichton commented May 5, 2017

bors commented May 5, 2017

GuillaumeGomez commented May 1, 2017 •

edited

Loading