Split should_panic doctests like merged doctests so we can use libtest API directly

Author: GuillaumeGomez

library/test/src/test_result.rs

@@ -158,6 +158,7 @@ pub(crate) fn get_result_from_exit_code(
     result
 }
 
+#[derive(Debug)]
 pub enum RustdocResult {
     /// The test failed to compile.
     CompileError,

src/librustdoc/doctest.rs

@@ -406,7 +406,7 @@ pub(crate) fn run_tests(
         // We failed to compile all compatible tests as one so we push them into the
         // `standalone_tests` doctests.
         debug!("Failed to compile compatible doctests for edition {} all at once", edition);
-        for (pos, (doctest, scraped_test)) in doctests.into_iter().enumerate() {
+        for (doctest, scraped_test) in doctests {
             doctest.generate_unique_doctest(
                 &scraped_test.text,
                 scraped_test.langstr.test_harness,
@@ -419,7 +419,6 @@ pub(crate) fn run_tests(
                 opts.clone(),
                 Arc::clone(rustdoc_options),
                 unused_extern_reports.clone(),
-                pos,
             ));
         }
     }
@@ -549,44 +548,33 @@ pub(crate) struct RunnableDocTest {
 }
 
 impl RunnableDocTest {
-    fn path_for_merged_doctest_bundle(&self, id: Option<usize>) -> PathBuf {
-        let name = if let Some(id) = id {
-            format!("doctest_bundle_id_{id}.rs")
-        } else {
-            format!("doctest_bundle_{}.rs", self.edition)
-        };
-        self.test_opts.outdir.path().join(name)
+    fn path_for_merged_doctest_bundle(&self) -> PathBuf {
+        self.test_opts.outdir.path().join(format!("doctest_bundle_{}.rs", self.edition))
     }
-    fn path_for_merged_doctest_runner(&self, id: Option<usize>) -> PathBuf {
-        let name = if let Some(id) = id {
-            format!("doctest_runner_id_{id}.rs")
-        } else {
-            format!("doctest_runner_{}.rs", self.edition)
-        };
-        self.test_opts.outdir.path().join(name)
+    fn path_for_merged_doctest_runner(&self) -> PathBuf {
+        self.test_opts.outdir.path().join(format!("doctest_runner_{}.rs", self.edition))
     }
     fn is_multiple_tests(&self) -> bool {
         self.merged_test_code.is_some()
     }
 }
 
 fn compile_merged_doctest_and_caller_binary(
-    mut child: process::Child,
+    child: process::Child,
     doctest: &RunnableDocTest,
     rustdoc_options: &RustdocOptions,
     rustc_binary: &Path,
     output_file: &Path,
     compiler_args: Vec<String>,
     test_code: &str,
     instant: Instant,
-    id: Option<usize>,
+    is_compile_fail: bool,
 ) -> Result<process::Output, (Duration, Result<(), RustdocResult>)> {
     // compile-fail tests never get merged, so this should always pass
-    let status = child.wait().expect("Failed to wait");
-
-    // the actual test runner is a separate component, built with nightly-only features;
-    // build it now
-    let runner_input_file = doctest.path_for_merged_doctest_runner(id);
+    let output = child.wait_with_output().expect("Failed to wait");
+    if is_compile_fail && !output.status.success() {
+        return Ok(output);
+    }
 
     let mut runner_compiler =
         wrapped_rustc_command(&rustdoc_options.test_builder_wrappers, rustc_binary);
@@ -595,13 +583,10 @@ fn compile_merged_doctest_and_caller_binary(
     runner_compiler.env("RUSTC_BOOTSTRAP", "1");
     runner_compiler.args(compiler_args);
     runner_compiler.args(["--crate-type=bin", "-o"]).arg(output_file);
-    let mut extern_path = if let Some(id) = id {
-        std::ffi::OsString::from(format!("--extern=doctest_bundle_id_{id}="))
+    let base_name = if is_compile_fail {
+        format!("rust_out")
     } else {
-        std::ffi::OsString::from(format!(
-            "--extern=doctest_bundle_{edition}=",
-            edition = doctest.edition
-        ))
+        format!("doctest_bundle_{edition}", edition = doctest.edition)
     };
 
     // Deduplicate passed -L directory paths, since usually all dependencies will be in the
@@ -621,36 +606,58 @@ fn compile_merged_doctest_and_caller_binary(
             }
         }
     }
-    let filename = if let Some(id) = id {
-        format!("libdoctest_bundle_id_{id}.rlib")
-    } else {
-        format!("libdoctest_bundle_{edition}.rlib", edition = doctest.edition)
-    };
-    let output_bundle_file = doctest.test_opts.outdir.path().join(filename);
+    let output_bundle_file = doctest.test_opts.outdir.path().join(format!("lib{base_name}.rlib"));
+    let mut extern_path = std::ffi::OsString::from(format!("--extern={base_name}="));
     extern_path.push(&output_bundle_file);
-    runner_compiler.arg(extern_path);
-    runner_compiler.arg(&runner_input_file);
-    if std::fs::write(&runner_input_file, test_code).is_err() {
-        // If we cannot write this file for any reason, we leave. All combined tests will be
-        // tested as standalone tests.
-        return Err((instant.elapsed(), Err(RustdocResult::CompileError)));
-    }
-    if !rustdoc_options.no_capture {
-        // If `no_capture` is disabled, then we don't display rustc's output when compiling
-        // the merged doctests.
-        runner_compiler.stderr(Stdio::null());
+    runner_compiler.arg(&extern_path);
+
+    if is_compile_fail {
+        add_rustdoc_env_vars(&mut runner_compiler, doctest);
+        runner_compiler.stderr(Stdio::piped());
+        runner_compiler.stdin(Stdio::piped());
+        runner_compiler.arg("-");
+    } else {
+        // The actual test runner is a separate component, built with nightly-only features;
+        // build it now
+        let runner_input_file = doctest.path_for_merged_doctest_runner();
+        runner_compiler.arg(&runner_input_file);
+        if std::fs::write(&runner_input_file, test_code).is_err() {
+            // If we cannot write this file for any reason, we leave. All combined tests will be
+            // tested as standalone tests.
+            return Err((instant.elapsed(), Err(RustdocResult::CompileError)));
+        }
+        if !rustdoc_options.no_capture {
+            // If `no_capture` is disabled, then we don't display rustc's output when compiling
+            // the merged doctests.
+            runner_compiler.stderr(Stdio::null());
+            runner_compiler.arg("--error-format=short");
+        }
     }
-    runner_compiler.arg("--error-format=short");
     debug!("compiler invocation for doctest runner: {runner_compiler:?}");
 
-    let status = if !status.success() {
-        status
+    let output = if !output.status.success() {
+        output
     } else {
         let mut child_runner = runner_compiler.spawn().expect("Failed to spawn rustc process");
-        child_runner.wait().expect("Failed to wait")
+        if is_compile_fail {
+            let stdin = child_runner.stdin.as_mut().expect("Failed to open stdin");
+            stdin.write_all(test_code.as_bytes()).expect("could write out test sources");
+        }
+        child_runner.wait_with_output().expect("Failed to wait")
     };
+    if is_compile_fail {
+        Ok(output)
+    } else {
+        Ok(process::Output { status: output.status, stdout: Vec::new(), stderr: Vec::new() })
+    }
+}
 
-    Ok(process::Output { status, stdout: Vec::new(), stderr: Vec::new() })
+fn add_rustdoc_env_vars(compiler: &mut Command, doctest: &RunnableDocTest) {
+    compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", &doctest.test_opts.path);
+    compiler.env(
+        "UNSTABLE_RUSTDOC_TEST_LINE",
+        format!("{}", doctest.line as isize - doctest.full_test_line_offset as isize),
+    );
 }
 
 /// Execute a `RunnableDoctest`.
@@ -664,7 +671,6 @@ fn run_test(
     rustdoc_options: &RustdocOptions,
     supports_color: bool,
     report_unused_externs: impl Fn(UnusedExterns),
-    doctest_id: usize,
 ) -> (Duration, Result<(), RustdocResult>) {
     let langstr = &doctest.langstr;
     // Make sure we emit well-formed executable names for our target.
@@ -695,11 +701,6 @@ fn run_test(
         compiler_args.extend_from_slice(&["-Z".to_owned(), "unstable-options".to_owned()]);
     }
 
-    if doctest.no_run && !langstr.compile_fail && rustdoc_options.persist_doctests.is_none() {
-        // FIXME: why does this code check if it *shouldn't* persist doctests
-        //        -- shouldn't it be the negation?
-        compiler_args.push("--emit=metadata".to_owned());
-    }
     compiler_args.extend_from_slice(&[
         "--target".to_owned(),
         match &rustdoc_options.target {
@@ -745,40 +746,47 @@ fn run_test(
 
     compiler.args(&compiler_args);
 
-    compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", &doctest.test_opts.path);
-    compiler.env(
-        "UNSTABLE_RUSTDOC_TEST_LINE",
-        format!("{}", doctest.line as isize - doctest.full_test_line_offset as isize),
-    );
+    let is_should_panic = !langstr.compile_fail && langstr.should_panic;
     // If this is a merged doctest, we need to write it into a file instead of using stdin
     // because if the size of the merged doctests is too big, it'll simply break stdin.
-    if doctest.is_multiple_tests() || (!langstr.compile_fail && langstr.should_panic) {
-        // It makes the compilation failure much faster if it is for a combined doctest.
-        compiler.arg("--error-format=short");
-        let input_file = doctest.path_for_merged_doctest_bundle(
-            if !langstr.compile_fail && langstr.should_panic { Some(doctest_id) } else { None },
-        );
-        if std::fs::write(&input_file, &doctest.full_test_code).is_err() {
-            // If we cannot write this file for any reason, we leave. All combined tests will be
-            // tested as standalone tests.
-            return (Duration::default(), Err(RustdocResult::CompileError));
-        }
-        if !rustdoc_options.no_capture {
-            // If `no_capture` is disabled, then we don't display rustc's output when compiling
-            // the merged doctests.
-            compiler.stderr(Stdio::null());
-        }
+    if doctest.is_multiple_tests() || is_should_panic {
         // bundled tests are an rlib, loaded by a separate runner executable
-        compiler
-            .arg("--crate-type=lib")
-            .arg("--out-dir")
-            .arg(doctest.test_opts.outdir.path())
-            .arg(input_file);
+        compiler.arg("--crate-type=lib").arg("--out-dir").arg(doctest.test_opts.outdir.path());
+
+        if !is_should_panic {
+            compiler.arg("--error-format=short");
+            if !rustdoc_options.no_capture {
+                // If `no_capture` is disabled, then we don't display rustc's output when compiling
+                // the merged doctests.
+                compiler.stderr(Stdio::null());
+                compiler.stdout(Stdio::null());
+            }
+            // It makes the compilation failure much faster if it is for a combined doctest.
+            let input_file = doctest.path_for_merged_doctest_bundle();
+            if std::fs::write(&input_file, &doctest.full_test_code).is_err() {
+                // If we cannot write this file for any reason, we leave. All combined tests will be
+                // tested as standalone tests.
+                return (Duration::default(), Err(RustdocResult::CompileError));
+            }
+            compiler.arg(input_file);
+        } else {
+            compiler.stdin(Stdio::piped());
+            compiler.stderr(Stdio::piped());
+            add_rustdoc_env_vars(&mut compiler, &doctest);
+            compiler.arg("-");
+        }
     } else {
+        add_rustdoc_env_vars(&mut compiler, &doctest);
         compiler.arg("--crate-type=bin").arg("-o").arg(&output_file);
         compiler.arg("-");
         compiler.stdin(Stdio::piped());
         compiler.stderr(Stdio::piped());
+
+        if doctest.no_run && !langstr.compile_fail && rustdoc_options.persist_doctests.is_none() {
+            // FIXME: why does this code check if it *shouldn't* persist doctests
+            //        -- shouldn't it be the negation?
+            compiler_args.push("--emit=metadata".to_owned());
+        }
     }
 
     debug!("compiler invocation for doctest: {compiler:?}");
@@ -794,45 +802,48 @@ fn run_test(
             compiler_args,
             merged_test_code,
             instant,
-            None,
+            false,
         ) {
             Ok(out) => out,
             Err(err) => return err,
         }
-    } else if !langstr.compile_fail && langstr.should_panic {
-        match compile_merged_doctest_and_caller_binary(
-            child,
-            &doctest,
-            rustdoc_options,
-            rustc_binary,
-            &output_file,
-            compiler_args,
-            &format!(
-                "\
+    } else {
+        let stdin = child.stdin.as_mut().expect("Failed to open stdin");
+        stdin.write_all(doctest.full_test_code.as_bytes()).expect("could write out test sources");
+
+        if !langstr.compile_fail && langstr.should_panic {
+            match compile_merged_doctest_and_caller_binary(
+                child,
+                &doctest,
+                rustdoc_options,
+                rustc_binary,
+                &output_file,
+                compiler_args,
+                &format!(
+                    "\
 #![feature(test)]
 extern crate test;
 
-use std::process::{{ExitCode, Termination}};
+use std::process::{{ExitCode, Termination, exit}};
 
 fn main() -> ExitCode {{
     if test::cannot_handle_should_panic() {{
-        ExitCode::SUCCESS
+        exit(test::ERROR_EXIT_CODE);
     }} else {{
-        extern crate doctest_bundle_id_{doctest_id} as doctest_bundle;
+        extern crate rust_out as doctest_bundle;
         doctest_bundle::main().report()
     }}
-}}"
-            ),
-            instant,
-            Some(doctest_id),
-        ) {
-            Ok(out) => out,
-            Err(err) => return err,
+}}",
+                ),
+                instant,
+                true,
+            ) {
+                Ok(out) => out,
+                Err(err) => return err,
+            }
+        } else {
+            child.wait_with_output().expect("Failed to read stdout")
         }
-    } else {
-        let stdin = child.stdin.as_mut().expect("Failed to open stdin");
-        stdin.write_all(doctest.full_test_code.as_bytes()).expect("could write out test sources");
-        child.wait_with_output().expect("Failed to read stdout")
     };
 
     struct Bomb<'a>(&'a str);
@@ -1138,7 +1149,6 @@ impl CreateRunnableDocTests {
             self.opts.clone(),
             Arc::clone(&self.rustdoc_options),
             self.unused_extern_reports.clone(),
-            self.standalone_tests.len(),
         )
     }
 }
@@ -1149,7 +1159,6 @@ fn generate_test_desc_and_fn(
     opts: GlobalTestOptions,
     rustdoc_options: Arc<RustdocOptions>,
     unused_externs: Arc<Mutex<Vec<UnusedExterns>>>,
-    doctest_id: usize,
 ) -> test::TestDescAndFn {
     let target_str = rustdoc_options.target.to_string();
     let rustdoc_test_options =
@@ -1184,7 +1193,6 @@ fn generate_test_desc_and_fn(
                 scraped_test,
                 rustdoc_options,
                 unused_externs,
-                doctest_id,
             )
         })),
     }
@@ -1197,7 +1205,6 @@ fn doctest_run_fn(
     scraped_test: ScrapedDocTest,
     rustdoc_options: Arc<RustdocOptions>,
     unused_externs: Arc<Mutex<Vec<UnusedExterns>>>,
-    doctest_id: usize,
 ) -> Result<(), String> {
     #[cfg(not(bootstrap))]
     if scraped_test.langstr.should_panic && test::cannot_handle_should_panic() {
@@ -1223,13 +1230,8 @@ fn doctest_run_fn(
         no_run: scraped_test.no_run(&rustdoc_options),
         merged_test_code: None,
     };
-    let (_, res) = run_test(
-        runnable_test,
-        &rustdoc_options,
-        doctest.supports_color,
-        report_unused_externs,
-        doctest_id,
-    );
+    let (_, res) =
+        run_test(runnable_test, &rustdoc_options, doctest.supports_color, report_unused_externs);
 
     if let Err(err) = res {
         eprint!("{err}");