57 lines
3.3 KiB
Markdown
57 lines
3.3 KiB
Markdown
# PR#138: Executor Tests without Waiting
|
|
|
|
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.
|
|
|
|
## The Problem
|
|
|
|
`BuildExecutor` costs more time in the nightly mutation build than any other class.
|
|
An estimate from the nightly of 2026-10-10 puts about a third of all minion time on it: `BuildExecutorUnitTest` alone about 14 of 81 minutes, and 35 of the 63 timed-out mutants are in `BuildExecutor`.
|
|
|
|
`BuildExecutorUnitTest` starts no process, but it waits:
|
|
|
|
- The timeout test waits out a real build timeout of one second.
|
|
- Two tests sleep 300 ms to see that a build still waits for its slot.
|
|
- `awaitStatus` polls for up to five seconds until a build has the expected status, also when the build already ended in another one.
|
|
|
|
A normal run of the spec takes about 3.7 s, more than half of it in these waits.
|
|
PIT runs the whole spec again for each of the 233 mutants it covers.
|
|
And a mutant that makes the builds end in the wrong status lets every test poll for five seconds, so the spec runs past PIT's timeout: the mutant ends `TIMED_OUT` after more than 20 s, and PIT starts a new minion.
|
|
|
|
## Non-Goals
|
|
|
|
- No change to how the executor runs builds: threads, slots and the drain stay as they are.
|
|
- The drain test keeps its pause of 500 ms: the drain polls every 200 ms, and only a pause shows that it does not restart the instance early.
|
|
- The tests of `BuildSlots`, the integration tests and `UiControllerRestTest` follow in their own PRs.
|
|
|
|
## The Scenarios
|
|
|
|
The behaviour of the executor does not change, so its scenarios stay as they are.
|
|
This PR changes only how three of them are verified:
|
|
|
|
- Scenario#108.08 (timeout): the fake process answers a wait with a timeout at once that the time is up, which is what a real process does once the timeout has passed.
|
|
- Scenario#119.02 and Scenario#119.04 (waiting reviews and CI builds): the test sees the build wait at its slot, through the number of builds waiting for a slot, instead of sleeping.
|
|
|
|
## The Solution
|
|
|
|
- `BuildSlots.waiting()` counts the builds waiting at a semaphore of any pool, and `BuildExecutor.waitingForSlots()` passes it on; both are internal.
|
|
- `awaitStatus` stops waiting as soon as the build has the expected status or any final one, and then compares.
|
|
- `awaitWaitingForSlots` waits until the given builds wait for a slot, and fails at once when one of them was let in.
|
|
- `FakeProcess(outlivesDeadline = true)` lets the timeout test run without waiting for its timeout.
|
|
|
|
A normal run of the spec now takes 2.1 s instead of 3.7 s.
|
|
A local PIT run of `BuildExecutor` alone, two minions, without history:
|
|
|
|
| Run | Duration | Mutants timed out |
|
|
|-------------|---------:|------------------:|
|
|
| Before | 27.5 min | 36 |
|
|
| With the PR | 23.7 min | 35 |
|
|
|
|
The timeouts stay because PIT runs the fastest covering spec first.
|
|
For `BuildExecutor` these are `PullRequestTargetBuildIntegrationTest` and `BuildExecutorArtifactIntegrationTest`, which wait up to 30 s for a build that a mutant never ends.
|
|
|
|
## Follow-up PRs
|
|
|
|
- Leave the two integration specs out of the mutation build, once unit tests cover what only they kill.
|