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

Have an option to output warnings from doctests #41574

Open
Susurrus opened this Issue Apr 27, 2017 · 9 comments

Comments

Projects
None yet
8 participants
@Susurrus
Contributor

Susurrus commented Apr 27, 2017

Running cargo test runs doctests by default, and shows warnings for regular code, but not for the doctests themselves. This can be enabled by running cargo test -- --nocapture. I'd like to have warnings also show in our CI tests, even for doctests, but setting -- --nocapture shows all the output which we don't want (and can't control, because they come from the underlying libraries complaining about our tests that are supposed to fail).

I'd think having warnings on for doctests by default would be good, but that wouldn't be backwards compatible. Maybe a --doctest-warnings option could be added in lieu of that?

I believe this was done in 5d145c1, so discussion there might be worth reading. It seems that there are a bunch of warnings that come up with doctests that should be ignored, but there are also ones that can appear that shouldn't be. The question is how do we address both needs?

@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Apr 28, 2017

I can put it back if you want. Just waiting for @rust-lang/docs team confirmation.

@frewsxcv

This comment has been minimized.

Member

frewsxcv commented Apr 28, 2017

This might be a tools team question, less-so a docs team one.

@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Apr 29, 2017

Let's make them join the issue then!

cc @rust-lang/tools

@carols10cents carols10cents changed the title from doctests don't print warnings by default to Have an option to output warnings from doctests Apr 29, 2017

@carols10cents

This comment has been minimized.

Member

carols10cents commented Apr 29, 2017

I've updated the title to better reflect what I think this request is, please correct it if I've misunderstood.

Many doc tests are small and would have warnings for unused variables, etc. I wouldn't want to see these by default, but I'd be ok with a flag to turn on just output of doc test warnings. I don't think I'd use it, but I wouldn't mind it being there.

Is there any way to get a sense of how many people would use this if it existed and the kinds of problems it would prevent, in order to prioritize this?

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Apr 29, 2017

The standard library handles this by denying all warnings, but it seems reasonable to have a flag. I don't think we can change the default behavior.

@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Apr 30, 2017

Ok, so I think we can consider this as acceptable. I'll add the flag.

frewsxcv added a commit to frewsxcv/rust that referenced this issue May 5, 2017

Rollup merge of rust-lang#41678 - GuillaumeGomez:rustdoc-test-warning…
…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
```

@steveklabnik steveklabnik added T-dev-tools and removed T-tools labels May 18, 2017

smangelsdorf pushed a commit to gotham-rs/gotham that referenced this issue Aug 11, 2017

@QuietMisdreavus

This comment has been minimized.

Member

QuietMisdreavus commented Mar 16, 2018

I'm digging into this CLI flag for #49028 and i'm struggling to figure out whether it's useful now. From digging into the implementation that remains, i can find two major features for it:

  • Enables --nocapture for doctests, which at the moment just allows it to show compilation warnings
  • Turns off the default #![allow(warnings)] that gets applied in regular documentation runs, allowing compilation warnings to be emitted

However, neither one is that useful, because for the first, you can just provide --test-args --nocapture (or -- --nocapture for cargo test) to get functionally the same output, and for the second one, the compilation output seems to still be suppressed anyway. Was there intended to be additional functionality for this?

@QuietMisdreavus

This comment has been minimized.

Member

QuietMisdreavus commented Mar 16, 2018

So here's the results of my testing that led me to this conclusion, and i want to know whether this flag is worth keeping, or whether we need to file additional bugs that can be worked on.

#![crate_type = "lib"]
#![doc(test(attr(warn(warnings))))] // suppress the default allow(unused)

/// (written on a spider's web) Some Struct
///
/// ```
/// let x = 5;
/// println!("sup");
/// ```
pub struct SomeStruct;

struct OtherStruct; // emit a warning when compiling
console output for various interactions with this rust file
$ rustdoc +nightly --version
rustdoc 1.26.0-nightly (392645394 2018-03-15)
$ rustc +nightly g.rs
warning: struct is never used: `OtherStruct`
  --> g.rs:12:1
   |
12 | struct OtherStruct;
   | ^^^^^^^^^^^^^^^^^^^
   |
   = note: #[warn(dead_code)] on by default

$ rustdoc +nightly g.rs
(no output)
$ rustdoc +nightly g.rs -Z unstable-options --display-warnings
(no output)
$ rustdoc +nightly --test g.rs

running 1 test
test g.rs - SomeStruct (line 6) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ rustdoc +nightly --test g.rs -Z unstable-options --display-warnings

running 1 test
test g.rs - SomeStruct (line 6) ... ok

successes:

---- g.rs - SomeStruct (line 6) stdout ----
        warning: unused variable: `x`
 --> g.rs:7:5
  |
3 | let x = 5;
  |     ^ help: consider using `_x` instead
  |
  = note: #[warn(unused_variables)] on by default



successes:
    g.rs - SomeStruct (line 6)

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ rustdoc +nightly --test g.rs --test-args "--nocapture"

running 1 test
warning: unused variable: `x`
 --> g.rs:7:5
  |
3 | let x = 5;
  |     ^ help: consider using `_x` instead
  |
  = note: #[warn(unused_variables)] on by default

test g.rs - SomeStruct (line 6) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Mar 19, 2018

Should we add an additional option to blockcodes as well? Like:

```rust,show_warnings
let x = 12;
```

kennytm added a commit to kennytm/rust that referenced this issue Mar 21, 2018

Rollup merge of rust-lang#49064 - QuietMisdreavus:piercing-the-veil, …
…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 added a commit to frewsxcv/rust that referenced this issue Mar 23, 2018

Rollup merge of rust-lang#49064 - QuietMisdreavus:piercing-the-veil, …
…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 added a commit to frewsxcv/rust that referenced this issue Mar 23, 2018

Rollup merge of rust-lang#49064 - QuietMisdreavus:piercing-the-veil, …
…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

bors added a commit that referenced this issue Mar 23, 2018

Auto merge of #49064 - QuietMisdreavus:piercing-the-veil, r=Guillaume…
…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 added a commit that referenced this issue Mar 23, 2018

Auto merge of #49064 - QuietMisdreavus:piercing-the-veil, r=Guillaume…
…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 added a commit that referenced this issue Mar 23, 2018

Auto merge of #49064 - QuietMisdreavus:piercing-the-veil, r=Guillaume…
…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 added a commit that referenced this issue Mar 23, 2018

Auto merge of #49064 - QuietMisdreavus:piercing-the-veil, r=Guillaume…
…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 added a commit that referenced this issue Mar 23, 2018

Auto merge of #49064 - QuietMisdreavus:piercing-the-veil, r=Guillaume…
…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 added a commit that referenced this issue Apr 5, 2018

Auto merge of #49064 - QuietMisdreavus:piercing-the-veil, r=Guillaume…
…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 added a commit that referenced this issue Apr 6, 2018

Auto merge of #49064 - QuietMisdreavus:piercing-the-veil, r=Guillaume…
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment