diff --git a/docs/prs/2026-09-04-PR#20-deflake-maxconcurrent-test.md b/docs/prs/2026-09-04-PR#20-deflake-maxconcurrent-test.md new file mode 100644 index 0000000..a2d2b62 --- /dev/null +++ b/docs/prs/2026-09-04-PR#20-deflake-maxconcurrent-test.md @@ -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)