Skip to content

Commit 2364571

Browse files
committed
RFC: Use cargo-nextest
Proposes using cargo-nextest for running tests in Patina. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
1 parent c624699 commit 2364571

1 file changed

Lines changed: 160 additions & 0 deletions

File tree

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
# RFC: Run Tests with `cargo-nextest`
2+
3+
This RFC proposes replacing `cargo test` with [cargo-nextest](https://nexte.st/) as the test runner behind Patina's
4+
`cargo make` test tasks, while keeping `cargo test` for running doctests since nextest can't run them.
5+
6+
## Change Log
7+
8+
- 2026-08-06: Initial draft of RFC.
9+
- 2026-08-11: Update "Unresolved Questions" to indicate that `#[serial]` should be removed in cases where nextest's
10+
isolation is sufficient. Add details about how nextest works with Miri in Technology Background.
11+
12+
## Motivation
13+
14+
Patina's has a few test-related tasks in `Makefile.toml`:
15+
16+
- `test`
17+
- `check_tests`
18+
- `check-no-default-features-tests`
19+
- `test-asan`
20+
- Coverage tasks
21+
22+
Those run tests through plain `cargo test`. `cargo test` works, but its output is hard to read at a glance for a
23+
workspace this size. It does not give a summary of which tests were slow, and offers no way to retry a test that fails
24+
intermittently.
25+
26+
In the past, a concern was that nextest is an external tool that had to be installed by hand. Patina's `Makefile.toml`
27+
now has an `install-tools` task that reads a `[tools]` table in `rust-toolchain.toml` and installs everything listed
28+
there automatically, so that particular objection no longer applies. This RFC picks the nextest idea back up on its own,
29+
without tying it to a larger build system rewrite.
30+
31+
## Technology Background
32+
33+
[cargo-nextest](https://nexte.st/) is a cargo subcommand that discovers and runs the same tests `cargo test` does,
34+
invoked as `cargo nextest run` instead of `cargo test`. It reads the same package, feature, and target selection flags,
35+
so most of the command line usage Patina already has does not need to change.
36+
37+
The core difference is how tests are executed. `cargo test` builds one binary per test target and runs every `#[test]`
38+
function inside it as a thread within a single process. `cargo nextest run` runs every individual test
39+
[in its own operating system process](https://nexte.st/docs/design/why-process-per-test/). This has a few consequences:
40+
41+
The first is isolation. If one test panics in a way that corrupts shared memory, triggers a hard abort, or segfaults,
42+
only that one process is affected. The rest of the run keeps going, and nextest reports exactly which test caused the
43+
failure. Patina already runs an `AddressSanitizer` pass over its tests (`cargo make test-asan`). Under `cargo test`, an
44+
ASan abort in one test can prevent further results from being reported in the binary. Using nextest, that test fails on
45+
its own and every other test still reports a result.
46+
47+
The second is that Nextest cannot run doctests, because doing so requires unstable compiler support that only works
48+
through the ordinary `cargo test` harness today. This is a known, permanent restriction tracked at
49+
[nextest-rs/nextest#16](https://github.com/nextest-rs/nextest/issues/16), not something specific to Patina. Doctests
50+
need to keep running through `cargo test --doc`.
51+
52+
The third is that Patina uses the [`serial_test`](https://crates.io/crates/serial_test) crate's `#[serial]` attribute in
53+
several places (`patina_adv_logger`, `patina_debugger`, the `patina_internal_cpu` interrupt tests, and the
54+
`patina_dxe_core` allocator usage tests) to stop specific tests from running at the same time as each other. `#[serial]`
55+
works by holding a lock inside the process. `serial_test`'s documentation recommends its `file_serial` variant instead
56+
for tests that run as separate processes. Since nextest always runs tests as separate processes, `#[serial]` no longer
57+
prevents two tests from running concurrently under nextest. In cases reviewed so far, the state those tests guard is a
58+
static that already lives entirely inside one process, so a new process isolates it on its own and the attribute appears
59+
to be redundant rather than actively wrong, but this has not been checked for every use of `#[serial]` in the repository
60+
though. See [Unresolved Questions](#unresolved-questions).
61+
62+
Nextest also adds a few capabilities `cargo test` does not have. It can automatically retry a test a set number of times
63+
and report whether it passed on retry, which is useful for flaky tests. It can produce JUnit XML output for tooling that
64+
expects it (like CI dashboards). It has a filter expression language for selecting tests by package, binary, or name
65+
pattern, that's more sophisticated than what `cargo test` supports. It also emulates enough of `cargo test`'s own
66+
command line arguments, including `-- --nocapture`, that existing arguments mostly carry over as long as a recent enough
67+
nextest version is used, which would be the case going forward in Patina. It is a well-supported, popular tool in the
68+
Rust ecosystem.
69+
70+
A final note is that nextest is beneficial for running tests under [Miri](https://github.com/rust-lang/miri). As
71+
described in [cargo-nextest - Miri and nextest](https://nexte.st/docs/integrations/miri/), because nextest runs each
72+
test in its own process and Miri is single-threaded, nextest can run multiple Miri tests in parallel across processes
73+
which can improve test run performance by 3-4x.
74+
75+
There is also a caveat to be aware of:
76+
77+
> Note, however, that cargo miri test is able to detect data races where two tests race on a shared resource. Miri with
78+
> nextest will not detect such races.
79+
80+
## Goals
81+
82+
1. Give clearer, more actionable output for test runs than `cargo test` provides today.
83+
2. Reduce the chance that a crash or resource conflict in one test hides or corrupts the results of other tests.
84+
3. Keep running the same set of tests Patina runs today, with the exception of doctests.
85+
4. Let every contributor and CI pipeline pick up the tool automatically, with no manual install step.
86+
5. Get a small overall reduction in total test execution time.
87+
88+
Since nextest can run tests in parallel across processes rather than across threads within a single process, it was
89+
found to provide a slight overall reduction in total test execution time.
90+
91+
- `cargo test` average: 29.18s
92+
- `cargo nextest run` average: 26.65s
93+
94+
> These numbers do not include the compilation tests (in `sdk/patina/tests/compile_fail_tests.rs`). They were taken
95+
> on a Windows 11 machine and only their relative difference is shown to be meaningful.
96+
97+
## Requirements
98+
99+
1. Every test that runs under `cargo test` today must keep running, except doctests, which nextest cannot execute and
100+
which continue to run through the existing `doc-test` task.
101+
2. `cargo-nextest` must install automatically through the existing `cargo make install-tools` task, with no separate
102+
step for contributors to follow by hand.
103+
3. Coverage collection through `cargo-llvm-cov` must keep working, including coverage collected from doctests.
104+
4. Existing developer workflows, such as running a single test by name, scoping a run to one package, or asking for
105+
output on a passing test, must keep working from the command line.
106+
5. AddressSanitizer testing (`cargo make test-asan`) must keep exercising both ordinary tests and doctests.
107+
108+
## Unresolved Questions
109+
110+
1. Should Patina adopt nextest's test group feature as a replacement for `#[serial]`? This is treated more as a follow
111+
up rather than something this RFC needs to resolve.
112+
113+
- The decision at the time of the RFC is remove `#[serial]` in cases where cargo-nextest's isolation is clearly
114+
sufficient.
115+
116+
## Prior Art (Existing PI C Implementation)
117+
118+
We've briefly considered other test runners as noted in the RFC introduction but never made a decision to adopt one.
119+
120+
## Alternatives
121+
122+
1. Stay with `cargo test`. This avoids adding a new tool, but keeps the output and process isolation limitations
123+
described above, and does not address the concern that originally motivated PR 497.
124+
2. Write custom tooling around `cargo test` to improve its output, for example a wrapper that parses and reformats its
125+
results. This would take ongoing effort to build and maintain, and would still run every test in a shared process, so
126+
it would not gain the isolation or performance benefits nextest provides.
127+
128+
## Rust Code Design
129+
130+
This proposal is a build and test tooling change. It does not require changes to production Patina source code. The
131+
changes are as follows.
132+
133+
In `rust-toolchain.toml`, add `cargo-nextest` to the `[tools]` table, the latest so `cargo make install-tools` installs
134+
it the same way it installs `cargo-llvm-cov`, `cargo-deny`, and the other pinned tools.
135+
136+
In `Makefile.toml`:
137+
138+
1. `test`, `check_tests`, and `check-no-default-features-tests` run `cargo nextest run` instead of `cargo test`.
139+
2. `test-asan` runs its ordinary tests through `cargo nextest run`, then runs a second, separate `cargo test --doc`.
140+
3. `test-cov` collects coverage for ordinary tests through `cargo llvm-cov nextest`, and a new task collects doctest
141+
coverage separately through `cargo llvm-cov --doc`, since nextest cannot run doctests itself. The report generation
142+
tasks (`coverage-lcov`, `coverage-html`) pass `--doctests` so both sets of collected data are merged into one report.
143+
4. `doc-test` is unchanged. It keeps running `cargo test --doc` because that is the only way to run doctests at all.
144+
145+
## Guide-Level Explanation
146+
147+
Running tests looks almost the same as it does today. `cargo make test`, `cargo make test -p patina_dxe_core`, and
148+
`cargo make test -p patina_dxe_core -- --nocapture` all keep working the way they do now.
149+
150+
`cargo make check`, `cargo make coverage`, and `cargo make test-asan` also keep working without any change to how you
151+
invoke them. The output changes and includes additional information. Instead of a stream of dots, you get a pass or fail
152+
line per test with how long it took, and a summary at the end.
153+
154+
`cargo make test` no longer runs doctests as part of the same command. Nextest cannot run them, so they continue to run
155+
through `cargo make doc-test`, which already existed as its own task before this change.
156+
157+
If you write a test that depends on another test having already run and left some state behind, nextest is likely to
158+
catch that quickly, because every test starts in its own process with no leftover state from anything else. If you hit
159+
this, the fix is the same one used there, reset the shared state explicitly at the start of the test rather than relying
160+
on it having already been set up by something else.

0 commit comments

Comments
 (0)