compiletest: Add an experimental new executor to replace libtest by Zalathar · Pull Request #139660 · rust-lang/rust

@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)

labels

Apr 11, 2025

@Zalathar Zalathar changed the title compiletest: Add an experimental "new" executor to replace libtest compiletest: Add an experimental new executor to replace libtest

Apr 11, 2025

jieyouxu

@rustbot rustbot 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-review

Status: Awaiting review from the assignee but also interested parties.

labels

Apr 14, 2025

@Zalathar

@Zalathar

@Zalathar

The new executor can be enabled by passing `--new-executor` or `-n` to
compiletest.

For example: `./x test ui -- -n`

@rustbot rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Apr 15, 2025

jieyouxu

@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

Apr 15, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request

Apr 15, 2025

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Apr 15, 2025
Rollup merge of rust-lang#139660 - Zalathar:new-executor, r=jieyouxu

compiletest: Add an experimental new executor to replace libtest

This PR adds a new "executor" to compiletest for running the list of collected tests, to eventually replace the current dependency on unstable libtest internals.

The new executor is currently inactive by default. It must be activated explicitly by passing `-n` or `--new-executor` to compiletest, e.g. `./x test ui -- -n`.

(After some amount of wider manual testing, the new executor will hopefully be made the default, and the libtest dependency can be removed. Contributors should not notice any change.)

The new executor is a stripped-down rewrite of the subset of libtest needed by compiletest.

# Supported functionality
- Specifying the number of concurrent tests with `RUST_TEST_THREADS`
- Filtering and skipping tests by name (substring or exact-match)
- Forcibly running ignored tests with `--ignored`
- Optional fail-fast with `--fail-fast`
- JSON output, compatible with bootstrap's parser for libtest output
- Running each test in its own thread
- Short backtraces that ignore the executor itself
- Slow test detection, with a hard-coded timeout of 60 seconds
- Capturing stdout/stderr, via `#![feature(internal_output_capture)]`
- Suppressing output capture with `--no-capture`

# Unsupported functionality
- Non-JSON output, as this is handled by bootstrap instead
- Separate code path for concurrency=1, as the concurrent path should handle this case naturally
- Fallback to running tests synchronously if new threads can't be spawned
- Special handling of hosts that don't support basic functionality like threads or timers
  - Our ability to test *targets* should be unaffected
- Graceful handling of some edge cases that could occur in arbitrary user-written unit tests, but would represent bugs in compiletest
- Due to the current need for output capture, the new executor is still not entirely written in stable Rust

---
r? jieyouxu

Kobzol

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request

Apr 19, 2025

ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request

Apr 22, 2025
compiletest: Fix deadline bugs in new executor

The experimental new executor for compiletest (rust-lang#139660) was found to have two major bugs in deadline handling for detecting slow tests:

- The comparison between `now` and test deadlines was reversed, causing no timeouts to ever be recognised.
- After fixing that bug, it was found that the existing code would issue timeouts for any test that had started more than 60 seconds ago, even if the test had finished long before its deadline was reached.

This PR fixes those bugs.

(The new executor is not yet enabled by default, so this PR has no immediate effect on contributors.)

---

I noted in rust-lang#139998 (comment) that I hoped to have some unit tests to accompany these fixes. Unfortunately that turned out to be infeasible, because `DeadlineQueue` is tightly coupled to concrete `mpsc::Receiver` APIs (in addition to `Instant::now`), and trying to mock all of those would make the code much more complicated.

I did, however, add a few assertions that would have caught the failure to remove tests from the queue after their deadline.

r? jieyouxu

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Apr 23, 2025
Rollup merge of rust-lang#140031 - Zalathar:deadline, r=jieyouxu

compiletest: Fix deadline bugs in new executor

The experimental new executor for compiletest (rust-lang#139660) was found to have two major bugs in deadline handling for detecting slow tests:

- The comparison between `now` and test deadlines was reversed, causing no timeouts to ever be recognised.
- After fixing that bug, it was found that the existing code would issue timeouts for any test that had started more than 60 seconds ago, even if the test had finished long before its deadline was reached.

This PR fixes those bugs.

(The new executor is not yet enabled by default, so this PR has no immediate effect on contributors.)

---

I noted in rust-lang#139998 (comment) that I hoped to have some unit tests to accompany these fixes. Unfortunately that turned out to be infeasible, because `DeadlineQueue` is tightly coupled to concrete `mpsc::Receiver` APIs (in addition to `Instant::now`), and trying to mock all of those would make the code much more complicated.

I did, however, add a few assertions that would have caught the failure to remove tests from the queue after their deadline.

r? jieyouxu

bors added a commit to rust-lang-ci/rust that referenced this pull request

Apr 23, 2025
compiletest: Use the new non-libtest executor by default

The new executor was implemented in rust-lang#139660, but required a manual opt-in. This PR activates the new executor by default, but leaves the old libtest-based executor in place (temporarily) to make reverting easier if something unexpectedly goes horribly wrong.

Currently the new executor can be explicitly disabled by passing the `-N` flag to compiletest (e.g. `./x test ui -- -N`), but eventually that flag will be removed, alongside the removal of the libtest dependency. The flag is mostly there to make manual comparative testing easier if something does go wrong.

As before, there *should* be no user-visible difference between the old executor and the new executor.

---

I didn't get much of a response to my [call for testing thread on Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/Call.20for.20testing.3A.20New.20test.20executor.20for.20compiletest/with/512452105), and the reports I did get (along with my own usage) indicate that there aren't any problems. So I think it's reasonable to move forward with making this the default, in the hopes of being able to remove the libtest dependency relatively soon.

When the libtest dependency is removed, it should be reasonable to build compiletest against pre-built stage0 std by default, even after the stage0 redesign. (Though we should probably have at least one CI job using in-tree stage1 std instead, to guard against the possibility of the `#![feature(internal_output_capture)]` API actually changing.)

bors added a commit to rust-lang-ci/rust that referenced this pull request

Apr 23, 2025
compiletest: Use the new non-libtest executor by default

The new executor was implemented in rust-lang#139660, but required a manual opt-in. This PR activates the new executor by default, but leaves the old libtest-based executor in place (temporarily) to make reverting easier if something unexpectedly goes horribly wrong.

Currently the new executor can be explicitly disabled by passing the `-N` flag to compiletest (e.g. `./x test ui -- -N`), but eventually that flag will be removed, alongside the removal of the libtest dependency. The flag is mostly there to make manual comparative testing easier if something does go wrong.

As before, there *should* be no user-visible difference between the old executor and the new executor.

---

I didn't get much of a response to my [call for testing thread on Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/Call.20for.20testing.3A.20New.20test.20executor.20for.20compiletest/with/512452105), and the reports I did get (along with my own usage) indicate that there aren't any problems. So I think it's reasonable to move forward with making this the default, in the hopes of being able to remove the libtest dependency relatively soon.

When the libtest dependency is removed, it should be reasonable to build compiletest against pre-built stage0 std by default, even after the stage0 redesign. (Though we should probably have at least one CI job using in-tree stage1 std instead, to guard against the possibility of the `#![feature(internal_output_capture)]` API actually changing.)

bors added a commit to rust-lang-ci/rust that referenced this pull request

Apr 26, 2025
compiletest: Re-land using the new non-libtest executor by default

This PR re-lands rust-lang#139998, which had the misfortune of triggering download-rustc in its CI jobs, so we didn't get proper test metrics for comparison with the old implementation. So that was PR was reverted in rust-lang#140233, with the intention of re-landing it alongside a dummy compiler change to inhibit download-rustc.

---

Original PR description for rust-lang#139998:
>The new executor was implemented in rust-lang#139660, but required a manual opt-in. This PR activates the new executor by default, but leaves the old libtest-based executor in place (temporarily) to make reverting easier if something unexpectedly goes horribly wrong.
>
>Currently the new executor can be explicitly disabled by passing the `-N` flag to compiletest (e.g. `./x test ui -- -N`), but eventually that flag will be removed, alongside the removal of the libtest dependency. The flag is mostly there to make manual comparative testing easier if something does go wrong.
>
>As before, there *should* be no user-visible difference between the old executor and the new executor.

---
r? jieyouxu