Auto merge of #143900 - GuillaumeGomez:fix-no-run, r=lolbinarycat,fmease

[rustdoc] Correctly handle `should_panic` doctest attribute and fix `--no-run` test flag on the 2024 edition

Fixes #143009.
Fixes #143858.

Since it includes fixes from #143453, it's taking it over (commits 2, 3 and 4 are from #143453).

For `--no-run`, we forgot to check the "global" options in the 2024 edition, fixed in the first commit.

For `should_panic` fix, the exit code check has been fixed.

cc `@TroyKomodo` (thanks so much for providing such a complete test, made my life a lot easier!)
r? `@notriddle`

Author: bors

library/std/src/error.rs

@@ -123,7 +123,7 @@ use crate::fmt::{self, Write};
 /// the `Debug` output means `Report` is an ideal starting place for formatting errors returned
 /// from `main`.
 ///
-/// ```should_panic
+/// ```
 /// #![feature(error_reporter)]
 /// use std::error::Report;
 /// # use std::error::Error;
@@ -154,10 +154,14 @@ use crate::fmt::{self, Write};
 /// #     Err(SuperError { source: SuperErrorSideKick })
 /// # }
 ///
-/// fn main() -> Result<(), Report<SuperError>> {
+/// fn run() -> Result<(), Report<SuperError>> {
 ///     get_super_error()?;
 ///     Ok(())
 /// }
+///
+/// fn main() {
+///     assert!(run().is_err());
+/// }
 /// ```
 ///
 /// This example produces the following output:
@@ -170,7 +174,7 @@ use crate::fmt::{self, Write};
 /// output format. If you want to make sure your `Report`s are pretty printed and include backtrace
 /// you will need to manually convert and enable those flags.
 ///
-/// ```should_panic
+/// ```
 /// #![feature(error_reporter)]
 /// use std::error::Report;
 /// # use std::error::Error;
@@ -201,12 +205,16 @@ use crate::fmt::{self, Write};
 /// #     Err(SuperError { source: SuperErrorSideKick })
 /// # }
 ///
-/// fn main() -> Result<(), Report<SuperError>> {
+/// fn run() -> Result<(), Report<SuperError>> {
 ///     get_super_error()
 ///         .map_err(Report::from)
 ///         .map_err(|r| r.pretty(true).show_backtrace(true))?;
 ///     Ok(())
 /// }
+///
+/// fn main() {
+///     assert!(run().is_err());
+/// }
 /// ```
 ///
 /// This example produces the following output:

src/librustdoc/doctest.rs

@@ -358,7 +358,7 @@ pub(crate) fn run_tests(
         );
 
         for (doctest, scraped_test) in &doctests {
-            tests_runner.add_test(doctest, scraped_test, &target_str);
+            tests_runner.add_test(doctest, scraped_test, &target_str, rustdoc_options);
         }
         let (duration, ret) = tests_runner.run_merged_tests(
             rustdoc_test_options,
@@ -828,7 +828,8 @@ fn run_test(
     match result {
         Err(e) => return (duration, Err(TestFailure::ExecutionError(e))),
         Ok(out) => {
-            if langstr.should_panic && out.status.success() {
+            // FIXME: use test::ERROR_EXIT_CODE once public
+            if langstr.should_panic && out.status.code() != Some(101) {
                 return (duration, Err(TestFailure::UnexpectedRunPass));
             } else if !langstr.should_panic && !out.status.success() {
                 return (duration, Err(TestFailure::ExecutionFailure(out)));
@@ -1138,7 +1139,7 @@ fn doctest_run_fn(
                 eprint!("Test compiled successfully, but it's marked `compile_fail`.");
             }
             TestFailure::UnexpectedRunPass => {
-                eprint!("Test executable succeeded, but it's marked `should_panic`.");
+                eprint!("Test didn't panic, but it's marked `should_panic`.");
             }
             TestFailure::MissingErrorCodes(codes) => {
                 eprint!("Some expected error codes were not found: {codes:?}");

src/librustdoc/doctest/runner.rs

@@ -39,6 +39,7 @@ impl DocTestRunner {
         doctest: &DocTestBuilder,
         scraped_test: &ScrapedDocTest,
         target_str: &str,
+        opts: &RustdocOptions,
     ) {
         let ignore = match scraped_test.langstr.ignore {
             Ignore::All => true,
@@ -62,6 +63,7 @@ impl DocTestRunner {
                 self.nb_tests,
                 &mut self.output,
                 &mut self.output_merged_tests,
+                opts,
             ),
         ));
         self.supports_color &= doctest.supports_color;
@@ -134,13 +136,21 @@ mod __doctest_mod {{
     }}
 
     #[allow(unused)]
-    pub fn doctest_runner(bin: &std::path::Path, test_nb: usize) -> ExitCode {{
+    pub fn doctest_runner(bin: &std::path::Path, test_nb: usize, should_panic: bool) -> ExitCode {{
         let out = std::process::Command::new(bin)
             .env(self::RUN_OPTION, test_nb.to_string())
             .args(std::env::args().skip(1).collect::<Vec<_>>())
             .output()
             .expect(\"failed to run command\");
-        if !out.status.success() {{
+        if should_panic {{
+            // FIXME: use test::ERROR_EXIT_CODE once public
+            if out.status.code() != Some(101) {{
+                eprintln!(\"Test didn't panic, but it's marked `should_panic`.\");
+                ExitCode::FAILURE
+            }} else {{
+                ExitCode::SUCCESS
+            }}
+        }} else if !out.status.success() {{
             if let Some(code) = out.status.code() {{
                 eprintln!(\"Test executable failed (exit status: {{code}}).\");
             }} else {{
@@ -223,6 +233,7 @@ fn generate_mergeable_doctest(
     id: usize,
     output: &mut String,
     output_merged_tests: &mut String,
+    opts: &RustdocOptions,
 ) -> String {
     let test_id = format!("__doctest_{id}");
 
@@ -256,22 +267,21 @@ fn main() {returns_result} {{
         )
         .unwrap();
     }
-    let not_running = ignore || scraped_test.langstr.no_run;
+    let not_running = ignore || scraped_test.no_run(opts);
     writeln!(
         output_merged_tests,
         "
 mod {test_id} {{
 pub const TEST: test::TestDescAndFn = test::TestDescAndFn::new_doctest(
-{test_name:?}, {ignore}, {file:?}, {line}, {no_run}, {should_panic},
+{test_name:?}, {ignore}, {file:?}, {line}, {no_run}, false,
 test::StaticTestFn(
     || {{{runner}}},
 ));
 }}",
         test_name = scraped_test.name,
         file = scraped_test.path(),
         line = scraped_test.line,
-        no_run = scraped_test.langstr.no_run,
-        should_panic = !scraped_test.langstr.no_run && scraped_test.langstr.should_panic,
+        no_run = scraped_test.no_run(opts),
         // Setting `no_run` to `true` in `TestDesc` still makes the test run, so we simply
         // don't give it the function to run.
         runner = if not_running {
@@ -280,11 +290,12 @@ test::StaticTestFn(
             format!(
                 "
 if let Some(bin_path) = crate::__doctest_mod::doctest_path() {{
-    test::assert_test_result(crate::__doctest_mod::doctest_runner(bin_path, {id}))
+    test::assert_test_result(crate::__doctest_mod::doctest_runner(bin_path, {id}, {should_panic}))
 }} else {{
     test::assert_test_result(doctest_bundle::{test_id}::__main_fn())
 }}
 ",
+                should_panic = scraped_test.langstr.should_panic,
             )
         },
     )

tests/run-make/rustdoc-should-panic/rmake.rs

@@ -0,0 +1,36 @@
+// Ensure that `should_panic` doctests only succeed if the test actually panicked.
+// Regression test for <https://github.com/rust-lang/rust/issues/143009>.
+
+//@ needs-target-std
+
+use run_make_support::rustdoc;
+
+fn check_output(output: String, edition: &str) {
+    let should_contain = &[
+        "test test.rs - bad_exit_code (line 1) ... FAILED",
+        "test test.rs - did_not_panic (line 6) ... FAILED",
+        "test test.rs - did_panic (line 11) ... ok",
+        "---- test.rs - bad_exit_code (line 1) stdout ----
+Test executable failed (exit status: 1).",
+        "---- test.rs - did_not_panic (line 6) stdout ----
+Test didn't panic, but it's marked `should_panic`.",
+        "test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out;",
+    ];
+    for text in should_contain {
+        assert!(
+            output.contains(text),
+            "output (edition: {edition}) doesn't contain {:?}\nfull output: {output}",
+            text
+        );
+    }
+}
+
+fn main() {
+    check_output(rustdoc().input("test.rs").arg("--test").run_fail().stdout_utf8(), "2015");
+
+    // Same check with the merged doctest feature (enabled with the 2024 edition).
+    check_output(
+        rustdoc().input("test.rs").arg("--test").edition("2024").run_fail().stdout_utf8(),
+        "2024",
+    );
+}

tests/run-make/rustdoc-should-panic/test.rs

@@ -0,0 +1,14 @@
+/// ```
+/// std::process::exit(1);
+/// ```
+fn bad_exit_code() {}
+
+/// ```should_panic
+/// std::process::exit(1);
+/// ```
+fn did_not_panic() {}
+
+/// ```should_panic
+/// panic!("yeay");
+/// ```
+fn did_panic() {}

tests/rustdoc-ui/doctest/failed-doctest-should-panic-2021.stdout

@@ -5,7 +5,7 @@ test $DIR/failed-doctest-should-panic-2021.rs - Foo (line 10) ... FAILED
 failures:
 
 ---- $DIR/failed-doctest-should-panic-2021.rs - Foo (line 10) stdout ----
-Test executable succeeded, but it's marked `should_panic`.
+Test didn't panic, but it's marked `should_panic`.
 
 failures:
     $DIR/failed-doctest-should-panic-2021.rs - Foo (line 10)

tests/rustdoc-ui/doctest/failed-doctest-should-panic.stdout

@@ -1,11 +1,12 @@
 
 running 1 test
-test $DIR/failed-doctest-should-panic.rs - Foo (line 12) - should panic ... FAILED
+test $DIR/failed-doctest-should-panic.rs - Foo (line 12) ... FAILED
 
 failures:
 
 ---- $DIR/failed-doctest-should-panic.rs - Foo (line 12) stdout ----
-note: test did not panic as expected at $DIR/failed-doctest-should-panic.rs:12:0
+Test didn't panic, but it's marked `should_panic`.
+
 
 failures:
     $DIR/failed-doctest-should-panic.rs - Foo (line 12)

tests/rustdoc-ui/doctest/no-run.edition2021.stdout

@@ -0,0 +1,12 @@
+
+running 7 tests
+test $DIR/no-run.rs - f (line 14) - compile ... ok
+test $DIR/no-run.rs - f (line 17) - compile ... ok
+test $DIR/no-run.rs - f (line 20) ... ignored
+test $DIR/no-run.rs - f (line 23) - compile ... ok
+test $DIR/no-run.rs - f (line 29) - compile fail ... ok
+test $DIR/no-run.rs - f (line 34) - compile ... ok
+test $DIR/no-run.rs - f (line 38) - compile ... ok
+
+test result: ok. 6 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in $TIME
+

tests/rustdoc-ui/doctest/no-run.edition2024.stdout

@@ -0,0 +1,18 @@
+
+running 5 tests
+test $DIR/no-run.rs - f (line 14) - compile ... ok
+test $DIR/no-run.rs - f (line 17) - compile ... ok
+test $DIR/no-run.rs - f (line 23) - compile ... ok
+test $DIR/no-run.rs - f (line 34) - compile ... ok
+test $DIR/no-run.rs - f (line 38) - compile ... ok
+
+test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
+
+
+running 2 tests
+test $DIR/no-run.rs - f (line 20) ... ignored
+test $DIR/no-run.rs - f (line 29) - compile fail ... ok
+
+test result: ok. 1 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in $TIME
+
+all doctests ran in $TIME; merged doctests compilation took $TIME

tests/rustdoc-ui/doctest/no-run.rs

@@ -0,0 +1,44 @@
+// This test ensures that the `--no-run` flag works the same between normal and merged doctests.
+// Regression test for <https://github.com/rust-lang/rust/issues/143858>.
+
+//@ check-pass
+//@ revisions: edition2021 edition2024
+//@ [edition2021]edition:2021
+//@ [edition2024]edition:2024
+//@ compile-flags:-Z unstable-options --test --no-run --test-args=--test-threads=1
+//@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR"
+//@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME"
+//@ normalize-stdout: "ran in \d+\.\d+s" -> "ran in $$TIME"
+//@ normalize-stdout: "compilation took \d+\.\d+s" -> "compilation took $$TIME"
+
+/// ```
+/// let a = true;
+/// ```
+/// ```should_panic
+/// panic!()
+/// ```
+/// ```ignore (incomplete-code)
+/// fn foo() {
+/// ```
+/// ```no_run
+/// loop {
+///     println!("Hello, world");
+/// }
+/// ```
+/// fails to compile
+/// ```compile_fail
+/// let x = 5;
+/// x += 2; // shouldn't compile!
+/// ```
+/// Ok the test does not run
+/// ```
+/// panic!()
+/// ```
+/// Ok the test does not run
+/// ```should_panic
+/// loop {
+///     println!("Hello, world");
+/// panic!()
+/// }
+/// ```
+pub fn f() {}