Files
werkator/docs/prs/2026-09-04-PR#20-deflake-maxconcurrent-test.md
2026-09-04 08:53:00 +02:00

2.6 KiB

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.