Proposal for a dedicated test suite for the parallel frontend
Proposal
Currently, the parallel front-end tests are included by UI tests. However, the tests of the front-end compiler should have an independent test set to verify compilation processes and results.
Context: https://github.com/rust-lang/rust/pull/132051 and https://github.com/rust-lang/rust/pull/143953
Especially, due to errors that may occur randomly under parallel compilation, there are some situations we face:
- Crashes: repeatedly execute to ensure no bugs.
- Deadlocks: interrupt it when a deadlock occurs.
- Diagnostic changes: check if it won’t change diagnostic contents, even if the sequence of the messages has been altered under a parallel environment.
- Differences in codegen: verify consistency of codegen between the parallel and serial compiler.
That means requiring a new execution and verification setup, which are different from UI tests.
And so on, we need to separate the parallel front-end tests into a new test suite. Moreover, many reported issues and cases ain’t collected into those tests, though some have been solved.
This MCP proposes to add the parallel front-end test suite and determine its mechanism.
Test coverage of the suite
The suite contains the cases that have been solved or are not reproducible so far. And those issues that are unsolved at the moment will be added once they are addressed.
Step 1: Enable ICE and deadlock tests on local
At the moment, a lot of reported issues with ICEs have been solved and thus they should be added into this suite.
- For the test method, repeatedly execute a test case and check compiler crashes to try to find more problems under parallel compilation.
- A detective to specify an iteration count for some tests, which need to be verified through more trials.
- An environment variable to enable it artificially, and it won’t run on CI.
This means we can verify the parallel frontend on local, especially while changes get involved in it.
Step 2: Enable ICE and deadlock tests on CI
Compared to step 1, there are somethings we wanna do:
- An iteration that is sufficient but doesn’t significantly affect CI time for every test
- For possible deadlocks during tests, a timeout mechanism is necessary to prevent unlimited waiting.(The most easy way is to kill CI process if timeout)
There are two steps to enable tests on CI:
- In the early step, reviewers can manually run the CI via a bot command on specific PRs. Additionally, someone from the working group of the parallel compiler will periodically (for example, weekly) submit a draft PR specifically for testing.
- Once we have sufficient experience to ensure it won't bother daily PRs, we can integrate it into the try tasks that are automatically triggered when merging a (rollup) PR.
Step 3: Enable the parallel frontend for existing tests
At this step, we can enable the parallel frontend for the existing ui test suite to compare diagnostic messages. Also, the codegen and assembly test suite will contain tests to compare builds generated by the serial and parallel compiler.
This test suite is intent to assist us stabilize the parallel front-end feature next.
Mentors or Reviewers
@SparrowLii
Process
The main points of the Major Change Process are as follows:
- [x] File an issue describing the proposal.
- [ ] A compiler team member who is knowledgeable in the area can second by writing
@rustbot secondor kickoff a team FCP with@rfcbot fcp $RESOLUTION.- Refer to Proposals, Approvals and Stabilization docs for when a second is sufficient, or when a full team FCP is required.
- [ ] Once an MCP is seconded, the Final Comment Period begins.
- Final Comment Period lasts for 10 days after all outstanding concerns are solved.
- Outstanding concerns will block the Final Comment Period from finishing. Once all concerns are resolved, the 10 day countdown is restarted.
- If no concerns are raised after 10 days since the resolution of the last outstanding concern, the MCP is considered approved.
You can read more about Major Change Proposals on forge.
[!CAUTION]
Concerns (2 active)
Managed by
@rustbot—see help for details.
[!IMPORTANT] This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
Concerns or objections can formally be registered here by adding a comment.
@rustbot concern reason-for-concern
<description of the concern>
Concerns can be lifted with:
@rustbot resolve reason-for-concern
See documentation at https://forge.rust-lang.org
@rustbot second
This has specified various possible validation concerns and not actually named specific ones that the UI test suite or another test suite is not sufficient for. It has kinda just said "well, they aren't". It says there will be codegen tests (which are an existing suite) and build-pass ("doesn't ICE") tests. These sound adequately handled by existing test suites. There are no special, particular concerns named that justify adding a new suite to compiletest. Yes, we should have more tests, but why a new suite?
To remove this concern, this MCP should name at least one specific reason that these tests cannot be done using other suites. At least, not without encountering other problems. It should be particular and not general, e.g. a good example would be a PR that bodges a test using existing means in one commit and then implements it using a separate suite in a successive commit, and some consensus that this PR is an improvement. I don't think that is required, just an example.
@rustbot concern name-a-specific-reason
We do not truly have a separate "parallel frontend" anymore. There is only how many threads the existing frontend is run with, controlled by -Zthreads. Conceptually, this proposal suggests we do not care about things like order of diagnostic output if we enable -Zthreads, but it is not clear that is the case. I have seen at least a few issues about diagnostic ordering. Maybe we do care? To remove this concern, this MCP should spend at least a sentence on justifying why we are okay with sacrificing certain forms of consistency in diagnostic output in tests just because we started using multiple threads. It should then explain why we are not willing to make this sacrifice in general (e.g. applied to all UI tests) or not willing to add the capability to identify diagnostics using some kind of DAG-like ordering to the UI test suite, possibly requiring a new directive or annotation of some sort.
@rustbot concern can-we-just-enhance-ui-suite