Skip to content

Add the parallel front-end test suite #143953

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ywxt
Copy link
Contributor

@ywxt ywxt commented Jul 15, 2025

update #118698
update #113349

This PR is the follow-up to #132051 .

This PR add all testable issues to UI tests to verify their irreproducibility. If no problems occur, and no feedback from other CI testing was received within a certain period of time, we can close the source PR

Now tests for the parallel front-end have been a dedicated test suite(named "parallel").

The related issues below have been irreproducible now.

This PR cleans up deadlock and ICE issues, although some reported problems related to reproducible builds still remain.

r? @jieyouxu

cc @SparrowLii @lqd

@rustbot
Copy link
Collaborator

rustbot commented Jul 15, 2025

jieyouxu is currently at their maximum review capacity.
They may take a while to respond.

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-meta Area: Issues & PRs about the rust-lang/rust repository itself 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 Jul 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 15, 2025

Some changes occurred in src/tools/opt-dist

cc @Kobzol

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rust-log-analyzer

This comment has been minimized.

@SparrowLii SparrowLii added the A-parallel-compiler Area: parallel compiler label Jul 15, 2025
@matthiaskrgr
Copy link
Member

do we have some kind of kill switch to quickly disable the entire parallel suite in case of spurious-looking one-in-50 test failures happening in ci?

@SparrowLii
Copy link
Member

@matthiaskrgr That sounds reasonable. I think controlling it through a env variable is a feasible method, e.g. RUSTC_PARALLEL_TEST.

for _ in 0..iteration_count {
let proc_res = self.compile_test(WillExecute::No, emit_metadata);
// Ensure there is no ICE during parallel complication.
self.check_no_compiler_crash(&proc_res, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a timeout here to prevent CI from getting stuck in a deadlock.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or does the current test set already have it? @jieyouxu

Copy link
Member

@jieyouxu jieyouxu Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current compiletest executor is basically libtest's. That is, tests are ran under test threads. There's a test running for too long warning message, but no hard kill-after-timeout, which IIUC is hard to implement with this setup because it'd require some kind of collaborative scheme.

For existing tests we've massaged them to not take that long, or disabled them if e.g. the compile time truly that long pointing to an issue tracking it.

@jieyouxu
Copy link
Member

I'm not at PC until much later today, but @SparrowLii I think it's worth opening an MCP for adding this test suite, especially because we may observe its failures in CI in unrelated PRs.

I'm still in favor of adding this test suite, but I would like to establish some consensus on what to do about test failures:

  • Do we disable offending test
  • Or in extreme cases, disable the whole test suite?

@ywxt ywxt force-pushed the parallel_tests branch from de899ac to 4f3f384 Compare July 16, 2025 02:36
@jieyouxu jieyouxu added needs-mcp This change is large enough that it needs a major change proposal before starting work. S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 16, 2025
@matthiaskrgr
Copy link
Member

to give a little bit more of background:

while fuzzing for bugs in the parallel compiler, sometimes you hit weird

  • crashes
  • deadlocks
  • diagnostic changes
  • differences in codegen

that only happen one in 10/ 20 / 30 / 40 times.

so even IF you have a test for something, you may actually regress it in some cases, but you only notice 30 runs later when a completely different area of the compiler is touched which will be extremely confusing.

I don't have a solution for this, but I'm not sure if "run everything 50 times just to be sure" is a scalable solution.

I'm also a bit worried about the implications of running N tests x M threads at the same time.
Usually we can run N tests at a time, but with threads, does that turn into 8*200 = up to 1600 threads at a time when running the suite?

Right now I can mostly rely on x.py test -j 1 not completely hogging my system but how would -j 1 work when we try to run 100 parallel ui tests with 20-200 threads?

@ywxt
Copy link
Contributor Author

ywxt commented Jul 17, 2025

so even IF you have a test for something, you may actually regress it in some cases, but you only notice 30 runs later when a completely different area of the compiler is touched which will be extremely confusing.

I don't have a solution for this, but I'm not sure if "run everything 50 times just to be sure" is a scalable solution.

I agree this that we need a proper solution. Although every test can have a specified number of iteration, it isn't available on it.

@ywxt ywxt changed the title Add the parallel front-end test suit Add the parallel front-end test suite Jul 17, 2025
@Kobzol
Copy link
Member

Kobzol commented Jul 17, 2025

Opening a MCP for this is definitely a good first step. I think that for testing the parallel frontend specifically, something akin to a 24/7 running fuzzing suite on a separate repository might be a better fit than having something that runs in r-l/r's auto CI jobs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-parallel-compiler Area: parallel compiler A-testsuite Area: The testsuite used to check the correctness of rustc needs-mcp This change is large enough that it needs a major change proposal before starting work. S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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
Development

Successfully merging this pull request may close these issues.

7 participants