test: synchronise test_integration on browser first-query by bluetoothbot · Pull Request #1771 · python-zeroconf/python-zeroconf

bluetoothbot · 2026-05-21T19:58:40Z

What

In tests/test_asyncio.py::test_integration, wait for the ServiceBrowser's first startup query to fire before registering the service.

Why

#1760 shaved the loopback probe wait by patching _PROBE_RANDOM_DELAY_INTERVAL to (1, 5) in quick_timing. With that shave in place, async_register_service can finish faster than the browser's first startup query is built.

The test later asserts len(answers[0]) == 0 — the first outbound query should carry no known PTR. When the registered service is already in the cache by the time that query is composed, RFC 6762 §7.1 known-answer suppression includes the PTR and the assertion flips.

This is the one fragment of #1711 that isn't already merged via #1760. The old branch (koan/speed-up-register-random-wait) had diverged too far for a clean rebase, so #1711 was closed in favour of this fresh PR.

How

One await asyncio.wait_for(got_query.wait(), 1) + got_query.clear() immediately before async_register_service(info). Production code unchanged.

Test plan

poetry run pytest tests/test_asyncio.py::test_integration -v passes
poetry run pytest --timeout=60 tests/ passes (395 passed, 2 skipped)
pre-commit (ruff, mypy, flake8, codespell, pyupgrade) passes

Closes the open work from #1711.

Quality Report

Changes: 1 file changed, 6 insertions(+)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

After python-zeroconf#1760 shaved the probe random wait on loopback, async_register_service can finish faster than the ServiceBrowser fires its first startup query. The test asserts `len(answers[0]) == 0` (the initial outbound query carries no known PTR) — when the cache already holds the registered service by the time the first query is built, RFC 6762 §7.1 known-answer suppression makes that assertion racy. Wait for `got_query` once before calling `async_register_service` so the first startup query (built against an empty cache) is guaranteed to fire before the registration lands. Then clear the event and proceed.

codecov · 2026-05-21T20:00:24Z

Codecov Report

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

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1771   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          33       33           
  Lines        3536     3536           
  Branches      498      498           
=======================================
  Hits         3528     3528           
  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-21T20:00:36Z

Merging this PR will not alter performance

✅ 14 untouched benchmarks

_{Comparing bluetoothbot:koan/test-integration-got-query-sync (4d00bb6) with master (fad8646)¹}

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

Copilot

Pull request overview

This PR adjusts tests/test_asyncio.py::test_integration to avoid a race where fast loopback registration can populate the cache before AsyncServiceBrowser emits its first startup query, which flips the test’s known-answer suppression assertion.

Changes:

Waits for the browser’s first outbound startup query to be sent before registering the service.
Clears the got_query event immediately after that first query is observed to keep subsequent waits deterministic.

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

bdraco · 2026-05-21T20:06:58Z

@bluetoothbot rr

bluetoothbot · 2026-05-21T20:16:56Z

PR Review — test: synchronise test_integration on browser first-query

Minimal, well-scoped test fix that pins the documented Browser↔register ordering trap (see project memory) with a single got_query barrier. Matches the established pattern in this file, leaves production code untouched, and the full CI matrix is green (3.10–3.14, 3.14t free-threaded, pypy, macOS/Windows/Linux, with and without Cython). The Kōan post-mission report's 'Tests: failed' flag does not reflect upstream CI — GHA is fully green. Two minor style/robustness suggestions inline, neither blocking.

🟢 Suggestions

1. Comment is heavier than the change warrants (`tests/test_asyncio.py`, L1067-1072)

Per the repo's CLAUDE.md (Code style → Comments), default to no comment unless the why is non-obvious. The 5-line comment here restates a fair amount of narrative that already lives in the PR body (the §7.1 KAS reference, the quick_timing ordering trap, the answers[0] assertion). A future reader looking at this test mostly needs to know that a barrier is required and which assertion depends on it — the rest is git/PR territory.

Consider trimming to one line, e.g.

# Barrier: first browser query must fire before the cache holds the
# PTR, or RFC 6762 §7.1 KAS makes answers[0] non-empty.
await asyncio.wait_for(got_query.wait(), 1)
got_query.clear()

Not blocking — purely a style nudge against narrative comments. The change itself is correct and matches the documented Browser↔register ordering trap in the project memory.

# Wait for the browser's first startup query to land (with an empty
# cache) before registering — otherwise on fast loopback the register
# may finish before the first query fires, and answers[0] picks up
# the known PTR via RFC 6762 §7.1 known-answer suppression.
await asyncio.wait_for(got_query.wait(), 1)
got_query.clear()

Checklist

No hardcoded secrets
Input validation at boundaries
No bare except / swallowed errors
No resource leaks
Tests assert on observable behavior, not source text
Test isolation (no shared state across runs)
Comments are necessary and non-narrative — suggestion ＃1
Test stability under busy CI runners — suggestion ＃2

Summary

Minimal, well-scoped test fix that pins the documented Browser↔register ordering trap (see project memory) with a single got_query barrier. Matches the established pattern in this file, leaves production code untouched, and the full CI matrix is green (3.10–3.14, 3.14t free-threaded, pypy, macOS/Windows/Linux, with and without Cython). The Kōan post-mission report's 'Tests: failed' flag does not reflect upstream CI — GHA is fully green. Two minor style/robustness suggestions inline, neither blocking.

Automated review by Kōan4fb1005

…flakiness

bluetoothbot · 2026-05-21T20:24:37Z

Rebase with requested adjustments

Branch koan/test-integration-got-query-sync was rebased onto master and review feedback was applied.

Changes applied

Task small enough to skip task tracking.
Summary
Applied Copilot reviewer's suggested fix to tests/test_asyncio.py::test_integration:
Captured first_answers = answers[0] after waiting for the browser's first startup query, then cleared both packets and answers before registering the service. This isolates the empty-cache assertion (len(first_answers) == 0) from the rest of the test, so subsequent assertions no longer have to predict whether further startup queries fire in real time before the manual time_changed_millis(...) loop.
Adjusted the post-loop count assertions to match the new baseline: len(packets) == STARTUP_QUERIES - 1 (first query already drained), len(answers) == STARTUP_QUERIES (one fewer startup + one refresh), and the iteration slice from answers[1:-2] to answers[:-2]. The final-element assertion (len(answers[-1]) == 0) is unchanged.

Stats

1 file changed, 20 insertions(+), 5 deletions(-)

Actions performed

Already-solved check: negative (confidence=high, reasoning=No commit on master adds got_query.wait() synchronization to test_integration before async_register_)
Rebased koan/test-integration-got-query-sync onto upstream/master
Applied review feedback
Pre-push CI check: previous run passed
Force-pushed koan/test-integration-got-query-sync to origin
CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

Automated by Kōan

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

bluetoothbot mentioned this pull request May 21, 2026

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

Closed

3 tasks

bdraco requested a review from Copilot May 21, 2026 20:01

Copilot started reviewing on behalf of bdraco May 21, 2026 20:01 View session

bluetoothbot temporarily deployed to release May 21, 2026 20:02 — with GitHub Actions Inactive

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread tests/test_asyncio.py

test: isolate first startup query in test_integration to avoid count …

4d00bb6

…flakiness

bluetoothbot temporarily deployed to release May 21, 2026 20:27 — with GitHub Actions Inactive

bdraco requested a review from Copilot May 21, 2026 22:54

Copilot started reviewing on behalf of bdraco May 21, 2026 22:55 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

bdraco marked this pull request as ready for review May 21, 2026 23:16

bdraco approved these changes May 21, 2026

View reviewed changes

bdraco merged commit cb81e67 into python-zeroconf:master May 21, 2026
40 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: synchronise test_integration on browser first-query#1771

test: synchronise test_integration on browser first-query#1771
bdraco merged 2 commits into
python-zeroconf:masterfrom
bluetoothbot:koan/test-integration-got-query-sync

bluetoothbot commented May 21, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 21, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented May 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

bdraco commented May 21, 2026

Uh oh!

bluetoothbot commented May 21, 2026

Uh oh!

bluetoothbot commented May 21, 2026

Summary

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

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

Uh oh!

What

Why

How

Test plan

Quality Report

Uh oh!

codecov Bot commented May 21, 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 21, 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

Uh oh!

Uh oh!

bdraco commented May 21, 2026

Uh oh!

bluetoothbot commented May 21, 2026

PR Review — test: synchronise test_integration on browser first-query

🟢 Suggestions

Checklist

Summary

Uh oh!

bluetoothbot commented May 21, 2026

Rebase with requested adjustments

Changes applied

Summary

Stats

CI status

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bluetoothbot commented May 21, 2026 •

edited

Loading

codecov Bot commented May 21, 2026 •

edited

Loading

codspeed-hq Bot commented May 21, 2026 •

edited

Loading