Skip to content

test: stop asserting one outcome of the pathsend ordering race - #52

Merged
h0rn3t merged 1 commit into
mainfrom
fix/flaky-pathsend-ordering-test
Sep 6, 2026
Merged

h0rn3t merged 1 commit into
mainfrom
fix/flaky-pathsend-ordering-test

Conversation

@h0rn3t

@h0rn3t h0rn3t commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Why

ci went red on main (run 33993750764) on the linux_3.12 job, while the same commit had passed on the pull request that produced it.

test_pathsend_background_races_finalization_behind_an_inner_middleware asserted that the background task always beats the forwarded pathsend message:

assert events.index("background") < events.index(send_event), events

Behind a BaseHTTPMiddleware that ordering is a genuine race — which is exactly what the test's own docstring and docs/guide/http-load.md describe. 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.Event the background task sets, turning the race into a deterministic observation:

events = await _pathsend_ordering(
    tmp_path, inner_middleware=True, commit_on_exit=True, await_background=True
)
assert events == ["commit", "background", "send(exists=False)"], events

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

  • 30/30 consecutive runs of tests/test_pathsend_responses.py pass (previously ~1 in 10 failed).
  • Full suite: 227 passed, 3 skipped.
  • ruff==0.16.6 check + format --check clean.

Note

The docs workflow was also red on the same push, for an unrelated reason: GitHub Pages was not enabled on the repository, so actions/deploy-pages 404'd. Pages has been enabled (build_type: workflow) and that run is now green — no code change needed.

`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

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (c694d20) to head (9e28c3e).
⚠️ Report is 1 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@h0rn3t
h0rn3t merged commit 45ac6a3 into main Sep 6, 2026
13 checks passed
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