Skip to content

Measure the overlap, not the wall clock, in the preprocess concurrency test - #187

Merged
adamjohnwright merged 1 commit into
mainfrom
fix/flaky-concurrency-timing
Sep 9, 2026
Merged

adamjohnwright merged 1 commit into
mainfrom
fix/flaky-concurrency-timing

Conversation

@adamjohnwright

Copy link
Copy Markdown
Contributor

This test failed on CI for #186, a change that touches nothing it covers:

AssertionError: took 0.63s, expected roughly 0.40s
assert 0.6318183140000002 < (0.2 * 2.8)

The two assertions immediately above it — that each step starts before the other finishes — passed. Only the wall-clock budget failed.

That budget covered two 0.2s sleeps plus asyncio.run start-up, three coroutine hand-offs and whatever else a shared runner was doing, so it measured the runner about as much as the code. It was also a weaker restatement of what the interval assertions already prove.

It now measures the overlap itself, from the same perf_counter marks:

overlap = min(safety[1], language[1]) - max(safety[0], language[0])
assert overlap > DELAY / 2

Two steps that ran back to back overlap by ~0; two started together overlap by ~DELAY.

Verified: 30 consecutive runs, 0 failures. And it still catches the regression it exists for — making preprocess await the two steps sequentially instead of asyncio.gather fails it with "language detection started after safety finished".

test_safety_and_language_overlap failed on CI for an unrelated PR:

    AssertionError: took 0.63s, expected roughly 0.40s
    assert 0.6318183140000002 < (0.2 * 2.8)

The two assertions above it -- that each step starts before the other
finishes -- passed. Only the wall-clock budget failed.

That budget covered two 0.2s sleeps plus asyncio.run start-up, three
coroutine hand-offs and whatever else a shared runner was doing, so it
measured the runner about as much as the code. It was also a weaker
restatement of what the interval assertions already prove.

It now measures the overlap itself, from the same perf_counter marks
the other assertions use. Two steps that ran back to back overlap by
~0; two started together overlap by ~DELAY.

Verified: 30 consecutive runs, 0 failures. Still catches the regression
it exists for -- awaiting the two steps sequentially instead of
asyncio.gather fails it.
@adamjohnwright
adamjohnwright merged commit 4bcc9b4 into main Sep 9, 2026
10 checks passed
@adamjohnwright
adamjohnwright deleted the fix/flaky-concurrency-timing branch September 9, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant