Fix potential infinite loop if incomplete lists returned (#920)

sdb9696 · rytilahti · web-flow · commit 1ba5c73279ae · 2024-05-19T10:34:52.000+01:00
Fixes the test framework to handle fixtures with incomplete lists better by checking for completeness and overriding the sum. Also adds a pytest-timeout dev dependency with timeout set to 10 seconds. Finally fixes smartprotocol to prevent an infinite loop if incomplete lists ever happens in the real world.

Co-authored-by: Teemu R. &lt;tpr@iki.fi&gt;
diff --git a/kasa/smartprotocol.py b/kasa/smartprotocol.py
@@ -229,6 +229,13 @@ async def _handle_response_lists(
                 iterate_list_pages=False,
             )
             next_batch = response[method]
+            # In case the device returns empty lists avoid infinite looping
+            if not next_batch[response_list_name]:
+                _LOGGER.error(
+                    f"Device {self._host} returned empty "
+                    + f"results list for method {method}"
+                )
+                break
             response_result[response_list_name].extend(next_batch[response_list_name])
 
     def _handle_response_error_code(self, resp_dict: dict, method, raise_on_error=True):
diff --git a/kasa/tests/conftest.py b/kasa/tests/conftest.py
@@ -58,6 +58,8 @@ def pytest_configure():
 
 
 def pytest_sessionfinish(session, exitstatus):
+    if not pytest.fixtures_missing_methods:
+        return
     msg = "\n"
     for fixture, methods in sorted(pytest.fixtures_missing_methods.items()):
         method_list = ", ".join(methods)
diff --git a/kasa/tests/fakeprotocol_smart.py b/kasa/tests/fakeprotocol_smart.py
@@ -28,6 +28,8 @@ def __init__(
         *,
         list_return_size=10,
         component_nego_not_included=False,
+        warn_fixture_missing_methods=True,
+        fix_incomplete_fixture_lists=True,
     ):
         super().__init__(
             config=DeviceConfig(
@@ -46,6 +48,8 @@ def __init__(
                 for comp in self.info["component_nego"]["component_list"]
             }
         self.list_return_size = list_return_size
+        self.warn_fixture_missing_methods = warn_fixture_missing_methods
+        self.fix_incomplete_fixture_lists = fix_incomplete_fixture_lists
 
     @property
     def default_port(self):
@@ -220,6 +224,18 @@ def _send_request(self, request_dict: dict):
                         if (params and (start_index := params.get("start_index")))
                         else 0
                     )
+                    # Fixtures generated before _handle_response_lists was implemented
+                    # could have incomplete lists.
+                    if (
+                        len(result[list_key]) < result["sum"]
+                        and self.fix_incomplete_fixture_lists
+                    ):
+                        result["sum"] = len(result[list_key])
+                        if self.warn_fixture_missing_methods:
+                            pytest.fixtures_missing_methods.setdefault(
+                                self.fixture_name, set()
+                            ).add(f"{method} (incomplete '{list_key}' list)")
+
                     result[list_key] = result[list_key][
                         start_index : start_index + self.list_return_size
                     ]
@@ -244,9 +260,10 @@ def _send_request(self, request_dict: dict):
                     "method": method,
                 }
             # Reduce warning spam by consolidating and reporting at the end of the run
-            if self.fixture_name not in pytest.fixtures_missing_methods:
-                pytest.fixtures_missing_methods[self.fixture_name] = set()
-            pytest.fixtures_missing_methods[self.fixture_name].add(method)
+            if self.warn_fixture_missing_methods:
+                pytest.fixtures_missing_methods.setdefault(
+                    self.fixture_name, set()
+                ).add(method)
             return retval
         elif method in ["set_qs_info", "fw_download"]:
             return {"error_code": 0}
diff --git a/kasa/tests/test_smartprotocol.py b/kasa/tests/test_smartprotocol.py
@@ -1,3 +1,5 @@
+import logging
+
 import pytest
 
 from ..credentials import Credentials
@@ -242,3 +244,49 @@ async def test_smart_protocol_lists_multiple_request(mocker, list_sum, batch_siz
     )
     assert query_spy.call_count == expected_count
     assert resp == response
+
+
+async def test_incomplete_list(mocker, caplog):
+    """Test for handling incomplete lists returned from queries."""
+    info = {
+        "get_preset_rules": {
+            "start_index": 0,
+            "states": [
+                {
+                    "brightness": 50,
+                },
+                {
+                    "brightness": 100,
+                },
+            ],
+            "sum": 7,
+        }
+    }
+    caplog.set_level(logging.ERROR)
+    transport = FakeSmartTransport(
+        info,
+        "dummy-name",
+        component_nego_not_included=True,
+        warn_fixture_missing_methods=False,
+    )
+    protocol = SmartProtocol(transport=transport)
+    resp = await protocol.query({"get_preset_rules": None})
+    assert resp
+    assert resp["get_preset_rules"]["sum"] == 2  # FakeTransport fixes sum
+    assert caplog.text == ""
+
+    # Test behaviour without FakeTranport fix
+    transport = FakeSmartTransport(
+        info,
+        "dummy-name",
+        component_nego_not_included=True,
+        warn_fixture_missing_methods=False,
+        fix_incomplete_fixture_lists=False,
+    )
+    protocol = SmartProtocol(transport=transport)
+    resp = await protocol.query({"get_preset_rules": None})
+    assert resp["get_preset_rules"]["sum"] == 7
+    assert (
+        "Device 127.0.0.123 returned empty results list for method get_preset_rules"
+        in caplog.text
+    )
diff --git a/poetry.lock b/poetry.lock
diff --git a/pyproject.toml b/pyproject.toml
@@ -57,6 +57,7 @@ pytest-mock = "*"
 codecov = "*"
 xdoctest = "*"
 coverage = {version = "*", extras = ["toml"]}
+pytest-timeout = "^2"
 
 [tool.poetry.extras]
 docs = ["sphinx", "sphinx_rtd_theme", "sphinxcontrib-programoutput", "myst-parser", "docutils"]
@@ -89,6 +90,7 @@ markers = [
     "requires_dummy: test requires dummy data to pass, skipped on real devices",
 ]
 asyncio_mode = "auto"
+timeout = 10
 
 [tool.doc8]
 paths = ["docs"]
