Improve error messages for failing should_panic doctests

Added missing FIXME comments

Author: GuillaumeGomez

src/librustdoc/doctest.rs

@@ -466,7 +466,7 @@ enum TestFailure {
     /// This typically means an assertion in the test failed or another form of panic occurred.
     ExecutionFailure(process::Output),
     /// The test is marked `should_panic` but the test binary executed successfully.
-    UnexpectedRunPass,
+    NoPanic(Option<String>),
 }
 
 enum DirState {
@@ -812,6 +812,9 @@ fn run_test(
         // (cfg!(target_family = "wasm") || cfg!(target_os = "zkvm"))
         //     && !cfg!(target_os = "emscripten")
         // ```
+        //
+        // FIXME: All this code is terrible and doesn't take into account `TargetTuple::TargetJson`.
+        // If `libtest` doesn't allow to handle this case, we'll need to use a rustc's API instead.
         && let TargetTuple::TargetTuple(ref s) = rustdoc_options.target
         && let mut iter = s.split('-')
         && let Some(arch) = iter.next()
@@ -876,12 +879,41 @@ fn run_test(
                     #[cfg(windows)]
                     Some(STATUS_FAIL_FAST_EXCEPTION) => {}
                     #[cfg(unix)]
-                    None if out.status.signal() == Some(SIGABRT) => {}
+                    None => match out.status.signal() {
+                        Some(SIGABRT) => {}
+                        Some(signal) => {
+                            return (
+                                duration,
+                                Err(TestFailure::NoPanic(Some(format!(
+                                    "Test didn't panic, but it's marked `should_panic` (exit signal: {signal}).",
+                                )))),
+                            );
+                        }
+                        None => {
+                            return (
+                                duration,
+                                Err(TestFailure::NoPanic(Some(format!(
+                                    "Test didn't panic, but it's marked `should_panic` and exited with no error code and no signal.",
+                                )))),
+                            );
+                        }
+                    },
+                    #[cfg(not(unix))]
+                    None => return (duration, Err(TestFailure::NoPanic(None))),
                     // Upon an abort, Fuchsia returns the status code
                     // `ZX_TASK_RETCODE_EXCEPTION_KILL`.
                     #[cfg(target_os = "fuchsia")]
                     Some(ZX_TASK_RETCODE_EXCEPTION_KILL) => {}
-                    _ => return (duration, Err(TestFailure::UnexpectedRunPass)),
+                    Some(exit_code) => {
+                        let err_msg = if !out.status.success() {
+                            Some(format!(
+                                "Test didn't panic, but it's marked `should_panic` (exit status: {exit_code}).",
+                            ))
+                        } else {
+                            None
+                        };
+                        return (duration, Err(TestFailure::NoPanic(err_msg)));
+                    }
                 }
             } else if !out.status.success() {
                 return (duration, Err(TestFailure::ExecutionFailure(out)));
@@ -1190,8 +1222,12 @@ fn doctest_run_fn(
             TestFailure::UnexpectedCompilePass => {
                 eprint!("Test compiled successfully, but it's marked `compile_fail`.");
             }
-            TestFailure::UnexpectedRunPass => {
-                eprint!("Test didn't panic, but it's marked `should_panic`.");
+            TestFailure::NoPanic(msg) => {
+                if let Some(msg) = msg {
+                    eprint!("{msg}");
+                } else {
+                    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

@@ -169,12 +169,31 @@ mod __doctest_mod {{
                 #[cfg(windows)]
                 Some(STATUS_FAIL_FAST_EXCEPTION) => ExitCode::SUCCESS,
                 #[cfg(unix)]
-                None if out.status.signal() == Some(SIGABRT) => ExitCode::SUCCESS,
+                None => match out.status.signal() {{
+                    Some(SIGABRT) => ExitCode::SUCCESS,
+                    Some(signal) => {{
+                        eprintln!(\"Test didn't panic, but it's marked `should_panic` (exit signal: {{signal}}).\");
+                        ExitCode::FAILURE
+                    }}
+                    None => {{
+                        eprintln!(\"Test didn't panic, but it's marked `should_panic` and exited with no error code and no signal.\");
+                        ExitCode::FAILURE
+                    }}
+                }},
+                #[cfg(not(unix))]
+                None => {{
+                    eprintln!(\"Test didn't panic, but it's marked `should_panic`.\");
+                    ExitCode::FAILURE
+                }}
                 // Upon an abort, Fuchsia returns the status code ZX_TASK_RETCODE_EXCEPTION_KILL.
                 #[cfg(target_os = \"fuchsia\")]
                 Some(ZX_TASK_RETCODE_EXCEPTION_KILL) => ExitCode::SUCCESS,
-                _ => {{
-                    eprintln!(\"Test didn't panic, but it's marked `should_panic`.\");
+                Some(exit_code) => {{
+                    if !out.status.success() {{
+                        eprintln!(\"Test didn't panic, but it's marked `should_panic` (exit status: {{exit_code}}).\");
+                    }} else {{
+                        eprintln!(\"Test didn't panic, but it's marked `should_panic`.\");
+                    }}
                     ExitCode::FAILURE
                 }}
             }}

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

@@ -19,7 +19,7 @@ fn check_output(edition: &str, panic_abort: bool) {
         "---- 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 didn't panic, but it's marked `should_panic` (exit status: 1).",
         "test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out;",
     ];
     for text in should_contain {

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

@@ -2,14 +2,17 @@
 // adapted to use that, and that normalize line can go away
 
 //@ edition: 2024
-//@ compile-flags:--test
+//@ compile-flags:--test --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"
 //@ failure-status: 101
 
-/// ```should_panic
-/// println!("Hello, world!");
-/// ```
-pub struct Foo;
+//! ```should_panic
+//! println!("Hello, world!");
+//! ```
+//!
+//! ```should_panic
+//! std::process::exit(2);
+//! ```

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

@@ -1,16 +1,21 @@
 
-running 1 test
-test $DIR/failed-doctest-should-panic.rs - Foo (line 12) ... FAILED
+running 2 tests
+test $DIR/failed-doctest-should-panic.rs - (line 12) ... FAILED
+test $DIR/failed-doctest-should-panic.rs - (line 16) ... FAILED
 
 failures:
 
----- $DIR/failed-doctest-should-panic.rs - Foo (line 12) stdout ----
+---- $DIR/failed-doctest-should-panic.rs - (line 12) stdout ----
 Test didn't panic, but it's marked `should_panic`.
 
+---- $DIR/failed-doctest-should-panic.rs - (line 16) stdout ----
+Test didn't panic, but it's marked `should_panic` (exit status: 2).
+
 
 failures:
-    $DIR/failed-doctest-should-panic.rs - Foo (line 12)
+    $DIR/failed-doctest-should-panic.rs - (line 12)
+    $DIR/failed-doctest-should-panic.rs - (line 16)
 
-test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
+test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
 
 all doctests ran in $TIME; merged doctests compilation took $TIME