Skip to content

Commit f1aa520

Browse files
committed
Propose compiletest directive handling rework project goal for 2025H1
1 parent bf4ae18 commit f1aa520

File tree

1 file changed

+148
-0
lines changed

1 file changed

+148
-0
lines changed
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
# Making compiletest more maintainable: reworking directive handling
2+
3+
| Metadata | |
4+
|----------|-------------------------|
5+
| Owner(s) | [@jieyouxu] |
6+
| Teams | [bootstrap], [compiler] |
7+
| Status | Proposed |
8+
9+
## Summary
10+
11+
*Short description of what you will do over the next 6 months.*
12+
13+
Rework [`compiletest`]'s directive handling to make it more maintainable, have better UX for
14+
compiler contributors, and fix some long-standing issues.
15+
16+
## Motivation
17+
18+
`rustc` relies on the test infrastructure implemented by the test harness [`compiletest`] (supported
19+
by bootstrap) to run the test suites under `tests/` (e.g. `ui` tests, `mir-opt` tests, `run-make`
20+
tests, etc.). However, [`compiletest`] is currently very [undertested] and [undermaintained], which
21+
is not ideal because we rely on the test suites to check `rustc`'s behavior. The current
22+
implementation in [`compiletest`] is also such that it's very hard and unpleasant to make changes
23+
(e.g. adding new directives) to provide up-to-date test infrastructure support for the needs of
24+
compiler (and rustdoc) contributors. The UX is not great either because of poor error handling and
25+
error reporting.
26+
27+
[undertested]: https://github.com/rust-lang/rust/issues/47606
28+
[undermaintained]: https://github.com/orgs/rust-lang/projects/53
29+
30+
### The status quo
31+
32+
The current status quo is that [`compiletest`] imposes significant friction for compiler (and
33+
rustdoc) contributors who want to run tests and diagnose test failures. [`compiletest`] error
34+
messages are opaque, terse and hard to read. We had to include a separate allow-list of known
35+
directives to detect unknown directives. We still sometimes let malformed directives through and
36+
silently do nothing. Argument splitting is naive and inconsistent. The implementation is very
37+
convoluted. Also there's still insufficient documentation.
38+
39+
See the [tracking issue of various directive handling related bugs][directive-bugs-tracking-issue].
40+
41+
[directive-bugs-tracking-issue]: https://github.com/rust-lang/rust/issues/131425
42+
43+
### The next 6 months
44+
45+
The key changes I want to achieve:
46+
47+
1. Directive handling is **testable** (at all) and in addition have strong test coverage.
48+
2. Directives have **stricter syntax** to reduce ambiguity and enable invalid directive detection or
49+
make invalid directive detection easier.
50+
3. Directives are **well-documented**. Move directive documentation close to directives themselves
51+
and make it possible to be generated alongside tool docs for `compiletest`, so it's less likely
52+
to become outdated and to enable documentation coverage enforcement.
53+
- Also, make sure that we have robust *self* documentation so it's not only one or two
54+
contributors who understands how things work inside `compiletest`...
55+
4. Generally improve directive handling **robustness**. Examples: fixing argument splitting in
56+
`compile-flags`, fix paths related to `aux-build`, etc.
57+
5. Test writers and reviewers can receive **better diagnostics**, for things like a directive is not
58+
accepted in a given test suite or *why* something in `compiletest` failed.
59+
60+
### The "shiny future" we are working towards
61+
62+
My long-term goal for [`compiletest`] is that I want it to make it significantly easier to
63+
maintain. Concretely, this means significantly better test coverage, easier to extend, better
64+
documentation. Hopefully, by being more maintainable, we are able to attract more active maintainers
65+
from both bootstrap and compiler teams and make the code base significantly more pleasant to work
66+
on.
67+
68+
For directive handling *specifically*, it should mean that:
69+
70+
- It's relatively straightforward and low friction to implement new directives, including test
71+
coverage and documentation. It should be easy to do the right thing.
72+
- [`compiletest`] should produce error messages that are easy to read and understand, possibly even
73+
making suggestions.
74+
- Directives should be documented (and enforced to be documented) via rustdoc which are made
75+
available on nightly-rustc docs so we can back-link from dev-guide and not have to maintain two
76+
sets of docs that are mutually inconsistent.
77+
78+
## Ownership and team asks
79+
80+
**Owner:** [@jieyouxu]
81+
82+
<!--
83+
*This section defines the specific work items that are planned and who is expected to do them. It
84+
should also include what will be needed from Rust teams. The table below shows some common sets of
85+
asks and work, but feel free to adjust it as needed. Every row in the table should either correspond
86+
to something done by a contributor or something asked of a team. For items done by a contributor,
87+
list the contributor, or ![Help wanted][] if you don't yet know who will do it. For things asked of
88+
teams, list ![Team][] and the name of the team. The things typically asked of teams are defined in
89+
the [Definitions](#definitions) section below.*
90+
-->
91+
92+
Note that [`compiletest`] is (in theory) currently co-maintained by both t-bootstrap and t-compiler,
93+
but AFAIK is (in practice) currently not really actively maintained by anyone else. The following
94+
team asks are probably mostly [compiler] for feedback on their use cases (as a test infra consumer)
95+
and [bootstrap] for implementation review.
96+
97+
| Subgoal | Owner(s) or team(s) | Notes |
98+
|------------------------------------------------------|---------------------------------------------|------------------------------------------------------------------------------------------------|
99+
| General discussion and moral support | ![Team][] [bootstrap], ![Team][] [compiler] | |
100+
| Consultations for desired test behaviors | ![Team][] [compiler], ![Team][] [rustdoc] | Test infra consumers |
101+
| Experimental prototype[^1] | | To see how approaches look like and gain experience/feedback |
102+
| ↳ Discussion and moral support | ![Team][] [bootstrap], ![Team][] [compiler] | |
103+
| ↳ Implementation | [@jieyouxu] | |
104+
| ↳ Standard reviews | ![Team][] [bootstrap], ![Team][] [compiler] | Probably mostly [bootstrap] or whoever is more interested in reviewing [`compiletest`] changes |
105+
| [`compiletest`] changes w/ experience from prototype | | |
106+
| ↳ Discussion and moral support | ![Team][] [bootstrap], ![Team][] [compiler] | |
107+
| ↳ Implementation | [@jieyouxu] | |
108+
| ↳ Standard reviews | ![Team][] [bootstrap], ![Team][] [compiler] | Probably mostly [bootstrap] or whoever is more interested in reviewing [`compiletest`] changes |
109+
| Inside Rust blog post for project outcome | ![Team][] [bootstrap], ![Team][] [compiler] | |
110+
111+
[^1]: I want to start with an out-of-tree experimental prototype to see how the pieces are fit
112+
together to make it easier to rapidly iterate and receive feedback without having to mess with
113+
the "live" [`compiletest`] that does not have sufficient test coverage.
114+
115+
### Definitions
116+
117+
Definitions for terms used above:
118+
119+
* *Discussion and moral support* is the lowest level offering, basically committing the team to nothing but good vibes and general support for this endeavor.
120+
* *Author RFC* and *Implementation* means actually writing the code, document, whatever.
121+
* *Design meeting* means holding a synchronous meeting to review a proposal and provide feedback (no decision expected).
122+
* *RFC decisions* means reviewing an RFC and deciding whether to accept.
123+
* *Org decisions* means reaching a decision on an organizational or policy matter.
124+
* *Secondary review* of an RFC means that the team is "tangentially" involved in the RFC and should be expected to briefly review.
125+
* *Stabilizations* means reviewing a stabilization and report and deciding whether to stabilize.
126+
* *Standard reviews* refers to reviews for PRs against the repository; these PRs are not expected to be unduly large or complicated.
127+
* *Prioritized nominations* refers to prioritized lang-team response to nominated issues, with the expectation that there will be *some* response from the next weekly triage meeting.
128+
* *Dedicated review* means identifying an individual (or group of individuals) who will review the changes, as they're expected to require significant context.
129+
* Other kinds of decisions:
130+
* [Lang team experiments](https://lang-team.rust-lang.org/how_to/experiment.html) are used to add nightly features that do not yet have an RFC. They are limited to trusted contributors and are used to resolve design details such that an RFC can be written.
131+
* Compiler [Major Change Proposal (MCP)](https://forge.rust-lang.org/compiler/mcp.html) is used to propose a 'larger than average' change and get feedback from the compiler team.
132+
* Library [API Change Proposal (ACP)](https://std-dev-guide.rust-lang.org/development/feature-lifecycle.html) describes a change to the standard library.
133+
134+
## Frequently asked questions
135+
136+
TODO: pending during project discussions
137+
138+
<!--
139+
### What do I do with this space?
140+
141+
*This is a good place to elaborate on your reasoning above -- for example, why did you put the
142+
design axioms in the order that you did? It's also a good place to put the answers to any questions
143+
that come up during discussion. The expectation is that this FAQ section will grow as the goal is
144+
discussed and eventually should contain a complete summary of the points raised along the way.*
145+
-->
146+
147+
[@jieyouxu]: https://github.com/jieyouxu
148+
[`compiletest`]: https://github.com/rust-lang/rust/tree/master/src/tools/compiletest

0 commit comments

Comments
 (0)