Correctly handle should_panic doctest attribute

Author: GuillaumeGomez

src/librustdoc/doctest.rs

@@ -773,7 +773,7 @@ fn run_test(
     match result {
         Err(e) => return Err(TestFailure::ExecutionError(e)),
         Ok(out) => {
-            if langstr.should_panic && out.status.success() {
+            if langstr.should_panic && out.status.code() != Some(101) {
                 return Err(TestFailure::UnexpectedRunPass);
             } else if !langstr.should_panic && !out.status.success() {
                 return Err(TestFailure::ExecutionFailure(out));
@@ -1083,7 +1083,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

@@ -131,13 +131,20 @@ 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 {{
+            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 {{
@@ -260,7 +267,7 @@ fn main() {returns_result} {{
         "
 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}}},
 ));
@@ -269,7 +276,6 @@ test::StaticTestFn(
         file = scraped_test.path(),
         line = scraped_test.line,
         no_run = scraped_test.no_run(opts),
-        should_panic = !scraped_test.langstr.no_run && scraped_test.langstr.should_panic,
         // 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 {
@@ -278,11 +284,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,
             )
         },
     )