Rollup merge of rust-lang#140706 - GuillaumeGomez:fix-missing-temp-dir-cleanup, r=notriddle

[rustdoc] Ensure that temporary doctest folder is correctly removed even if doctests failed

Fixes rust-lang#139899.

The bug was due to the fact that if any doctest fails for any reason, we call `exit` (or it's called inside `libtest` if not edition 2024), meaning that `TempDir`'s destructor isn't called, and therefore the temporary folder isn't cleaned up.

Took me a while to figure out how to reproduce but finally I was able to reproduce the bug with:

`````rust
#![doc(test(attr(deny(warnings))))]

//! ```
//! let a = 12;
//! ```
`````

And then I ensured that panicking doctests were cleaned up as well:

`````rust
//! ```
//! panic!();
//! ```
`````

And finally I checked if it was fixed for merged doctests too (`--edition 2024`).

To make this work, I needed to add a new public function in `libtest` too which would call a function once all tests have been run.

So only issue is: I have absolutely no idea how we can add a regression test for this fix. If anyone has an idea...

r? `@notriddle`

Author: GuillaumeGomez

library/test/src/lib.rs

@@ -98,6 +98,15 @@ const SECONDARY_TEST_BENCH_BENCHMARKS_VAR: &str = "__RUST_TEST_BENCH_BENCHMARKS"
 // The default console test runner. It accepts the command line
 // arguments and a vector of test_descs.
 pub fn test_main(args: &[String], tests: Vec<TestDescAndFn>, options: Option<Options>) {
+    test_main_with_exit_callback(args, tests, options, || {})
+}
+
+pub fn test_main_with_exit_callback<F: FnOnce()>(
+    args: &[String],
+    tests: Vec<TestDescAndFn>,
+    options: Option<Options>,
+    exit_callback: F,
+) {
     let mut opts = match cli::parse_opts(args) {
         Some(Ok(o)) => o,
         Some(Err(msg)) => {
@@ -151,6 +160,7 @@ pub fn test_main(args: &[String], tests: Vec<TestDescAndFn>, options: Option<Opt
         let res = console::run_tests_console(&opts, tests);
         // Prevent Valgrind from reporting reachable blocks in users' unit tests.
         drop(panic::take_hook());
+        exit_callback();
         match res {
             Ok(true) => {}
             Ok(false) => process::exit(ERROR_EXIT_CODE),

src/librustdoc/doctest.rs

@@ -262,11 +262,21 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
         Ok(None) => return,
         Err(error) => {
             eprintln!("{error}");
+            // Since some files in the temporary folder are still owned and alive, we need
+            // to manually remove the folder.
+            let _ = std::fs::remove_dir_all(temp_dir.path());
             std::process::exit(1);
         }
     };
 
-    run_tests(opts, &rustdoc_options, &unused_extern_reports, standalone_tests, mergeable_tests);
+    run_tests(
+        opts,
+        &rustdoc_options,
+        &unused_extern_reports,
+        standalone_tests,
+        mergeable_tests,
+        Some(temp_dir),
+    );
 
     let compiling_test_count = compiling_test_count.load(Ordering::SeqCst);
 
@@ -316,6 +326,8 @@ pub(crate) fn run_tests(
     unused_extern_reports: &Arc<Mutex<Vec<UnusedExterns>>>,
     mut standalone_tests: Vec<test::TestDescAndFn>,
     mergeable_tests: FxIndexMap<Edition, Vec<(DocTestBuilder, ScrapedDocTest)>>,
+    // We pass this argument so we can drop it manually before using `exit`.
+    mut temp_dir: Option<TempDir>,
 ) {
     let mut test_args = Vec::with_capacity(rustdoc_options.test_args.len() + 1);
     test_args.insert(0, "rustdoctest".to_string());
@@ -382,9 +394,14 @@ pub(crate) fn run_tests(
     // `running 0 tests...`.
     if ran_edition_tests == 0 || !standalone_tests.is_empty() {
         standalone_tests.sort_by(|a, b| a.desc.name.as_slice().cmp(b.desc.name.as_slice()));
-        test::test_main(&test_args, standalone_tests, None);
+        test::test_main_with_exit_callback(&test_args, standalone_tests, None, || {
+            // We ensure temp dir destructor is called.
+            std::mem::drop(temp_dir.take());
+        });
     }
     if nb_errors != 0 {
+        // We ensure temp dir destructor is called.
+        std::mem::drop(temp_dir);
         // libtest::ERROR_EXIT_CODE is not public but it's the same value.
         std::process::exit(101);
     }
@@ -450,7 +467,7 @@ enum TestFailure {
 }
 
 enum DirState {
-    Temp(tempfile::TempDir),
+    Temp(TempDir),
     Perm(PathBuf),
 }
 

src/librustdoc/doctest/markdown.rs

@@ -116,6 +116,7 @@ pub(crate) fn test(input: &Input, options: Options) -> Result<(), String> {
         &Arc::new(Mutex::new(Vec::new())),
         standalone_tests,
         mergeable_tests,
+        None,
     );
     Ok(())
 }

tests/run-make/rustdoc-tempdir-removal/compile-error.rs

@@ -0,0 +1,5 @@
+#![doc(test(attr(deny(warnings))))]
+
+//! ```
+//! let a = 12;
+//! ```

tests/run-make/rustdoc-tempdir-removal/rmake.rs

@@ -0,0 +1,34 @@
+// This test ensures that no temporary folder is "left behind" when doctests fail for any reason.
+
+//@ only-linux
+
+use std::path::Path;
+
+use run_make_support::{path, rfs, rustdoc};
+
+fn run_doctest_and_check_tmpdir(tmp_dir: &Path, doctest: &str, edition: &str) {
+    let output =
+        rustdoc().input(doctest).env("TMPDIR", tmp_dir).arg("--test").edition(edition).run_fail();
+
+    output.assert_exit_code(101).assert_stdout_contains(
+        "test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out",
+    );
+
+    rfs::read_dir_entries(tmp_dir, |entry| {
+        panic!("Found an item inside the temporary folder: {entry:?}");
+    });
+}
+
+fn run_doctest_and_check_tmpdir_for_edition(tmp_dir: &Path, edition: &str) {
+    run_doctest_and_check_tmpdir(tmp_dir, "compile-error.rs", edition);
+    run_doctest_and_check_tmpdir(tmp_dir, "run-error.rs", edition);
+}
+
+fn main() {
+    let tmp_dir = path("tmp");
+    rfs::create_dir(&tmp_dir);
+
+    run_doctest_and_check_tmpdir_for_edition(&tmp_dir, "2018");
+    // We use the 2024 edition to check that it's also working for merged doctests.
+    run_doctest_and_check_tmpdir_for_edition(&tmp_dir, "2024");
+}

tests/run-make/rustdoc-tempdir-removal/run-error.rs

@@ -0,0 +1,3 @@
+//! ```
+//! panic!();
+//! ```