docs(prs): the deflake-maxconcurrent-test PR-doc
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
9221550a2a
commit
5c73c4cc21
@@ -0,0 +1,42 @@
|
|||||||
|
> **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 build for commit `7028ca8` on `main` failed on the mih09 production instance with
|
||||||
|
`BuildExecutorTest > with maxConcurrent 1 a second branch stays PENDING until the first finished`,
|
||||||
|
while a retry of the very same commit passed, and the test passes locally.
|
||||||
|
|
||||||
|
The test was timing-dependent.
|
||||||
|
It started a build for `branch-a` whose build command was `sleep 1`, immediately started a second build for `branch-b`,
|
||||||
|
and then asserted — without any synchronization at all — that `branch-b` was still `PENDING`.
|
||||||
|
|
||||||
|
That assertion only held as long as the test thread reached it within the one second `branch-a` slept.
|
||||||
|
On a shared host under CPU contention the executor can get through `branch-a` entirely (queued, running, slept, succeeded) first,
|
||||||
|
and `branch-b` is then already `RUNNING` or `SUCCESS` when the assertion runs.
|
||||||
|
The failure therefore says nothing about the executor; it is pure scheduling noise that costs a build and a retry every time it hits.
|
||||||
|
|
||||||
|
## Non-Goals
|
||||||
|
|
||||||
|
- No change to production code — `BuildExecutor` is not touched, its queueing behaviour is unchanged.
|
||||||
|
- No sweep of the other timing-sensitive tests in the suite; only the one that actually flaked is fixed.
|
||||||
|
|
||||||
|
## The Solution
|
||||||
|
|
||||||
|
`branch-a` no longer sleeps for a fixed time, it blocks until the test says so:
|
||||||
|
its build command is `until [ -f gate ]; do sleep 0.05; done`, and the build workspace is the test's working directory.
|
||||||
|
|
||||||
|
The test now
|
||||||
|
1. waits (via `eventually`) until `branch-a` is `RUNNING`, so the single executor slot is provably occupied,
|
||||||
|
2. asserts that `branch-b` is `PENDING` — which cannot race anything, because `branch-a` cannot finish before the gate file exists,
|
||||||
|
3. creates the gate file, and only then awaits both builds' `SUCCESS`.
|
||||||
|
|
||||||
|
The assertion on the event transitions (`branch-b` goes `RUNNING` only after `branch-a` reached `SUCCESS`) is unchanged.
|
||||||
|
A blocking gate was chosen over a mocked `BuildRunner` because it keeps the test on the real `ProcessBuildRunner`,
|
||||||
|
so it still covers the actual process handling rather than only the executor's bookkeeping.
|
||||||
|
|
||||||
|
Verified by running `BuildExecutorTest` five times on an idle machine and three more times with twice `nproc` busy-loops saturating the CPU,
|
||||||
|
which is the condition that produced the original failure.
|
||||||
|
|
||||||
|
- [BuildExecutorTest](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorTest.kt)
|
||||||
Reference in New Issue
Block a user