Skip to content

Commit 014e229

Browse files
authored
Unrolled build for rust-lang#140706
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`
2 parents e9f8103 + 442ae63 commit 014e229

File tree

6 files changed

+73
-3
lines changed

6 files changed

+73
-3
lines changed

library/test/src/lib.rs

+10
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,15 @@ const SECONDARY_TEST_BENCH_BENCHMARKS_VAR: &str = "__RUST_TEST_BENCH_BENCHMARKS"
9898
// The default console test runner. It accepts the command line
9999
// arguments and a vector of test_descs.
100100
pub fn test_main(args: &[String], tests: Vec<TestDescAndFn>, options: Option<Options>) {
101+
test_main_with_exit_callback(args, tests, options, || {})
102+
}
103+
104+
pub fn test_main_with_exit_callback<F: FnOnce()>(
105+
args: &[String],
106+
tests: Vec<TestDescAndFn>,
107+
options: Option<Options>,
108+
exit_callback: F,
109+
) {
101110
let mut opts = match cli::parse_opts(args) {
102111
Some(Ok(o)) => o,
103112
Some(Err(msg)) => {
@@ -151,6 +160,7 @@ pub fn test_main(args: &[String], tests: Vec<TestDescAndFn>, options: Option<Opt
151160
let res = console::run_tests_console(&opts, tests);
152161
// Prevent Valgrind from reporting reachable blocks in users' unit tests.
153162
drop(panic::take_hook());
163+
exit_callback();
154164
match res {
155165
Ok(true) => {}
156166
Ok(false) => process::exit(ERROR_EXIT_CODE),

src/librustdoc/doctest.rs

+20-3
Original file line numberDiff line numberDiff line change
@@ -262,11 +262,21 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
262262
Ok(None) => return,
263263
Err(error) => {
264264
eprintln!("{error}");
265+
// Since some files in the temporary folder are still owned and alive, we need
266+
// to manually remove the folder.
267+
let _ = std::fs::remove_dir_all(temp_dir.path());
265268
std::process::exit(1);
266269
}
267270
};
268271

269-
run_tests(opts, &rustdoc_options, &unused_extern_reports, standalone_tests, mergeable_tests);
272+
run_tests(
273+
opts,
274+
&rustdoc_options,
275+
&unused_extern_reports,
276+
standalone_tests,
277+
mergeable_tests,
278+
Some(temp_dir),
279+
);
270280

271281
let compiling_test_count = compiling_test_count.load(Ordering::SeqCst);
272282

@@ -316,6 +326,8 @@ pub(crate) fn run_tests(
316326
unused_extern_reports: &Arc<Mutex<Vec<UnusedExterns>>>,
317327
mut standalone_tests: Vec<test::TestDescAndFn>,
318328
mergeable_tests: FxIndexMap<Edition, Vec<(DocTestBuilder, ScrapedDocTest)>>,
329+
// We pass this argument so we can drop it manually before using `exit`.
330+
mut temp_dir: Option<TempDir>,
319331
) {
320332
let mut test_args = Vec::with_capacity(rustdoc_options.test_args.len() + 1);
321333
test_args.insert(0, "rustdoctest".to_string());
@@ -382,9 +394,14 @@ pub(crate) fn run_tests(
382394
// `running 0 tests...`.
383395
if ran_edition_tests == 0 || !standalone_tests.is_empty() {
384396
standalone_tests.sort_by(|a, b| a.desc.name.as_slice().cmp(b.desc.name.as_slice()));
385-
test::test_main(&test_args, standalone_tests, None);
397+
test::test_main_with_exit_callback(&test_args, standalone_tests, None, || {
398+
// We ensure temp dir destructor is called.
399+
std::mem::drop(temp_dir.take());
400+
});
386401
}
387402
if nb_errors != 0 {
403+
// We ensure temp dir destructor is called.
404+
std::mem::drop(temp_dir);
388405
// libtest::ERROR_EXIT_CODE is not public but it's the same value.
389406
std::process::exit(101);
390407
}
@@ -450,7 +467,7 @@ enum TestFailure {
450467
}
451468

452469
enum DirState {
453-
Temp(tempfile::TempDir),
470+
Temp(TempDir),
454471
Perm(PathBuf),
455472
}
456473

src/librustdoc/doctest/markdown.rs

+1
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ pub(crate) fn test(input: &Input, options: Options) -> Result<(), String> {
116116
&Arc::new(Mutex::new(Vec::new())),
117117
standalone_tests,
118118
mergeable_tests,
119+
None,
119120
);
120121
Ok(())
121122
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#![doc(test(attr(deny(warnings))))]
2+
3+
//! ```
4+
//! let a = 12;
5+
//! ```
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// This test ensures that no temporary folder is "left behind" when doctests fail for any reason.
2+
3+
//@ only-linux
4+
5+
use std::path::Path;
6+
7+
use run_make_support::{path, rfs, rustdoc};
8+
9+
fn run_doctest_and_check_tmpdir(tmp_dir: &Path, doctest: &str, edition: &str) {
10+
let output =
11+
rustdoc().input(doctest).env("TMPDIR", tmp_dir).arg("--test").edition(edition).run_fail();
12+
13+
output.assert_exit_code(101).assert_stdout_contains(
14+
"test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out",
15+
);
16+
17+
rfs::read_dir_entries(tmp_dir, |entry| {
18+
panic!("Found an item inside the temporary folder: {entry:?}");
19+
});
20+
}
21+
22+
fn run_doctest_and_check_tmpdir_for_edition(tmp_dir: &Path, edition: &str) {
23+
run_doctest_and_check_tmpdir(tmp_dir, "compile-error.rs", edition);
24+
run_doctest_and_check_tmpdir(tmp_dir, "run-error.rs", edition);
25+
}
26+
27+
fn main() {
28+
let tmp_dir = path("tmp");
29+
rfs::create_dir(&tmp_dir);
30+
31+
run_doctest_and_check_tmpdir_for_edition(&tmp_dir, "2018");
32+
// We use the 2024 edition to check that it's also working for merged doctests.
33+
run_doctest_and_check_tmpdir_for_edition(&tmp_dir, "2024");
34+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
//! ```
2+
//! panic!();
3+
//! ```

0 commit comments

Comments
 (0)