Repository navigation
test: stop asserting one outcome of the pathsend ordering race - #52
Merged
Merged
Conversation
`test_pathsend_background_races_finalization_behind_an_inner_middleware` asserted that the background task always beats the forwarded pathsend message. Behind a `BaseHTTPMiddleware` that ordering is a genuine race, so the assertion was a coin flip: it turned main red on linux_3.12 while the same commit had passed on the pull request, and it reproduces locally about once in ten runs. Make the observation deterministic instead. The tracked commit now waits on an event the background task sets, so the test proves the background task *can* run while finalization is in flight rather than hoping it wins the scheduler. If the limitation documented in docs/guide/http-load.md ever goes away the wait times out and the test fails, so the canary the old assertion was there to provide is preserved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #52 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 657 657
=========================================
Hits 657 657 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Why
ciwent red onmain(run 33993750764) on thelinux_3.12job, while the same commit had passed on the pull request that produced it.test_pathsend_background_races_finalization_behind_an_inner_middlewareasserted that the background task always beats the forwarded pathsend message:Behind a
BaseHTTPMiddlewarethat ordering is a genuine race — which is exactly what the test's own docstring anddocs/guide/http-load.mddescribe. So the assertion was a coin flip on the scheduler. It reproduces locally roughly once in ten runs; CI got the other side of the flip.What changed
The tracked commit now waits on an
asyncio.Eventthe background task sets, turning the race into a deterministic observation:This proves the background task can run while finalization is in flight, instead of hoping it wins the scheduler — and it keeps the canary the old assertion existed for. If the limitation ever disappears, the wait times out and the test fails; verified by running the same helper with
inner_middleware=False, where the ordering is guaranteed and the test duly fails.Behaviour under test is unchanged — this is a test-only fix.
Verification
tests/test_pathsend_responses.pypass (previously ~1 in 10 failed).ruff==0.16.6check + format --check clean.Note
The
docsworkflow was also red on the same push, for an unrelated reason: GitHub Pages was not enabled on the repository, soactions/deploy-pages404'd. Pages has been enabled (build_type: workflow) and that run is now green — no code change needed.