Skip to content

Support alternate Speedtest plugins #80

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Snuffy2 merged 5 commits into from
Jul 19, 2026
Merged

Support alternate Speedtest plugins #80

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2a8c604
Support alternate Speedtest plugins
Snuffy2 Jul 19, 2026
b3faf20
Preserve Speedtest server fields
Snuffy2 Jul 19, 2026
d5a075d
Remove obsolete Speedtest parser
Snuffy2 Jul 19, 2026
62ebbd0
Simplify Speedtest field normalization
Snuffy2 Jul 19, 2026
dfd37f9
Harden Speedtest log parsing
Snuffy2 Jul 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 41 additions & 11 deletions aiopnsense/speedtest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from ._typing import AiopnsenseClientProtocol
from .helpers import _LOGGER, _log_errors, try_to_float, try_to_int

SPEEDTEST_SHOW_RECENT_ENDPOINT = "/api/speedtest/service/showrecent"
SPEEDTEST_SHOW_LOG_ENDPOINT = "/api/speedtest/service/showlog"
SPEEDTEST_SHOW_STAT_ENDPOINT = "/api/speedtest/service/showstat"
SPEEDTEST_RUN_ENDPOINT = "/api/speedtest/service/run"

Expand All @@ -39,20 +39,22 @@ async def get_speedtest(self) -> dict[str, Any]:
include min, max, sample count, and period bounds. Returns
``{"available": False}`` when the plugin endpoint is missing.
"""
if not await self._is_get_endpoint_available(SPEEDTEST_SHOW_RECENT_ENDPOINT):
if not await self._is_get_endpoint_available(SPEEDTEST_SHOW_LOG_ENDPOINT):
_LOGGER.debug("Speedtest not installed")
return {"available": False}

show_recent = await self._safe_dict_get(SPEEDTEST_SHOW_RECENT_ENDPOINT)
latest_result = self._parse_showlog_latest(
await self._safe_list_get(SPEEDTEST_SHOW_LOG_ENDPOINT)
)
if await self._is_get_endpoint_available(SPEEDTEST_SHOW_STAT_ENDPOINT):
show_stat = await self._safe_dict_get(SPEEDTEST_SHOW_STAT_ENDPOINT)
else:
_LOGGER.debug("Speedtest statistics endpoint unavailable")
show_stat = {}

server_id, server_name = self._parse_recent_server(show_recent.get("server"))
date = show_recent.get("date") if isinstance(show_recent.get("date"), str) else None
url = show_recent.get("url") if isinstance(show_recent.get("url"), str) else None
server_id, server_name = self._parse_speedtest_server(latest_result.get("server"))
date = latest_result.get("date") if isinstance(latest_result.get("date"), str) else None
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", {})
Expand All @@ -65,7 +67,7 @@ async def get_speedtest(self) -> dict[str, Any]:
"average": {},
}
for metric in ("download", "upload", "latency"):
recent_value = try_to_float(show_recent.get(metric))
recent_value = try_to_float(latest_result.get(metric))
stat_metric = show_stat.get(metric, {})

output["last"][metric] = {
Expand All @@ -91,11 +93,39 @@ async def get_speedtest(self) -> dict[str, Any]:
}
return output

def _parse_recent_server(self, server_text: Any) -> tuple[str | None, str | None]:
"""Parse the ``showrecent.server`` field into server ID and name.
def _parse_showlog_latest(self, show_log: Any) -> dict[str, Any]:
"""Normalize the newest row returned by the Speedtest ``showlog`` endpoint.

Args:
show_log (Any): Raw Speedtest history payload, ordered newest first.

Returns:
dict[str, Any]: Latest result using the legacy ``showrecent`` field
names, or an empty mapping when no valid row is available.
Comment thread
greptile-apps[bot] marked this conversation as resolved.
"""
if not isinstance(show_log, list) or not show_log:
return {}
latest = show_log[0]
if not isinstance(latest, list) or len(latest) < 9:
return {}

server_id = str(latest[2]).strip() if isinstance(latest[2], (str, int)) else ""
server_name = latest[3].strip() if isinstance(latest[3], str) else ""
server = " ".join(part for part in (server_id, server_name) if part) or None
Comment thread
Snuffy2 marked this conversation as resolved.
Outdated
return {
"date": latest[0],
"server": server,
"download": latest[5],
"upload": latest[6],
"latency": latest[7],
"url": latest[8],
}

def _parse_speedtest_server(self, server_text: Any) -> tuple[str | None, str | None]:
"""Parse normalized Speedtest server text into server ID and name.

Args:
server_text (Any): Raw ``showrecent.server`` text, commonly either
server_text (Any): Raw server text, commonly either
``"<id> <name>"`` or just the server name.

Returns:
Expand Down Expand Up @@ -123,7 +153,7 @@ async def run_speedtest(self) -> dict[str, Any]:
endpoint, or an empty mapping when the plugin endpoint is
unavailable or returns a malformed payload.
"""
if not await self._is_get_endpoint_available(SPEEDTEST_SHOW_RECENT_ENDPOINT):
if not await self._is_get_endpoint_available(SPEEDTEST_SHOW_LOG_ENDPOINT):
_LOGGER.debug("Speedtest not installed")
return {}

Expand Down
168 changes: 96 additions & 72 deletions tests/test_speedtest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Tests for `aiopnsense.speedtest`."""

from collections.abc import Callable
from typing import Any
from unittest.mock import AsyncMock, call

import pytest
Expand All @@ -17,44 +18,47 @@ async def test_get_speedtest_skips_calls_when_endpoint_missing(make_client) -> N
client, _session = make_mock_session_client(make_client)
try:
client._is_get_endpoint_available = AsyncMock(return_value=False)
client._safe_dict_get = AsyncMock()
client._safe_list_get = AsyncMock()

result = await client.get_speedtest()

assert result == {"available": False}
client._safe_dict_get.assert_not_awaited()
client._is_get_endpoint_available.assert_awaited_once_with(
"/api/speedtest/service/showrecent"
)
client._safe_list_get.assert_not_awaited()
client._is_get_endpoint_available.assert_awaited_once_with("/api/speedtest/service/showlog")
finally:
await client.async_close()


@pytest.mark.asyncio
async def test_get_speedtest_normalizes_recent_and_stat_payloads(make_client) -> None:
"""get_speedtest should normalize showrecent and showstat payload fields."""
async def test_get_speedtest_normalizes_latest_and_stat_payloads(make_client) -> None:
"""get_speedtest should normalize shared showlog and showstat payload fields."""
client, _session = make_mock_session_client(make_client)
try:
client._is_get_endpoint_available = AsyncMock(side_effect=[True, True])
client._safe_dict_get = AsyncMock(
side_effect=[
{
"date": "2026-03-14T03:09:45",
"server": "72800 RippleFiber, Newark, NJ",
"download": "836.05",
"upload": "832.97",
"latency": "4.0",
"url": "https://www.speedtest.net/result/c/abc",
},
{
"samples": 10717,
"period": {"oldest": "2023-01-22 00:29:00", "youngest": "2026-03-14 03:09:45"},
"latency": {"avg": 13.42, "min": 2.35, "max": 1266.74},
"download": {"avg": 723.83, "min": 4.18, "max": 942.02},
"upload": {"avg": 706.7, "min": 1.54, "max": 890.32},
},
client._safe_list_get = AsyncMock(
return_value=[
[
"2026-03-14T03:09:45",
"198.51.100.10",
"72800",
"RippleFiber, Newark, NJ",
"United States",
"836.05",
"832.97",
"4.0",
"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"},
"latency": {"avg": 13.42, "min": 2.35, "max": 1266.74},
"download": {"avg": 723.83, "min": 4.18, "max": 942.02},
"upload": {"avg": 706.7, "min": 1.54, "max": 890.32},
}
)

result = await client.get_speedtest()

Expand All @@ -73,20 +77,15 @@ async def test_get_speedtest_normalizes_recent_and_stat_payloads(make_client) ->


@pytest.mark.parametrize(
("endpoint_side_effect", "safe_dict_get_payload", "showstat_available"),
("endpoint_side_effect", "showstat_available"),
[
pytest.param(
[True, True],
[
{"download": "1", "upload": "2", "latency": "3"},
{},
],
True,
id="showstat-available",
),
pytest.param(
[True, False],
{"download": "1", "upload": "2", "latency": "3"},
False,
id="showstat-missing",
),
Expand All @@ -96,16 +95,13 @@ async def test_get_speedtest_normalizes_recent_and_stat_payloads(make_client) ->
async def test_get_speedtest_probes_showstat_before_fetching_optional_payload(
make_client: ClientType,
endpoint_side_effect: list[bool],
safe_dict_get_payload: list[dict[str, str]] | dict[str, str],
showstat_available: bool,
) -> None:
"""Validate ``get_speedtest`` probes ``showstat`` before optional fetches.

Args:
make_client (ClientType): Fixture factory returning ``OPNsenseClient`` instances.
endpoint_side_effect (list[bool]): Endpoint availability responses in call order.
safe_dict_get_payload (list[dict[str, str]] | dict[str, str]): Mocked payloads for
endpoint fetches.
showstat_available (bool): Whether the ``showstat`` endpoint should be fetched.

Returns:
Expand All @@ -114,29 +110,39 @@ async def test_get_speedtest_probes_showstat_before_fetching_optional_payload(
client, _session = make_mock_session_client(make_client)
try:
client._is_get_endpoint_available = AsyncMock(side_effect=endpoint_side_effect)
if showstat_available:
client._safe_dict_get = AsyncMock(side_effect=safe_dict_get_payload)
else:
client._safe_dict_get = AsyncMock(return_value=safe_dict_get_payload)
client._safe_list_get = AsyncMock(
return_value=[
[
"2026-03-14T03:09:45",
"198.51.100.10",
"72800",
"Test ISP, New York, NY",
"United States",
"1",
"2",
"3",
"https://www.speedtest.net/result/c/abc",
]
]
)
client._safe_dict_get = AsyncMock(return_value={})

result = await client.get_speedtest()

assert result["available"] is True
assert client._is_get_endpoint_available.await_args_list == [
call("/api/speedtest/service/showrecent"),
call("/api/speedtest/service/showlog"),
call("/api/speedtest/service/showstat"),
]
client._safe_list_get.assert_awaited_once_with("/api/speedtest/service/showlog")

if showstat_available:
assert client._safe_dict_get.await_args_list == [
call("/api/speedtest/service/showrecent"),
call("/api/speedtest/service/showstat"),
]
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_awaited_once_with("/api/speedtest/service/showrecent")
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
Expand All @@ -150,25 +156,30 @@ async def test_get_speedtest_normalizes_malformed_payloads(make_client) -> None:
client, _session = make_mock_session_client(make_client)
try:
client._is_get_endpoint_available = AsyncMock(side_effect=[True, True])
client._safe_dict_get = AsyncMock(
side_effect=[
{
"date": 12345,
"server": "Regional POP - NYC",
"download": "bad-number",
"upload": "12.5",
"latency": None,
"url": 999,
},
{
"samples": "not-an-int",
"period": "bad-period-shape",
"download": "bad-download-shape",
"upload": None,
"latency": ["bad-latency-shape"],
},
client._safe_list_get = AsyncMock(
return_value=[
[
12345,
"198.51.100.10",
None,
"Regional POP - NYC",
"United States",
"bad-number",
"12.5",
None,
999,
]
]
)
client._safe_dict_get = AsyncMock(
return_value={
"samples": "not-an-int",
"period": "bad-period-shape",
"download": "bad-download-shape",
"upload": None,
"latency": ["bad-latency-shape"],
}
)

result = await client.get_speedtest()

Expand All @@ -192,21 +203,37 @@ async def test_get_speedtest_normalizes_malformed_payloads(make_client) -> None:


@pytest.mark.asyncio
async def test_parse_recent_server_variants(make_client) -> None:
"""_parse_recent_server should parse known server formats safely."""
async def test_parse_speedtest_server_variants(make_client) -> None:
"""_parse_speedtest_server should parse known server formats safely."""
client, _session = make_mock_session_client(make_client)
try:
assert client._parse_recent_server(None) == (None, None)
assert client._parse_recent_server(" ") == (None, None)
assert client._parse_recent_server("10001 Test ISP, NY") == ("10001", "Test ISP, NY")
assert client._parse_recent_server("Unstructured Server Name") == (
assert client._parse_speedtest_server(None) == (None, None)
assert client._parse_speedtest_server(" ") == (None, None)
assert client._parse_speedtest_server("10001 Test ISP, NY") == (
"10001",
"Test ISP, NY",
)
assert client._parse_speedtest_server("Unstructured Server Name") == (
None,
"Unstructured Server Name",
)
finally:
await client.async_close()


@pytest.mark.parametrize("show_log", [None, {}, [], ["malformed-row"], [["too", "short"]]])
@pytest.mark.asyncio
async def test_parse_showlog_latest_rejects_malformed_rows(
make_client: ClientType, show_log: Any
) -> None:
"""_parse_showlog_latest should reject missing or malformed history rows."""
client, _session = make_mock_session_client(make_client)
try:
assert client._parse_showlog_latest(show_log) == {}
finally:
await client.async_close()


@pytest.mark.asyncio
async def test_run_speedtest_uses_extended_timeout(make_client) -> None:
"""run_speedtest should use custom timeout helper for long-running endpoint calls."""
Expand All @@ -218,6 +245,7 @@ async def test_run_speedtest_uses_extended_timeout(make_client) -> None:
result = await client.run_speedtest()

assert result == {"timestamp": "x"}
client._is_get_endpoint_available.assert_awaited_once_with("/api/speedtest/service/showlog")
client._safe_dict_get_with_timeout.assert_awaited_once_with(
"/api/speedtest/service/run", timeout_seconds=180
)
Expand All @@ -237,9 +265,7 @@ async def test_run_speedtest_returns_empty_when_endpoint_missing(make_client) ->

assert result == {}
client._safe_dict_get_with_timeout.assert_not_awaited()
client._is_get_endpoint_available.assert_awaited_once_with(
"/api/speedtest/service/showrecent"
)
client._is_get_endpoint_available.assert_awaited_once_with("/api/speedtest/service/showlog")
finally:
await client.async_close()

Expand All @@ -255,9 +281,7 @@ async def test_run_speedtest_returns_empty_for_non_mapping_response(make_client)
result = await client.run_speedtest()

assert result == {}
client._is_get_endpoint_available.assert_awaited_once_with(
"/api/speedtest/service/showrecent"
)
client._is_get_endpoint_available.assert_awaited_once_with("/api/speedtest/service/showlog")
client._safe_dict_get_with_timeout.assert_awaited_once_with(
"/api/speedtest/service/run", timeout_seconds=180
)
Expand Down
Loading