Skip to content

Thousands of /tmp/rustdoctest* directories #139899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
m-ou-se opened this issue Apr 16, 2025 · 9 comments · Fixed by #140706
Closed

Thousands of /tmp/rustdoctest* directories #139899

m-ou-se opened this issue Apr 16, 2025 · 9 comments · Fixed by #140706
Labels
A-doctests Area: Documentation tests, run by rustdoc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented Apr 16, 2025

While working on rustc, I often have:

$ ls /tmp/rustdoctest* | wc -l
6197

😬

Almost all these directories are completely empty.

We should probably do a better job at cleaning them up. Or move them into the build directory instead of littering all over /tmp.
@m-ou-se m-ou-se added A-doctests Area: Documentation tests, run by rustdoc C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 16, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 16, 2025
@GuillaumeGomez
Copy link
Member

How do you run doctests and under which edition? Asking because we recently changed how we were running doctests so like that I know where to look at.

@GuillaumeGomez GuillaumeGomez added C-cleanup Category: PRs that clean code up or issues documenting cleanup. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. C-bug Category: This is a bug. labels Apr 16, 2025
@m-ou-se
Copy link
Member Author

m-ou-se commented Apr 16, 2025

@GuillaumeGomez I just run ./x.py test in the rust-lang/rust repo.

@GuillaumeGomez
Copy link
Member

Ah. So I guess I'll check if rustc is using the old or new doctests soon enough. :)

@GuillaumeGomez
Copy link
Member

Problem is that if doctests fail, exit is called and temporary folder is not cleaned up. No clue how I will add a regression test for that but at least the fix is done.

@GuillaumeGomez GuillaumeGomez mentioned this issue May 6, 2025
Merged
@GuillaumeGomez
Copy link
Member

Fix opened in #140706.

@lolbinarycat
Copy link
Contributor

No clue how I will add a regression test for that but at least the fix is done.

run_make test with TMPDIR set to a local dir, probably?

@GuillaumeGomez
Copy link
Member

Good idea! Gonna give it a try locally.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue May 7, 2025
@GuillaumeGomez
Add regression test for rust-lang#139899
1ba215c
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue May 7, 2025
@GuillaumeGomez
Add regression test for rust-lang#139899
442ae63
@jieyouxu
Copy link
Member

jieyouxu commented May 7, 2025

cf. #138475 :D

@GuillaumeGomez
Copy link
Member

Nice! Can't wait to see what you'll come up with!

@bors bors closed this as completed in 83e3d0e May 8, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 8, 2025
@rust-timer
Unrolled build for rust-lang#140706
014e229 
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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-doctests Area: Documentation tests, run by rustdoc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
testing-devex backlog
Status: Done
Milestone
No milestone
5 participants
@m-ou-se @GuillaumeGomez @lolbinarycat @jieyouxu @rustbot