From 1ba5c73279ae21973841e37d41f84cf024e48bc8 Mon Sep 17 00:00:00 2001 From: Steven B <51370195+sdb9696@users.noreply.github.com> Date: Sun, 19 May 2024 10:34:52 +0100 Subject: [PATCH] Fix potential infinite loop if incomplete lists returned (#920) 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. --- kasa/smartprotocol.py | 7 +++++ kasa/tests/conftest.py | 2 ++ kasa/tests/fakeprotocol_smart.py | 23 +++++++++++++-- kasa/tests/test_smartprotocol.py | 48 ++++++++++++++++++++++++++++++++ poetry.lock | 28 +++++++++++++++++-- pyproject.toml | 2 ++ 6 files changed, 105 insertions(+), 5 deletions(-) diff --git a/kasa/smartprotocol.py b/kasa/smartprotocol.py index 472d9320..b1cde04d 100644 --- a/kasa/smartprotocol.py +++ b/kasa/smartprotocol.py @@ -229,6 +229,13 @@ class SmartProtocol(BaseProtocol): 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 index 7829eac1..578a82c6 100644 --- 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 index 23394450..693410b4 100644 --- a/kasa/tests/fakeprotocol_smart.py +++ b/kasa/tests/fakeprotocol_smart.py @@ -28,6 +28,8 @@ class FakeSmartTransport(BaseTransport): *, 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 @@ class FakeSmartTransport(BaseTransport): 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 @@ class FakeSmartTransport(BaseTransport): 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 @@ class FakeSmartTransport(BaseTransport): "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 index ca62ba02..a2bcacfa 100644 --- 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 index 6bd770b5..90667c80 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.7.1 and should not be changed by hand. [[package]] name = "aiohttp" @@ -1541,6 +1541,20 @@ termcolor = ">=2.1.0" [package.extras] dev = ["black", "flake8", "pre-commit"] +[[package]] +name = "pytest-timeout" +version = "2.3.1" +description = "pytest plugin to abort hanging tests" +optional = false +python-versions = ">=3.7" +files = [ + {file = "pytest-timeout-2.3.1.tar.gz", hash = "sha256:12397729125c6ecbdaca01035b9e5239d4db97352320af155b3f5de1ba5165d9"}, + {file = "pytest_timeout-2.3.1-py3-none-any.whl", hash = "sha256:68188cb703edfc6a18fad98dc25a3c61e9f24d644b0b70f33af545219fc7813e"}, +] + +[package.dependencies] +pytest = ">=7.0.0" + [[package]] name = "pytz" version = "2024.1" @@ -1564,6 +1578,7 @@ files = [ {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:69b023b2b4daa7548bcfbd4aa3da05b3a74b772db9e23b982788168117739938"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:81e0b275a9ecc9c0c0c07b4b90ba548307583c125f54d5b6946cfee6360c733d"}, {file = "PyYAML-6.0.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:ba336e390cd8e4d1739f42dfe9bb83a3cc2e80f567d8805e11b46f4a943f5515"}, + {file = "PyYAML-6.0.1-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:326c013efe8048858a6d312ddd31d56e468118ad4cdeda36c719bf5bb6192290"}, {file = "PyYAML-6.0.1-cp310-cp310-win32.whl", hash = "sha256:bd4af7373a854424dabd882decdc5579653d7868b8fb26dc7d0e99f823aa5924"}, {file = "PyYAML-6.0.1-cp310-cp310-win_amd64.whl", hash = "sha256:fd1592b3fdf65fff2ad0004b5e363300ef59ced41c2e6b3a99d4089fa8c5435d"}, {file = "PyYAML-6.0.1-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:6965a7bc3cf88e5a1c3bd2e0b5c22f8d677dc88a455344035f03399034eb3007"}, @@ -1571,8 +1586,15 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:42f8152b8dbc4fe7d96729ec2b99c7097d656dc1213a3229ca5383f973a5ed6d"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:062582fca9fabdd2c8b54a3ef1c978d786e0f6b3a1510e0ac93ef59e0ddae2bc"}, {file = "PyYAML-6.0.1-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:d2b04aac4d386b172d5b9692e2d2da8de7bfb6c387fa4f801fbf6fb2e6ba4673"}, + {file = "PyYAML-6.0.1-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:e7d73685e87afe9f3b36c799222440d6cf362062f78be1013661b00c5c6f678b"}, {file = "PyYAML-6.0.1-cp311-cp311-win32.whl", hash = "sha256:1635fd110e8d85d55237ab316b5b011de701ea0f29d07611174a1b42f1444741"}, {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, + {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, + {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, + {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, + {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, + {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, + {file = "PyYAML-6.0.1-cp312-cp312-win_amd64.whl", hash = "sha256:0d3304d8c0adc42be59c5f8a4d9e3d7379e6955ad754aa9d6ab7a398b59dd1df"}, {file = "PyYAML-6.0.1-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:50550eb667afee136e9a77d6dc71ae76a44df8b3e51e41b77f6de2932bfe0f47"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:1fe35611261b29bd1de0070f0b2f47cb6ff71fa6595c077e42bd0c419fa27b98"}, {file = "PyYAML-6.0.1-cp36-cp36m-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:704219a11b772aea0d8ecd7058d0082713c3562b4e271b849ad7dc4a5c90c13c"}, @@ -1589,6 +1611,7 @@ files = [ {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a0cd17c15d3bb3fa06978b4e8958dcdc6e0174ccea823003a106c7d4d7899ac5"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:28c119d996beec18c05208a8bd78cbe4007878c6dd15091efb73a30e90539696"}, {file = "PyYAML-6.0.1-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:7e07cbde391ba96ab58e532ff4803f79c4129397514e1413a7dc761ccd755735"}, + {file = "PyYAML-6.0.1-cp38-cp38-musllinux_1_1_x86_64.whl", hash = "sha256:49a183be227561de579b4a36efbb21b3eab9651dd81b1858589f796549873dd6"}, {file = "PyYAML-6.0.1-cp38-cp38-win32.whl", hash = "sha256:184c5108a2aca3c5b3d3bf9395d50893a7ab82a38004c8f61c258d4428e80206"}, {file = "PyYAML-6.0.1-cp38-cp38-win_amd64.whl", hash = "sha256:1e2722cc9fbb45d9b87631ac70924c11d3a401b2d7f410cc0e3bbf249f2dca62"}, {file = "PyYAML-6.0.1-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:9eb6caa9a297fc2c2fb8862bc5370d0303ddba53ba97e71f08023b6cd73d16a8"}, @@ -1596,6 +1619,7 @@ files = [ {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:5773183b6446b2c99bb77e77595dd486303b4faab2b086e7b17bc6bef28865f6"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:b786eecbdf8499b9ca1d697215862083bd6d2a99965554781d0d8d1ad31e13a0"}, {file = "PyYAML-6.0.1-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:bc1bf2925a1ecd43da378f4db9e4f799775d6367bdb94671027b73b393a7c42c"}, + {file = "PyYAML-6.0.1-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:04ac92ad1925b2cff1db0cfebffb6ffc43457495c9b3c39d3fcae417d7125dc5"}, {file = "PyYAML-6.0.1-cp39-cp39-win32.whl", hash = "sha256:faca3bdcf85b2fc05d06ff3fbc1f83e1391b3e724afa3feba7d13eeab355484c"}, {file = "PyYAML-6.0.1-cp39-cp39-win_amd64.whl", hash = "sha256:510c9deebc5c0225e8c96813043e62b680ba2f9c50a08d3724c7f28a747d1486"}, {file = "PyYAML-6.0.1.tar.gz", hash = "sha256:bfdf460b1736c775f2ba9f6a92bca30bc2095067b8a9d77876d1fad6cc3b4a43"}, @@ -2132,4 +2156,4 @@ speedups = ["kasa-crypt", "orjson"] [metadata] lock-version = "2.0" python-versions = "^3.8" -content-hash = "d627e4165dade7eaaf21708f00bc919bc3fffb3e8a805e186dfb56e5e1781bbe" +content-hash = "ba5c0da1e413e466834d0954528c7ace6dd9e01d9fb2e626f4c6b23044803aef" diff --git a/pyproject.toml b/pyproject.toml index 5b5f4d3e..783477e1 100644 --- 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"]