Second Mutation-CRAP refactoring. - `BuildExecutor.execute` (647.55, C 40) -> `admit`, `run`, `settle` + release in finally (5.67). - `BuildSlots`: global + per-repository slots, sized once, repo slot first, lease releases both; unit tests. - Behaviour unchanged (BuildExecutorIntegrationTest green). - Nightly gate `maxMutationCrap` 650 -> 57 (next highest: DockerSocketLocator.locate, InitCommand.runningJarPath at 56). PR-doc: `docs/prs/2026-10-04-PR#106-build-run-steps.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #106
105 lines
5.7 KiB
Markdown
105 lines
5.7 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
|
|
|
|
`BuildExecutor.execute` had the highest Mutation-CRAP of Werkator: 647.55, at a complexity of 40.
|
|
One method waited for the slots, marked the build running, prepared the worktree, ran the commands, decided the terminal status, stored the artifacts with their one special failure, and released everything in a `finally`, guarded by four nullable variables.
|
|
PR#103 had added the per-repository slot to it, and every new rule made it longer.
|
|
Its mutation score of 0.28 is partly a measuring artefact: `BuildExecutorIntegrationTest`, which covers it, is left out of the nightly PIT run (PR#94); but at a complexity of 40 no test could bring it below 15.
|
|
|
|
## Non-Goals
|
|
|
|
- No change in behaviour: every status, every phase, every order of release stays as it was.
|
|
- The rest of `BuildExecutor` (`runBuildCommands`, `shutdown`, `cancel`, `startBuild`) stays; most of its low scores come from the excluded test, and its own PR decides how to measure it.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: The slots of the instance and of each repository
|
|
|
|
#### Scenario#106.01: The slots are sized once, from the first repository that asks
|
|
|
|
So that `executor.maxConcurrent` is the instance's setting, not the last repository's.
|
|
|
|
- **Given** two repositories with different `executor.maxConcurrent`
|
|
- **When** the first of them acquires a slot
|
|
- **Then** its value sizes the slots for both, and the configured value answers only before
|
|
|
|
##### Verified by
|
|
|
|
- [BuildSlotsUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildSlotsUnitTest.kt) — Scenario#106.01
|
|
|
|
#### Scenario#106.02: A second build of a repository waits for its repository, another repository's does not
|
|
|
|
So that a waiting build never sits on a global slot.
|
|
|
|
- **Given** `maxConcurrent: 2` and `maxConcurrentPerRepository: 1`, and one build of a repository holding its slots
|
|
- **When** a second build of that repository and then a build of another one ask
|
|
- **Then** the second waits, the other gets the second global slot, and the second runs once the first gives back its slots
|
|
|
|
##### Verified by
|
|
|
|
- [BuildSlotsUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildSlotsUnitTest.kt) — Scenario#106.02
|
|
|
|
#### Scenario#106.03: A build cancelled while it waits for its repository gives the slot back and takes no global one
|
|
|
|
So that a cancelled build costs no capacity.
|
|
|
|
- **Given** a build waiting for its repository's only slot
|
|
- **When** it is cancelled and the slot frees up
|
|
- **Then** it gets no lease, and the next build of the repository runs at once
|
|
|
|
##### Verified by
|
|
|
|
- [BuildSlotsUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildSlotsUnitTest.kt) — Scenario#106.03
|
|
|
|
#### Scenario#106.04: A lease gives back both slots
|
|
|
|
So that no slot leaks, and without a per-repository limit only the global cap applies.
|
|
|
|
- **Given** slots with and without `maxConcurrentPerRepository`
|
|
- **When** builds acquire and release
|
|
- **Then** a released lease frees both slots, and without the key two builds of one repository run at once
|
|
|
|
##### Verified by
|
|
|
|
- [BuildSlotsUnitTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildSlotsUnitTest.kt) — Scenario#106.04
|
|
|
|
### Feature: A build runs in three steps
|
|
|
|
The behaviour is unchanged; [BuildExecutorIntegrationTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorIntegrationTest.kt) holds it, and it caught an order of evaluation the first draft got wrong.
|
|
|
|
## The Solution
|
|
|
|
- `execute` reads as the run it is: `admit`, `run`, `settle`, and the release in a `finally`.
|
|
- `admit` waits for the build's turn and marks it running; it answers null for a build cancelled while it waited, or when the instance is going.
|
|
- `run` prepares the worktree and runs the commands; an exception ends the run, never the worker.
|
|
- `settle` writes the journal's word, the terminal status, the artifacts (`storeArtifacts`, with the escape that takes a success back) and the phases.
|
|
- `BuildSlots` holds `executor.maxConcurrent` and `executor.maxConcurrentPerRepository`: sized once, the repository's slot before the global one, a lease that gives back both.
|
|
- A build whose slots cannot even be sized, a configuration that cannot be read, still ends FAILED rather than PENDING in no queue.
|
|
- The gate `maxMutationCrap` falls from 650 to 57: the highest scores left are `DockerSocketLocator.locate` and `InitCommand.runningJarPath`, 56 each.
|
|
|
|
Measured with PIT as the nightly build runs it, on `BuildExecutor` and `BuildSlots`:
|
|
|
|
| Method | Before | After |
|
|
| ----------------------- | ------ | ----- |
|
|
| `BuildExecutor.execute` | 647.55 | 5.67 |
|
|
| `BuildExecutor.admit` | — | 7.54 |
|
|
| `BuildExecutor.run` | — | 3.69 |
|
|
| `BuildExecutor.settle` | — | 12.19 |
|
|
| `BuildSlots.acquire` | — | 6.29 |
|
|
| `BuildSlots.limitsFor` | — | 4.07 |
|
|
|
|
`settle` is red, below the limit: it is covered only by the excluded `BuildExecutorIntegrationTest`.
|
|
|
|
## Additional Changes
|
|
|
|
- `BuildSlotsUnitTest` waits in threads of its own and lets every waiting thread finish: a first version waited in the common `ForkJoinPool`, which has two workers on the three CPUs of werkator.javagil.de, and the whole test task hung behind it (reproduced with `-Djava.util.concurrent.ForkJoinPool.common.parallelism=2`).
|
|
- Version 1.34.3 with its release note.
|
|
|
|
## Follow-up PRs
|
|
|
|
- The rest of `BuildExecutor`, with a decision on how the nightly run measures it.
|
|
- `ConfigLoader`, by its domain rules: pinning, trigger and follow-up rules, legacy migration, credentials.
|