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

Remove --display-doctest-warnings #91259

Merged
merged 1 commit into from
Nov 27, 2021
Merged

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Nov 26, 2021

--display-doctest-warnings can be replicated in full with other existing features, there's no
need to have a separate option for it. This removes the option and documents the combination of other features to replicate it.

This also fixes a bug where --test-args=--show-output had no effect.

cc @ollie27, #73314 (comment)
Fixes #41574

r? @GuillaumeGomez

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-doctests Area: Documentation tests, run by rustdoc labels Nov 26, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2021
@camelid
Copy link
Member

camelid commented Nov 26, 2021

This also fixes a bug where --test-args=--show-output had no effect.

Do you think you could separate that into its own commit? It'd make it easier to understand what's being changed.

@jyn514
Copy link
Member Author

jyn514 commented Nov 26, 2021

Do you think you could separate that into its own commit? It'd make it easier to understand what's being changed.

Not really? This was the fix:

diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs
index de4a3732e73..56ccdfae1d8 100644
--- a/src/librustdoc/doctest.rs
+++ b/src/librustdoc/doctest.rs
@@ -208,29 +205,19 @@
     Ok(())
 }
 
-crate fn run_tests(
-    mut test_args: Vec<String>,
-    nocapture: bool,
-    display_doctest_warnings: bool,
-    tests: Vec<test::TestDescAndFn>,
-) {
+crate fn run_tests(mut test_args: Vec<String>, nocapture: bool, tests: Vec<test::TestDescAndFn>) {
     test_args.insert(0, "rustdoctest".to_string());
     if nocapture {
         test_args.push("--nocapture".to_string());
     }
-    test::test_main(
-        &test_args,
-        tests,
-        Some(test::Options::new().display_output(display_doctest_warnings)),
-    );
+    test::test_main(&test_args, tests, None);
 }
 

The problem was that display_output(display_doctest_warnings) was overriding the argument passed on the command line.

@GuillaumeGomez
Copy link
Member

You even provided the documentation alongside, it's great, thanks!

r=me once CI pass

This can be replicated in full with other existing features, there's no
need to have a separate option for it.

This also fixes a bug where `--test-args=--show-output` had no effect,
and updates the documentation.
@jyn514
Copy link
Member Author

jyn514 commented Nov 26, 2021

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Nov 26, 2021

📌 Commit 7e4bf4b 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 Nov 26, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2021
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90611 (Fix another ICE in rustdoc scrape_examples)
 - rust-lang#91197 (rustdoc: Rename `Type::ResolvedPath` to `Type::Path` and don't re-export it)
 - rust-lang#91223 (Fix headings indent)
 - rust-lang#91240 (Saner formatting for UTF8_CHAR_WIDTH table)
 - rust-lang#91248 (Bump compiler-builtins to 0.1.53)
 - rust-lang#91252 (Fix bug where submodules wouldn't be updated when running x.py from a subdirectory)
 - rust-lang#91259 (Remove `--display-doctest-warnings`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 092477d into rust-lang:master Nov 27, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 27, 2021
@jyn514 jyn514 deleted the doctest-warnings branch April 22, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doctests Area: Documentation tests, run by rustdoc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have an option to output warnings from doctests
7 participants