-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
base: master
Are you sure you want to change the base?
Conversation
|
This comment has been minimized.
This comment has been minimized.
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? |
@matthiaskrgr That sounds reasonable. I think controlling it through a env variable is a feasible method, e.g. |
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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
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:
|
Co-authored-by: SparrowLii <[email protected]>
to give a little bit more of background: while fuzzing for bugs in the parallel compiler, sometimes you hit weird
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 Right now I can mostly rely on |
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. |
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. |
update #118698
update #113349
This PR is the follow-up to #132051 .
Now tests for the parallel front-end have been a dedicated test suite(named "parallel").
The related issues below have been irreproducible now.
value must be in cache after waiting
#111528deadlock detected without any query
#120759deadlock detected as we're unable to find a query cycle to break
#129911internal error: entered unreachable code
#142064collecting exported symbols for crate
/collect_and_partition_mono_items
#118205assertion failed: found_cycle
#115223no index for a field
#120760variances_of
returns&[ty::Variance]
' #124423only 'variances_of' returns '&[ty::Variance]'
#127971Could not send CguMessage to main thread
#142949None
#120786attempted to read from stolen value: rustc_middle::thir::Thir
:-Zthir-unsafeck=yes
#111520Unexpected type for constructor
#120601This PR cleans up deadlock and ICE issues, although some reported problems related to reproducible builds still remain.
r? @jieyouxu
cc @SparrowLii @lqd