From 19a6266f4e8c14e20a3b29792443b83e2fe4b4ad Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Mon, 20 Jul 2026 20:11:08 -0400 Subject: [PATCH 1/5] Add timezone to Speedtest dates --- aiopnsense/speedtest.py | 24 ++++++++++++++- tests/test_speedtest.py | 67 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/aiopnsense/speedtest.py b/aiopnsense/speedtest.py index 42548a0..ecf4cf0 100644 --- a/aiopnsense/speedtest.py +++ b/aiopnsense/speedtest.py @@ -15,6 +15,7 @@ from __future__ import annotations from collections.abc import MutableMapping +from datetime import datetime from typing import Any from ._typing import AiopnsenseClientProtocol @@ -58,7 +59,7 @@ async def get_speedtest(self) -> dict[str, Any]: server_name = latest_result.get("server") if not isinstance(server_name, str): server_name = None - date = latest_result.get("date") if isinstance(latest_result.get("date"), str) else None + date = await self._normalize_speedtest_date(latest_result.get("date")) url = latest_result.get("url") if isinstance(latest_result.get("url"), str) else None samples = try_to_int(show_stat.get("samples")) @@ -98,6 +99,27 @@ async def get_speedtest(self) -> dict[str, Any]: } return output + async def _normalize_speedtest_date(self, value: object) -> str | None: + """Return a timezone-aware ISO 8601 Speedtest timestamp. + + Args: + value (object): Raw date value returned by the Speedtest plugin. + + Returns: + str | None: ISO 8601 timestamp including a UTC offset, or ``None`` + when the value is missing or malformed. + """ + if not isinstance(value, str): + return None + try: + parsed_date = datetime.fromisoformat(value) + except ValueError: + _LOGGER.debug("Failed to parse Speedtest date: %s", value) + return None + if parsed_date.tzinfo is None: + parsed_date = parsed_date.replace(tzinfo=await self._get_opnsense_timezone()) + return parsed_date.isoformat() + def _parse_showlog_latest(self, show_log: object) -> dict[str, Any]: """Normalize the newest row returned by the Speedtest ``showlog`` endpoint. diff --git a/tests/test_speedtest.py b/tests/test_speedtest.py index 2fa15b8..3ab2c91 100644 --- a/tests/test_speedtest.py +++ b/tests/test_speedtest.py @@ -1,6 +1,7 @@ """Tests for `aiopnsense.speedtest`.""" from collections.abc import Callable +from datetime import timedelta, timezone from unittest.mock import AsyncMock, call import pytest @@ -58,6 +59,7 @@ async def test_get_speedtest_normalizes_latest_and_stat_payloads(make_client) -> "upload": {"avg": 706.7, "min": 1.54, "max": 890.32}, } ) + client._get_opnsense_timezone = AsyncMock(return_value=timezone(timedelta(hours=-4))) result = await client.get_speedtest() @@ -65,6 +67,7 @@ async def test_get_speedtest_normalizes_latest_and_stat_payloads(make_client) -> assert result["last"]["download"]["value"] == 836.05 assert result["last"]["download"]["server_id"] == "72800" assert result["last"]["download"]["server"] == "RippleFiber, Newark, NJ" + assert result["last"]["download"]["date"] == "2026-03-14T03:09:45-04:00" assert result["average"]["download"]["value"] == 723.83 assert result["average"]["download"]["min"] == 4.18 assert result["average"]["download"]["max"] == 942.02 @@ -125,6 +128,7 @@ async def test_get_speedtest_probes_showstat_before_fetching_optional_payload( ] ) client._safe_dict_get = AsyncMock(return_value={}) + client._get_opnsense_timezone = AsyncMock(return_value=timezone(timedelta(hours=-4))) result = await client.get_speedtest() @@ -133,6 +137,7 @@ async def test_get_speedtest_probes_showstat_before_fetching_optional_payload( call("/api/speedtest/service/showlog"), call("/api/speedtest/service/showstat"), ] + client._get_opnsense_timezone.assert_awaited_once_with() client._safe_list_get.assert_awaited_once_with("/api/speedtest/service/showlog") if showstat_available: @@ -149,6 +154,68 @@ async def test_get_speedtest_probes_showstat_before_fetching_optional_payload( await client.async_close() +@pytest.mark.asyncio +async def test_get_speedtest_preserves_timezone_aware_date(make_client) -> None: + """get_speedtest should preserve an existing timestamp UTC offset.""" + client, _session = make_mock_session_client(make_client) + try: + client._is_get_endpoint_available = AsyncMock(side_effect=[True, False]) + client._safe_list_get = AsyncMock( + return_value=[ + [ + "2026-03-14T03:09:45+01:30", + "198.51.100.10", + "72800", + "Test ISP", + "United States", + "1", + "2", + "3", + "https://www.speedtest.net/result/c/abc", + ] + ] + ) + client._get_opnsense_timezone = AsyncMock() + + result = await client.get_speedtest() + + assert result["last"]["download"]["date"] == "2026-03-14T03:09:45+01:30" + client._get_opnsense_timezone.assert_not_awaited() + finally: + await client.async_close() + + +@pytest.mark.asyncio +async def test_get_speedtest_drops_malformed_date(make_client) -> None: + """get_speedtest should omit malformed timestamp values.""" + client, _session = make_mock_session_client(make_client) + try: + client._is_get_endpoint_available = AsyncMock(side_effect=[True, False]) + client._safe_list_get = AsyncMock( + return_value=[ + [ + "not-a-date", + "198.51.100.10", + "72800", + "Test ISP", + "United States", + "1", + "2", + "3", + "https://www.speedtest.net/result/c/abc", + ] + ] + ) + client._get_opnsense_timezone = AsyncMock() + + result = await client.get_speedtest() + + assert result["last"]["download"]["date"] is None + client._get_opnsense_timezone.assert_not_awaited() + finally: + await client.async_close() + + @pytest.mark.asyncio async def test_get_speedtest_normalizes_malformed_payloads(make_client) -> None: """get_speedtest should coerce malformed or missing values to None safely.""" From b11d35718db000117ff6d5a09e778317c367c36e Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Mon, 20 Jul 2026 20:21:18 -0400 Subject: [PATCH 2/5] Normalize Speedtest period dates --- aiopnsense/helpers.py | 25 ++++++++++++++++++++++++- aiopnsense/speedtest.py | 37 +++++++++++-------------------------- tests/test_helpers.py | 31 ++++++++++++++++++++++++++++++- tests/test_speedtest.py | 12 +++++++----- 4 files changed, 72 insertions(+), 33 deletions(-) diff --git a/aiopnsense/helpers.py b/aiopnsense/helpers.py index 76ec69c..9f61702 100644 --- a/aiopnsense/helpers.py +++ b/aiopnsense/helpers.py @@ -2,7 +2,7 @@ import asyncio from collections.abc import Callable, MutableMapping -from datetime import UTC, datetime +from datetime import UTC, datetime, tzinfo from functools import wraps import ipaddress import logging @@ -294,6 +294,29 @@ def timestamp_to_datetime(timestamp: int | None) -> datetime | None: return utc_datetime.astimezone() +def normalize_datetime(value: object, default_tz: tzinfo) -> str | None: + """Return a timezone-aware ISO 8601 datetime string. + + Args: + value (object): Raw datetime value to normalize. + default_tz (tzinfo): Timezone assigned when ``value`` is naive. + + Returns: + str | None: ISO 8601 timestamp including a UTC offset, or ``None`` + when the value is missing or malformed. + """ + if not isinstance(value, str): + return None + try: + parsed_date = datetime.fromisoformat(value) + except ValueError: + _LOGGER.debug("Failed to parse datetime: %s", value) + return None + if parsed_date.tzinfo is None: + parsed_date = parsed_date.replace(tzinfo=default_tz) + return parsed_date.isoformat() + + def try_to_int(value: Any | None, retval: int | None = None) -> int | None: """Convert a value to ``int`` and return a fallback on conversion failure. diff --git a/aiopnsense/speedtest.py b/aiopnsense/speedtest.py index ecf4cf0..1ee3381 100644 --- a/aiopnsense/speedtest.py +++ b/aiopnsense/speedtest.py @@ -15,11 +15,10 @@ from __future__ import annotations from collections.abc import MutableMapping -from datetime import datetime from typing import Any from ._typing import AiopnsenseClientProtocol -from .helpers import _LOGGER, _log_errors, try_to_float, try_to_int +from .helpers import _LOGGER, _log_errors, normalize_datetime, try_to_float, try_to_int SPEEDTEST_SHOW_LOG_ENDPOINT = "/api/speedtest/service/showlog" SPEEDTEST_SHOW_STAT_ENDPOINT = "/api/speedtest/service/showstat" @@ -59,13 +58,20 @@ async def get_speedtest(self) -> dict[str, Any]: server_name = latest_result.get("server") if not isinstance(server_name, str): server_name = None - date = await self._normalize_speedtest_date(latest_result.get("date")) + opnsense_tz = await self._get_opnsense_timezone() + date = normalize_datetime(latest_result.get("date"), opnsense_tz) url = latest_result.get("url") if isinstance(latest_result.get("url"), str) else None samples = try_to_int(show_stat.get("samples")) period = show_stat.get("period", {}) - oldest = period.get("oldest") if isinstance(period, MutableMapping) else None - youngest = period.get("youngest") if isinstance(period, MutableMapping) else None + oldest = normalize_datetime( + period.get("oldest") if isinstance(period, MutableMapping) else None, + opnsense_tz, + ) + youngest = normalize_datetime( + period.get("youngest") if isinstance(period, MutableMapping) else None, + opnsense_tz, + ) output: dict[str, Any] = { "available": True, @@ -99,27 +105,6 @@ async def get_speedtest(self) -> dict[str, Any]: } return output - async def _normalize_speedtest_date(self, value: object) -> str | None: - """Return a timezone-aware ISO 8601 Speedtest timestamp. - - Args: - value (object): Raw date value returned by the Speedtest plugin. - - Returns: - str | None: ISO 8601 timestamp including a UTC offset, or ``None`` - when the value is missing or malformed. - """ - if not isinstance(value, str): - return None - try: - parsed_date = datetime.fromisoformat(value) - except ValueError: - _LOGGER.debug("Failed to parse Speedtest date: %s", value) - return None - if parsed_date.tzinfo is None: - parsed_date = parsed_date.replace(tzinfo=await self._get_opnsense_timezone()) - return parsed_date.isoformat() - def _parse_showlog_latest(self, show_log: object) -> dict[str, Any]: """Normalize the newest row returned by the Speedtest ``showlog`` endpoint. diff --git a/tests/test_helpers.py b/tests/test_helpers.py index a3005c4..3ba7343 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,7 +1,7 @@ """Tests for `aiopnsense.helpers` utility and decorator helpers.""" from collections.abc import Callable -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta, timezone import inspect import logging from unittest.mock import MagicMock @@ -89,6 +89,35 @@ def test_timestamp_to_datetime() -> None: assert aiopnsense_helpers.timestamp_to_datetime(None) is None +@pytest.mark.parametrize( + ("value", "expected"), + [ + ("2026-03-14T03:09:45", "2026-03-14T03:09:45-04:00"), + ("2023-01-22 00:29:00", "2023-01-22T00:29:00-04:00"), + ("2026-03-14T03:09:45+01:30", "2026-03-14T03:09:45+01:30"), + ("not-a-date", None), + (12345, None), + ], +) +def test_normalize_datetime(value: object, expected: str | None) -> None: + """Normalize naive and aware datetimes while rejecting malformed values. + + Args: + value (object): Raw datetime value under test. + expected (str | None): Expected timezone-aware ISO result. + + Returns: + None: This test validates helper output via assertions. + """ + assert ( + aiopnsense_helpers.normalize_datetime( + value, + timezone(timedelta(hours=-4)), + ) + == expected + ) + + @pytest.mark.parametrize( ("firmware_version", "expected"), [ diff --git a/tests/test_speedtest.py b/tests/test_speedtest.py index 3ab2c91..944ee5d 100644 --- a/tests/test_speedtest.py +++ b/tests/test_speedtest.py @@ -3,6 +3,7 @@ from collections.abc import Callable from datetime import timedelta, timezone from unittest.mock import AsyncMock, call +from zoneinfo import ZoneInfo import pytest @@ -59,7 +60,7 @@ async def test_get_speedtest_normalizes_latest_and_stat_payloads(make_client) -> "upload": {"avg": 706.7, "min": 1.54, "max": 890.32}, } ) - client._get_opnsense_timezone = AsyncMock(return_value=timezone(timedelta(hours=-4))) + client._get_opnsense_timezone = AsyncMock(return_value=ZoneInfo("America/New_York")) result = await client.get_speedtest() @@ -72,8 +73,8 @@ async def test_get_speedtest_normalizes_latest_and_stat_payloads(make_client) -> assert result["average"]["download"]["min"] == 4.18 assert result["average"]["download"]["max"] == 942.02 assert result["average"]["download"]["samples"] == 10717 - assert result["average"]["download"]["oldest"] == "2023-01-22 00:29:00" - assert result["average"]["download"]["youngest"] == "2026-03-14 03:09:45" + assert result["average"]["download"]["oldest"] == "2023-01-22T00:29:00-05:00" + assert result["average"]["download"]["youngest"] == "2026-03-14T03:09:45-04:00" finally: await client.async_close() @@ -180,7 +181,7 @@ async def test_get_speedtest_preserves_timezone_aware_date(make_client) -> None: result = await client.get_speedtest() assert result["last"]["download"]["date"] == "2026-03-14T03:09:45+01:30" - client._get_opnsense_timezone.assert_not_awaited() + client._get_opnsense_timezone.assert_awaited_once_with() finally: await client.async_close() @@ -211,7 +212,7 @@ async def test_get_speedtest_drops_malformed_date(make_client) -> None: result = await client.get_speedtest() assert result["last"]["download"]["date"] is None - client._get_opnsense_timezone.assert_not_awaited() + client._get_opnsense_timezone.assert_awaited_once_with() finally: await client.async_close() @@ -246,6 +247,7 @@ async def test_get_speedtest_normalizes_malformed_payloads(make_client) -> None: "latency": ["bad-latency-shape"], } ) + client._get_opnsense_timezone = AsyncMock(return_value=timezone(timedelta(hours=-4))) result = await client.get_speedtest() From 0f29825ed29eba43d5f901ac3efc509d5acf6830 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Mon, 20 Jul 2026 20:41:34 -0400 Subject: [PATCH 3/5] Avoid host timezone fallback for Speedtest --- aiopnsense/_typing.py | 4 ++ aiopnsense/helpers.py | 8 +++- aiopnsense/speedtest.py | 2 +- aiopnsense/system.py | 88 ++++++++++++++++++++++++++++------------- tests/test_helpers.py | 14 +++++++ tests/test_speedtest.py | 63 +++++++++++++++++++++++++---- tests/test_system.py | 54 +++++++++++++++++++++++++ 7 files changed, 195 insertions(+), 38 deletions(-) diff --git a/aiopnsense/_typing.py b/aiopnsense/_typing.py index 2f4d7af..db2c1f7 100644 --- a/aiopnsense/_typing.py +++ b/aiopnsense/_typing.py @@ -44,6 +44,10 @@ async def _safe_list_post( self, path: str, payload: MutableMapping[str, Any] | None = None ) -> list: ... + async def _get_resolved_opnsense_timezone( + self, datetime_str: str | None = None + ) -> tzinfo | None: ... + async def _get_opnsense_timezone(self, datetime_str: str | None = None) -> tzinfo: ... async def get_host_firmware_version(self) -> str | None: ... diff --git a/aiopnsense/helpers.py b/aiopnsense/helpers.py index 9f61702..b25f776 100644 --- a/aiopnsense/helpers.py +++ b/aiopnsense/helpers.py @@ -294,12 +294,14 @@ def timestamp_to_datetime(timestamp: int | None) -> datetime | None: return utc_datetime.astimezone() -def normalize_datetime(value: object, default_tz: tzinfo) -> str | None: +def normalize_datetime(value: object, default_tz: tzinfo | None) -> str | None: """Return a timezone-aware ISO 8601 datetime string. Args: value (object): Raw datetime value to normalize. - default_tz (tzinfo): Timezone assigned when ``value`` is naive. + default_tz (tzinfo | None): Timezone assigned when ``value`` is naive. + When ``None``, naive values are returned as ``None`` so callers can + distinguish missing or unresolvable timezones. Returns: str | None: ISO 8601 timestamp including a UTC offset, or ``None`` @@ -313,6 +315,8 @@ def normalize_datetime(value: object, default_tz: tzinfo) -> str | None: _LOGGER.debug("Failed to parse datetime: %s", value) return None if parsed_date.tzinfo is None: + if default_tz is None: + return None parsed_date = parsed_date.replace(tzinfo=default_tz) return parsed_date.isoformat() diff --git a/aiopnsense/speedtest.py b/aiopnsense/speedtest.py index 1ee3381..12ac6be 100644 --- a/aiopnsense/speedtest.py +++ b/aiopnsense/speedtest.py @@ -58,7 +58,7 @@ async def get_speedtest(self) -> dict[str, Any]: server_name = latest_result.get("server") if not isinstance(server_name, str): server_name = None - opnsense_tz = await self._get_opnsense_timezone() + opnsense_tz = await self._get_resolved_opnsense_timezone() date = normalize_datetime(latest_result.get("date"), opnsense_tz) url = latest_result.get("url") if isinstance(latest_result.get("url"), str) else None diff --git a/aiopnsense/system.py b/aiopnsense/system.py index e2d120e..c76e5d5 100644 --- a/aiopnsense/system.py +++ b/aiopnsense/system.py @@ -330,49 +330,83 @@ def _get_local_timezone(self) -> tzinfo: """ return timezone(datetime.now().astimezone().utcoffset() or timedelta()) - async def _get_opnsense_timezone(self, datetime_str: str | None = None) -> tzinfo: - """Resolve timezone information from OPNsense system time data. + def _parse_opnsense_tz(self, datetime_str: str | None) -> tzinfo | None: + """Parse a timezone from a system timestamp string. Args: - datetime_str (str | None, optional): Datetime string parsed from API output. + datetime_str (str | None): Raw timestamp value from the system + endpoint. Returns: - tzinfo: Resolved timezone object for OPNsense system data. + tzinfo | None: Parsed timezone when available; ``None`` when the + string is missing, naive, or cannot be parsed. + """ + if not datetime_str: + return None + + try: + with warnings.catch_warnings(): + warnings.simplefilter("error", UnknownTimezoneWarning) + parsed_time = parse(datetime_str, tzinfos=AMBIGUOUS_TZINFOS) + if parsed_time.tzinfo is not None: + return parsed_time.tzinfo + _LOGGER.debug("No timezone data in OPNsense datetime '%s'", datetime_str) + except (ValueError, TypeError, ParserError, UnknownTimezoneWarning) as err: + _LOGGER.debug( + "Failed to parse OPNsense timezone from datetime '%s': %s: %s", + datetime_str, + type(err).__name__, + err, + ) + return None + + async def _get_resolved_opnsense_timezone( + self, datetime_str: str | None = None + ) -> tzinfo | None: + """Resolve OPNsense timezone only when it can be determined. + + Args: + datetime_str (str | None, optional): Datetime string from OPNsense to + parse instead of reading the system-time endpoint. + + Returns: + tzinfo | None: Resolved firewall timezone, or ``None`` when the + timezone cannot be resolved. """ if datetime_str is None: if not await self._is_get_endpoint_available(SYSTEM_TIME_ENDPOINT): _LOGGER.debug("System time endpoint unavailable for timezone resolution") - return self._get_local_timezone() + return None try: - datetime_raw = (await self._safe_dict_get(SYSTEM_TIME_ENDPOINT)).get("datetime") + datetime_payload = await self._safe_dict_get(SYSTEM_TIME_ENDPOINT) except (OPNsenseError, aiohttp.ClientError, TimeoutError) as err: _LOGGER.debug( "Failed to fetch OPNsense system time for timezone resolution: %s: %s", type(err).__name__, err, ) - return self._get_local_timezone() - datetime_str = datetime_raw if isinstance(datetime_raw, str) else None + return None + datetime_value = ( + datetime_payload.get("datetime") + if isinstance(datetime_payload, MutableMapping) + else None + ) + datetime_str = datetime_value if isinstance(datetime_value, str) else None - if datetime_str: - try: - with warnings.catch_warnings(): - warnings.simplefilter("error", UnknownTimezoneWarning) - parsed_time = parse(datetime_str, tzinfos=AMBIGUOUS_TZINFOS) - if parsed_time.tzinfo is not None: - return parsed_time.tzinfo - _LOGGER.debug( - "No timezone data in OPNsense datetime '%s', using local fallback", - datetime_str, - ) - except (ValueError, TypeError, ParserError, UnknownTimezoneWarning) as err: - _LOGGER.debug( - "Failed to parse OPNsense timezone from datetime '%s': %s: %s", - datetime_str, - type(err).__name__, - err, - ) - return self._get_local_timezone() + return self._parse_opnsense_tz(datetime_str) + + async def _get_opnsense_timezone(self, datetime_str: str | None = None) -> tzinfo: + """Resolve timezone information from OPNsense system time data. + + Args: + datetime_str (str | None, optional): Datetime string parsed from API output. + + Returns: + tzinfo: Resolved timezone object for OPNsense system data. + """ + return ( + await self._get_resolved_opnsense_timezone(datetime_str) or self._get_local_timezone() + ) @_log_errors async def get_device_unique_id(self, expected_id: str | None = None) -> str | None: diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 3ba7343..c0e1ce8 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -118,6 +118,20 @@ def test_normalize_datetime(value: object, expected: str | None) -> None: ) +@pytest.mark.parametrize( + ("value", "default_tz", "expected"), + [ + ("2026-03-14T03:09:45", None, None), + ("2026-03-14T03:09:45+01:30", None, "2026-03-14T03:09:45+01:30"), + ], +) +def test_normalize_datetime_without_default_tz( + value: object, default_tz: timezone | None, expected: str | None +) -> None: + """Return ``None`` for naive values when no default timezone is available.""" + assert aiopnsense_helpers.normalize_datetime(value, default_tz) == expected + + @pytest.mark.parametrize( ("firmware_version", "expected"), [ diff --git a/tests/test_speedtest.py b/tests/test_speedtest.py index 944ee5d..2bd1563 100644 --- a/tests/test_speedtest.py +++ b/tests/test_speedtest.py @@ -60,7 +60,9 @@ async def test_get_speedtest_normalizes_latest_and_stat_payloads(make_client) -> "upload": {"avg": 706.7, "min": 1.54, "max": 890.32}, } ) - client._get_opnsense_timezone = AsyncMock(return_value=ZoneInfo("America/New_York")) + client._get_resolved_opnsense_timezone = AsyncMock( + return_value=ZoneInfo("America/New_York") + ) result = await client.get_speedtest() @@ -129,7 +131,9 @@ async def test_get_speedtest_probes_showstat_before_fetching_optional_payload( ] ) client._safe_dict_get = AsyncMock(return_value={}) - client._get_opnsense_timezone = AsyncMock(return_value=timezone(timedelta(hours=-4))) + client._get_resolved_opnsense_timezone = AsyncMock( + return_value=timezone(timedelta(hours=-4)) + ) result = await client.get_speedtest() @@ -138,7 +142,7 @@ async def test_get_speedtest_probes_showstat_before_fetching_optional_payload( call("/api/speedtest/service/showlog"), call("/api/speedtest/service/showstat"), ] - client._get_opnsense_timezone.assert_awaited_once_with() + client._get_resolved_opnsense_timezone.assert_awaited_once_with() client._safe_list_get.assert_awaited_once_with("/api/speedtest/service/showlog") if showstat_available: @@ -176,12 +180,12 @@ async def test_get_speedtest_preserves_timezone_aware_date(make_client) -> None: ] ] ) - client._get_opnsense_timezone = AsyncMock() + client._get_resolved_opnsense_timezone = AsyncMock() result = await client.get_speedtest() assert result["last"]["download"]["date"] == "2026-03-14T03:09:45+01:30" - client._get_opnsense_timezone.assert_awaited_once_with() + client._get_resolved_opnsense_timezone.assert_awaited_once_with() finally: await client.async_close() @@ -207,12 +211,53 @@ async def test_get_speedtest_drops_malformed_date(make_client) -> None: ] ] ) - client._get_opnsense_timezone = AsyncMock() + client._get_resolved_opnsense_timezone = AsyncMock() result = await client.get_speedtest() assert result["last"]["download"]["date"] is None - client._get_opnsense_timezone.assert_awaited_once_with() + client._get_resolved_opnsense_timezone.assert_awaited_once_with() + finally: + await client.async_close() + + +@pytest.mark.asyncio +async def test_get_speedtest_preserves_aware_date_and_drops_naive_periods_when_timezone_unresolved( + make_client, +) -> None: + """When OPNsense timezone is unresolved, keep aware date fields but drop naive period fields.""" + client, _session = make_mock_session_client(make_client) + try: + client._is_get_endpoint_available = AsyncMock(side_effect=[True, False]) + client._safe_list_get = AsyncMock( + return_value=[ + [ + "2026-03-14T03:09:45+01:30", + "198.51.100.10", + "72800", + "Test ISP", + "United States", + "1", + "2", + "3", + "https://www.speedtest.net/result/c/abc", + ] + ] + ) + client._safe_dict_get = AsyncMock( + return_value={ + "samples": 10717, + "period": {"oldest": "2023-01-22 00:29:00", "youngest": "2026-03-14 03:09:45"}, + } + ) + client._get_resolved_opnsense_timezone = AsyncMock(return_value=None) + + result = await client.get_speedtest() + + assert result["last"]["download"]["date"] == "2026-03-14T03:09:45+01:30" + assert result["average"]["download"]["oldest"] is None + assert result["average"]["download"]["youngest"] is None + client._get_resolved_opnsense_timezone.assert_awaited_once_with() finally: await client.async_close() @@ -247,7 +292,9 @@ async def test_get_speedtest_normalizes_malformed_payloads(make_client) -> None: "latency": ["bad-latency-shape"], } ) - client._get_opnsense_timezone = AsyncMock(return_value=timezone(timedelta(hours=-4))) + client._get_resolved_opnsense_timezone = AsyncMock( + return_value=timezone(timedelta(hours=-4)) + ) result = await client.get_speedtest() diff --git a/tests/test_system.py b/tests/test_system.py index 6e8f887..9021695 100644 --- a/tests/test_system.py +++ b/tests/test_system.py @@ -207,6 +207,60 @@ async def test_get_opnsense_timezone_fallback_for_mapped_error(make_client: Clie await client.async_close() +@pytest.mark.asyncio +async def test_get_resolved_opnsense_timezone_returns_none_on_endpoint_unavailable( + make_client: ClientType, +) -> None: + """Verify resolved-only timezone lookup returns ``None`` when endpoint is unavailable.""" + client, _session = make_mock_session_client(make_client) + try: + client._is_get_endpoint_available = AsyncMock(return_value=False) + + resolved_tz = await client._get_resolved_opnsense_timezone() + + assert resolved_tz is None + client._is_get_endpoint_available.assert_awaited_once_with( + "/api/diagnostics/system/system_time" + ) + finally: + await client.async_close() + + +@pytest.mark.asyncio +async def test_get_resolved_opnsense_timezone_returns_none_on_fetch_error( + make_client: ClientType, +) -> None: + """Verify resolved-only timezone lookup returns ``None`` when fetch fails.""" + client, _session = make_mock_session_client(make_client) + try: + client._is_get_endpoint_available = AsyncMock(return_value=True) + client._safe_dict_get = AsyncMock(side_effect=aiohttp.ClientError("transient fetch error")) + + resolved_tz = await client._get_resolved_opnsense_timezone() + + assert resolved_tz is None + client._safe_dict_get.assert_awaited_once_with("/api/diagnostics/system/system_time") + finally: + await client.async_close() + + +@pytest.mark.asyncio +async def test_get_resolved_opnsense_timezone_returns_none_on_malformed_datetime( + make_client: ClientType, +) -> None: + """Verify resolved-only timezone lookup returns ``None`` for malformed data.""" + client, _session = make_mock_session_client(make_client) + try: + client._is_get_endpoint_available = AsyncMock(return_value=True) + client._safe_dict_get = AsyncMock(return_value={"datetime": "not-a-datetime"}) + + resolved_tz = await client._get_resolved_opnsense_timezone() + + assert resolved_tz is None + finally: + await client.async_close() + + @pytest.mark.asyncio @pytest.mark.parametrize( ("datetime_str", "expected_dt", "expected_offset"), From 71384a29836d6f3b66a3d3af63bc04a344bb928a Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Mon, 20 Jul 2026 20:48:23 -0400 Subject: [PATCH 4/5] Simplify Speedtest result normalization --- aiopnsense/speedtest.py | 54 ++++++++++++----------------------------- tests/test_helpers.py | 35 ++++++++++---------------- 2 files changed, 29 insertions(+), 60 deletions(-) diff --git a/aiopnsense/speedtest.py b/aiopnsense/speedtest.py index 12ac6be..c460244 100644 --- a/aiopnsense/speedtest.py +++ b/aiopnsense/speedtest.py @@ -18,7 +18,7 @@ from typing import Any from ._typing import AiopnsenseClientProtocol -from .helpers import _LOGGER, _log_errors, normalize_datetime, try_to_float, try_to_int +from .helpers import _LOGGER, _log_errors, dict_get, normalize_datetime, try_to_float, try_to_int SPEEDTEST_SHOW_LOG_ENDPOINT = "/api/speedtest/service/showlog" SPEEDTEST_SHOW_STAT_ENDPOINT = "/api/speedtest/service/showstat" @@ -53,25 +53,14 @@ async def get_speedtest(self) -> dict[str, Any]: show_stat = {} server_id = latest_result.get("server_id") - if not isinstance(server_id, str): - server_id = None server_name = latest_result.get("server") - if not isinstance(server_name, str): - server_name = None opnsense_tz = await self._get_resolved_opnsense_timezone() date = normalize_datetime(latest_result.get("date"), opnsense_tz) url = latest_result.get("url") if isinstance(latest_result.get("url"), str) else None - samples = try_to_int(show_stat.get("samples")) - period = show_stat.get("period", {}) - oldest = normalize_datetime( - period.get("oldest") if isinstance(period, MutableMapping) else None, - opnsense_tz, - ) - youngest = normalize_datetime( - period.get("youngest") if isinstance(period, MutableMapping) else None, - opnsense_tz, - ) + samples = try_to_int(dict_get(show_stat, "samples")) + oldest = normalize_datetime(dict_get(show_stat, "period.oldest"), opnsense_tz) + youngest = normalize_datetime(dict_get(show_stat, "period.youngest"), opnsense_tz) output: dict[str, Any] = { "available": True, @@ -80,8 +69,6 @@ async def get_speedtest(self) -> dict[str, Any]: } for metric in ("download", "upload", "latency"): recent_value = try_to_float(latest_result.get(metric)) - stat_metric = show_stat.get(metric, {}) - output["last"][metric] = { "value": recent_value, "date": date, @@ -90,15 +77,9 @@ async def get_speedtest(self) -> dict[str, Any]: "url": url, } output["average"][metric] = { - "value": try_to_float( - stat_metric.get("avg") if isinstance(stat_metric, MutableMapping) else None - ), - "min": try_to_float( - stat_metric.get("min") if isinstance(stat_metric, MutableMapping) else None - ), - "max": try_to_float( - stat_metric.get("max") if isinstance(stat_metric, MutableMapping) else None - ), + "value": try_to_float(dict_get(show_stat, f"{metric}.avg")), + "min": try_to_float(dict_get(show_stat, f"{metric}.min")), + "max": try_to_float(dict_get(show_stat, f"{metric}.max")), "oldest": oldest, "youngest": youngest, "samples": samples, @@ -122,19 +103,16 @@ def _parse_showlog_latest(self, show_log: object) -> dict[str, Any]: if not isinstance(latest, list) or len(latest) < 9: return {} - raw_server_id = latest[2].strip() if isinstance(latest[2], str) else latest[2] - if isinstance(raw_server_id, bool) or not isinstance(raw_server_id, int | str): + server_id = ( + str(latest[2]).strip() + if isinstance(latest[2], int | str) and not isinstance(latest[2], bool) + else None + ) + if server_id == "": server_id = None - else: - server_id = str(raw_server_id) - if not server_id: - server_id = None - - raw_server = latest[3].strip() if isinstance(latest[3], str) else latest[3] - if not isinstance(raw_server, str): - server = None - else: - server = raw_server or None + raw_server = latest[3] + server = raw_server.strip() if isinstance(raw_server, str) else None + server = server or None return { "date": latest[0], "server_id": server_id, diff --git a/tests/test_helpers.py b/tests/test_helpers.py index c0e1ce8..5d83400 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -90,20 +90,25 @@ def test_timestamp_to_datetime() -> None: @pytest.mark.parametrize( - ("value", "expected"), + ("value", "default_tz", "expected"), [ - ("2026-03-14T03:09:45", "2026-03-14T03:09:45-04:00"), - ("2023-01-22 00:29:00", "2023-01-22T00:29:00-04:00"), - ("2026-03-14T03:09:45+01:30", "2026-03-14T03:09:45+01:30"), - ("not-a-date", None), - (12345, None), + ("2026-03-14T03:09:45", timezone(timedelta(hours=-4)), "2026-03-14T03:09:45-04:00"), + ("2023-01-22 00:29:00", timezone(timedelta(hours=-4)), "2023-01-22T00:29:00-04:00"), + ("2026-03-14T03:09:45+01:30", timezone(timedelta(hours=-4)), "2026-03-14T03:09:45+01:30"), + ("2026-03-14T03:09:45", None, None), + ("2026-03-14T03:09:45+01:30", None, "2026-03-14T03:09:45+01:30"), + ("not-a-date", timezone(timedelta(hours=-4)), None), + (12345, timezone(timedelta(hours=-4)), None), ], ) -def test_normalize_datetime(value: object, expected: str | None) -> None: +def test_normalize_datetime( + value: object, default_tz: timezone | None, expected: str | None +) -> None: """Normalize naive and aware datetimes while rejecting malformed values. Args: value (object): Raw datetime value under test. + default_tz (timezone | None): Fallback timezone for naive values. expected (str | None): Expected timezone-aware ISO result. Returns: @@ -112,26 +117,12 @@ def test_normalize_datetime(value: object, expected: str | None) -> None: assert ( aiopnsense_helpers.normalize_datetime( value, - timezone(timedelta(hours=-4)), + default_tz, ) == expected ) -@pytest.mark.parametrize( - ("value", "default_tz", "expected"), - [ - ("2026-03-14T03:09:45", None, None), - ("2026-03-14T03:09:45+01:30", None, "2026-03-14T03:09:45+01:30"), - ], -) -def test_normalize_datetime_without_default_tz( - value: object, default_tz: timezone | None, expected: str | None -) -> None: - """Return ``None`` for naive values when no default timezone is available.""" - assert aiopnsense_helpers.normalize_datetime(value, default_tz) == expected - - @pytest.mark.parametrize( ("firmware_version", "expected"), [ From 36997db8c3ef378ebb0581b582f50dce84ae2003 Mon Sep 17 00:00:00 2001 From: Snuffy2 Date: Mon, 20 Jul 2026 21:01:20 -0400 Subject: [PATCH 5/5] Keep Speedtest timezone lookup optional --- aiopnsense/system.py | 8 ++++---- tests/test_system.py | 26 +++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/aiopnsense/system.py b/aiopnsense/system.py index c76e5d5..fc9e310 100644 --- a/aiopnsense/system.py +++ b/aiopnsense/system.py @@ -374,14 +374,14 @@ async def _get_resolved_opnsense_timezone( timezone cannot be resolved. """ if datetime_str is None: - if not await self._is_get_endpoint_available(SYSTEM_TIME_ENDPOINT): - _LOGGER.debug("System time endpoint unavailable for timezone resolution") - return None try: + if not await self._is_get_endpoint_available(SYSTEM_TIME_ENDPOINT): + _LOGGER.debug("System time endpoint unavailable for timezone resolution") + return None datetime_payload = await self._safe_dict_get(SYSTEM_TIME_ENDPOINT) except (OPNsenseError, aiohttp.ClientError, TimeoutError) as err: _LOGGER.debug( - "Failed to fetch OPNsense system time for timezone resolution: %s: %s", + "Failed to access OPNsense system time for timezone resolution: %s: %s", type(err).__name__, err, ) diff --git a/tests/test_system.py b/tests/test_system.py index 9021695..320ec6d 100644 --- a/tests/test_system.py +++ b/tests/test_system.py @@ -8,7 +8,7 @@ import aiohttp import pytest -from aiopnsense import OPNsenseClient, OPNsenseInvalidURL +from aiopnsense import OPNsenseClient, OPNsenseInvalidURL, OPNsensePrivilegeMissing from aiopnsense.exceptions import OPNsenseMissingDeviceUniqueID from tests.conftest import make_mock_session_client @@ -226,6 +226,30 @@ async def test_get_resolved_opnsense_timezone_returns_none_on_endpoint_unavailab await client.async_close() +@pytest.mark.asyncio +async def test_get_resolved_opnsense_timezone_returns_none_on_probe_error( + make_client: ClientType, +) -> None: + """Verify a mapped endpoint probe error does not abort timezone enrichment.""" + client, _session = make_mock_session_client(make_client) + try: + client.toggle_throwing_errors(True) + client._is_get_endpoint_available = AsyncMock( + side_effect=OPNsensePrivilegeMissing("Missing privilege for system time") + ) + client._safe_dict_get = AsyncMock() + + resolved_tz = await client._get_resolved_opnsense_timezone() + + assert resolved_tz is None + client._is_get_endpoint_available.assert_awaited_once_with( + "/api/diagnostics/system/system_time" + ) + client._safe_dict_get.assert_not_awaited() + finally: + await client.async_close() + + @pytest.mark.asyncio async def test_get_resolved_opnsense_timezone_returns_none_on_fetch_error( make_client: ClientType,