Skip to content

Add parallel rustc ui tests #120664

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 7, 2024
Merged

Add parallel rustc ui tests #120664

merged 1 commit into from
Feb 7, 2024

Conversation

SparrowLii
Copy link
Member

Updates #118698

Add some ui tests for parallel rustc front end

This is a relatively large feature so I think it's worth creating a new entity in tests/ui folder, so we need to modify the limit in tidy.

@rustbot
Copy link
Collaborator

rustbot commented Feb 5, 2024

r? @michaelwoerister

(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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 5, 2024
@michaelwoerister
Copy link
Member

Thanks for the PR, @SparrowLii!

It looks like we don't have a parallelization group in triagebot.toml. Do you know a reviewer with the necessary context?

@SparrowLii
Copy link
Member Author

ok
r? @nnethercote
cc @cjgillot @bjorn3 @Kobzol

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Feb 5, 2024 •
edited
Loading

How will we deal with spurious failures of these in CI that only happen every 10th/100th run or so?

@SparrowLii
Copy link
Member Author

SparrowLii commented Feb 5, 2024 •
edited
Loading

Some use cases can be tested multiple times (such as 10 or 100 times) by modifying the compile-test tool, but there are still some errors (such as once in 10000 times) that are difficult to guarantee correct.

So I think this PR is just the beginning

@matthiaskrgr
Copy link
Member

Maybe we can also increase the threadcount to 50 or so to increase the chance of concurrency problems?

@SparrowLii
Copy link
Member Author

Yeah of course we should.

We can add some simple use cases first to ensure that the basic functions will not be overturned (such as #118648)

@rust-log-analyzer

This comment has been minimized.

# to view
@rust-log-analyzer

This comment has been minimized.

# to view
@SparrowLii
Copy link
Member Author

SparrowLii commented Feb 5, 2024 •
edited
Loading

The no ImplicitCtxt stored in tls error showed here so I think we need to solve #111522 first :(

Otherwise we cannot guarantee the stability of the test results

The reason for this error is that the test case workerlocal-outer-thread-pool-issue-94654.rs relies on the deadlock_handler function (since it has queries cycle error), which triggers the error in #111522. Other test cases should not be affected.

@nnethercote
Copy link
Contributor

I think the commits should be squashed together before merging. Otherwise, more tests is good. It seems like the concern from yesterday has been resolved? If so, r=me once the commits are squashed.

@SparrowLii
Copy link
Member Author

Thanks! We need to enhance the ui testing tool to support more tests (I listed it in #118698), so I would like to add them in another new PR

@SparrowLii
Copy link
Member Author

@bors r=nnethercote

@bors
Copy link
Collaborator

bors commented Feb 7, 2024

📌 Commit 073f5cd has been approved by nnethercote

It is now in the queue for this repository.

@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 Feb 7, 2024
@bors
Copy link
Collaborator

bors commented Feb 7, 2024

☔ The latest upstream changes (presumably #120722) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 7, 2024
@SparrowLii
Copy link
Member Author

@bors r=nnethercote

@bors
Copy link
Collaborator

bors commented Feb 7, 2024

📌 Commit a59d006 has been approved by nnethercote

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 7, 2024
Nadrieril added a commit to Nadrieril/rust that referenced this pull request Feb 7, 2024
@Nadrieril
Rollup merge of rust-lang#120664 - SparrowLii:parallel_test, r=nnethe…
8ba999e 
…rcote

Add parallel rustc ui tests

Updates rust-lang#118698

Add some ui tests for parallel rustc front end

This is a relatively large feature so I think it's worth creating a new entity in tests/ui folder, so we need to modify the limit in tidy.
@Nadrieril Nadrieril mentioned this pull request Feb 7, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
@bors
Auto merge of rust-lang#120743 - Nadrieril:rollup-h2mxjc2, r=Nadrieril
e8ec9f4 
Rollup of 13 pull requests

Successful merges:

 - rust-lang#110482 (Add armv8r-none-eabihf target for the Cortex-R52.)
 - rust-lang#119162 (Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`)
 - rust-lang#120302 (various const interning cleanups)
 - rust-lang#120455 ( Add FileCheck annotations to MIR-opt SROA tests)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120479 (Suggest turning `if let` into irrefutable `let` if appropriate)
 - rust-lang#120564 (coverage: Split out counter increment sites from BCB node/edge counters)
 - rust-lang#120633 (pattern_analysis: gather up place-relevant info)
 - rust-lang#120664 (Add parallel rustc ui tests)
 - rust-lang#120721 (fix `llvm_out` to use the correct LLVM root)
 - rust-lang#120726 (Don't use bashism in checktools.sh)
 - rust-lang#120733 (MirPass: make name more const)
 - rust-lang#120735 (Remove some `unchecked_claim_error_was_emitted` calls)

Failed merges:

 - rust-lang#120727 (exhaustiveness: Prefer "`0..MAX` not covered" to "`_` not covered")

r? `@ghost`
`@rustbot` modify labels: rollup
@Nadrieril Nadrieril mentioned this pull request Feb 7, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
@bors
Auto merge of rust-lang#120748 - Nadrieril:rollup-dj0qwv5, r=Nadrieril
8ace7ea 
Rollup of 13 pull requests

Successful merges:

 - rust-lang#110482 (Add armv8r-none-eabihf target for the Cortex-R52.)
 - rust-lang#119162 (Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`)
 - rust-lang#120302 (various const interning cleanups)
 - rust-lang#120455 ( Add FileCheck annotations to MIR-opt SROA tests)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120479 (Suggest turning `if let` into irrefutable `let` if appropriate)
 - rust-lang#120564 (coverage: Split out counter increment sites from BCB node/edge counters)
 - rust-lang#120633 (pattern_analysis: gather up place-relevant info)
 - rust-lang#120664 (Add parallel rustc ui tests)
 - rust-lang#120726 (Don't use bashism in checktools.sh)
 - rust-lang#120733 (MirPass: make name more const)
 - rust-lang#120735 (Remove some `unchecked_claim_error_was_emitted` calls)
 - rust-lang#120746 (Record coroutine kind in coroutine generics)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit eecab31 into rust-lang:master Feb 7, 2024
@rustbot rustbot added this to the 1.78.0 milestone Feb 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
@rust-timer
Unrolled build for rust-lang#120664
50dbec4 
Rollup merge of rust-lang#120664 - SparrowLii:parallel_test, r=nnethercote

Add parallel rustc ui tests

Updates rust-lang#118698

Add some ui tests for parallel rustc front end

This is a relatively large feature so I think it's worth creating a new entity in tests/ui folder, so we need to modify the limit in tidy.
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Noratrieb Noratrieb Noratrieb left review comments

Assignees

@nnethercote nnethercote

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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.78.0
Development

Successfully merging this pull request may close these issues.
8 participants
@SparrowLii @rustbot @michaelwoerister @matthiaskrgr @rust-log-analyzer @nnethercote @bors @Noratrieb