From 90b5d945d9721fb800e7d56c5a54f666cb4a49e2 Mon Sep 17 00:00:00 2001 From: Franck Nijhof Date: Sat, 3 Oct 2026 11:22:51 +0000 Subject: [PATCH 1/2] Skip empty queue entries, and test what the review found missing An empty entry in the notification queue made the warning about it crash, taking every valid notification in the queue down with it. A review of the recent changes also found regressions the tests would not have caught. Each now has a test that fails when it happens: endpoints that skip the parse helper, catching only missing fields in the cloud, a Wi-Fi block that is not an object, broken JSON raised as the wrong kind of error, socket errors other than DNS on the stream, an invalid height on a stream area, and Update missing from the package. --- src/demetriek/device.py | 5 ++- tests/test_cloud.py | 40 +++++++++++++++++++---- tests/test_device.py | 70 +++++++++++++++++++++++++++++++++++++++++ tests/test_lametric.py | 5 ++- tests/test_stream.py | 13 ++++++-- 5 files changed, 123 insertions(+), 10 deletions(-) diff --git a/src/demetriek/device.py b/src/demetriek/device.py index 4d48ba42..a958247f 100644 --- a/src/demetriek/device.py +++ b/src/demetriek/device.py @@ -687,9 +687,12 @@ async def notification_queue(self) -> list[Notification]: try: notifications.append(self._parse(Notification, notification)) except LaMetricError: + notification_id = ( + notification.get("id") if isinstance(notification, dict) else None + ) _LOGGER.warning( "Skipping notification %s, its format is not supported", - notification.get("id"), + notification_id, ) return notifications diff --git a/tests/test_cloud.py b/tests/test_cloud.py index 6daec32c..67c7370a 100644 --- a/tests/test_cloud.py +++ b/tests/test_cloud.py @@ -159,9 +159,12 @@ async def test_invalid_json_response( """Test a broken JSON response raises a LaMetricError, without retrying.""" responses.get(f"{CLOUD_URL}/", status=200, body="{", repeat=True) - with pytest.raises(LaMetricError, match="invalid JSON"): + with pytest.raises(LaMetricError, match="invalid JSON") as error: await cloud._request("/") + # Not a subclass, broken JSON is no reason to ask for new credentials. + assert type(error.value) is LaMetricError + assert len(next(iter(responses.requests.values()))) == 1 @@ -183,18 +186,43 @@ async def test_get_current_user(responses: aioresponses, cloud: LaMetricCloud) - assert User.from_dict(user.to_dict()) == user +@pytest.mark.parametrize( + ("body", "match"), + [ + ('{"id": 1}', 'Field "apps_count" of type int is missing'), + ( + ( + '{"id": 1, "apps_count": "many", "email": "", "name": "",' + ' "private_apps_count": 0, "private_device_count": 0}' + ), + 'Field "apps_count" of type int in User has invalid value', + ), + ], +) async def test_get_current_user_unexpected_data( - responses: aioresponses, cloud: LaMetricCloud + responses: aioresponses, cloud: LaMetricCloud, body: str, match: str ) -> None: """Test data the library does not understand raises a LaMetricError.""" - responses.get(f"{CLOUD_URL}/api/v2/users/me", status=200, body='{"id": 1}') + responses.get(f"{CLOUD_URL}/api/v2/users/me", status=200, body=body) - with pytest.raises( - LaMetricError, match='Field "apps_count" of type int is missing' - ): + with pytest.raises(LaMetricError, match=match): await cloud.current_user() +async def test_get_devices_unexpected_data( + responses: aioresponses, cloud: LaMetricCloud +) -> None: + """Test a device the library does not understand raises a LaMetricError.""" + responses.get(f"{CLOUD_URL}/api/v2/users/me/devices", status=200, body="[{}]") + responses.get(f"{CLOUD_URL}/api/v2/users/me/devices/42", status=200, body="{}") + + with pytest.raises(LaMetricError, match="data this library does not understand"): + await cloud.devices() + + with pytest.raises(LaMetricError, match="data this library does not understand"): + await cloud.device(device_id=42) + + async def test_get_devices(responses: aioresponses, cloud: LaMetricCloud) -> None: """Test getting devices from the logged in account.""" responses.get( diff --git a/tests/test_device.py b/tests/test_device.py index ebc29521..ca22ec8b 100644 --- a/tests/test_device.py +++ b/tests/test_device.py @@ -27,6 +27,7 @@ NotificationSoundCategory, Simple, Sound, + Update, ) from demetriek.const import ( NotificationType, @@ -97,6 +98,60 @@ async def test_api_not_an_object( await device.api() +@pytest.mark.parametrize( + ("call", "path", "body"), + [ + ("display", "/api/v2/device/display", "{}"), + ("audio", "/api/v2/device/audio", '{"volume": "loud"}'), + ("bluetooth", "/api/v2/device/bluetooth", "{}"), + ("wifi", "/api/v2/device/wifi", "{}"), + ("apps", "/api/v2/device/apps", '{"com.lametric.clock": {}}'), + ("stream", "/api/v2/device/stream", "{}"), + ("notification_current", "/api/v2/device/notifications/current", '{"id": 1}'), + ], +) +async def test_unexpected_data( + responses: aioresponses, + device: LaMetricDevice, + call: str, + path: str, + body: str, +) -> None: + """Test every endpoint raises a LaMetricError on data it does not understand.""" + responses.get(f"{DEVICE_URL}{path}", status=200, body=body) + + with pytest.raises(LaMetricError, match="data this library does not understand"): + await getattr(device, call)() + + +async def test_get_device_wifi_not_an_object( + responses: aioresponses, device: LaMetricDevice +) -> None: + """Test a Wi-Fi block that is not an object raises a LaMetricError.""" + data = json.loads(load_fixture("device.json")) + data["wifi"] = "unexpected" + responses.get(f"{DEVICE_URL}/api/v2/device", status=200, body=json.dumps(data)) + + with pytest.raises(LaMetricError, match='Field "wifi"'): + await device.device() + + +async def test_get_device_update( + responses: aioresponses, device: LaMetricDevice +) -> None: + """Test an available firmware update is exposed as an Update.""" + responses.get( + f"{DEVICE_URL}/api/v2/device", + status=200, + body=load_fixture("device_sa5_1.json"), + ) + + result = await device.device() + + assert isinstance(result.update, Update) + assert result.update.version == "3.2.1" + + async def test_notify(responses: aioresponses, device: LaMetricDevice) -> None: """Test sending notification serialization.""" url = f"{DEVICE_URL}/api/v2/device/notifications" @@ -389,6 +444,21 @@ async def test_notification_queue_skips_unsupported( assert "Skipping notification 26" in caplog.text +async def test_notification_queue_skips_null( + responses: aioresponses, device: LaMetricDevice +) -> None: + """Test an empty entry in the queue does not break the whole queue.""" + responses.get( + f"{DEVICE_URL}/api/v2/device/notifications", + status=200, + body='[null, {"id": "25", "model": {"frames": [{"text": "first"}]}}]', + ) + + notifications = await device.notification_queue() + + assert [notification.notification_id for notification in notifications] == [25] + + async def test_dismiss_all_notifications_unsupported( responses: aioresponses, device: LaMetricDevice ) -> None: diff --git a/tests/test_lametric.py b/tests/test_lametric.py index a7d97bf2..d9538f5a 100644 --- a/tests/test_lametric.py +++ b/tests/test_lametric.py @@ -179,9 +179,12 @@ async def test_invalid_json_response( """Test a broken JSON response raises a LaMetricError, without retrying.""" responses.get(f"{DEVICE_URL}/", status=200, body="{", repeat=True) - with pytest.raises(LaMetricError, match="invalid JSON"): + with pytest.raises(LaMetricError, match="invalid JSON") as error: await device._request("/") + # Not a subclass, broken JSON is no reason to ask for new credentials. + assert type(error.value) is LaMetricError + assert len(next(iter(responses.requests.values()))) == 1 diff --git a/tests/test_stream.py b/tests/test_stream.py index a513de7a..631fda21 100644 --- a/tests/test_stream.py +++ b/tests/test_stream.py @@ -206,6 +206,10 @@ def test_build_lmsp_packet_encoded() -> None: [StreamArea(data=b"", width=-1, height=0)], "width must be between 0 and 65535, got -1", ), + ( + [StreamArea(data=b"", width=0, height=-1)], + "height must be between 0 and 65535, got -1", + ), ], ) def test_build_lmsp_packet_invalid(areas: list[StreamArea], match: str) -> None: @@ -256,11 +260,16 @@ async def test_stream_send() -> None: assert packet == DOCUMENTED_HEADER + frame -async def test_stream_connect_error(monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.mark.parametrize( + "exception", [socket.gaierror(), OSError("Socket unavailable")] +) +async def test_stream_connect_error( + monkeypatch: pytest.MonkeyPatch, exception: OSError +) -> None: """Test a socket that cannot be opened raises a connection error.""" async def fail(*_args: object, **_kwargs: object) -> None: - raise socket.gaierror + raise exception monkeypatch.setattr(asyncio.get_running_loop(), "create_datagram_endpoint", fail) stream = LaMetricStream(host="lametric.invalid", session=_session(9999)) From 0b4e7a0d0b7a4923780cdf2a65d26356e3896e85 Mon Sep 17 00:00:00 2001 From: Franck Nijhof Date: Sat, 3 Oct 2026 11:30:20 +0000 Subject: [PATCH 2/2] Check the exact error type the pytest way, so pylint is happy --- tests/test_cloud.py | 2 +- tests/test_lametric.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_cloud.py b/tests/test_cloud.py index 67c7370a..6811b5ba 100644 --- a/tests/test_cloud.py +++ b/tests/test_cloud.py @@ -163,7 +163,7 @@ async def test_invalid_json_response( await cloud._request("/") # Not a subclass, broken JSON is no reason to ask for new credentials. - assert type(error.value) is LaMetricError + assert error.type is LaMetricError assert len(next(iter(responses.requests.values()))) == 1 diff --git a/tests/test_lametric.py b/tests/test_lametric.py index d9538f5a..7f24fa8d 100644 --- a/tests/test_lametric.py +++ b/tests/test_lametric.py @@ -183,7 +183,7 @@ async def test_invalid_json_response( await device._request("/") # Not a subclass, broken JSON is no reason to ask for new credentials. - assert type(error.value) is LaMetricError + assert error.type is LaMetricError assert len(next(iter(responses.requests.values()))) == 1