Skip to content
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.

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

compiletest: Don't allow tests with overlapping prefix names #109509

Merged
merged 2 commits into from
Mar 30, 2023

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Mar 23, 2023

Some tests will delete their output directory before starting. The output directory is based on the test names. If one test is the prefix of another test, then when that test starts, it could try to delete the output directory of the other test with the longer path, or otherwise clash with it while the two tests are trying to create/delete/modify the same directory.

In practice, this manifested as a random error on macOS where two tests were trying to create/delete/create rustdoc/primitive and rustdoc/primitive/no_std, which resulted in an EINVAL (InvalidInput) error.

This renames some of the offending tests, adds compiletest-ignore-dir to prevent compiletest from processing some files, and adds a check to prevent this from happening in the future.

Fixes #109397

Some tests will delete their output directory before starting.
The output directory is based on the test names.
If one test is the prefix of another test, then when that test
starts, it could try to delete the output directory of the other
test with the longer path.
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 23, 2023
Some tests will delete their output directory before starting.
The output directory is based on the test names.
If one test is the prefix of another test, then when that test
starts, it could try to delete the output directory of the other
test with the longer path.
@Mark-Simulacrum
Copy link
Member

@bors r+

Thanks for including the validation logic!

@bors
Copy link
Contributor

bors commented Mar 26, 2023

📌 Commit 1692d0c has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 26, 2023

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2023
Rollup of 7 pull requests

Successful merges:

 - rust-lang#106985 (Enhanced doucmentation of binary search methods for `slice` and `VecDeque` for unsorted instances)
 - rust-lang#109509 (compiletest: Don't allow tests with overlapping prefix names)
 - rust-lang#109719 (RELEASES: Add "Only support Android NDK 25 or newer" to 1.68.0)
 - rust-lang#109748 (Don't ICE on `DiscriminantKind` projection in new solver)
 - rust-lang#109749 (Canonicalize float var as float in new solver)
 - rust-lang#109761 (Drop binutils on powerpc-unknown-freebsd)
 - rust-lang#109766 (Fix title for openharmony.md)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2981d77 into rust-lang:master Mar 30, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 30, 2023
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request May 18, 2023
…rk-Simulacrum

Include better context for "already exists" error in compiletest

I encountered the following error from `x.py test tests/ui` today.

```console
---- [ui] tests/ui/impl-trait/multiple-lifetimes/multiple-lifetimes.rs stdout ----
thread '[ui] tests/ui/impl-trait/multiple-lifetimes/multiple-lifetimes.rs' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 17, kind: AlreadyExists, message: "File exists" }', src/tools/compiletest/src/runtest.rs:134:43
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

I found it impossible to unblock myself without knowing which directory it was stuck on.

Error message after this PR:

```console
---- [ui] tests/ui/impl-trait/multiple-lifetimes/multiple-lifetimes.rs stdout ----
thread '[ui] tests/ui/impl-trait/multiple-lifetimes/multiple-lifetimes.rs' panicked at 'called `Result::unwrap()` on an `Err` value: failed to create output base directory /git/rust/build/x86_64-unknown-linux-gnu/test/ui/impl-trait/multiple-lifetimes/multiple-lifetimes

Caused by:
    File exists (os error 17)', src/tools/compiletest/src/runtest.rs:139:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

Now I was able to run `rm build/x86_64-unknown-linux-gnu/test/ui/impl-trait/multiple-lifetimes/multiple-lifetimes` and unblock myself.

Seems to be related to rust-lang#109509 moving *tests/ui/impl-trait/multiple-lifetimes.rs* to *tests/ui/impl-trait/multiple-lifetimes/multiple-lifetimes.rs*.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
4 participants