DEV Community

Jeff
Jeff

Posted on Originally published at powerduck.com

The agent "fixed" the race condition with a 200ms sleep. CI went green. Tuesday morning it fell over.

The flaky test was the kind that everyone on the team had learned to rerun.

  it('replays the event into the read model', async () => {
    await bus.publish(event);
    const view = await readmodel.byId(event.aggregateId);
    expect(view.count).toBe(1);
  });
Enter fullscreen mode Exit fullscreen mode

It failed maybe one run in twenty on CI. Locally it never failed at all. The usual ritual was: hit retry, move on.

When the agent got assigned to it, it found the gap in about two minutes. The publish resolves before the consumer has flushed to the read model, so by the time the assertion runs, the row isn't there yet. The patch it produced was three lines:

  await bus.publish(event);
+ await new Promise(r => setTimeout(r, 200));
  const view = await readmodel.byId(event.aggregateId);
Enter fullscreen mode Exit fullscreen mode

CI went green. The flaky test stopped flaking. The PR merged. We moved on to the next thing.

Why this felt like a win

For about two weeks it was a win. The test was stable. Nobody had to rerun the build. The agent had diagnosed a real ordering problem and inserted something that made the ordering deterministic, at least on the machine where CI ran.

The problem is that a sleep is not a synchronization primitive. It's a bet about how slow the system on the other side is going to be. On a quiet runner, 200ms is generous. On a loaded runner, or in a container with a noisy neighbor, or once the read model got bigger and the flush slowed down, 200ms stopped being generous. It started being exactly the wrong number.

The first symptom was not a test failure. It was a support ticket on a Tuesday: the dashboard shows zero orders right after a purchase. A refresh fixed it. The read model had been written; the UI had just read it a few hundred milliseconds too early. That is the shape of a sleep bug in production. The system is correct on average, and wrong at the worst possible moment.

What the agent should have done instead

There are three honest answers to how you know the consumer has processed the event, and none of them is a magic number.

1. Wait on the thing you actually care about. If the read model is the assertion, wait on the read model:

await eventually(() => readmodel.byId(event.aggregateId), {
  timeout: 2000,
  interval: 25,
});
Enter fullscreen mode Exit fullscreen mode

eventually polls until the row exists or the budget runs out. On a fast machine it resolves in one tick. On a slow machine it waits the real time it needs. It fails loudly when the budget runs out, instead of silently passing and hoping.

2. Make the consumer emit a signal. In tests, use an in-memory bus that resolves its publish promise only after every handler has finished. This is the cleanest version: the test controls the timing, the production code stays the same, and you never depend on wall clock.

3. Stop racing in production. A sleep in a test is one problem. A sleep in application code is another. If the read path has to wait 200ms for a write to land, you have a consistency problem, and the fix is a stronger read (read-your-writes, a follow-up query on the source of truth, or an actual outbox) — not a delay.

The review habit this earns you

I now read any PR that contains setTimeout, sleep, wait(200), or delay as a smell, regardless of who wrote it. The question is not whether it passes CI. It is what that number is protecting against, and what happens when the thing it protects against is slower than the number.

That question is much easier to ask when the diff is three lines generated by an agent, because the agent has no stake in the number it picked. It pulled 200ms out of a distribution of numbers people use on the internet, and it happened to be right on the machines we tested on. It is not a synchronization strategy.

This is also where having a contract for the boundary between services pays off. If the read model is an HTTP endpoint, the cheapest regression safety net is to record real responses in tests and re-run them against the new code — the kind of local, spec-first loop we built into Powerduck specifically because an agent silently renaming a field is the cousin of this same class of bug.

The line I'd leave you with

A sleep makes the test deterministic on your laptop. A real wait makes the system deterministic in the world. When the agent reaches for setTimeout, that is the moment to ask it what the test would look like if we removed the wall clock entirely. The answer is usually twenty lines longer than the sleep, and it is the version that won't surface as a Tuesday support ticket.

Top comments (3)

Collapse
 
arhancanli profile image
Arhan Canli •

A cheap way to catch a sleep fix before it merges: run the test once with the consumer deliberately slowed (inject 500 ms into the handler, or wrap the bus in a delaying proxy). A real synchronization fix still passes; a setTimeout(200) fails every time, so the review bot or the human reviewer sees the bet instead of a green check. It also gives you the number your Tuesday ticket supplied for free: how slow can the consumer get before the test lies?

One caveat on eventually: if it only polls the read model, a stale row from an earlier test in the same process satisfies it. Keying the poll on event.aggregateId (as you do) or resetting the model between tests keeps the wait tied to this event. Did the agent's patch come with any test of its own, or only the three-line diff?

Collapse
 
williamsj04 profile image
Jessica Williams •

The eventually() helper is the version I would push for in review too. One detail worth adding: keep the timeout budget generous (a few seconds) and the interval small, because a generous budget costs nothing on a fast machine but stops the test from flaking on a loaded runner. The sleep and the polling loop look similar in the diff, but only one of them waits for the real condition.

Collapse
 
pm25coder profile image
pm25coder •

The eventually() shape is the one I would push for too, and I think it is worth putting a number on why, because "a generous budget costs nothing on a fast machine" is exactly the asymmetry — and it is bigger than it feels.

I modelled the consumer's flush latency (95% uniform 2–60 ms, 5% uniform 60–900 ms for the noisy-neighbour/GC tail), then ran the three arms on a real asyncio loop, 150 iterations each, latency drawn from that model:

arm                            failures          wall per run
setTimeout 200 ms              4/150 (2.7%)          208 ms
poll 25 ms / budget 2000 ms    0/150                  55 ms
poll  5 ms / budget 2000 ms    0/150                  44 ms
Enter fullscreen mode Exit fullscreen mode

The model says 4.2 % for the sleep; 4/150 sits inside one standard deviation of that, so the loop agrees with the arithmetic rather than with my hopes.

Two things fall out that the post does not spell out.

The sleep charges every green run; the budget charges only the slow ones. 208 ms against 55 ms per test is the same protection, paid by 100 % of runs versus roughly the actual wait. The poll is not the slower option here — a fixed wait is, because its number has to cover the tail while it is charged at the median. One refinement to @williamsj04's point: the budget is free, the interval is not. Dropping the interval from 25 ms to 5 ms cut 11 ms per run (55 → 44 ms), which is small but is the only part of the polling loop a passing run actually pays for.

"It stayed green for two weeks" is a measurement, and it is bounded. With zero failures in N runs, the 95 % upper bound on the per-run failure rate is about 3/N (the rule of three): 200 green runs means the sleep could have been failing 1 run in 66 the entire time. So two weeks of green does not show the number was adequate. At best it shows the patch bought a rate below the original 1-in-20 — which is precisely the rate a team calls flaky and reruns by reflex.

The corollary I found useful: to carry the promise a 2000 ms budget carries, the fixed number has to sit at the tail rather than at the typical wait — about 900 ms in this model, 28× the median wait, charged on every single run. That is why nobody writes it, and why a reviewer cannot be argued out of a sleep by a slightly larger sleep.

One addition to option 2: a bus that resolves only after every handler has finished makes the timing deterministic by removing the interleaving the bug lived in. That is right for this assertion, but if the thing you are hunting is an ordering or duplicate-delivery bug, that arm cannot produce the schedule that triggers it — worth keeping at least one test on the real bus.