Repository navigation
Measure the overlap, not the wall clock, in the preprocess concurrency test - #187
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This test failed on CI for #186, a change that touches nothing it covers:
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.runstart-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_countermarks: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
preprocessawait the two steps sequentially instead ofasyncio.gatherfails it with "language detection started after safety finished".