miandmhoennig
733bbc25f8
- `BuildExecutorUnitTest`: the real BuildExecutor against a fake build runner (no processes, ~6 s), so the nightly PIT run measures command order, cancel, shutdown, duplicates, timeout and the restart drain. - `ActiveBuilds`: register of queued and running builds (duplicates, active keys, executing, cancellation) instead of a map asked in nine places. - BuildExecutorIntegrationTest stays excluded from PIT: locally PIT on BuildExecutor ran >100 min with it vs 12 min without. - runBuildCommands 46.55 -> 20.78, shutdown 42 -> 7, cancel 30 -> 4, startBuild 28.7 -> 3; gate 47 -> 46. PR-doc: `docs/prs/2026-10-04-PR#108-build-executor-fast-tests.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #108
153 lines
6.9 KiB
Markdown
153 lines
6.9 KiB
Markdown
> **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
|
|
|
|
The nightly mutation build leaves out `BuildExecutorIntegrationTest` (PR#94), the only test that runs `BuildExecutor`.
|
|
The executor's own decisions therefore looked untested: `runBuildCommands` 46.55, `shutdown` 42, `cancel` 30, `startBuild` 28.67.
|
|
Taking the test back into the run is no way out: measured locally on `BuildExecutor` alone, PIT needed 12 minutes without it and had not finished after more than 100 minutes with it, 72 mutants having run into PIT's time limit.
|
|
Every one of those tests starts real processes and waits for them.
|
|
The register of active builds, a map of artifact keys, was asked in nine places, each with its own loop.
|
|
|
|
## Non-Goals
|
|
|
|
- `BuildExecutorIntegrationTest` stays, and stays out of the nightly run; it holds the executor together with real processes.
|
|
- The process handling (`destroyProcessTree`, `waitForBounded`, the processor-time sampling) follows in its own PR.
|
|
- No change in behaviour.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: The register of active builds
|
|
|
|
#### Scenario#108.01: A build of the same pool and commit is a duplicate while it is active and not cancelled
|
|
|
|
So that a double-triggered restart does not queue a commit twice, and a cancelled build does not block it.
|
|
|
|
- **Given** a queued build of `main` at `abc`
|
|
- **When** the register is asked for a duplicate
|
|
- **Then** the same pool and commit in the same repository is one; another pool, commit or repository is not; a cancelled build is not
|
|
|
|
##### Verified by
|
|
|
|
- [ActiveBuildsUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/ActiveBuildsUnitTest.kt) — Scenario#108.01
|
|
|
|
#### Scenario#108.02: A key is active in its own repository only, and until it is removed
|
|
|
|
So that a route of one repository never reaches a build of another.
|
|
|
|
- **Given** a listed build, one queued and one executing
|
|
- **When** the register is asked
|
|
- **Then** the key is active in its repository only, until removed, and only the executing build counts as executing
|
|
|
|
##### Verified by
|
|
|
|
- [ActiveBuildsUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/ActiveBuildsUnitTest.kt) — Scenario#108.02
|
|
|
|
#### Scenario#108.03: A cancellation says whether the build was still queued or already executing
|
|
|
|
So that the executor records a queued build at once and terminates an executing one.
|
|
|
|
- **Given** a queued and an executing build
|
|
- **When** each is cancelled, and an unknown key
|
|
- **Then** the answers are queued, executing, and none, and both builds are marked cancelled
|
|
|
|
##### Verified by
|
|
|
|
- [ActiveBuildsUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/ActiveBuildsUnitTest.kt) — Scenario#108.03
|
|
|
|
### Feature: The executor's decisions, without a process
|
|
|
|
#### Scenario#108.04: A build runs its clean command, then its build command, and the exit codes decide its status
|
|
|
|
So that the order of the commands and the logs they write are held where the nightly run sees them.
|
|
|
|
- **Given** a runner whose commands end at once with chosen exit codes
|
|
- **When** builds run with and without a clean command, and with their own log names
|
|
- **Then** clean runs before build, a failed clean ends the build, the exit codes decide success or failure, and the live log and the named logs hold what they should
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) — Scenario#108.04
|
|
|
|
#### Scenario#108.05: Cancelling a queued build records it at once and never starts it; an executing one is terminated
|
|
|
|
So that a cancellation is final either way.
|
|
|
|
- **Given** an executing build that ends only when destroyed, and a build queued behind it
|
|
- **When** both are cancelled
|
|
- **Then** the queued one is CANCELLED at once and never started, the executing one is destroyed and CANCELLED
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) — Scenario#108.05
|
|
|
|
#### Scenario#108.06: A second start of the same commit is the same build while it is active
|
|
|
|
So that a double click does not build twice, and a cancelled build can be started again.
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) — Scenario#108.06
|
|
|
|
#### Scenario#108.07: A shutdown interrupts the executing build and leaves the queued one pending
|
|
|
|
So that the startup recovery finds both.
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) — Scenario#108.07
|
|
|
|
#### Scenario#108.08: A build that outlives its timeout is stopped and recorded as timed out
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) — Scenario#108.08
|
|
|
|
#### Scenario#108.09: A build cancelled after its clean command never starts its build command
|
|
|
|
So that a cancellation between the two commands is not overrun by the second one.
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) — Scenario#108.09
|
|
|
|
#### Scenario#108.10: After a deployment the instance restarts only once every other build has ended
|
|
|
|
So that a deployment never stops another repository's build.
|
|
|
|
- **Given** a build of another branch executing
|
|
- **When** a deployment with `restartsInstance` succeeds
|
|
- **Then** the instance drains, starts no further build, and asks for the restart only after the other build has ended
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorUnitTest.kt) — Scenario#108.10
|
|
|
|
## The Solution
|
|
|
|
- `ActiveBuilds` is the register of accepted, unfinished builds: duplicates, active keys, executing builds, and what a cancellation found; `BuildExecutor` asks it instead of a map.
|
|
- `BuildExecutorUnitTest` runs the real `BuildExecutor` against a build runner that starts no process: its processes end at once with a chosen exit code, or only when destroyed.
|
|
The spec takes about six seconds, so the nightly run keeps it.
|
|
|
|
Measured with PIT as the nightly build runs it, on `BuildExecutor`:
|
|
|
|
| Method | Before | After |
|
|
| ------------------ | ------ | ----- |
|
|
| `runBuildCommands` | 46.55 | 20.78 |
|
|
| `shutdown` | 42 | 7.05 |
|
|
| `cancel` | 30 | 4 |
|
|
| `startBuild` | 28.67 | 2.98 |
|
|
| the drain's wait | 20 | 8.13 |
|
|
|
|
`runBuildCommands` is still above 15; its rest is the process handling of the next PR.
|
|
The gate `maxMutationCrap` falls from 47 to 46: the highest score left is `NginxProxyManager.cleanupStaleContainers` at 45.04.
|
|
|
|
## Additional Changes
|
|
|
|
- Version 1.34.5 with its release note.
|
|
|
|
## Follow-up PRs
|
|
|
|
- The process handling of `BuildExecutor` in a class of its own, with a short spec against real, short processes that the nightly run keeps.
|