test: shave loopback timing overhead from remaining slow tests by bluetoothbot · Pull Request #1760 · python-zeroconf/python-zeroconf

bluetoothbot · 2026-05-20T16:09:06Z

Summary

Issue #1707 listed 20 tests over 0.5s. Several of them weren't pinning RFC behavior — they were paying production-grade delays on a 127.0.0.1 fake network. This PR cuts the ones that can be safely shortened and leaves the RFC-pinned ones (response aggregation, duplicate-question suppression, TC-deferral, 4-retry budget) alone.

Fixes #1707

Changes

Extract the 150-250ms probe collision-avoidance delay in Zeroconf._async_check_service into a new module-level _PROBE_RANDOM_DELAY_INTERVAL constant (src/zeroconf/_core.py). The existing quick_timing fixture now patches it to (1, 5), dropping ~200ms per register_service call on loopback. Production behaviour unchanged.
Add quick_request_timing to:
- tests/services/test_info.py::TestServiceInfo::test_get_info_partial
- tests/test_services.py::ListenerTest::test_integration_with_listener_class
- tests/test_asyncio.py::test_service_info_async_request
  …so get_service_info calls no longer pay the 200ms _LISTENER_TIME + 20-120ms jitter on loopback.
Tighten the deliberate forced-timeout in test_service_info_async_request from 1500ms to 300ms — under quick_request_timing the first QM lands in <20ms, so 300ms is comfortable.
Patch _TASK_AWAIT_TIMEOUT to 50ms in test_shutdown_loop so the inner asyncio.wait on the deliberately never-completing _still_running() task returns quickly instead of blocking the default 1s.

Slow test before / after (local measurements, top of `--durations=20` for the issue's list)

Test	Before	After
`test_service_info_async_request`	2.19s	0.78s
`test_get_info_partial`	1.50s	1.05s
`test_integration_with_listener_class`	1.68s	1.16s
`test_shutdown_loop`	1.15s	0.15s
`test_get_info_single`	1.01s	0.03s
`test_async_zeroconf_service_types`	0.52s	0.29s
`test_async_service_registration_name_conflict`	0.68s	0.09s
`test_verify_name_change_with_lots_of_names`	0.80s	0.36s
`test_name_conflicts`	0.47s	0.09s
`test_integration_with_listener` (types)	1.28s	0.19s
`test_integration_with_subtype_and_listener`	1.27s	0.20s
`test_integration_with_listener_v6_records`	1.27s	0.20s
`test_asking_qu_questions`	0.52s	0.06s

Tests intentionally left at their original runtime because they pin RFC 6762 timing:

test_response_aggregation_timings / _multiple (§14)
test_we_try_four_times_with_random_delay (3000ms / 4 retries)
test_get_info_suppressed_by_question_history (§7.1, _DUPLICATE_QUESTION_INTERVAL)
test_tc_bit_defers_last_response_missing (§18.5, _TC_DELAY_RANDOM_INTERVAL)

Test plan

poetry run pytest --timeout=60 -q — 385 passed, 2 skipped, no regressions
poetry run ruff check + poetry run ruff format --check clean on touched files
Targeted re-run of the 20 tests from the issue shows the expected drops

Generated by Kōan /fix

Quality Report

Changes: 6 files changed, 32 insertions(+), 17 deletions(-)

Code scan: clean

Tests: passed (4 PASSED)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

codecov · 2026-05-20T16:11:16Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.77%. Comparing base (5444495) to head (c8e442d).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1760   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          33       33           
  Lines        3508     3509    +1     
  Branches      493      493           
=======================================
+ Hits         3500     3501    +1     
  Misses          5        5           
  Partials        3        3

☔ View full report in Codecov by Sentry.
📢 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.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codspeed-hq · 2026-05-20T16:11:17Z

Merging this PR will not alter performance

✅ 14 untouched benchmarks

_{Comparing bluetoothbot:koan/fix-issue-1707 (c8e442d) with master (5444495)¹}

No successful run was found on master (e95297f) during the generation of this report, so 5444495 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Reduces end-to-end runtime of several loopback-based tests by patching production-grade timing delays (probe/jitter/timeouts) down to test-friendly values while keeping RFC-timing tests unchanged.

Changes:

Extracted the probe collision-avoidance jitter into _PROBE_RANDOM_DELAY_INTERVAL and patched it in the quick_timing fixture.
Added quick_request_timing to selected tests and shortened an intentional timeout in test_service_info_async_request.
Reduced shutdown-loop waiting overhead by patching _TASK_AWAIT_TIMEOUT in a loop shutdown test.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/utils/test_asyncio.py	Speeds up shutdown-loop test by shrinking sleeps/timeouts and patching `_TASK_AWAIT_TIMEOUT`.
tests/test_services.py	Applies `quick_request_timing` fixture to a listener integration test.
tests/test_asyncio.py	Applies `quick_request_timing` and reduces a forced timeout in an async service-info request test.
tests/services/test_info.py	Applies `quick_request_timing` to a partial info lookup test.
tests/conftest.py	Updates `quick_timing` fixture to also patch the new probe-delay interval.
src/zeroconf/_core.py	Introduces `_PROBE_RANDOM_DELAY_INTERVAL` and uses it for pre-probe random wait.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bdraco · 2026-05-20T16:19:48Z

@bluetoothbot rr

bluetoothbot · 2026-05-20T17:37:51Z

PR Review — test: shave loopback timing overhead from remaining slow tests

Clean test-speedup PR. The _PROBE_RANDOM_DELAY_INTERVAL extraction is the right shape — keeps the production tuple in _core.py, lets quick_timing patch it, and the .pxd doesn't need to change because _core.py isn't in TO_CYTHONIZE. The quick_request_timing rollouts at test_get_info_partial, test_integration_with_listener_class, and test_service_info_async_request are well-justified, and the matching 1500ms→300ms timeout tightening in the TOCTOU race-condition block has a clear new comment explaining the new arithmetic. test_shutdown_loop rewrite is correct: moving runcoro_thread_ready.set() after run_coroutine_threadsafe and replacing the time.sleep(0.1) buffer with a patched _TASK_AWAIT_TIMEOUT is a sound trade. RFC-pinned tests are correctly left alone. Two suggestion-level notes inline; neither blocks merge.

🟢 Suggestions

1. Task-scheduling race in _run_coro thread (`tests/utils/test_asyncio.py`, L89-95)

Good fix moving runcoro_thread_ready.set() after run_coroutine_threadsafe(...) — that closes the worst of the race Copilot flagged. One residual concern: run_coroutine_threadsafe only queues call_soon_threadsafe(callback), where callback is what actually does ensure_future(coro, loop=loop). So when the worker thread calls .set(), the concurrent.futures.Future exists but the asyncio Task may not be on the loop yet. If the main thread races into shutdown_loop(loop) before the loop processes the call_soon callback, asyncio.all_tasks(loop) will be empty and the test silently passes a different code path than intended.

If you want to make this fully deterministic, have _still_running() set a second threading.Event at its first yield point and wait on that from the main thread:

task_started = threading.Event()

async def _still_running():
    task_started.set()
    await asyncio.sleep(5)

# main thread
runcoro_thread_ready.wait()
task_started.wait()
with patch.object(aioutils, "_TASK_AWAIT_TIMEOUT", 0.05):
    aioutils.shutdown_loop(loop)

Not a blocker — the original code had the same class of race buffered by time.sleep(0.1), and in practice call_soon_threadsafe wakes the loop fast enough that this almost never fires. Worth a follow-up if the test starts flaking on the 3.14t / PyPy matrix entries.

future = asyncio.run_coroutine_threadsafe(_still_running(), loop)
runcoro_thread_ready.set()

2. Comment can be tightened to one line (`src/zeroconf/_core.py`, L107-110)

Per CLAUDE.md (Code style / Comments): "Default to writing no comments. RFC citations are useful when the reason for a timing constant or framing decision is 'the spec says so' — leave those in." The RFC citation is the load-bearing bit here; the rest restates quick_timing's job. Consider collapsing to:

# RFC 6762 §8.1: random 0-250ms pre-probe delay against thundering herd.
_PROBE_RANDOM_DELAY_INTERVAL = (150, 250)  # ms

The (150, 250) value already speaks for itself — readers don't need a paragraph to know that 150 is a more conservative floor than the spec's 0, nor that test fixtures can patch.object a module constant. Not a blocker.

# RFC 6762 §8.1 thundering-herd avoidance: wait a random
# 0-250ms before the first probe so simultaneously-started
# responders don't collide. We default to 150-250ms to
# preserve existing timing assumptions; tests on loopback
# may patch this lower via the `quick_timing` fixture.
_PROBE_RANDOM_DELAY_INTERVAL = (150, 250)  # ms

Checklist

No hardcoded secrets or unsafe deserialization
Error paths handle cleanup correctly
No untested branches introduced
Tests verify behavior, not source-code presence
No race conditions in test setup — suggestion ＃1
Comment style matches CLAUDE.md (terse, why-only) — suggestion ＃2
Cython .pxd sync — not required (constant lives in non-Cythonized _core.py)
RFC-pinned timing tests untouched
Free-threaded (3.14t) safety — no new module-level mutable state

Summary

Clean test-speedup PR. The _PROBE_RANDOM_DELAY_INTERVAL extraction is the right shape — keeps the production tuple in _core.py, lets quick_timing patch it, and the .pxd doesn't need to change because _core.py isn't in TO_CYTHONIZE. The quick_request_timing rollouts at test_get_info_partial, test_integration_with_listener_class, and test_service_info_async_request are well-justified, and the matching 1500ms→300ms timeout tightening in the TOCTOU race-condition block has a clear new comment explaining the new arithmetic. test_shutdown_loop rewrite is correct: moving runcoro_thread_ready.set() after run_coroutine_threadsafe and replacing the time.sleep(0.1) buffer with a patched _TASK_AWAIT_TIMEOUT is a sound trade. RFC-pinned tests are correctly left alone. Two suggestion-level notes inline; neither blocks merge.

Automated review by Kōan038579d
a30eb88

bluetoothbot · 2026-05-20T17:39:53Z

Rebase with requested adjustments

Branch koan/fix-issue-1707 was rebased onto master and review feedback was applied.

Changes applied

Task small, no tracking needed.
Summary of changes addressing review comments:
src/zeroconf/_core.py — Rephrased _PROBE_RANDOM_DELAY_INTERVAL comment per Copilot: removed "enforces 150ms floor" language (code doesn't enforce, it defaults), now says "We default to 150-250ms to preserve existing timing assumptions; tests on loopback may patch this lower."
tests/conftest.py — Updated both quick_timing and quick_request_timing docstrings per Copilot to document both opt-in mechanisms (argument injection and @pytest.mark.usefixtures(...)).
tests/utils/test_asyncio.py — Fixed race per Copilot: moved runcoro_thread_ready.set() to after asyncio.run_coroutine_threadsafe(...) submits the future, removed the now-unneeded time.sleep(0.05) since ordering is guaranteed. Also reworded the _TASK_AWAIT_TIMEOUT patch comment from "returns immediately" to "returns within 50ms" to match the patched value.

Stats

6 files changed, 40 insertions(+), 21 deletions(-)

Actions performed

Already-solved check: negative (confidence=high, reasoning=No recent master commit shortens loopback test timings via _PROBE_RANDOM_DELAY_INTERVAL or quick_req)
Rebased koan/fix-issue-1707 onto upstream/master
Applied review feedback
Pre-push CI check: previous run passed
Force-pushed koan/fix-issue-1707 to origin
CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

Automated by Kōan

bdraco · 2026-05-20T18:58:29Z

@bluetoothbot rr

Issue python-zeroconf#1707 listed 20 tests over 0.5s. The fixes here cut the ones that aren't pinning RFC behavior: - Extract the 150-250ms probe collision-avoidance delay in Zeroconf._async_check_service to _PROBE_RANDOM_DELAY_INTERVAL so the existing quick_timing fixture can shrink it on loopback. Drops 150-250ms per register_service call. - Add quick_request_timing to test_get_info_partial, test_integration_with_listener_class, and test_service_info_async_request — get_service_info calls no longer pay the 200ms _LISTENER_TIME + 20-120ms jitter. - Tighten the deliberate forced-timeout in test_service_info_async_request from 1500ms to 300ms now that quick_request_timing brings the first QM into the window in <20ms. - Patch _TASK_AWAIT_TIMEOUT in test_shutdown_loop so the asyncio.wait on the never-completing _still_running() task returns at 50ms instead of the default 1s. Tests that pin RFC 6762 timing (response aggregation, duplicate-question suppression, TC-deferral, 4-retry budget) are intentionally left alone — their runtime is the assertion.

bluetoothbot · 2026-05-20T19:12:54Z

Rebase with requested adjustments

Branch koan/fix-issue-1707 was rebased onto master and review feedback was applied.

Stats

6 files changed, 40 insertions(+), 21 deletions(-)

Actions performed

Already-solved check: negative (confidence=high, reasoning=No recent master commit addresses loopback test timing overhead via _PROBE_RANDOM_DELAY_INTERVAL ext)
Rebased koan/fix-issue-1707 onto upstream/master
Pre-push CI check: previous run passed
Force-pushed koan/fix-issue-1707 to origin
CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

Automated by Kōan

bdraco requested a review from Copilot May 20, 2026 16:10

bluetoothbot temporarily deployed to release May 20, 2026 16:13 — with GitHub Actions Inactive

Copilot started reviewing on behalf of bdraco May 20, 2026 16:17 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

Comment thread tests/utils/test_asyncio.py Outdated

Comment thread tests/utils/test_asyncio.py Outdated

Comment thread src/zeroconf/_core.py Outdated

Comment thread tests/conftest.py Outdated

bdraco added traffic-reduction and removed traffic-reduction labels May 20, 2026

bluetoothbot force-pushed the koan/fix-issue-1707 branch from a958dcd to a30eb88 Compare May 20, 2026 17:39

bluetoothbot temporarily deployed to release May 20, 2026 17:44 — with GitHub Actions Inactive

bluetoothbot added 2 commits May 20, 2026 19:11

test: apply review feedback on shutdown-loop race and fixture docs

c8e442d

bluetoothbot force-pushed the koan/fix-issue-1707 branch from a30eb88 to c8e442d Compare May 20, 2026 19:12

bdraco marked this pull request as ready for review May 20, 2026 19:17

bdraco merged commit 343dc7a into python-zeroconf:master May 20, 2026
33 checks passed

bluetoothbot had a problem deploying to release May 20, 2026 19:21 — with GitHub Actions Failure

bluetoothbot deleted the koan/fix-issue-1707 branch May 20, 2026 19:28

This was referenced May 21, 2026

test: shave register/shutdown/TC waits on loopback #1711

Closed

test: synchronise test_integration on browser first-query #1771

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: shave loopback timing overhead from remaining slow tests#1760

test: shave loopback timing overhead from remaining slow tests#1760
bdraco merged 2 commits into
python-zeroconf:masterfrom
bluetoothbot:koan/fix-issue-1707

bluetoothbot commented May 20, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 20, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented May 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bdraco commented May 20, 2026

Uh oh!

bluetoothbot commented May 20, 2026 •

edited

Loading

Uh oh!

bluetoothbot commented May 20, 2026

Uh oh!

bdraco commented May 20, 2026

Uh oh!

bluetoothbot commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

bluetoothbot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Slow test before / after (local measurements, top of --durations=20 for the issue's list)

Test plan

Quality Report

Uh oh!

codecov Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bdraco commented May 20, 2026

Uh oh!

bluetoothbot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — test: shave loopback timing overhead from remaining slow tests

🟢 Suggestions

Checklist

Summary

Uh oh!

bluetoothbot commented May 20, 2026

Rebase with requested adjustments

Changes applied

Stats

CI status

Uh oh!

bdraco commented May 20, 2026

Uh oh!

bluetoothbot commented May 20, 2026

Rebase with requested adjustments

Stats

CI status

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bluetoothbot commented May 20, 2026 •

edited

Loading

Slow test before / after (local measurements, top of `--durations=20` for the issue's list)

codecov Bot commented May 20, 2026 •

edited

Loading

codspeed-hq Bot commented May 20, 2026 •

edited

Loading

bluetoothbot commented May 20, 2026 •

edited

Loading