From 5c45abe2b73eae1e556996f5d597c1e454d3c64f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 20 May 2026 10:39:21 -0500 Subject: [PATCH] refactor: extract loopback Zeroconf fixtures and mock_incoming_msg helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pattern `zc = Zeroconf(interfaces=["127.0.0.1"]); ...; zc.close()` appears 100+ times across the test suite, and the async variant roughly 30 times. Four files duplicate a byte-identical `mock_incoming_msg(records) -> DNSIncoming` helper that wraps records in a `DNSOutgoing(_FLAGS_QR_RESPONSE)` and parses the resulting wire bytes back through `DNSIncoming`. This PR: - Adds `mock_incoming_msg` to `tests/__init__.py`, replacing the four local copies in `tests/services/test_browser.py` and `tests/services/test_info.py`. - Adds `zc_loopback` (sync) and `aiozc_loopback` (async) fixtures to `tests/conftest.py` that yield a loopback-only Zeroconf instance and close it on teardown via try/finally. `close()` / `async_close()` remain idempotent, so tests are free to call them mid-test (e.g. to test post-close behavior) without conflicting with the fixture's cleanup. - Adopts `aiozc_loopback` in `tests/test_engine.py` to demonstrate the fixture's use — converts the two tests that previously had explicit try/finally + async_close blocks. The async fixture uses `@pytest_asyncio.fixture` so it works under pytest-asyncio's strict mode (the project default). Replaces and closes #1757. --- tests/__init__.py | 11 ++++++++- tests/conftest.py | 36 ++++++++++++++++++++++++++-- tests/services/test_browser.py | 9 +------ tests/services/test_info.py | 25 +------------------ tests/test_engine.py | 44 ++++++++++++++-------------------- 5 files changed, 64 insertions(+), 61 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index d2af5889..d68444d0 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -26,12 +26,13 @@ import platform import socket import time +from collections.abc import Iterable from functools import cache from unittest import mock import ifaddr -from zeroconf import DNSIncoming, DNSQuestion, DNSRecord, Zeroconf +from zeroconf import DNSIncoming, DNSOutgoing, DNSQuestion, DNSRecord, Zeroconf, const from zeroconf._history import QuestionHistory _MONOTONIC_RESOLUTION = time.get_clock_info("monotonic").resolution @@ -70,6 +71,14 @@ def suppresses(self, question: DNSQuestion, now: float, known_answers: set[DNSRe return False +def mock_incoming_msg(records: Iterable[DNSRecord]) -> DNSIncoming: + """Build a `DNSIncoming` response message from a list of `DNSRecord`s.""" + generated = DNSOutgoing(const._FLAGS_QR_RESPONSE) + for record in records: + generated.add_answer_at_time(record, 0) + return DNSIncoming(generated.packets()[0]) + + def _inject_responses(zc: Zeroconf, msgs: list[DNSIncoming]) -> None: """Inject a DNSIncoming response.""" assert zc.loop is not None diff --git a/tests/conftest.py b/tests/conftest.py index 31c2a17b..c281e2d4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,15 +3,17 @@ from __future__ import annotations import threading -from collections.abc import Generator +from collections.abc import AsyncGenerator, Generator from unittest.mock import patch import pytest +import pytest_asyncio -from zeroconf import _core, const +from zeroconf import Zeroconf, _core, const from zeroconf._handlers import query_handler from zeroconf._services import browser as service_browser from zeroconf._services import info as service_info +from zeroconf.asyncio import AsyncZeroconf @pytest.fixture(autouse=True) @@ -23,6 +25,36 @@ def verify_threads_ended(): assert not threads +@pytest.fixture +def zc_loopback() -> Generator[Zeroconf]: + """Yield a loopback `Zeroconf` and close it on teardown. + + Replaces the inline `zc = Zeroconf(interfaces=["127.0.0.1"])` + + explicit `zc.close()` pattern duplicated across the suite. Calling + `zc.close()` inside a test is still safe — `close()` is idempotent. + """ + zc = Zeroconf(interfaces=["127.0.0.1"]) + try: + yield zc + finally: + zc.close() + + +@pytest_asyncio.fixture +async def aiozc_loopback() -> AsyncGenerator[AsyncZeroconf]: + """Yield a loopback `AsyncZeroconf` and close it on teardown. + + Replaces the inline `aiozc = AsyncZeroconf(interfaces=["127.0.0.1"])` + + explicit `await aiozc.async_close()` pattern duplicated across the + suite. Calling `async_close()` inside a test is still safe. + """ + aiozc = AsyncZeroconf(interfaces=["127.0.0.1"]) + try: + yield aiozc + finally: + await aiozc.async_close() + + @pytest.fixture def run_isolated(): """Change the mDNS port to run the test in isolation.""" diff --git a/tests/services/test_browser.py b/tests/services/test_browser.py index 32c122a4..342b2fc1 100644 --- a/tests/services/test_browser.py +++ b/tests/services/test_browser.py @@ -8,7 +8,6 @@ import socket import time import unittest -from collections.abc import Iterable from threading import Event from typing import cast from unittest.mock import patch @@ -36,6 +35,7 @@ _inject_response, _wait_for_start, has_working_ipv6, + mock_incoming_msg, time_changed_millis, ) @@ -54,13 +54,6 @@ def teardown_module(): log.setLevel(original_logging_level) -def mock_incoming_msg(records: Iterable[r.DNSRecord]) -> r.DNSIncoming: - generated = r.DNSOutgoing(const._FLAGS_QR_RESPONSE) - for record in records: - generated.add_answer_at_time(record, 0) - return r.DNSIncoming(generated.packets()[0]) - - def test_service_browser_cancel_multiple_times(): """Test we can cancel a ServiceBrowser multiple times before close.""" diff --git a/tests/services/test_info.py b/tests/services/test_info.py index dae42afd..56146852 100644 --- a/tests/services/test_info.py +++ b/tests/services/test_info.py @@ -8,7 +8,6 @@ import socket import threading import unittest -from collections.abc import Iterable from ipaddress import ip_address from threading import Event from unittest.mock import patch @@ -23,7 +22,7 @@ from zeroconf._utils.net import IPVersion from zeroconf.asyncio import AsyncZeroconf -from .. import QUICK_REQUEST_TIMEOUT_MS, _inject_response, has_working_ipv6 +from .. import QUICK_REQUEST_TIMEOUT_MS, _inject_response, has_working_ipv6, mock_incoming_msg log = logging.getLogger("zeroconf") original_logging_level = logging.NOTSET @@ -279,14 +278,6 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT, v6_flow_scope=()): # patch the zeroconf send with patch.object(zc, "async_send", send): - def mock_incoming_msg(records: Iterable[r.DNSRecord]) -> r.DNSIncoming: - generated = r.DNSOutgoing(const._FLAGS_QR_RESPONSE) - - for record in records: - generated.add_answer_at_time(record, 0) - - return r.DNSIncoming(generated.packets()[0]) - def get_service_info_helper(zc, type, name): nonlocal service_info service_info = zc.get_service_info(type, name) @@ -422,14 +413,6 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT, v6_flow_scope=()): # patch the zeroconf send with patch.object(zc, "async_send", send): - def mock_incoming_msg(records: Iterable[r.DNSRecord]) -> r.DNSIncoming: - generated = r.DNSOutgoing(const._FLAGS_QR_RESPONSE) - - for record in records: - generated.add_answer_at_time(record, 0) - - return r.DNSIncoming(generated.packets()[0]) - def get_service_info_helper(zc, type, name, timeout): nonlocal service_info service_info = zc.get_service_info(type, name, timeout) @@ -552,12 +535,6 @@ def test_get_info_single(self): ), ] - def mock_incoming_msg(records: Iterable[r.DNSRecord]) -> r.DNSIncoming: - generated = r.DNSOutgoing(const._FLAGS_QR_RESPONSE) - for record in records: - generated.add_answer_at_time(record, 0) - return r.DNSIncoming(generated.packets()[0]) - sent_queries: list[r.DNSOutgoing] = [] def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT, v6_flow_scope=()): diff --git a/tests/test_engine.py b/tests/test_engine.py index 750d3393..66cfa356 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -74,39 +74,31 @@ async def test_reaper(): @pytest.mark.asyncio -async def test_setup_releases_socket_ownership() -> None: +async def test_setup_releases_socket_ownership(aiozc_loopback: AsyncZeroconf) -> None: """Engine releases its pending-socket refs once each socket has a transport.""" - aiozc = AsyncZeroconf(interfaces=["127.0.0.1"]) - try: - await aiozc.zeroconf.async_wait_for_start() - engine = aiozc.zeroconf.engine - assert engine._listen_socket is None - assert engine._respond_sockets == [] - assert engine.readers - assert engine.senders - finally: - await aiozc.async_close() + await aiozc_loopback.zeroconf.async_wait_for_start() + engine = aiozc_loopback.zeroconf.engine + assert engine._listen_socket is None + assert engine._respond_sockets == [] + assert engine.readers + assert engine.senders @pytest.mark.asyncio -async def test_async_close_propagates_outer_cancellation() -> None: +async def test_async_close_propagates_outer_cancellation(aiozc_loopback: AsyncZeroconf) -> None: """Outer-task cancellation while awaiting setup propagates to the caller.""" - aiozc = AsyncZeroconf(interfaces=["127.0.0.1"]) + await aiozc_loopback.zeroconf.async_wait_for_start() + engine = aiozc_loopback.zeroconf.engine + loop = asyncio.get_running_loop() + original_task = engine._setup_task + fake_task = loop.create_future() + fake_task.set_exception(asyncio.CancelledError()) + engine._setup_task = fake_task # type: ignore[assignment] try: - await aiozc.zeroconf.async_wait_for_start() - engine = aiozc.zeroconf.engine - loop = asyncio.get_running_loop() - original_task = engine._setup_task - fake_task = loop.create_future() - fake_task.set_exception(asyncio.CancelledError()) - engine._setup_task = fake_task # type: ignore[assignment] - try: - with pytest.raises(asyncio.CancelledError): - await engine._async_close() - finally: - engine._setup_task = original_task + with pytest.raises(asyncio.CancelledError): + await engine._async_close() finally: - await aiozc.async_close() + engine._setup_task = original_task @pytest.mark.asyncio