From e3e89ba3031bdcef0921e26bb833a9b4dfb250fc Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:06:58 -0400 Subject: [PATCH 01/16] test: parameterize dump CLI error mappings --- tests/test_scripts_aiopnsense_dump.py | 145 +++++++++----------------- 1 file changed, 48 insertions(+), 97 deletions(-) diff --git a/tests/test_scripts_aiopnsense_dump.py b/tests/test_scripts_aiopnsense_dump.py index c160574..728f46c 100644 --- a/tests/test_scripts_aiopnsense_dump.py +++ b/tests/test_scripts_aiopnsense_dump.py @@ -2,15 +2,16 @@ from __future__ import annotations +import argparse import asyncio +from collections.abc import Callable import importlib.util import json import logging from pathlib import Path -from types import ModuleType -import argparse import sys -from typing import Any +from types import ModuleType +from typing import Any, Self from unittest.mock import AsyncMock, MagicMock import pytest @@ -96,10 +97,10 @@ def __init__(self) -> None: self.enter = AsyncMock(return_value=self) self.exit = AsyncMock(return_value=None) - async def __aenter__(self) -> "FakeClientSession": + async def __aenter__(self) -> Self: return await self.enter() - async def __aexit__(self, *_args: Any) -> None: + async def __aexit__(self, *_args: object) -> None: await self.exit() @@ -351,7 +352,7 @@ def __init__(self) -> None: self.closed = False self.opened = False - def __aiter__(self) -> "StalledStream": + def __aiter__(self) -> StalledStream: """Return the stream iterator. Returns: @@ -924,106 +925,56 @@ async def raise_config_error() -> None: assert "bad config" in str(excinfo.value) -def test_main_turns_opnsense_error_into_system_exit( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """``main`` converts aiopnsense live failures into concise ``SystemExit``. - - Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise OPNsense failure. - """ - module = load_dump_module() - - async def raise_opnsense_error() -> None: - raise module.OPNsenseError("connection failed") - - monkeypatch.setattr(module, "async_main", raise_opnsense_error) - - with pytest.raises(SystemExit) as excinfo: - module.main() - - assert "OPNsenseError: connection failed" in str(excinfo.value) - - -def test_main_turns_aiohttp_client_error_into_system_exit( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """``main`` converts aiohttp live failures into concise ``SystemExit``. - - Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise transport failure. - """ - module = load_dump_module() - - async def raise_client_error() -> None: - raise module.aiohttp.ClientConnectionError("connection failed") - - monkeypatch.setattr(module, "async_main", raise_client_error) - - with pytest.raises(SystemExit) as excinfo: - module.main() - - assert "ClientConnectionError: connection failed" in str(excinfo.value) - - -def test_main_turns_runtime_error_into_system_exit( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """``main`` converts runtime failures into concise ``SystemExit``. - - Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise runtime failure. - """ - module = load_dump_module() - - async def raise_runtime_error() -> None: - raise RuntimeError("runtime validation failed") - - monkeypatch.setattr(module, "async_main", raise_runtime_error) - - with pytest.raises(SystemExit) as excinfo: - module.main() - - assert "RuntimeError: runtime validation failed" in str(excinfo.value) - - -def test_main_turns_timeout_error_into_system_exit( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """``main`` converts timeout failures into concise ``SystemExit``. - - Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise timeout failure. - """ - module = load_dump_module() - - async def raise_timeout_error() -> None: - raise TimeoutError("request timed out") - - monkeypatch.setattr(module, "async_main", raise_timeout_error) - - with pytest.raises(SystemExit) as excinfo: - module.main() - - assert "TimeoutError: request timed out" in str(excinfo.value) - - -def test_main_converts_os_error_to_system_exit( +@pytest.mark.parametrize( + ("error_factory", "expected_error"), + [ + pytest.param( + lambda module: module.OPNsenseError("connection failed"), + "OPNsenseError: connection failed", + id="opnsense-error", + ), + pytest.param( + lambda module: module.aiohttp.ClientConnectionError("connection failed"), + "ClientConnectionError: connection failed", + id="aiohttp-client-error", + ), + pytest.param( + lambda _module: RuntimeError("runtime validation failed"), + "RuntimeError: runtime validation failed", + id="runtime-error", + ), + pytest.param( + lambda _module: TimeoutError("request timed out"), + "TimeoutError: request timed out", + id="timeout-error", + ), + pytest.param( + lambda _module: OSError("output is unavailable"), + "OSError: output is unavailable", + id="os-error", + ), + ], +) +def test_main_turns_errors_into_system_exit( monkeypatch: pytest.MonkeyPatch, + error_factory: Callable[[ModuleType], Exception], + expected_error: str, ) -> None: - """``main`` converts I/O write failures into concise ``SystemExit``. + """``main`` converts expected live failures into concise ``SystemExit``. Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise I/O failure. + monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` fail. + error_factory (Callable[[ModuleType], Exception]): Factory for the expected failure. + expected_error (str): Error text expected in the resulting ``SystemExit``. """ module = load_dump_module() - async def raise_os_error() -> None: - raise OSError("output is unavailable") + async def raise_error() -> None: + raise error_factory(module) - monkeypatch.setattr(module, "async_main", raise_os_error) + monkeypatch.setattr(module, "async_main", raise_error) with pytest.raises(SystemExit) as excinfo: module.main() - assert "OSError: output is unavailable" in str(excinfo.value) + assert expected_error in str(excinfo.value) From ce9e0777c1f517ecb4130b08ff61559e42f8de20 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:09:53 -0400 Subject: [PATCH 02/16] test: simplify traffic probe fallbacks --- tests/test_traffic.py | 111 +++++++++++------------------------------- 1 file changed, 28 insertions(+), 83 deletions(-) diff --git a/tests/test_traffic.py b/tests/test_traffic.py index 39c459c..0a1493e 100644 --- a/tests/test_traffic.py +++ b/tests/test_traffic.py @@ -9,13 +9,13 @@ import aiohttp import pytest -from aiopnsense import OPNsenseClient, OPNsenseError, OPNsenseTimeoutError -from tests.conftest import FakeStreamResponseFactory, MakeClientFactory +from aiopnsense import OPNsenseError, OPNsenseTimeoutError from aiopnsense.traffic import ( DIAGNOSTICS_TRAFFIC_ENDPOINT, DIAGNOSTICS_TRAFFIC_STREAM_ENDPOINT_PREFIX, normalize_traffic_payload, ) +from tests.conftest import FakeStreamResponseFactory, MakeClientFactory def test_diagnostics_traffic_stream_endpoint_prefix() -> None: @@ -152,9 +152,6 @@ async def test_get_interface_traffic_probes_and_normalizes( """ client = make_client() try: - assert isinstance(client, OPNsenseClient) - assert hasattr(client, "get_interface_traffic") - client._is_get_endpoint_available = AsyncMock(return_value=True) client._safe_dict_get = AsyncMock( return_value={ @@ -213,8 +210,6 @@ async def test_get_interface_traffic_handles_unavailable_endpoint( """ client = make_client() try: - assert isinstance(client, OPNsenseClient) - client._is_get_endpoint_available = AsyncMock(return_value=False) client._safe_dict_get = AsyncMock() @@ -226,44 +221,27 @@ async def test_get_interface_traffic_handles_unavailable_endpoint( await client.async_close() +@pytest.mark.parametrize( + "probe_error", + [ + pytest.param(TimeoutError("probe timeout"), id="timeout"), + pytest.param(OPNsenseError("probe failed"), id="opnsense-error"), + ], +) @pytest.mark.asyncio -async def test_get_interface_traffic_returns_empty_sample_on_probe_timeout( - make_client: Callable[..., Any], -) -> None: - """Probe timeout should return an empty sample when throw_errors is disabled. - - Args: - make_client (Callable[..., Any]): Client factory whose endpoint probe times out. - """ - client = make_client() - try: - assert isinstance(client, OPNsenseClient) - - client._is_get_endpoint_available = AsyncMock(side_effect=TimeoutError("probe timeout")) - client._safe_dict_get = AsyncMock() - - traffic = await client.get_interface_traffic() - assert traffic == {"time": None, "interfaces": {}} - client._is_get_endpoint_available.assert_awaited_once_with(DIAGNOSTICS_TRAFFIC_ENDPOINT) - client._safe_dict_get.assert_not_awaited() - finally: - await client.async_close() - - -@pytest.mark.asyncio -async def test_get_interface_traffic_returns_empty_sample_on_probed_opnsense_error( +async def test_get_interface_traffic_returns_empty_sample_on_probe_error( make_client: MakeClientFactory, + probe_error: Exception, ) -> None: - """Mapped OPNsense errors from endpoint probing should fallback when throw_errors is disabled. + """Probe errors should return an empty sample when throw_errors is disabled. Args: - make_client (MakeClientFactory): Client factory whose probe raises ``OPNsenseError``. + make_client (MakeClientFactory): Client factory whose endpoint probe fails. + probe_error (Exception): Error raised by the endpoint probe. """ client = make_client() try: - assert isinstance(client, OPNsenseClient) - - client._is_get_endpoint_available = AsyncMock(side_effect=OPNsenseError("probe failed")) + client._is_get_endpoint_available = AsyncMock(side_effect=probe_error) client._safe_dict_get = AsyncMock() traffic = await client.get_interface_traffic() @@ -285,8 +263,6 @@ async def test_get_interface_traffic_raises_when_throw_errors_is_enabled( """ client = make_client(throw_errors=True) try: - assert isinstance(client, OPNsenseClient) - client._is_get_endpoint_available = AsyncMock(side_effect=TimeoutError("probe timeout")) with pytest.raises(OPNsenseTimeoutError): @@ -306,8 +282,6 @@ async def test_get_interface_traffic_raises_same_opnsense_error_when_throw_error """ client = make_client(throw_errors=True) try: - assert isinstance(client, OPNsenseClient) - probe_error = OPNsenseError("probe failed") client._is_get_endpoint_available = AsyncMock(side_effect=probe_error) @@ -399,7 +373,6 @@ async def fake_stream(_path: str, **_: Any) -> AsyncIterator[dict[str, Any]]: client = make_client() try: - assert isinstance(client, OPNsenseClient) client._is_get_endpoint_available = AsyncMock(return_value=True) client._stream_json_events = cast(Any, fake_stream) @@ -454,7 +427,6 @@ async def test_stream_interface_traffic_real_stream_path_keeps_non_ascii_name( ) client = make_client(session=session) try: - assert isinstance(client, OPNsenseClient) client._is_get_endpoint_available = AsyncMock(return_value=True) samples = [sample async for sample in client.stream_interface_traffic(poll_interval=1)] @@ -497,7 +469,6 @@ async def fake_stream(_path: str, **_: Any) -> AsyncIterator[dict[str, Any]]: client = make_client() try: - assert isinstance(client, OPNsenseClient) client._is_get_endpoint_available = AsyncMock(return_value=True) client._stream_json_events = cast(Any, fake_stream) @@ -534,7 +505,6 @@ async def fake_stream(_path: str, **_: Any) -> AsyncIterator[dict[str, Any]]: client = make_client() try: - assert isinstance(client, OPNsenseClient) client._is_get_endpoint_available = AsyncMock(return_value=True) client._stream_json_events = cast(Any, fake_stream) @@ -586,7 +556,6 @@ async def fake_stream(_path: str, **_: Any) -> AsyncIterator[dict[str, Any]]: client = make_client() try: - assert isinstance(client, OPNsenseClient) client._is_get_endpoint_available = AsyncMock(return_value=True) client._stream_json_events = cast(Any, fake_stream) @@ -734,7 +703,6 @@ async def test_stream_interface_traffic_resets_interval_after_stream_json_reset( ) client = make_client(session=session) try: - assert isinstance(client, OPNsenseClient) client._is_get_endpoint_available = AsyncMock(return_value=True) samples = [sample async for sample in client.stream_interface_traffic(poll_interval=1)] @@ -759,8 +727,6 @@ async def test_stream_interface_traffic_returns_when_endpoint_unavailable( """ client = make_client() try: - assert isinstance(client, OPNsenseClient) - client._is_get_endpoint_available = AsyncMock(return_value=False) client._stream_json_events = AsyncMock() @@ -772,44 +738,27 @@ async def test_stream_interface_traffic_returns_when_endpoint_unavailable( await client.async_close() +@pytest.mark.parametrize( + "probe_error", + [ + pytest.param(TimeoutError("probe timeout"), id="timeout"), + pytest.param(OPNsenseError("probe failed"), id="opnsense-error"), + ], +) @pytest.mark.asyncio -async def test_stream_interface_traffic_returns_empty_iteration_on_probe_timeout( - make_client: Callable[..., Any], -) -> None: - """Probe timeout should return an empty stream when throw_errors is disabled. - - Args: - make_client (Callable[..., Any]): Client factory whose stream probe times out. - """ - client = make_client() - try: - assert isinstance(client, OPNsenseClient) - - client._is_get_endpoint_available = AsyncMock(side_effect=TimeoutError("probe timeout")) - client._stream_json_events = AsyncMock() - - samples = [sample async for sample in client.stream_interface_traffic(poll_interval=1)] - - assert samples == [] - client._stream_json_events.assert_not_called() - finally: - await client.async_close() - - -@pytest.mark.asyncio -async def test_stream_interface_traffic_returns_empty_iteration_on_opnsense_error( +async def test_stream_interface_traffic_returns_empty_iteration_on_probe_error( make_client: MakeClientFactory, + probe_error: Exception, ) -> None: - """Mapped OPNsense errors from endpoint probing should fallback when throw_errors is disabled. + """Probe errors should return an empty stream when throw_errors is disabled. Args: - make_client (MakeClientFactory): Client factory whose stream probe raises ``OPNsenseError``. + make_client (MakeClientFactory): Client factory whose stream probe fails. + probe_error (Exception): Error raised by the endpoint probe. """ client = make_client() try: - assert isinstance(client, OPNsenseClient) - - client._is_get_endpoint_available = AsyncMock(side_effect=OPNsenseError("probe failed")) + client._is_get_endpoint_available = AsyncMock(side_effect=probe_error) client._stream_json_events = AsyncMock() samples = [sample async for sample in client.stream_interface_traffic(poll_interval=1)] @@ -831,8 +780,6 @@ async def test_stream_interface_traffic_raises_same_opnsense_error_when_throw_er """ client = make_client(throw_errors=True) try: - assert isinstance(client, OPNsenseClient) - probe_error = OPNsenseError("probe failed") client._is_get_endpoint_available = AsyncMock(side_effect=probe_error) @@ -854,8 +801,6 @@ async def test_stream_interface_traffic_raises_when_probe_timeout_and_throw_erro """ client = make_client(throw_errors=True) try: - assert isinstance(client, OPNsenseClient) - client._is_get_endpoint_available = AsyncMock(side_effect=TimeoutError("probe timeout")) with pytest.raises(OPNsenseTimeoutError): From 25ce08352713973de2c017bd0ed488262f4a35f0 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:12:05 -0400 Subject: [PATCH 03/16] test: remove queue caller-frame coupling --- tests/test_client_queue.py | 67 ++++++++------------------------------ 1 file changed, 13 insertions(+), 54 deletions(-) diff --git a/tests/test_client_queue.py b/tests/test_client_queue.py index c67baba..a159c39 100644 --- a/tests/test_client_queue.py +++ b/tests/test_client_queue.py @@ -8,15 +8,8 @@ import pytest -from aiopnsense import ( - OPNsenseError, - client_queue as aiopnsense_client_queue, -) -from tests.conftest import ( - FakeStreamResponseFactory, - MakeClientFactory, - make_mock_session_client, -) +from aiopnsense import OPNsenseError, client_queue as aiopnsense_client_queue +from tests.conftest import FakeStreamResponseFactory, MakeClientFactory, make_mock_session_client @pytest.mark.asyncio @@ -231,7 +224,7 @@ async def test_get_enqueues_and_processes(returned: Any, make_client: MakeClient make_client (MakeClientFactory): Fixture factory returning ``OPNsenseClient`` instances. Returns: - None: This test asserts queue integration and caller propagation for ``_get``. + None: This test asserts queue integration for ``_get``. """ client, _session = make_mock_session_client(make_client) task: asyncio.Task | None = None @@ -240,23 +233,7 @@ async def test_get_enqueues_and_processes(returned: Any, make_client: MakeClient q: asyncio.Queue = asyncio.Queue() client._request_queue = q - called = {} - - async def fake_do_get(path: Any, caller: str = "x") -> Any: - # capture the caller name supplied by _get - """Fake do get. - - Args: - path (Any): API endpoint path to request. - caller (str): Caller name used for diagnostics and logging. - - Returns: - Any: Mock value returned to support test behavior. - """ - called["caller"] = caller - return returned - - client._do_get = AsyncMock(side_effect=fake_do_get) + client._do_get = AsyncMock(return_value=returned) # start the real processor task task = asyncio.get_running_loop().create_task(client._process_queue()) @@ -265,8 +242,8 @@ async def fake_do_get(path: Any, caller: str = "x") -> Any: res = await client._get("/testpath") assert res == returned - # Lock the direct-call caller label derived from the fixed frame depth. - assert called.get("caller") == "test_get_enqueues_and_processes" + client._do_get.assert_awaited_once() + assert client._do_get.await_args.args[0] == "/testpath" finally: if task is not None and not task.done(): task.cancel() @@ -310,14 +287,14 @@ async def test_get_text_rejects_unexpected_type(make_client: MakeClientFactory) async def test_get_uses_unknown_when_caller_frame_is_unavailable( monkeypatch: pytest.MonkeyPatch, make_client: MakeClientFactory, - error_type: type[ValueError] | type[AttributeError], + error_type: type[ValueError | AttributeError], ) -> None: """Verify ``_get`` uses ``Unknown`` caller when frame lookup fails. Args: monkeypatch (pytest.MonkeyPatch): Fixture for patching frame lookup. make_client (MakeClientFactory): Fixture factory returning ``OPNsenseClient`` instances. - error_type (type[ValueError] | type[AttributeError]): Frame lookup error to raise. + error_type (type[ValueError | AttributeError]): Frame lookup error to raise. Returns: None: This test asserts caller fallback behavior. @@ -397,24 +374,7 @@ async def test_post_enqueues_and_processes(returned: Any, make_client: MakeClien q: asyncio.Queue = asyncio.Queue() client._request_queue = q - captured: dict[str, Any] = {} - - async def fake_do_post(path: Any, payload: Any = None, caller: str = "x") -> Any: - """Fake do post. - - Args: - path (Any): API endpoint path to request. - payload (Any): Request payload sent to the API endpoint. - caller (str): Caller name used for diagnostics and logging. - - Returns: - Any: Mock value returned to support test behavior. - """ - captured["caller"] = caller - captured["payload"] = payload - return returned - - client._do_post = AsyncMock(side_effect=fake_do_post) + client._do_post = AsyncMock(return_value=returned) task = asyncio.get_running_loop().create_task(client._process_queue()) @@ -422,9 +382,8 @@ async def fake_do_post(path: Any, payload: Any = None, caller: str = "x") -> Any res = await client._post("/postpath", payload=payload) assert res == returned - assert captured.get("payload") == payload - # Lock the direct-call caller label derived from the fixed frame depth. - assert captured.get("caller") == "test_post_enqueues_and_processes" + client._do_post.assert_awaited_once() + assert client._do_post.await_args.args[:2] == ("/postpath", payload) finally: if task is not None and not task.done(): task.cancel() @@ -438,14 +397,14 @@ async def fake_do_post(path: Any, payload: Any = None, caller: str = "x") -> Any async def test_post_uses_unknown_when_caller_frame_is_unavailable( monkeypatch: pytest.MonkeyPatch, make_client: MakeClientFactory, - error_type: type[ValueError] | type[AttributeError], + error_type: type[ValueError | AttributeError], ) -> None: """Verify ``_post`` uses ``Unknown`` caller when frame lookup fails. Args: monkeypatch (pytest.MonkeyPatch): Fixture for patching frame lookup. make_client (MakeClientFactory): Fixture factory returning ``OPNsenseClient`` instances. - error_type (type[ValueError] | type[AttributeError]): Frame lookup error to raise. + error_type (type[ValueError | AttributeError]): Frame lookup error to raise. Returns: None: This test asserts caller fallback behavior for ``_post``. From 7c96a9e951a58098e61d1c6d11f70e41514957e9 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:13:47 -0400 Subject: [PATCH 04/16] test: parameterize public exception checks --- tests/test_exceptions.py | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 0bf940e..b6cd095 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -3,31 +3,39 @@ from unittest.mock import MagicMock import aiohttp -import pytest from aiohttp.client_reqrep import ConnectionKey +import pytest import aiopnsense as aiopnsense_module from aiopnsense.exceptions import _map_opnsense_exception, _opnsense_http_error -def test_voucher_server_error() -> None: - """Raise OPNsenseVoucherServerError to ensure the exception class exists. - - Raises: - aiopnsense_module.OPNsenseVoucherServerError: Raised to verify the public exception exists. - """ - with pytest.raises(aiopnsense_module.OPNsenseVoucherServerError): - raise aiopnsense_module.OPNsenseVoucherServerError - +@pytest.mark.parametrize( + "exception_type", + [ + pytest.param( + aiopnsense_module.OPNsenseVoucherServerError, + id="voucher-server-error", + ), + pytest.param( + aiopnsense_module.OPNsenseMissingDeviceUniqueID, + id="missing-device-unique-id", + ), + ], +) +def test_public_specific_exceptions_can_be_raised_and_caught( + exception_type: type[aiopnsense_module.OPNsenseError], +) -> None: + """Raise and catch each specific exception exported by the public API. -def test_missing_device_unique_id_error() -> None: - """Raise OPNsenseMissingDeviceUniqueID to ensure the exception class exists. + Args: + exception_type (type[aiopnsense_module.OPNsenseError]): Public exception class to verify. Raises: - aiopnsense_module.OPNsenseMissingDeviceUniqueID: Raised to verify the public exception exists. + exception_type: Raised to verify the public exception class. """ - with pytest.raises(aiopnsense_module.OPNsenseMissingDeviceUniqueID): - raise aiopnsense_module.OPNsenseMissingDeviceUniqueID + with pytest.raises(exception_type): + raise exception_type def test_invalid_argument_is_public_opnsense_type_error() -> None: From 531b1d8f37997f75e3463cf0be555c243e9553f4 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:15:28 -0400 Subject: [PATCH 05/16] test: parameterize constructor option validation --- tests/test_client_base.py | 53 +++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/tests/test_client_base.py b/tests/test_client_base.py index f0eec22..c687bee 100644 --- a/tests/test_client_base.py +++ b/tests/test_client_base.py @@ -7,14 +7,8 @@ import aiohttp import pytest -from aiopnsense import ( - OPNsenseClient, - client as aiopnsense_client, -) -from aiopnsense.const import ( - OPNSENSE_LTD_FIRMWARE, - OPNSENSE_MIN_FIRMWARE, -) +from aiopnsense import OPNsenseClient, client as aiopnsense_client +from aiopnsense.const import OPNSENSE_LTD_FIRMWARE, OPNSENSE_MIN_FIRMWARE from aiopnsense.exceptions import ( OPNsenseBelowMinFirmware, OPNsenseConnectionError, @@ -457,29 +451,38 @@ async def test_client_constructor_throw_errors_configuration( @pytest.mark.asyncio -async def test_client_constructor_invalid_throw_errors_raises_type_error( +@pytest.mark.parametrize( + ("kwargs", "error_message"), + [ + pytest.param( + {"throw_errors": "false"}, + "`throw_errors` must be a bool", + id="throw-errors", + ), + pytest.param( + {"initial": "false"}, + "`initial` must be a bool", + id="legacy-initial", + ), + ], +) +async def test_client_constructor_invalid_boolean_option_raises_type_error( + kwargs: dict[str, str], + error_message: str, make_client: MakeClientFactory, ) -> None: - """Verify invalid ``throw_errors`` values raise ``TypeError``. + """Verify invalid constructor boolean options raise ``TypeError``. Args: - make_client (MakeClientFactory): Fixture factory used to pass an invalid constructor option. - """ - with pytest.raises(TypeError, match="`throw_errors` must be a bool"): - make_client(throw_errors="false") + kwargs (dict[str, str]): Invalid constructor option and value. + error_message (str): Exact validation error expected for the option. + make_client (MakeClientFactory): Fixture factory used to exercise option validation. - -@pytest.mark.asyncio -async def test_client_constructor_invalid_initial_raises_type_error( - make_client: MakeClientFactory, -) -> None: - """Verify invalid deprecated ``initial`` values raise ``TypeError``. - - Args: - make_client (MakeClientFactory): Fixture factory used to exercise legacy-option validation. + Returns: + None: This test validates constructor option type enforcement. """ - with pytest.raises(TypeError, match="`initial` must be a bool"): - make_client(initial="false") + with pytest.raises(TypeError, match=error_message): + make_client(**kwargs) @pytest.mark.asyncio From 8db47dc5fb01c3a43c5d0911858de4a00ac3e784 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:17:32 -0400 Subject: [PATCH 06/16] test: parameterize duration formatting cases --- tests/test_helpers.py | 63 +++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index c9c5f75..2a75e6d 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -5,8 +5,8 @@ import inspect import logging import traceback -from unittest.mock import MagicMock from typing import Any, NoReturn +from unittest.mock import MagicMock import aiohttp import pytest @@ -16,49 +16,42 @@ OPNsenseError, OPNsenseInvalidURL, OPNsenseTimeoutError, + helpers as aiopnsense_helpers, ) -from aiopnsense import helpers as aiopnsense_helpers from tests.conftest import make_mock_session_client ClientType = Callable[..., OPNsenseClient] -def test_human_friendly_duration() -> None: - """Convert seconds into a human-friendly duration string.""" - assert aiopnsense_helpers.human_friendly_duration(65) == "1 minute, 5 seconds" - assert aiopnsense_helpers.human_friendly_duration(0) == "0 seconds" - assert "month" in aiopnsense_helpers.human_friendly_duration(2419200) - +@pytest.mark.parametrize( + ("seconds", "expected"), + [ + pytest.param(0, "0 seconds", id="zero-seconds"), + pytest.param(1, "1 second", id="singular-second"), + pytest.param(2, "2 seconds", id="plural-seconds"), + pytest.param(60, "1 minute", id="singular-minute"), + pytest.param(61, "1 minute, 1 second", id="minute-and-singular-second"), + pytest.param(65, "1 minute, 5 seconds", id="minute-and-plural-seconds"), + pytest.param(3600, "1 hour", id="singular-hour"), + pytest.param(7200, "2 hours", id="plural-hours"), + pytest.param(86400, "1 day", id="singular-day"), + pytest.param(604800, "1 week", id="singular-week"), + pytest.param(1209600, "2 weeks", id="plural-weeks"), + pytest.param(2419200, "1 month", id="singular-month"), + pytest.param(4838400, "2 months", id="plural-months"), + ], +) +def test_human_friendly_duration(seconds: int, expected: str) -> None: + """Convert seconds to exact human-friendly duration strings. -def test_human_friendly_duration_singular_and_plural() -> None: - """Verify singular and plural forms for all supported units. + Args: + seconds (int): Duration to format, in seconds. + expected (str): Expected human-friendly duration. - This covers seconds, minutes, hours, days, weeks and months and ensures - the function emits the singular form when the value is 1 and plural - otherwise. + Returns: + None: This test validates formatted output via assertions. """ - # seconds - assert aiopnsense_helpers.human_friendly_duration(1) == "1 second" - assert aiopnsense_helpers.human_friendly_duration(2) == "2 seconds" - - # minutes + seconds - assert aiopnsense_helpers.human_friendly_duration(60) == "1 minute" - assert aiopnsense_helpers.human_friendly_duration(61) == "1 minute, 1 second" - - # hours - assert aiopnsense_helpers.human_friendly_duration(3600) == "1 hour" - assert aiopnsense_helpers.human_friendly_duration(7200) == "2 hours" - - # days - assert aiopnsense_helpers.human_friendly_duration(86400) == "1 day" - - # weeks - assert aiopnsense_helpers.human_friendly_duration(604800) == "1 week" - assert aiopnsense_helpers.human_friendly_duration(1209600) == "2 weeks" - - # months (28-day month used in implementation) - assert aiopnsense_helpers.human_friendly_duration(2419200) == "1 month" - assert aiopnsense_helpers.human_friendly_duration(4838400) == "2 months" + assert aiopnsense_helpers.human_friendly_duration(seconds) == expected def test_get_ip_key() -> None: From 775a29563ba8a0a5b49ea264203fa5e52699c786 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:19:36 -0400 Subject: [PATCH 07/16] test: parameterize legacy firmware failures --- tests/test_client_endpoint.py | 41 ++++++++++++++--------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/tests/test_client_endpoint.py b/tests/test_client_endpoint.py index 1ed7690..5a7fd2e 100644 --- a/tests/test_client_endpoint.py +++ b/tests/test_client_endpoint.py @@ -963,49 +963,40 @@ async def test_set_use_snake_case_selects_expected_endpoint_style( await client.async_close() +@pytest.mark.parametrize( + ("firmware_version", "expected_use_snake_case"), + [ + pytest.param("invalid-version", True, id="invalid-version"), + pytest.param(None, None, id="missing-version"), + ], +) @pytest.mark.asyncio async def test_set_use_snake_case_initial_raises_unknown_firmware( make_client: MakeClientFactory, + firmware_version: str | None, + expected_use_snake_case: bool | None, ) -> None: - """Verify legacy initial setup still raises for invalid firmware versions. + """Verify legacy initial setup raises when firmware cannot be identified. Args: make_client (MakeClientFactory): Fixture factory returning ``OPNsenseClient`` instances. + firmware_version (str | None): Invalid or missing firmware version returned by the client. + expected_use_snake_case (bool | None): Expected state to assert, when covered by the + original case. Returns: None: This test asserts the preserved compatibility path. """ client = make_client() try: - client.get_host_firmware_version = AsyncMock(return_value="invalid-version") - - with pytest.raises(OPNsenseUnknownFirmware): - await client._set_use_snake_case(initial=True) - - client.get_host_firmware_version.assert_awaited_once_with() - assert client._use_snake_case is True - finally: - await client.async_close() - - -@pytest.mark.asyncio -async def test_set_use_snake_case_initial_raises_unknown_firmware_when_version_missing( - make_client: MakeClientFactory, -) -> None: - """Verify missing firmware version still raises unknown-firmware in legacy init mode. - - Args: - make_client (MakeClientFactory): Fixture factory used to simulate a client without firmware data. - """ - - client = make_client() - try: - client.get_host_firmware_version = AsyncMock(return_value=None) + client.get_host_firmware_version = AsyncMock(return_value=firmware_version) with pytest.raises(OPNsenseUnknownFirmware): await client._set_use_snake_case(initial=True) client.get_host_firmware_version.assert_awaited_once_with() + if expected_use_snake_case is not None: + assert client._use_snake_case is expected_use_snake_case finally: await client.async_close() From 5a6052be134ae89f5189bfdf290e2020fb9e6f73 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:21:29 -0400 Subject: [PATCH 08/16] test: parameterize SMART post-only probes --- tests/test_smart.py | 85 ++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/tests/test_smart.py b/tests/test_smart.py index 1d28ba5..b72d684 100644 --- a/tests/test_smart.py +++ b/tests/test_smart.py @@ -183,14 +183,47 @@ async def test_smart_fails_closed_when_endpoint_is_unavailable( @pytest.mark.asyncio -async def test_get_smart_does_not_probe_post_only_endpoint(make_client: ClientType) -> None: - """SMART list queries should not use GET endpoint probes for POST-only APIs. +@pytest.mark.parametrize( + ("operation", "response", "expected", "preflight_endpoint", "post_args"), + [ + pytest.param( + "list", + {"devices": [{"ident": "nvme0", "device": "nvme0", "status": "PASSED"}]}, + [{"ident": "nvme0", "device": "nvme0", "status": "PASSED"}], + "/api/smart/service/list", + ("/api/smart/service/list/1",), + id="list", + ), + pytest.param( + "info", + {"output": {"smart_status": "PASSED"}}, + {"smart_status": "PASSED"}, + "/api/smart/service/info", + ("/api/smart/service/info", {"device": "nvme0", "type": "a", "json": True}), + id="info", + ), + ], +) +async def test_smart_does_not_probe_post_only_endpoint( + make_client: ClientType, + operation: str, + response: dict[str, object], + expected: object, + preflight_endpoint: str, + post_args: tuple[object, ...], +) -> None: + """SMART operations should not use GET endpoint probes for POST-only APIs. Args: make_client (ClientType): Fixture factory returning ``OPNsenseClient`` instances. + operation (str): SMART operation under test. + response (dict[str, object]): Mocked response from the POST request. + expected (object): Expected result from the SMART operation. + preflight_endpoint (str): Expected endpoint for the POST availability check. + post_args (tuple[object, ...]): Expected arguments for the SMART POST request. Returns: - None: This test validates that SMART list requests do not depend on GET probes. + None: This test validates that SMART POST requests do not depend on GET probes. """ client, _session = make_mock_session_client(make_client) try: @@ -198,15 +231,17 @@ async def test_get_smart_does_not_probe_post_only_endpoint(make_client: ClientTy side_effect=AssertionError("GET probe should not run") ) client._is_post_endpoint_available = AsyncMock(return_value=True) - client._safe_dict_post = AsyncMock( - return_value={"devices": [{"ident": "nvme0", "device": "nvme0", "status": "PASSED"}]} - ) + client._safe_dict_post = AsyncMock(return_value=response) - assert await client.get_smart() == [ - {"ident": "nvme0", "device": "nvme0", "status": "PASSED"} - ] - client._is_post_endpoint_available.assert_awaited_once_with("/api/smart/service/list") - client._safe_dict_post.assert_awaited_once_with("/api/smart/service/list/1") + if operation == "list": + got = await client.get_smart() + else: + got = await client.get_smart_info("nvme0") + + assert got == expected + client._is_get_endpoint_available.assert_not_awaited() + client._is_post_endpoint_available.assert_awaited_once_with(preflight_endpoint) + client._safe_dict_post.assert_awaited_once_with(*post_args) finally: await client.async_close() @@ -267,34 +302,6 @@ async def test_get_smart_info_wraps_non_mapping_output(make_client: ClientType) await client.async_close() -@pytest.mark.asyncio -async def test_get_smart_info_does_not_probe_post_only_endpoint(make_client: ClientType) -> None: - """SMART info queries should not use GET endpoint probes for POST-only APIs. - - Args: - make_client (ClientType): Fixture factory returning ``OPNsenseClient`` instances. - - Returns: - None: This test validates that SMART info requests do not depend on GET probes. - """ - client, _session = make_mock_session_client(make_client) - try: - client._is_get_endpoint_available = AsyncMock( - side_effect=AssertionError("GET probe should not run") - ) - client._is_post_endpoint_available = AsyncMock(return_value=True) - client._safe_dict_post = AsyncMock(return_value={"output": {"smart_status": "PASSED"}}) - - assert await client.get_smart_info("nvme0") == {"smart_status": "PASSED"} - client._is_post_endpoint_available.assert_awaited_once_with("/api/smart/service/info") - client._safe_dict_post.assert_awaited_once_with( - "/api/smart/service/info", - {"device": "nvme0", "type": "a", "json": True}, - ) - finally: - await client.async_close() - - @pytest.mark.asyncio async def test_get_smart_fails_closed_when_list_endpoint_unavailable( make_client: ClientType, From 8ead622f97d76738124742a507ffbb47189e0dd9 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:23:52 -0400 Subject: [PATCH 09/16] test: parameterize API call CLI errors --- tests/test_scripts_opnsense_api_call.py | 148 ++++++++++-------------- 1 file changed, 64 insertions(+), 84 deletions(-) diff --git a/tests/test_scripts_opnsense_api_call.py b/tests/test_scripts_opnsense_api_call.py index 85d2412..99a7be2 100644 --- a/tests/test_scripts_opnsense_api_call.py +++ b/tests/test_scripts_opnsense_api_call.py @@ -2,14 +2,15 @@ from __future__ import annotations +from collections.abc import Callable import importlib.util -import runpy from pathlib import Path +import runpy import sys from types import ModuleType -from typing import Any -import aiohttp +from typing import Any, Self +import aiohttp import pytest @@ -66,10 +67,10 @@ def __init__( self._text = text self._json_error = json_error - async def __aenter__(self) -> "FakeResponse": + async def __aenter__(self) -> Self: return self - async def __aexit__(self, *_args: Any) -> None: + async def __aexit__(self, *_args: object) -> None: return None async def json(self, *_args: Any, **_kwargs: Any) -> Any: @@ -128,13 +129,12 @@ def __init__( self.enter_count = 0 self.exit_count = 0 - async def __aenter__(self) -> "FakeSession": + async def __aenter__(self) -> Self: self.enter_count += 1 return self - async def __aexit__(self, *_args: Any) -> None: + async def __aexit__(self, *_args: object) -> None: self.exit_count += 1 - return None def get(self, url: str, **kwargs: object) -> FakeResponse: """Record GET call and return response context. @@ -453,25 +453,6 @@ async def test_call_api_preserves_non_2xx_text_body() -> None: assert result["text"] == "resource not found" -def test_main_converts_live_config_error_to_system_exit(monkeypatch: pytest.MonkeyPatch) -> None: - """main() maps LiveConfigError from async_main to SystemExit. - - Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise configuration failure. - """ - module = load_api_call_module() - - async def raise_error() -> None: - raise module.LiveConfigError("bad config") - - monkeypatch.setattr(module, "async_main", raise_error) - - with pytest.raises(SystemExit) as excinfo: - module.main() - - assert "bad config" in str(excinfo.value) - - @pytest.mark.parametrize( ("payload", "name"), [('{"x":1}', "object"), ("{bad_json}", "invalid-json"), ("[1, 2, 3]", "non-object")], @@ -796,72 +777,71 @@ def test_entrypoint_exits_with_help_status_zero() -> None: assert excinfo.value.code == 0 -def test_main_converts_client_error_to_system_exit( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """main() converts aiohttp client transport exceptions into SystemExit. - - Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise a client error. - """ - module = load_api_call_module() - - async def raise_connector_error() -> None: - raise module.aiohttp.ClientConnectorError( - module.aiohttp.client_reqrep.ConnectionKey( - host="localhost", - port=443, - is_ssl=False, - ssl=None, - proxy=None, - proxy_auth=None, - proxy_headers_hash=None, +@pytest.mark.parametrize( + ("exception_factory", "expected_message", "exact"), + [ + pytest.param( + lambda module: module.LiveConfigError("bad config"), + "bad config", + False, + id="live-config-error", + ), + pytest.param( + lambda module: module.aiohttp.ClientConnectorError( + module.aiohttp.client_reqrep.ConnectionKey( + host="localhost", + port=443, + is_ssl=False, + ssl=None, + proxy=None, + proxy_auth=None, + proxy_headers_hash=None, + ), + OSError("boom"), ), - OSError("boom"), - ) - - monkeypatch.setattr(module, "async_main", raise_connector_error) - with pytest.raises(SystemExit) as excinfo: - module.main() - - assert "ClientConnectorError" in str(excinfo.value) - - -def test_main_converts_timeout_error_to_system_exit( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """main() converts timeout exceptions into concise SystemExit messages. - - Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise a timeout. - """ - module = load_api_call_module() - - async def raise_timeout_error() -> None: - raise TimeoutError("timeout while waiting for response") - - monkeypatch.setattr(module, "async_main", raise_timeout_error) - with pytest.raises(SystemExit) as excinfo: - module.main() - - assert "TimeoutError" in str(excinfo.value) - - -def test_main_converts_os_error_to_system_exit( + "ClientConnectorError", + False, + id="client-connector-error", + ), + pytest.param( + lambda _module: TimeoutError("timeout while waiting for response"), + "TimeoutError", + False, + id="timeout-error", + ), + pytest.param( + lambda _module: OSError("output is unavailable"), + "OSError: output is unavailable", + True, + id="os-error", + ), + ], +) +def test_main_converts_expected_errors_to_system_exit( monkeypatch: pytest.MonkeyPatch, + exception_factory: Callable[[ModuleType], Exception], + expected_message: str, + exact: bool, ) -> None: - """main() converts I/O errors into concise SystemExit messages. + """main() maps expected failures to concise user-visible SystemExit messages. Args: - monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise an I/O error. + monkeypatch (pytest.MonkeyPatch): Fixture that makes ``async_main`` raise the failure. + exception_factory (Callable[[ModuleType], Exception]): Factory for the failure case. + expected_message (str): Exact message or substring expected in ``SystemExit``. + exact (bool): Whether the complete exit message must match. """ module = load_api_call_module() - async def raise_os_error() -> None: - raise OSError("output is unavailable") + async def raise_error() -> None: + raise exception_factory(module) - monkeypatch.setattr(module, "async_main", raise_os_error) + monkeypatch.setattr(module, "async_main", raise_error) with pytest.raises(SystemExit) as excinfo: module.main() - assert str(excinfo.value) == "OSError: output is unavailable" + message = str(excinfo.value) + if exact: + assert message == expected_message + else: + assert expected_message in message From 5619ca6196a5a24d4e4246a356ea953a6927fbbe Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:28:25 -0400 Subject: [PATCH 10/16] test: remove brittle stream parser checks --- tests/test_client_transport.py | 66 ++++++---------------------------- 1 file changed, 10 insertions(+), 56 deletions(-) diff --git a/tests/test_client_transport.py b/tests/test_client_transport.py index ed569ab..3a92d95 100644 --- a/tests/test_client_transport.py +++ b/tests/test_client_transport.py @@ -1,20 +1,17 @@ """Tests for client transport helpers and HTTP response handling.""" +from collections.abc import Callable, MutableMapping import json -from collections.abc import MutableMapping from types import TracebackType -from typing import Any, Callable +from typing import Any from unittest.mock import AsyncMock, MagicMock import aiohttp import pytest import aiopnsense.client_transport - from aiopnsense.client_transport import _STREAM_JSON_EVENT_RESET_KEY -from aiopnsense.const import ( - DEFAULT_REQUEST_TIMEOUT_SECONDS, -) +from aiopnsense.const import DEFAULT_REQUEST_TIMEOUT_SECONDS from aiopnsense.exceptions import ( OPNsenseConnectionError, OPNsenseError, @@ -303,36 +300,6 @@ def raise_transport_error(*_args: Any, **_kwargs: Any) -> Any: await client.async_close() -@pytest.mark.asyncio -async def test_get_from_stream_parsing( - make_client: MakeClientFactory, - fake_stream_response_factory: FakeStreamResponseFactory, -) -> None: - """Verify stream parsing returns the second valid SSE payload as mapping. - - Args: - make_client (MakeClientFactory): Fixture factory returning ``OPNsenseClient`` instances. - fake_stream_response_factory (FakeStreamResponseFactory): Fixture function building stream - response stubs. - - Returns: - None: This test asserts stream payload parsing behavior. - """ - client, session = make_mock_session_client(make_client) - - # use shared factory to construct a fake streaming response - session.get = lambda *a, **k: fake_stream_response_factory( - [b'data: {"a": 1}\n\n', b'data: {"b": 2}\n\n'] - ) - try: - res = await client._do_get_from_stream("/stream", caller="tst") - # implementation returns the second 'data' message parsed as JSON - assert isinstance(res, MutableMapping) - assert res.get("b") == 2 - finally: - await client.async_close() - - @pytest.mark.asyncio async def test_get_from_stream_ignores_first_message( make_client: MakeClientFactory, @@ -918,8 +885,8 @@ async def test_stream_json_events_reassembles_split_multibyte_utf8_event( }, ensure_ascii=False, ).encode("utf-8") - event_payload = f"data: {event_json.decode('utf-8')}\n\n".encode("utf-8") - accent_index = event_payload.index("é".encode("utf-8")) + 1 + event_payload = f"data: {event_json.decode('utf-8')}\n\n".encode() + accent_index = event_payload.index("é".encode()) + 1 chunks = [ b'data: {"time": 9, "interfaces": {"wan": {"bytes received": 0, "bytes transmitted": 0}}}\n\n', event_payload[:accent_index], @@ -964,8 +931,8 @@ async def test_stream_json_events_ignores_trailing_incomplete_utf8_chunk( {"time": 12, "interfaces": {"wan": {"description": "Café"}}}, ensure_ascii=False, ).encode("utf-8") - incomplete_payload = f"data: {incomplete_event_json.decode('utf-8')}\n\n".encode("utf-8") - leading_byte_index = incomplete_payload.index("é".encode("utf-8")) + incomplete_payload = f"data: {incomplete_event_json.decode('utf-8')}\n\n".encode() + leading_byte_index = incomplete_payload.index("é".encode()) leading_byte_index += 1 session = MagicMock() @@ -1107,27 +1074,16 @@ async def test_stream_json_events_yields_eof_complete_event( @pytest.mark.asyncio -async def test_stream_json_events_preserves_leading_sse_space_after_data_prefix( +async def test_stream_json_events_preserves_leading_spaces_in_json_values( make_client: Callable[..., Any], fake_stream_response_factory: Callable[..., FakeResponse], - monkeypatch: pytest.MonkeyPatch, ) -> None: - """Parser should preserve only one leading space after `data:` when present. + """SSE payload whitespace should not strip leading spaces from JSON values. Args: make_client (Callable[..., Any]): Factory used to configure the SSE stream client. fake_stream_response_factory (Callable[..., FakeResponse]): Factory supplying a data field with leading spaces. - monkeypatch (pytest.MonkeyPatch): Fixture used to capture payloads passed to JSON decoding. """ - captured_payloads: list[str] = [] - original_loads = aiopnsense.client_transport.json.loads - - def fake_loads(response_str: str) -> dict[str, Any]: - captured_payloads.append(response_str) - return original_loads(response_str) - - monkeypatch.setattr("aiopnsense.client_transport.json.loads", fake_loads) - session = MagicMock() session.get = lambda *a, **k: fake_stream_response_factory( [b'data: {"time": 22, "name": " leading"}\n\n'] @@ -1138,9 +1094,7 @@ def fake_loads(response_str: str) -> dict[str, Any]: event async for event in client._stream_json_events("/api/diagnostics/traffic/stream/1") ] - assert len(events) == 1 - assert events[0] == {"time": 22, "name": " leading"} - assert captured_payloads == [' {"time": 22, "name": " leading"}'] + assert events == [{"time": 22, "name": " leading"}] finally: await client.async_close() From b449b58085d314caaa1e54cf69e03e957ca8d3ae Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:30:25 -0400 Subject: [PATCH 11/16] test: simplify error decorator coverage --- tests/test_helpers.py | 103 ++++++++---------------------------------- 1 file changed, 20 insertions(+), 83 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 2a75e6d..9ba7602 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -409,11 +409,21 @@ async def boom(self, value: str) -> str: @pytest.mark.asyncio -async def test_log_errors_timeout_re_raise_and_suppress(make_client: ClientType) -> None: - """Verify ``_log_errors`` re-raises or suppresses ``TimeoutError`` by configuration. +@pytest.mark.parametrize( + "timeout_error", + [ + pytest.param(TimeoutError("boom"), id="timeout-error"), + pytest.param(aiohttp.ServerTimeoutError("srv"), id="server-timeout-error"), + ], +) +async def test_log_errors_timeout_re_raise_and_suppress( + make_client: ClientType, timeout_error: TimeoutError +) -> None: + """Verify ``_log_errors`` maps or suppresses timeout-family errors by configuration. Args: make_client (ClientType): Fixture factory returning ``OPNsenseClient`` instances. + timeout_error (TimeoutError): Timeout-family error raised by the wrapped coroutine. Returns: None: This test validates timeout error propagation behavior. @@ -422,7 +432,7 @@ async def test_log_errors_timeout_re_raise_and_suppress(make_client: ClientType) try: async def raising_timeout(*args: Any, **kwargs: Any) -> NoReturn: - """Raising timeout. + """Raise the configured timeout-family error. Args: args (Any): Positional arguments accepted by `raising_timeout`. @@ -432,16 +442,16 @@ async def raising_timeout(*args: Any, **kwargs: Any) -> NoReturn: NoReturn: This helper always raises ``TimeoutError``. Raises: - TimeoutError: Always raised to test timeout handling. + timeout_error: Always raised to test timeout handling. """ - raise TimeoutError("boom") + raise timeout_error # wrap the coroutine with the decorator decorated = aiopnsense_helpers._log_errors(raising_timeout) # When error throwing is enabled we expect a public timeout error. client._throw_errors = True - with pytest.raises(OPNsenseTimeoutError, match="boom"): + with pytest.raises(OPNsenseTimeoutError, match=str(timeout_error)): await decorated(client) # When error throwing is disabled the decorator suppresses ``TimeoutError``. @@ -478,82 +488,9 @@ async def boom(self) -> None: @pytest.mark.asyncio -async def test_log_errors_server_timeout_re_raise_and_suppress(make_client: ClientType) -> None: - """Verify ``_log_errors`` re-raises or suppresses ``ServerTimeoutError`` by configuration. - - Args: - make_client (ClientType): Fixture factory returning ``OPNsenseClient`` instances. - - Returns: - None: This test validates server-timeout error propagation behavior. - """ - client, _ = make_mock_session_client(make_client, url="http://x") - try: - - async def raising_server_timeout(*args: Any, **kwargs: Any) -> Any: - """Raising server timeout. - - Args: - args (Any): Positional arguments accepted by `raising_server_timeout`. - kwargs (Any): Keyword arguments accepted by `raising_server_timeout`. - - Returns: - Any: This coroutine only raises the configured timeout. - - Raises: - aiohttp.ServerTimeoutError: Always raised to test server-timeout handling. - """ - raise aiohttp.ServerTimeoutError("srv") - - decorated = aiopnsense_helpers._log_errors(raising_server_timeout) - - client._throw_errors = True - with pytest.raises(OPNsenseTimeoutError, match="srv"): - await decorated(client) - - client._throw_errors = False - assert await decorated(client) is None - finally: - await client.async_close() - - -@pytest.mark.parametrize( - ("raw_url", "forbidden"), - [ - ("https://alice:secret@api.example/opn", ("alice", "secret")), - ("https://alice secret@api.example/opn", ("alice secret",)), - ("'https://alice:secret@api.example/opn'", ("alice", "secret")), - ('"https://alice:secret@api.example/opn"', ("alice", "secret")), - ("", ("alice", "secret")), - ("`https://alice:secret@api.example/opn`", ("alice", "secret")), - ("https://alice@api.example/opn", ("alice",)), - ("https://alice:@api.example/opn", ("alice",)), - ("https://alice:pa@ss@api.example/opn", ("alice", "pa@ss")), - ("https://u%40lice:p%40ss@api.example/opn", ("u%40lice", "p%40ss")), - ("https://u:pa@ss@api.example/path@with@ats", ("u", "pa@ss")), - ("https://alice:secret@[2001:db8::1]:443/path", ("alice", "secret")), - ("https://alice:secret@[bad", ("alice", "secret")), - ( - "https://public.example/path https://alice:secret@api.example/opn", - ("alice", "secret"), - ), - ("https://alice?bad:secret@api.example/opn", ("alice?bad", "secret")), - ("https://alice#bad:secret@api.example/opn", ("alice#bad", "secret")), - ("https://alice:pa/ss@api.example/opn", ("alice", "pa/ss")), - ( - "https://alice:secret@api.example/opn https://bob:pass@other.example/opn", - ("alice", "secret", "bob", "pass"), - ), - ], -) -@pytest.mark.asyncio -async def test_log_errors_redacts_url_userinfo(raw_url: str, forbidden: tuple[str, ...]) -> None: - """Verify _log_errors maps invalid URLs using a constant-safe message. - - Args: - raw_url (str): URL containing credentials to redact. - forbidden (tuple[str, ...]): Fragments that must not appear in the mapped message. - """ +async def test_log_errors_redacts_url_userinfo() -> None: + """Verify _log_errors integration maps a representative credentialed invalid URL safely.""" + raw_url = "https://alice:secret@api.example/opn" class Dummy: """Small wrapper for testing redaction in error logs and mapping.""" @@ -576,7 +513,7 @@ async def boom(self) -> None: message = str(exc_info.value) assert message == "Invalid OPNsense URL" assert raw_url not in message - for token in forbidden: + for token in ("alice", "secret"): assert token not in message From 7d1b8caa508be7349396809893c47ea93be94af7 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:31:58 -0400 Subject: [PATCH 12/16] test: remove vnStat parser call coupling --- tests/test_vnstat.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_vnstat.py b/tests/test_vnstat.py index a1a4cf9..53bcb60 100644 --- a/tests/test_vnstat.py +++ b/tests/test_vnstat.py @@ -3,7 +3,7 @@ from collections.abc import Callable from datetime import UTC, datetime, timedelta from typing import Any -from unittest.mock import AsyncMock, Mock +from unittest.mock import AsyncMock import pytest @@ -93,7 +93,7 @@ async def test_get_vnstat_metrics_yearly_parsing(make_client: ClientType) -> Non assert parsed["period"] == "yearly" assert len(rows) == 4 assert rows[0]["label"] == "2023" - assert rows[0]["total_bytes"] == int(round(63.25 * tib)) + assert rows[0]["total_bytes"] == round(63.25 * tib) assert rows[3]["label"] == "2026" assert rows[3]["avg_rate_bits_per_second"] == 14150000 client._safe_dict_get.assert_awaited_once_with("/api/vnstat/service/yearly") @@ -213,8 +213,6 @@ async def fake_safe_get(path: str, *_args: Any, **_kwargs: Any) -> dict[str, Any return {} client._safe_dict_get = AsyncMock(side_effect=fake_safe_get) - client._parse_daily_label = Mock(wraps=client._parse_daily_label) - client._parse_month_label = Mock(wraps=client._parse_month_label) vnstat = await client.get_vnstat() gib = 1024**3 @@ -236,8 +234,6 @@ async def fake_safe_get(path: str, *_args: Any, **_kwargs: Any) -> dict[str, Any assert igc1_metrics["vnstat_yesterday"]["total_bytes"] == 1 * gib assert igc1_metrics["vnstat_last_month"]["total_bytes"] == 2 * gib assert igc1_metrics["vnstat_last_hour"]["total_bytes"] == int(1.5 * gib) - assert client._parse_daily_label.call_count == 4 - assert client._parse_month_label.call_count == 4 finally: await client.async_close() From d3f193db182475ac0b97606cb6e51cbe98b93604 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:33:34 -0400 Subject: [PATCH 13/16] test: remove Speedtest probe ordering coupling --- tests/test_speedtest.py | 48 +++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/tests/test_speedtest.py b/tests/test_speedtest.py index b70d5e7..bc795cc 100644 --- a/tests/test_speedtest.py +++ b/tests/test_speedtest.py @@ -2,7 +2,7 @@ from collections.abc import Callable from datetime import timedelta, timezone -from unittest.mock import AsyncMock, call +from unittest.mock import AsyncMock from zoneinfo import ZoneInfo import pytest @@ -90,39 +90,35 @@ async def test_get_speedtest_normalizes_latest_and_stat_payloads(make_client: Cl @pytest.mark.parametrize( - ("endpoint_side_effect", "showstat_available"), + "showstat_available", [ - pytest.param( - [True, True], - True, - id="showstat-available", - ), - pytest.param( - [True, False], - False, - id="showstat-missing", - ), + pytest.param(True, id="showstat-available"), + pytest.param(False, id="showstat-missing"), ], ) @pytest.mark.asyncio -async def test_get_speedtest_probes_showstat_before_fetching_optional_payload( +async def test_get_speedtest_conditionally_fetches_optional_showstat_payload( make_client: ClientType, - endpoint_side_effect: list[bool], showstat_available: bool, ) -> None: - """Validate ``get_speedtest`` probes ``showstat`` before optional fetches. + """Validate ``get_speedtest`` conditionally fetches the ``showstat`` payload. Args: make_client (ClientType): Fixture factory returning ``OPNsenseClient`` instances. - endpoint_side_effect (list[bool]): Endpoint availability responses in call order. showstat_available (bool): Whether the ``showstat`` endpoint should be fetched. Returns: - None: This test validates endpoint probing order and conditional fetches. + None: This test validates endpoint availability and conditional fetches. """ client, _session = make_mock_session_client(make_client) try: - client._is_get_endpoint_available = AsyncMock(side_effect=endpoint_side_effect) + endpoint_availability = { + "/api/speedtest/service/showlog": True, + "/api/speedtest/service/showstat": showstat_available, + } + client._is_get_endpoint_available = AsyncMock( + side_effect=lambda endpoint: endpoint_availability[endpoint] + ) client._safe_list_get = AsyncMock( return_value=[ [ @@ -146,23 +142,19 @@ async def test_get_speedtest_probes_showstat_before_fetching_optional_payload( result = await client.get_speedtest() assert result["available"] is True - assert client._is_get_endpoint_available.await_args_list == [ - call("/api/speedtest/service/showlog"), - call("/api/speedtest/service/showstat"), - ] + assert client._is_get_endpoint_available.await_count == 2 + client._is_get_endpoint_available.assert_any_await("/api/speedtest/service/showlog") + client._is_get_endpoint_available.assert_any_await("/api/speedtest/service/showstat") client._get_resolved_opnsense_timezone.assert_awaited_once_with() client._safe_list_get.assert_awaited_once_with("/api/speedtest/service/showlog") if showstat_available: client._safe_dict_get.assert_awaited_once_with("/api/speedtest/service/showstat") - assert result["last"]["download"]["value"] == 1.0 - assert result["last"]["upload"]["value"] == 2.0 - assert result["last"]["latency"]["value"] == 3.0 else: client._safe_dict_get.assert_not_awaited() - assert result["last"]["download"]["value"] == 1.0 - assert result["last"]["upload"]["value"] == 2.0 - assert result["last"]["latency"]["value"] == 3.0 + assert result["last"]["download"]["value"] == 1.0 + assert result["last"]["upload"]["value"] == 2.0 + assert result["last"]["latency"]["value"] == 3.0 finally: await client.async_close() From edcec40cad63da7ad1f854999bcb64dfcc4ecb38 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:45:19 -0400 Subject: [PATCH 14/16] [prek-autofix] apply automatic fixes --- aiopnsense/__init__.py | 4 ++-- aiopnsense/client.py | 6 +----- aiopnsense/client_transport.py | 3 +-- aiopnsense/nut.py | 2 +- aiopnsense/system.py | 7 ++----- aiopnsense/traffic.py | 2 +- scripts/_opnsense_live_common.py | 11 ++++++----- scripts/aiopnsense_dump.py | 9 +++++---- scripts/opnsense_api_call.py | 4 ++-- tests/conftest.py | 6 +++--- tests/test_dhcp.py | 2 +- tests/test_nut.py | 2 +- tests/test_scripts_live_common.py | 2 +- tests/test_system.py | 4 ++-- 14 files changed, 29 insertions(+), 35 deletions(-) diff --git a/aiopnsense/__init__.py b/aiopnsense/__init__.py index 3fae22a..a590310 100644 --- a/aiopnsense/__init__.py +++ b/aiopnsense/__init__.py @@ -5,8 +5,8 @@ OPNsenseBelowMinFirmware, OPNsenseConnectionError, OPNsenseError, - OPNsenseInvalidAuth, OPNsenseInvalidArgument, + OPNsenseInvalidAuth, OPNsenseInvalidURL, OPNsenseMissingDeviceUniqueID, OPNsensePrivilegeMissing, @@ -21,8 +21,8 @@ "OPNsenseClient", "OPNsenseConnectionError", "OPNsenseError", - "OPNsenseInvalidAuth", "OPNsenseInvalidArgument", + "OPNsenseInvalidAuth", "OPNsenseInvalidURL", "OPNsenseMissingDeviceUniqueID", "OPNsensePrivilegeMissing", diff --git a/aiopnsense/client.py b/aiopnsense/client.py index 2a48da0..9151e4c 100644 --- a/aiopnsense/client.py +++ b/aiopnsense/client.py @@ -9,11 +9,7 @@ from .client_base import ClientBaseMixin from .const import OPNSENSE_LTD_FIRMWARE, OPNSENSE_MIN_FIRMWARE from .dhcp import DHCPMixin -from .exceptions import ( - OPNsenseBelowMinFirmware, - OPNsenseUnknownFirmware, - _map_opnsense_exception, -) +from .exceptions import OPNsenseBelowMinFirmware, OPNsenseUnknownFirmware, _map_opnsense_exception from .firewall import FirewallMixin from .firmware import FirmwareMixin from .helpers import _LOGGER, firmware_is_at_least diff --git a/aiopnsense/client_transport.py b/aiopnsense/client_transport.py index 87fbc88..be3338d 100644 --- a/aiopnsense/client_transport.py +++ b/aiopnsense/client_transport.py @@ -210,8 +210,7 @@ def _drain_buffer( if not line.startswith("data:"): continue value = line[len("data:") :] - if value.startswith(" "): - value = value[1:] + value = value.removeprefix(" ") data_lines.append(value) if not data_lines: diff --git a/aiopnsense/nut.py b/aiopnsense/nut.py index 2abe0c8..be54129 100644 --- a/aiopnsense/nut.py +++ b/aiopnsense/nut.py @@ -1,7 +1,7 @@ """NUT plugin methods for OPNsenseClient.""" -import re from collections.abc import Mapping +import re from typing import Any from ._typing import AiopnsenseClientProtocol diff --git a/aiopnsense/system.py b/aiopnsense/system.py index e29ee59..b0e0af1 100644 --- a/aiopnsense/system.py +++ b/aiopnsense/system.py @@ -9,11 +9,8 @@ from dateutil.parser import ParserError, UnknownTimezoneWarning, parse from ._typing import AiopnsenseClientProtocol -from .const import ( - AMBIGUOUS_TZINFOS, - OPNSENSE_26_1_11_COMPAT_FIRMWARE, -) -from .exceptions import OPNsenseMissingDeviceUniqueID, OPNsenseError +from .const import AMBIGUOUS_TZINFOS, OPNSENSE_26_1_11_COMPAT_FIRMWARE +from .exceptions import OPNsenseError, OPNsenseMissingDeviceUniqueID from .helpers import ( _LOGGER, _log_errors, diff --git a/aiopnsense/traffic.py b/aiopnsense/traffic.py index 48d5d95..8f292d3 100644 --- a/aiopnsense/traffic.py +++ b/aiopnsense/traffic.py @@ -5,10 +5,10 @@ from collections.abc import AsyncIterator, Mapping from typing import Any +from ._typing import AiopnsenseClientProtocol from .client_transport import _STREAM_JSON_EVENT_RESET_KEY from .const import DEFAULT_REQUEST_TIMEOUT_SECONDS from .exceptions import OPNsenseError, _map_opnsense_exception -from ._typing import AiopnsenseClientProtocol from .helpers import _LOGGER, try_to_float, try_to_int DIAGNOSTICS_TRAFFIC_ENDPOINT = "/api/diagnostics/traffic/interface" diff --git a/scripts/_opnsense_live_common.py b/scripts/_opnsense_live_common.py index 213ce71..4b7fff4 100644 --- a/scripts/_opnsense_live_common.py +++ b/scripts/_opnsense_live_common.py @@ -2,18 +2,19 @@ from __future__ import annotations -import json -import os -import re from collections.abc import Mapping from dataclasses import dataclass from functools import cache +import json +import os from pathlib import Path +import re import sys from typing import TYPE_CHECKING, Any if TYPE_CHECKING: import aiohttp + from aiopnsense import OPNsenseClient @@ -255,7 +256,7 @@ def write_output(payload: Any, output_path: Path | None = None) -> None: @cache -def _get_client_class() -> type["OPNsenseClient"]: +def _get_client_class() -> type[OPNsenseClient]: """Return the lazily imported OPNsense client class. Returns: @@ -266,7 +267,7 @@ def _get_client_class() -> type["OPNsenseClient"]: return OPNsenseClient -def create_client(config: LiveConfig, session: aiohttp.ClientSession) -> "OPNsenseClient": +def create_client(config: LiveConfig, session: aiohttp.ClientSession) -> OPNsenseClient: """Create an OPNsense client for live scripts. Args: diff --git a/scripts/aiopnsense_dump.py b/scripts/aiopnsense_dump.py index fd1ded5..7cc8862 100755 --- a/scripts/aiopnsense_dump.py +++ b/scripts/aiopnsense_dump.py @@ -5,11 +5,11 @@ import argparse import asyncio +from dataclasses import dataclass import importlib import logging -import time -from dataclasses import dataclass from pathlib import Path +import time from typing import Any _common = importlib.import_module("_opnsense_live_common") @@ -25,8 +25,9 @@ if __name__ == "__main__": reexec_with_repo_venv(Path(__file__)) -import aiohttp # noqa: E402 -from aiopnsense.exceptions import OPNsenseError # noqa: E402 +import aiohttp + +from aiopnsense.exceptions import OPNsenseError _LOGGER = logging.getLogger(__name__) diff --git a/scripts/opnsense_api_call.py b/scripts/opnsense_api_call.py index 675aea4..789511a 100755 --- a/scripts/opnsense_api_call.py +++ b/scripts/opnsense_api_call.py @@ -27,7 +27,7 @@ if __name__ == "__main__": reexec_with_repo_venv(Path(__file__)) -import aiohttp # noqa: E402 +import aiohttp _NO_PAYLOAD = object() @@ -164,7 +164,7 @@ def load_payload( async def call_api( session: aiohttp.ClientSession, - config: "LiveConfig", + config: LiveConfig, endpoint: str, method: str, payload: dict[str, Any] | object, diff --git a/tests/conftest.py b/tests/conftest.py index 2a5653d..8ba5681 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -45,7 +45,7 @@ def legacy_dnsbl_payload() -> dict[str, Any]: class FakeClientSession: """Minimal fake aiohttp session for unit tests.""" - async def __aenter__(self) -> "FakeClientSession": + async def __aenter__(self) -> FakeClientSession: """Enter async context. Returns: @@ -276,7 +276,7 @@ def done(self) -> bool: """ return False - def __await__(self) -> Generator[None, None, None]: + def __await__(self) -> Generator[None]: """Await. Returns: @@ -394,7 +394,7 @@ def _make( @pytest.fixture -async def make_client() -> AsyncGenerator[Callable[..., aiopnsense.OPNsenseClient], None]: +async def make_client() -> AsyncGenerator[Callable[..., aiopnsense.OPNsenseClient]]: """Return a factory that constructs an OPNsenseClient for tests. Yields: diff --git a/tests/test_dhcp.py b/tests/test_dhcp.py index 0d216c2..e7237a2 100644 --- a/tests/test_dhcp.py +++ b/tests/test_dhcp.py @@ -295,7 +295,7 @@ async def test_keep_latest_leases_handles_list_values(make_client: ClientType) - async def test_is_reserved_lease_handles_legacy_and_list_flags( make_client: ClientType, raw_reserved: object, - expected: bool, # noqa: FBT001 - pytest injects parametrized values by name. + expected: bool, ) -> None: """Verify reserved lease detection supports legacy and new value shapes. diff --git a/tests/test_nut.py b/tests/test_nut.py index 0f05f08..8caa859 100644 --- a/tests/test_nut.py +++ b/tests/test_nut.py @@ -1,7 +1,7 @@ """Tests for `aiopnsense.nut`.""" -import logging from collections.abc import Callable +import logging from typing import Any from unittest.mock import AsyncMock diff --git a/tests/test_scripts_live_common.py b/tests/test_scripts_live_common.py index a30e7ca..84a79b4 100644 --- a/tests/test_scripts_live_common.py +++ b/tests/test_scripts_live_common.py @@ -2,8 +2,8 @@ from __future__ import annotations -import importlib.util import dataclasses +import importlib.util import json from pathlib import Path import sys diff --git a/tests/test_system.py b/tests/test_system.py index a1cef55..541d262 100644 --- a/tests/test_system.py +++ b/tests/test_system.py @@ -1,7 +1,7 @@ """Tests for `aiopnsense.system`.""" from collections.abc import Callable -from datetime import datetime, timedelta, timezone +from datetime import UTC, datetime, timedelta from typing import Any from unittest.mock import AsyncMock @@ -522,7 +522,7 @@ async def test_gateways_notices_and_close_notice_all(make_client: ClientType) -> "n1": { "statusCode": 1, "message": "m", - "timestamp": int(datetime.now(tz=timezone.utc).timestamp()), + "timestamp": int(datetime.now(tz=UTC).timestamp()), } } ) From 0e5694549af4e217b830cf4d6a5bb226c90f2af4 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 19:52:44 -0400 Subject: [PATCH 15/16] style: resolve prek lint findings --- aiopnsense/dhcp.py | 8 ++-- aiopnsense/firewall.py | 15 ++----- aiopnsense/system.py | 10 ++--- aiopnsense/vnstat.py | 10 ++--- docs/source/_ext/opnsense_client_api.py | 5 ++- docs/source/conf.py | 4 +- scripts/opnsense_api_call.py | 6 ++- tests/conftest.py | 12 ++--- tests/test_firewall.py | 6 ++- tests/test_nut.py | 33 ++++---------- tests/test_scripts_live_common.py | 60 ++++++++++--------------- tests/test_system.py | 51 +++++++++++++++++---- 12 files changed, 109 insertions(+), 111 deletions(-) diff --git a/aiopnsense/dhcp.py b/aiopnsense/dhcp.py index cf4ddbb..941e785 100644 --- a/aiopnsense/dhcp.py +++ b/aiopnsense/dhcp.py @@ -544,10 +544,10 @@ async def _get_isc_dhcpv4_leases(self, opnsense_tz: tzinfo | None = None) -> lis try: dt: datetime = datetime.strptime( lease_info.get("ends", None), "%Y/%m/%d %H:%M:%S" - ) + ).replace(tzinfo=opnsense_tz) except TypeError, ValueError: continue - lease["expires"] = dt.replace(tzinfo=opnsense_tz) + lease["expires"] = dt if lease["expires"] < current_time: continue else: @@ -604,10 +604,10 @@ async def _get_isc_dhcpv6_leases(self, opnsense_tz: tzinfo | None = None) -> lis try: dt: datetime = datetime.strptime( lease_info.get("ends", None), "%Y/%m/%d %H:%M:%S" - ) + ).replace(tzinfo=opnsense_tz) except TypeError, ValueError: continue - lease["expires"] = dt.replace(tzinfo=opnsense_tz) + lease["expires"] = dt if lease["expires"] < current_time: continue else: diff --git a/aiopnsense/firewall.py b/aiopnsense/firewall.py index 7595a79..60bd4a2 100644 --- a/aiopnsense/firewall.py +++ b/aiopnsense/firewall.py @@ -375,10 +375,7 @@ async def toggle_firewall_rule(self, uuid: str, toggle_on_off: str | None = None return False apply_resp = await self._safe_dict_post(FIREWALL_FILTER_APPLY_ENDPOINT) - if apply_resp.get("status", "").strip() != "OK": - return False - - return True + return apply_resp.get("status", "").strip() == "OK" async def toggle_nat_rule( self, nat_rule_type: str, uuid: str, toggle_on_off: str | None = None @@ -425,10 +422,7 @@ async def toggle_nat_rule( apply_resp = await self._safe_dict_post( f"{FIREWALL_NAT_TOGGLE_RULE_ENDPOINT_PREFIX}{nat_rule_type}{FIREWALL_NAT_APPLY_ENDPOINT_SUFFIX}" ) - if apply_resp.get("status", "").strip() != "OK": - return False - - return True + return apply_resp.get("status", "").strip() == "OK" async def kill_states(self, ip_addr: str) -> MutableMapping[str, Any]: """Kill the active states of the IP address. @@ -513,7 +507,4 @@ async def toggle_alias(self, alias: str, toggle_on_off: str | None = None) -> bo return False reconfigure_resp = await self._safe_dict_post(FIREWALL_ALIAS_RECONFIGURE_ENDPOINT) - if reconfigure_resp.get("status") != "ok": - return False - - return True + return reconfigure_resp.get("status") == "ok" diff --git a/aiopnsense/system.py b/aiopnsense/system.py index b0e0af1..5a93cd8 100644 --- a/aiopnsense/system.py +++ b/aiopnsense/system.py @@ -445,7 +445,7 @@ async def get_device_unique_id(self, expected_id: str | None = None) -> str | No ) return expected_id - device_unique_id = sorted(mac_addresses)[0] + device_unique_id = min(mac_addresses) _LOGGER.debug("[get_device_unique_id] device_unique_id (first): %s", device_unique_id) return device_unique_id @@ -616,9 +616,7 @@ async def system_reboot(self) -> bool: """ response = await self._safe_dict_post(CORE_SYSTEM_REBOOT_ENDPOINT) _LOGGER.debug("[system_reboot] response: %s", response) - if response.get("status", "") == "ok": - return True - return False + return response.get("status", "") == "ok" @_log_errors async def system_halt(self) -> None: @@ -674,9 +672,7 @@ async def send_wol(self, interface: str, mac: str) -> bool: _LOGGER.debug("[send_wol] payload: %s", payload) response = await self._safe_dict_post(WOL_SET_ENDPOINT, payload) _LOGGER.debug("[send_wol] response: %s", response) - if response.get("status", "") == "ok": - return True - return False + return response.get("status", "") == "ok" @_log_errors async def get_notices(self) -> dict[str, Any]: diff --git a/aiopnsense/vnstat.py b/aiopnsense/vnstat.py index c7d2d31..8a59b86 100644 --- a/aiopnsense/vnstat.py +++ b/aiopnsense/vnstat.py @@ -3,7 +3,7 @@ from __future__ import annotations from collections.abc import Mapping, MutableMapping, Sequence -from datetime import date, datetime, timedelta, tzinfo +from datetime import UTC, date, datetime, timedelta, tzinfo import re from typing import Any @@ -291,7 +291,7 @@ def _to_bytes(self, value: str, unit: str) -> int | None: factor = _BYTE_FACTORS.get(unit.upper()) if parsed_value is None or factor is None: return None - return int(round(parsed_value * factor)) + return round(parsed_value * factor) def _to_bits_per_second(self, value: str, unit: str) -> int | None: """Convert vnStat rate strings into integer bits-per-second. @@ -309,7 +309,7 @@ def _to_bits_per_second(self, value: str, unit: str) -> int | None: factor = _RATE_FACTORS.get(unit.upper()) if parsed_value is None or factor is None: return None - return int(round(parsed_value * factor)) + return round(parsed_value * factor) def _pick_daily_row( self, @@ -488,7 +488,7 @@ def _parse_daily_label(self, label: Any) -> date | None: return None for fmt in ("%m/%d/%y", "%Y-%m-%d"): try: - return datetime.strptime(label, fmt).date() + return datetime.strptime(label, fmt).replace(tzinfo=UTC).date() except ValueError: continue return None @@ -508,7 +508,7 @@ def _parse_month_label(self, label: Any) -> tuple[int, int] | None: return None for fmt in ("%Y-%m", "%b '%y", "%B '%y"): try: - parsed = datetime.strptime(label, fmt) + parsed = datetime.strptime(label, fmt).replace(tzinfo=UTC) except ValueError: continue else: diff --git a/docs/source/_ext/opnsense_client_api.py b/docs/source/_ext/opnsense_client_api.py index 4f45925..5ada52d 100644 --- a/docs/source/_ext/opnsense_client_api.py +++ b/docs/source/_ext/opnsense_client_api.py @@ -2,9 +2,10 @@ from __future__ import annotations +from collections.abc import Callable from importlib import import_module import inspect -from typing import Any +from typing import Any, ClassVar from docutils import nodes from docutils.parsers.rst import Directive, directives @@ -59,7 +60,7 @@ class OPNsenseClientAPIDirective(Directive): required_arguments = 1 optional_arguments = 0 final_argument_whitespace = False - option_spec = { + option_spec: ClassVar[dict[str, Callable[[str], str]]] = { "client": directives.unchanged_required, } diff --git a/docs/source/conf.py b/docs/source/conf.py index 6207295..8129bf4 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -2,7 +2,7 @@ from __future__ import annotations -from datetime import datetime +from datetime import UTC, datetime import inspect import logging from pathlib import Path @@ -21,7 +21,7 @@ project: str = "aiopnsense" author: str = "Snuffy2" -copyright: str = f"{datetime.now():%Y}, {author}" +copyright: str = f"{datetime.now(UTC):%Y}, {author}" extensions: list[str] = [ "sphinx.ext.autodoc", diff --git a/scripts/opnsense_api_call.py b/scripts/opnsense_api_call.py index 789511a..0adb027 100755 --- a/scripts/opnsense_api_call.py +++ b/scripts/opnsense_api_call.py @@ -121,14 +121,16 @@ def _load_json_object(payload_text: str, source: str) -> dict[str, Any]: Raises: - ValueError: If the payload is invalid JSON or does not decode to an object. + ValueError: If the payload is invalid JSON or does not decode to an + object. """ try: value = json.loads(payload_text) except json.JSONDecodeError as err: raise ValueError(f"Invalid JSON in {source}: {err}") from err if not isinstance(value, dict): - raise ValueError(f"{source} must be a JSON object") + # ValueError is the CLI's documented payload-validation contract. + raise ValueError(f"{source} must be a JSON object") # noqa: TRY004 return value diff --git a/tests/conftest.py b/tests/conftest.py index 8ba5681..7f09c13 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,7 @@ import contextlib from dataclasses import dataclass from types import TracebackType -from typing import Any, cast +from typing import Any, Self, cast from unittest.mock import MagicMock import aiohttp @@ -45,11 +45,11 @@ def legacy_dnsbl_payload() -> dict[str, Any]: class FakeClientSession: """Minimal fake aiohttp session for unit tests.""" - async def __aenter__(self) -> FakeClientSession: + async def __aenter__(self) -> Self: """Enter async context. Returns: - FakeClientSession: The context-managed instance. + Self: The context-managed instance. """ return self @@ -159,11 +159,11 @@ def __init__( if include_request_info: self.request_info = _FakeRequestInfo(real_url=URL(request_url)) - async def __aenter__(self) -> FakeResponse: + async def __aenter__(self) -> Self: """Enter the asynchronous context. Returns: - FakeResponse: The context-managed instance. + Self: The context-managed instance. """ return self @@ -280,7 +280,7 @@ def __await__(self) -> Generator[None]: """Await. Returns: - Generator[None, None, None]: Iterator used by the await + Generator[None]: Iterator used by the await protocol. """ if False: diff --git a/tests/test_firewall.py b/tests/test_firewall.py index 23a70d9..9f9e4a6 100644 --- a/tests/test_firewall.py +++ b/tests/test_firewall.py @@ -123,8 +123,10 @@ async def test_get_firewall_rules_skips_invalid_rows( }, ), ( - "@uuid;enabled;tcpflags2;tcpflags_any;categories;description\n" - 'rule-2612;1;SA;1;web;"Allow; web"\n', + ( + "@uuid;enabled;tcpflags2;tcpflags_any;categories;description\n" + 'rule-2612;1;SA;1;web;"Allow; web"\n' + ), { "uuid": "rule-2612", "enabled": "1", diff --git a/tests/test_nut.py b/tests/test_nut.py index 8caa859..a52d1de 100644 --- a/tests/test_nut.py +++ b/tests/test_nut.py @@ -178,35 +178,20 @@ async def test_get_nut_ups_status_parses_colon_in_value_and_ignores_invalid_line """ client, _session = make_mock_session_client(make_client) try: + raw_response = """\ +battery.charge: 100 +\x20\x20 +Error: UPS unavailable +ups.message: on battery: replace battery +this-line-is-invalid +ups.load: 12""" client._is_get_endpoint_available = AsyncMock(return_value=True) - client._safe_dict_get = AsyncMock( - return_value={ - "response": "\n".join( - [ - "battery.charge: 100", - " ", - "Error: UPS unavailable", - "ups.message: on battery: replace battery", - "this-line-is-invalid", - "ups.load: 12", - ] - ) - } - ) + client._safe_dict_get = AsyncMock(return_value={"response": raw_response}) nut_status = await client.get_nut_ups_status() assert nut_status == { - "response": "\n".join( - [ - "battery.charge: 100", - " ", - "Error: UPS unavailable", - "ups.message: on battery: replace battery", - "this-line-is-invalid", - "ups.load: 12", - ] - ), + "response": raw_response, "status": { "battery.charge": "100", "ups.message": "on battery: replace battery", diff --git a/tests/test_scripts_live_common.py b/tests/test_scripts_live_common.py index 84a79b4..c225909 100644 --- a/tests/test_scripts_live_common.py +++ b/tests/test_scripts_live_common.py @@ -122,16 +122,13 @@ def test_load_env_file_parses_simple_shell_style_values(tmp_path: Path) -> None: common = load_common_module() env_file = tmp_path / "aiopnsense.env" env_file.write_text( - "\n".join( - [ - "# local credentials", - "AIOPNSENSE_URL=https://firewall.example.test", - "AIOPNSENSE_API_KEY='key value'", - 'AIOPNSENSE_API_SECRET="secret value"', - "AIOPNSENSE_VERIFY_SSL=false # local cert", - "", - ] - ), + """\ +# local credentials +AIOPNSENSE_URL=https://firewall.example.test +AIOPNSENSE_API_KEY='key value' +AIOPNSENSE_API_SECRET="secret value" +AIOPNSENSE_VERIFY_SSL=false # local cert +""", encoding="utf-8", ) @@ -306,15 +303,12 @@ def test_load_config_builds_config_from_fallback_names(tmp_path: Path) -> None: common = load_common_module() env_file = tmp_path / "aiopnsense.env" env_file.write_text( - "\n".join( - [ - "OPNSENSE_URL=https://firewall.example.test", - "OPNSENSE_API_KEY=key", - "OPNSENSE_API_SECRET=secret", - "OPNSENSE_VERIFY_SSL=no", - "", - ] - ), + """\ +OPNSENSE_URL=https://firewall.example.test +OPNSENSE_API_KEY=key +OPNSENSE_API_SECRET=secret +OPNSENSE_VERIFY_SSL=no +""", encoding="utf-8", ) @@ -335,14 +329,11 @@ def test_load_config_defaults_verify_ssl_true(tmp_path: Path) -> None: common = load_common_module() env_file = tmp_path / "aiopnsense.env" env_file.write_text( - "\n".join( - [ - "AIOPNSENSE_URL=https://firewall.example.test", - "AIOPNSENSE_API_KEY=key", - "AIOPNSENSE_API_SECRET=secret", - "", - ] - ), + """\ +AIOPNSENSE_URL=https://firewall.example.test +AIOPNSENSE_API_KEY=key +AIOPNSENSE_API_SECRET=secret +""", encoding="utf-8", ) @@ -360,15 +351,12 @@ def test_load_config_empty_canonical_verify_ssl_is_invalid(tmp_path: Path) -> No common = load_common_module() env_file = tmp_path / "aiopnsense.env" env_file.write_text( - "\n".join( - [ - "AIOPNSENSE_URL=https://firewall.example.test", - "AIOPNSENSE_API_KEY=key", - "AIOPNSENSE_API_SECRET=secret", - "AIOPNSENSE_VERIFY_SSL=", - "", - ] - ), + """\ +AIOPNSENSE_URL=https://firewall.example.test +AIOPNSENSE_API_KEY=key +AIOPNSENSE_API_SECRET=secret +AIOPNSENSE_VERIFY_SSL= +""", encoding="utf-8", ) diff --git a/tests/test_system.py b/tests/test_system.py index 541d262..119e91f 100644 --- a/tests/test_system.py +++ b/tests/test_system.py @@ -305,14 +305,46 @@ async def test_get_resolved_opnsense_timezone_returns_none_on_malformed_datetime @pytest.mark.parametrize( ("datetime_str", "expected_dt", "expected_offset"), [ - ("2026-06-07 12:00:00 ADT", datetime(2026, 6, 7, 12, 0, 0), timedelta(hours=-3)), - ("2026-01-07 12:00:00 AEDT", datetime(2026, 1, 7, 12, 0, 0), timedelta(hours=11)), - ("2026-06-07 12:00:00 CEST", datetime(2026, 6, 7, 12, 0, 0), timedelta(hours=2)), - ("2026-06-07 12:00:00 CDT", datetime(2026, 6, 7, 12, 0, 0), timedelta(hours=-5)), - ("2026-06-07 12:00:00 EEST", datetime(2026, 6, 7, 12, 0, 0), timedelta(hours=3)), - ("2026-06-07 12:00:00 MDT", datetime(2026, 6, 7, 12, 0, 0), timedelta(hours=-6)), - ("2026-01-07 12:00:00 NZDT", datetime(2026, 1, 7, 12, 0, 0), timedelta(hours=13)), - ("2026-06-07 12:00:00 PDT", datetime(2026, 6, 7, 12, 0, 0), timedelta(hours=-7)), + ( + "2026-06-07 12:00:00 ADT", + datetime(2026, 6, 7, 12, 0, 0, tzinfo=UTC), + timedelta(hours=-3), + ), + ( + "2026-01-07 12:00:00 AEDT", + datetime(2026, 1, 7, 12, 0, 0, tzinfo=UTC), + timedelta(hours=11), + ), + ( + "2026-06-07 12:00:00 CEST", + datetime(2026, 6, 7, 12, 0, 0, tzinfo=UTC), + timedelta(hours=2), + ), + ( + "2026-06-07 12:00:00 CDT", + datetime(2026, 6, 7, 12, 0, 0, tzinfo=UTC), + timedelta(hours=-5), + ), + ( + "2026-06-07 12:00:00 EEST", + datetime(2026, 6, 7, 12, 0, 0, tzinfo=UTC), + timedelta(hours=3), + ), + ( + "2026-06-07 12:00:00 MDT", + datetime(2026, 6, 7, 12, 0, 0, tzinfo=UTC), + timedelta(hours=-6), + ), + ( + "2026-01-07 12:00:00 NZDT", + datetime(2026, 1, 7, 12, 0, 0, tzinfo=UTC), + timedelta(hours=13), + ), + ( + "2026-06-07 12:00:00 PDT", + datetime(2026, 6, 7, 12, 0, 0, tzinfo=UTC), + timedelta(hours=-7), + ), ], ) async def test_get_opnsense_timezone_supports_known_daylight_abbreviations( @@ -326,7 +358,8 @@ async def test_get_opnsense_timezone_supports_known_daylight_abbreviations( Args: make_client (ClientType): Fixture factory returning ``OPNsenseClient`` instances. datetime_str (str): Datetime string parsed from mocked API output. - expected_dt (datetime): Naive datetime used to evaluate the resolved timezone offset. + expected_dt (datetime): Datetime whose wall time is used to evaluate the + resolved timezone offset. expected_offset (timedelta): Expected UTC offset for the parsed timezone. Returns: From 99d595e672c0721c7cc992d42c7093d5d9f4a853 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Sat, 8 Aug 2026 20:03:20 -0400 Subject: [PATCH 16/16] test: document parameterized CLI error helpers --- tests/test_scripts_aiopnsense_dump.py | 5 +++++ tests/test_scripts_opnsense_api_call.py | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/test_scripts_aiopnsense_dump.py b/tests/test_scripts_aiopnsense_dump.py index 728f46c..98dde0b 100644 --- a/tests/test_scripts_aiopnsense_dump.py +++ b/tests/test_scripts_aiopnsense_dump.py @@ -970,6 +970,11 @@ def test_main_turns_errors_into_system_exit( module = load_dump_module() async def raise_error() -> None: + """Raise the failure generated by the test case's factory. + + Raises: + error_factory: The parameterized failure generated for this test case. + """ raise error_factory(module) monkeypatch.setattr(module, "async_main", raise_error) diff --git a/tests/test_scripts_opnsense_api_call.py b/tests/test_scripts_opnsense_api_call.py index 99a7be2..586af25 100644 --- a/tests/test_scripts_opnsense_api_call.py +++ b/tests/test_scripts_opnsense_api_call.py @@ -834,6 +834,11 @@ def test_main_converts_expected_errors_to_system_exit( module = load_api_call_module() async def raise_error() -> None: + """Raise the failure generated by the test case's factory. + + Raises: + exception_factory: The parameterized failure generated for this test case. + """ raise exception_factory(module) monkeypatch.setattr(module, "async_main", raise_error)