From 71ae06fa83e82a77c0da42a65cce6ded20cbf1b6 Mon Sep 17 00:00:00 2001
From: "Steven B." <51370195+sdb9696@users.noreply.github.com>
Date: Mon, 11 Nov 2024 17:41:31 +0000
Subject: [PATCH] Fix test framework running against real devices (#1235)

---
 kasa/device.py                       |  5 +++
 kasa/iot/iotdevice.py                |  6 +++
 kasa/smart/smartdevice.py            | 11 +++++
 tests/device_fixtures.py             | 60 +++++++++++++++++++++++-----
 tests/fixtureinfo.py                 | 20 +++++++---
 tests/smart/modules/test_firmware.py |  1 +
 tests/test_aestransport.py           |  2 +
 tests/test_bulb.py                   |  2 +-
 tests/test_childdevice.py            |  7 +++-
 tests/test_cli.py                    |  4 ++
 tests/test_common_modules.py         | 47 ++++++++++++++--------
 tests/test_device_factory.py         |  4 ++
 tests/test_discovery.py              |  3 ++
 tests/test_klapprotocol.py           |  3 ++
 tests/test_protocol.py               | 11 +++--
 tests/test_smartdevice.py            |  2 +
 tests/test_smartprotocol.py          | 10 ++---
 tests/test_sslaestransport.py        |  3 ++
 18 files changed, 158 insertions(+), 43 deletions(-)

diff --git a/kasa/device.py b/kasa/device.py
index 72c56717..ca16bb6b 100644
--- a/kasa/device.py
+++ b/kasa/device.py
@@ -320,6 +320,11 @@ class Device(ABC):
     def model(self) -> str:
         """Returns the device model."""
 
+    @property
+    @abstractmethod
+    def _model_region(self) -> str:
+        """Return device full model name and region."""
+
     @property
     @abstractmethod
     def alias(self) -> str | None:
diff --git a/kasa/iot/iotdevice.py b/kasa/iot/iotdevice.py
index 4ee403db..20284c1d 100755
--- a/kasa/iot/iotdevice.py
+++ b/kasa/iot/iotdevice.py
@@ -455,6 +455,12 @@ class IotDevice(Device):
         sys_info = self._sys_info
         return str(sys_info["model"])
 
+    @property
+    @requires_update
+    def _model_region(self) -> str:
+        """Return device full model name and region."""
+        return self.model
+
     @property  # type: ignore
     def alias(self) -> str | None:
         """Return device name (alias)."""
diff --git a/kasa/smart/smartdevice.py b/kasa/smart/smartdevice.py
index 35524ee8..e497b8e8 100644
--- a/kasa/smart/smartdevice.py
+++ b/kasa/smart/smartdevice.py
@@ -492,6 +492,17 @@ class SmartDevice(Device):
         """Returns the device model."""
         return str(self._info.get("model"))
 
+    @property
+    def _model_region(self) -> str:
+        """Return device full model name and region."""
+        if (disco := self._discovery_info) and (
+            disco_model := disco.get("device_model")
+        ):
+            return disco_model
+        # Some devices have the region in the specs element.
+        region = f"({specs})" if (specs := self._info.get("specs")) else ""
+        return f"{self.model}{region}"
+
     @property
     def alias(self) -> str | None:
         """Returns the device alias or nickname."""
diff --git a/tests/device_fixtures.py b/tests/device_fixtures.py
index 1726ee8c..4d335d5c 100644
--- a/tests/device_fixtures.py
+++ b/tests/device_fixtures.py
@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import os
 from collections.abc import AsyncGenerator
 
 import pytest
@@ -142,7 +143,7 @@ ALL_DEVICES_SMART = (
 )
 ALL_DEVICES = ALL_DEVICES_IOT.union(ALL_DEVICES_SMART)
 
-IP_MODEL_CACHE: dict[str, str] = {}
+IP_FIXTURE_CACHE: dict[str, FixtureInfo] = {}
 
 
 def parametrize_combine(parametrized: list[pytest.MarkDecorator]):
@@ -448,6 +449,39 @@ def get_fixture_info(fixture, protocol):
             return fixture_info
 
 
+def get_nearest_fixture_to_ip(dev):
+    if isinstance(dev, SmartDevice):
+        protocol_fixtures = filter_fixtures("", protocol_filter={"SMART"})
+    elif isinstance(dev, SmartCamera):
+        protocol_fixtures = filter_fixtures("", protocol_filter={"SMARTCAMERA"})
+    else:
+        protocol_fixtures = filter_fixtures("", protocol_filter={"IOT"})
+    assert protocol_fixtures, "Unknown device type"
+
+    # This will get the best fixture with a match on model region
+    if model_region_fixtures := filter_fixtures(
+        "", model_filter={dev._model_region}, fixture_list=protocol_fixtures
+    ):
+        return next(iter(model_region_fixtures))
+
+    # This will get the best fixture based on model starting with the name.
+    if "(" in dev.model:
+        model, _, _ = dev.model.partition("(")
+    else:
+        model = dev.model
+    if model_fixtures := filter_fixtures(
+        "", model_startswith_filter=model, fixture_list=protocol_fixtures
+    ):
+        return next(iter(model_fixtures))
+
+    if device_type_fixtures := filter_fixtures(
+        "", device_type_filter={dev.device_type}, fixture_list=protocol_fixtures
+    ):
+        return next(iter(device_type_fixtures))
+
+    return next(iter(protocol_fixtures))
+
+
 @pytest.fixture(params=filter_fixtures("main devices"), ids=idgenerator)
 async def dev(request) -> AsyncGenerator[Device, None]:
     """Device fixture.
@@ -459,24 +493,28 @@ async def dev(request) -> AsyncGenerator[Device, None]:
     dev: Device
 
     ip = request.config.getoption("--ip")
-    username = request.config.getoption("--username")
-    password = request.config.getoption("--password")
+    username = request.config.getoption("--username") or os.environ.get("KASA_USERNAME")
+    password = request.config.getoption("--password") or os.environ.get("KASA_PASSWORD")
     if ip:
-        model = IP_MODEL_CACHE.get(ip)
-        d = None
-        if not model:
-            d = await _discover_update_and_close(ip, username, password)
-            IP_MODEL_CACHE[ip] = model = d.model
+        fixture = IP_FIXTURE_CACHE.get(ip)
 
-        if model not in fixture_data.name:
+        d = None
+        if not fixture:
+            d = await _discover_update_and_close(ip, username, password)
+            IP_FIXTURE_CACHE[ip] = fixture = get_nearest_fixture_to_ip(d)
+        assert fixture
+        if fixture.name != fixture_data.name:
             pytest.skip(f"skipping file {fixture_data.name}")
-        dev = d if d else await _discover_update_and_close(ip, username, password)
+            dev = None
+        else:
+            dev = d if d else await _discover_update_and_close(ip, username, password)
     else:
         dev = await get_device_for_fixture(fixture_data)
 
     yield dev
 
-    await dev.disconnect()
+    if dev:
+        await dev.disconnect()
 
 
 def get_parent_and_child_modules(device: Device, module_name):
diff --git a/tests/fixtureinfo.py b/tests/fixtureinfo.py
index 9f4d3952..cb75b423 100644
--- a/tests/fixtureinfo.py
+++ b/tests/fixtureinfo.py
@@ -104,8 +104,10 @@ def filter_fixtures(
     data_root_filter: str | None = None,
     protocol_filter: set[str] | None = None,
     model_filter: set[str] | None = None,
+    model_startswith_filter: str | None = None,
     component_filter: str | ComponentFilter | None = None,
     device_type_filter: Iterable[DeviceType] | None = None,
+    fixture_list: list[FixtureInfo] = FIXTURE_DATA,
 ):
     """Filter the fixtures based on supplied parameters.
 
@@ -127,12 +129,15 @@ def filter_fixtures(
             and (model := model_filter_list[0])
             and len(model.split("_")) == 3
         ):
-            # return exact match
+            # filter string includes hw and fw, return exact match
             return fixture_data.name == f"{model}.json"
         file_model_region = fixture_data.name.split("_")[0]
         file_model = file_model_region.split("(")[0]
         return file_model in model_filter
 
+    def _model_startswith_match(fixture_data: FixtureInfo, starts_with: str):
+        return fixture_data.name.startswith(starts_with)
+
     def _component_match(
         fixture_data: FixtureInfo, component_filter: str | ComponentFilter
     ):
@@ -175,13 +180,17 @@ def filter_fixtures(
     filtered = []
     if protocol_filter is None:
         protocol_filter = {"IOT", "SMART"}
-    for fixture_data in FIXTURE_DATA:
+    for fixture_data in fixture_list:
         if data_root_filter and data_root_filter not in fixture_data.data:
             continue
         if fixture_data.protocol not in protocol_filter:
             continue
         if model_filter is not None and not _model_match(fixture_data, model_filter):
             continue
+        if model_startswith_filter is not None and not _model_startswith_match(
+            fixture_data, model_startswith_filter
+        ):
+            continue
         if component_filter and not _component_match(fixture_data, component_filter):
             continue
         if device_type_filter and not _device_type_match(
@@ -191,8 +200,9 @@ def filter_fixtures(
 
         filtered.append(fixture_data)
 
-    print(f"# {desc}")
-    for value in filtered:
-        print(f"\t{value.name}")
+    if desc:
+        print(f"# {desc}")
+        for value in filtered:
+            print(f"\t{value.name}")
     filtered.sort()
     return filtered
diff --git a/tests/smart/modules/test_firmware.py b/tests/smart/modules/test_firmware.py
index c1961b41..8f6fe6eb 100644
--- a/tests/smart/modules/test_firmware.py
+++ b/tests/smart/modules/test_firmware.py
@@ -74,6 +74,7 @@ async def test_update_available_without_cloud(dev: SmartDevice):
         pytest.param(False, pytest.raises(KasaException), id="not-available"),
     ],
 )
+@pytest.mark.requires_dummy()
 async def test_firmware_update(
     dev: SmartDevice,
     mocker: MockerFixture,
diff --git a/tests/test_aestransport.py b/tests/test_aestransport.py
index 1bc6d8dd..302f195a 100644
--- a/tests/test_aestransport.py
+++ b/tests/test_aestransport.py
@@ -29,6 +29,8 @@ from kasa.exceptions import (
 )
 from kasa.httpclient import HttpClient
 
+pytestmark = [pytest.mark.requires_dummy]
+
 DUMMY_QUERY = {"foobar": {"foo": "bar", "bar": "foo"}}
 
 key = b"8\x89\x02\xfa\xf5Xs\x1c\xa1 H\x9a\x82\xc7\xd9\t"
diff --git a/tests/test_bulb.py b/tests/test_bulb.py
index 9e6dd7c2..64c012fd 100644
--- a/tests/test_bulb.py
+++ b/tests/test_bulb.py
@@ -32,7 +32,7 @@ from .conftest import (
 from .test_iotdevice import SYSINFO_SCHEMA
 
 
-@bulb
+@bulb_iot
 async def test_bulb_sysinfo(dev: Device):
     assert dev.sys_info is not None
     SYSINFO_SCHEMA_BULB(dev.sys_info)
diff --git a/tests/test_childdevice.py b/tests/test_childdevice.py
index 797e8dff..05743abb 100644
--- a/tests/test_childdevice.py
+++ b/tests/test_childdevice.py
@@ -125,8 +125,13 @@ async def test_parent_property(dev: Device):
 
 
 @has_children_smart
+@pytest.mark.requires_dummy()
 async def test_child_time(dev: Device, freezer: FrozenDateTimeFactory):
-    """Test a child device gets the time from it's parent module."""
+    """Test a child device gets the time from it's parent module.
+
+    This is excluded from real device testing as the test often fail if the
+    device time is not in the past.
+    """
     if not dev.children:
         pytest.skip(f"Device {dev} fixture does not have any children")
 
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 7a0b0dde..d22bb112 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -51,6 +51,10 @@ from .conftest import (
     turn_on,
 )
 
+# The cli tests should be testing the cli logic rather than a physical device
+# so mark the whole file for skipping with real devices.
+pytestmark = [pytest.mark.requires_dummy]
+
 
 @pytest.fixture()
 def runner():
diff --git a/tests/test_common_modules.py b/tests/test_common_modules.py
index 5e2622b9..168b4090 100644
--- a/tests/test_common_modules.py
+++ b/tests/test_common_modules.py
@@ -1,7 +1,6 @@
 from datetime import datetime
 
 import pytest
-from freezegun.api import FrozenDateTimeFactory
 from pytest_mock import MockerFixture
 from zoneinfo import ZoneInfo
 
@@ -326,22 +325,38 @@ async def test_light_preset_save(dev: Device, mocker: MockerFixture):
     assert new_preset_state.color_temp == new_preset.color_temp
 
 
-async def test_set_time(dev: Device, freezer: FrozenDateTimeFactory):
+async def test_set_time(dev: Device):
     """Test setting the device time."""
-    freezer.move_to("2021-01-09 12:00:00+00:00")
     time_mod = dev.modules[Module.Time]
-    tz_info = time_mod.timezone
-    now = datetime.now(tz=tz_info)
-    now = now.replace(microsecond=0)
-    assert time_mod.time != now
 
-    await time_mod.set_time(now)
-    await dev.update()
-    assert time_mod.time == now
+    original_time = time_mod.time
+    original_timezone = time_mod.timezone
 
-    zone = ZoneInfo("Europe/Berlin")
-    now = datetime.now(tz=zone)
-    now = now.replace(microsecond=0)
-    await time_mod.set_time(now)
-    await dev.update()
-    assert time_mod.time == now
+    test_time = datetime.fromisoformat("2021-01-09 12:00:00+00:00")
+    test_time = test_time.astimezone(original_timezone)
+
+    try:
+        assert time_mod.time != test_time
+
+        await time_mod.set_time(test_time)
+        await dev.update()
+        assert time_mod.time == test_time
+
+        if (
+            isinstance(original_timezone, ZoneInfo)
+            and original_timezone.key != "Europe/Berlin"
+        ):
+            test_zonezone = ZoneInfo("Europe/Berlin")
+        else:
+            test_zonezone = ZoneInfo("Europe/London")
+
+        # Just update the timezone
+        new_time = time_mod.time.astimezone(test_zonezone)
+        await time_mod.set_time(new_time)
+        await dev.update()
+        assert time_mod.time == new_time
+    finally:
+        # Reset back to the original
+        await time_mod.set_time(original_time)
+        await dev.update()
+        assert time_mod.time == original_time
diff --git a/tests/test_device_factory.py b/tests/test_device_factory.py
index 35031cd0..8690e580 100644
--- a/tests/test_device_factory.py
+++ b/tests/test_device_factory.py
@@ -35,6 +35,10 @@ from kasa.discover import DiscoveryResult
 
 from .conftest import DISCOVERY_MOCK_IP
 
+# Device Factory tests are not relevant for real devices which run against
+# a single device that has already been created via the factory.
+pytestmark = [pytest.mark.requires_dummy]
+
 
 def _get_connection_type_device_class(discovery_info):
     if "result" in discovery_info:
diff --git a/tests/test_discovery.py b/tests/test_discovery.py
index 0318de35..7f69977e 100644
--- a/tests/test_discovery.py
+++ b/tests/test_discovery.py
@@ -53,6 +53,9 @@ from .conftest import (
     wallswitch_iot,
 )
 
+# A physical device has to respond to discovery for the tests to work.
+pytestmark = [pytest.mark.requires_dummy]
+
 UNSUPPORTED = {
     "result": {
         "device_id": "xx",
diff --git a/tests/test_klapprotocol.py b/tests/test_klapprotocol.py
index 55df0b34..524a6be3 100644
--- a/tests/test_klapprotocol.py
+++ b/tests/test_klapprotocol.py
@@ -32,6 +32,9 @@ from kasa.smartprotocol import SmartProtocol
 
 DUMMY_QUERY = {"foobar": {"foo": "bar", "bar": "foo"}}
 
+# Transport tests are not designed for real devices
+pytestmark = [pytest.mark.requires_dummy]
+
 
 class _mock_response:
     def __init__(self, status, content: bytes):
diff --git a/tests/test_protocol.py b/tests/test_protocol.py
index 6c79885d..7638a4bf 100644
--- a/tests/test_protocol.py
+++ b/tests/test_protocol.py
@@ -687,10 +687,13 @@ def test_deprecated_protocol():
 @device_iot
 async def test_iot_queries_redaction(dev: IotDevice, caplog: pytest.LogCaptureFixture):
     """Test query sensitive info redaction."""
-    device_id = "123456789ABCDEF"
-    cast(FakeIotTransport, dev.protocol._transport).proto["system"]["get_sysinfo"][
-        "deviceId"
-    ] = device_id
+    if isinstance(dev.protocol._transport, FakeIotTransport):
+        device_id = "123456789ABCDEF"
+        cast(FakeIotTransport, dev.protocol._transport).proto["system"]["get_sysinfo"][
+            "deviceId"
+        ] = device_id
+    else:  # real device with --ip
+        device_id = dev.sys_info["deviceId"]
 
     # Info no message logging
     caplog.set_level(logging.INFO)
diff --git a/tests/test_smartdevice.py b/tests/test_smartdevice.py
index d96542e5..616db77e 100644
--- a/tests/test_smartdevice.py
+++ b/tests/test_smartdevice.py
@@ -26,6 +26,7 @@ from .conftest import (
 
 
 @device_smart
+@pytest.mark.requires_dummy()
 async def test_try_get_response(dev: SmartDevice, caplog):
     mock_response: dict = {
         "get_device_info": SmartErrorCode.PARAMS_ERROR,
@@ -37,6 +38,7 @@ async def test_try_get_response(dev: SmartDevice, caplog):
 
 
 @device_smart
+@pytest.mark.requires_dummy()
 async def test_update_no_device_info(dev: SmartDevice, mocker: MockerFixture):
     mock_response: dict = {
         "get_device_usage": {},
diff --git a/tests/test_smartprotocol.py b/tests/test_smartprotocol.py
index 19e62a3d..ab68b34b 100644
--- a/tests/test_smartprotocol.py
+++ b/tests/test_smartprotocol.py
@@ -1,5 +1,4 @@
 import logging
-from typing import cast
 
 import pytest
 import pytest_mock
@@ -420,10 +419,11 @@ async def test_smart_queries_redaction(
     dev: SmartDevice, caplog: pytest.LogCaptureFixture
 ):
     """Test query sensitive info redaction."""
-    device_id = "123456789ABCDEF"
-    cast(FakeSmartTransport, dev.protocol._transport).info["get_device_info"][
-        "device_id"
-    ] = device_id
+    if isinstance(dev.protocol._transport, FakeSmartTransport):
+        device_id = "123456789ABCDEF"
+        dev.protocol._transport.info["get_device_info"]["device_id"] = device_id
+    else:  # real device
+        device_id = dev.device_id
 
     # Info no message logging
     caplog.set_level(logging.INFO)
diff --git a/tests/test_sslaestransport.py b/tests/test_sslaestransport.py
index 30c74234..49605d37 100644
--- a/tests/test_sslaestransport.py
+++ b/tests/test_sslaestransport.py
@@ -27,6 +27,9 @@ from kasa.experimental.sslaestransport import (
 from kasa.httpclient import HttpClient
 from kasa.protocol import DEFAULT_CREDENTIALS, get_default_credentials
 
+# Transport tests are not designed for real devices
+pytestmark = [pytest.mark.requires_dummy]
+
 MOCK_ADMIN_USER = get_default_credentials(DEFAULT_CREDENTIALS["TAPOCAMERA"]).username
 MOCK_PWD = "correct_pwd"  # noqa: S105
 MOCK_USER = "mock@example.com"