From 44fcd53d0089d1a9dad26a2f78e1b576e81a55dc Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 18:57:39 +0200 Subject: [PATCH 01/29] docs: add design spec for HA test-harness migration Design for replacing PR #394 (coordinator/entity tests) with a pytest-homeassistant-custom-component based approach: real hass + MockConfigEntry, patch at the Zaptec client boundary, assert on public state. Includes an OS-guarded Windows compat shim so the harness runs in native-Windows py314 and on Linux CI. Establishes reusable infra for the later #395 replacement. Bug #410 kept test-only (xfail) pending maintainer input on availability semantics. Co-Authored-By: Claude Opus 4.8 --- ...-07-25-ha-test-harness-migration-design.md | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md diff --git a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md new file mode 100644 index 00000000..c73955ce --- /dev/null +++ b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md @@ -0,0 +1,120 @@ +# Design: Migrate coordinator/entity tests to the HA test harness + +**Date:** 2026-07-25 +**Status:** Approved (brainstorming complete) +**Scope of this spec:** the replacement for PR #394 (coordinator + entity tests). Establishes the reusable infrastructure that a later, separate PR (replacing #395, the platform-entity tests) will build on. + +## Background & motivation + +Maintainer review on PR #394 (CHANGES_REQUESTED, 2026-07-25) asked how gold/platinum HA integrations test coordinators and entities, aiming at a high-quality standard. + +The current test suite is hand-rolled: it instantiates `ZaptecUpdateCoordinator` and `ZaptecBaseEntity` directly and asserts against private methods (`# noqa: SLF001` throughout `tests/test_entity.py`), with a `MagicMock`-based fake `hass` and a `FakeConfigEntry` in `tests/conftest.py`. This is white-box, implementation-coupled testing. + +Gold/platinum HA integrations instead use `pytest-homeassistant-custom-component` (pytest-hacc): a real `hass`, `MockConfigEntry`, and tests that set the integration up through the normal `async_setup` path with the cloud API mocked, then assert on **public state** (`hass.states.get(...)`, entity/device registries), often via `syrupy` snapshot tests. + +The original reason for the hand-rolled mocks was that pytest-hacc did not run on the maintainer's native-Windows dev environment (`homeassistant` imports `fcntl`, which is Unix-only). That is a local-dev constraint, not a project one: CI runs on Linux where pytest-hacc works, and the Windows issue is solvable with a small, OS-guarded compatibility shim (the sibling `luxtronik` integration already does exactly this). + +PRs #394 and #395 have been converted to **draft** and will be replaced by PRs built on this approach. + +## Goals + +- Bring the coordinator + entity tests to gold/platinum shape: behavior-first, through the real HA harness. +- Establish reusable test infrastructure (`mock_zaptec` + `setup_integration` + Windows shim) that the #395 replacement reuses without re-solving anything. +- Match or beat current coverage on `coordinator.py` / `entity.py` (100% / 98%) — but via observable behavior, not private-method assertions. +- Tests must run green in **native-Windows py314** (via the shim) *and* Linux CI. + +## Non-goals (out of scope for this spec) + +- The six platform files (`sensor/switch/number/button/binary_sensor/update`) — that is the #395 replacement, a separate PR. +- Config-flow / `__init__` coverage. +- Snapshot tests (deferred to the #395 replacement, where full-state snapshots pay off). +- Fixing bug #410 (see "Bug #410" below — this PR stays test-only). + +## Approach (selected) + +**Real harness, behavior-first.** Adopt pytest-hacc, patch the integration at the `Zaptec` client boundary, and assert on public state. Chosen over (B) a like-for-like fixture swap and (C) staying hand-rolled, because it is the only option that reaches the target standard and it turns the #410 gap into a real, self-catching test. + +## Design + +### 1. Test infrastructure (the foundation) + +- **`requirements_test.txt`** — add `pytest-homeassistant-custom-component` and a **pinned** `homeassistant` version matching what CI already resolves (validate workflow tests Python 3.13/3.14). Pin rather than float, so a new HA/plugin release can't silently break the harness. +- **`conftest.py` (repo root, new)** — port luxtronik's OS-guarded shim: under `sys.platform == "win32"`, stub `fcntl` / `resource` and wrap `socket.socketpair`, then `pytest_plugins = "pytest_homeassistant_custom_component.plugins"`. Completely no-op on Linux, so CI is unaffected. `pytest_plugins` is only honored in the rootdir conftest, so this cannot live in `tests/conftest.py`. +- **pytest config** (`pyproject.toml` or `pytest.ini`) — add `-p no:homeassistant` to block the broken plugin autoload; the root conftest re-loads it explicitly *after* shimming. Confirm during planning that the repo has no conflicting existing pytest config. +- **`tests/conftest.py`** — replace the hand-rolled `hass` / `FakeConfigEntry` with: + - the harness's real `hass` fixture, + - a **`mock_zaptec`** fixture: `MagicMock(spec=Zaptec)` pre-populated with a representative installation + charger object graph (and, because `Zaptec` is a `Mapping[str, ZaptecBase]`, implementing `__getitem__` / `__iter__` / `values()` to yield the fake `Charger` / `Installation` objects the platforms enumerate), + - a **`mock_config_entry`** (`MockConfigEntry`) and a **`setup_integration(hass, mock_zaptec)`** helper that patches the client into the setup path and awaits `async_setup`. + +**Why this shim is justified now (and #403 was not):** a standalone shim PR (#403) was closed because pytest-hacc was not a real dependency, so CI didn't install it and the unconditional plugin import broke Linux CI. Here pytest-hacc becomes a genuine `requirements_test.txt` dependency (CI installs it, Linux import works natively) and the shim is `win32`-guarded (never runs on Linux). Both failure modes are avoided. + +**Key risk & mitigation:** the whole approach hinges on the shim making pytest-hacc run in native-Windows py314. **Plan step 1 is a throwaway feasibility probe** (a trivial `async def test_hass(hass)` under the shim) before any real test is written. Fallback if it fails: run tests in a devcontainer on the user's Raspberry Pi (HA-in-Docker, separate port). Rated low-risk because luxtronik already runs pytest-hacc in this exact py314 env. + +### 2. The #394-replacement tests (coordinator + entity, behavior-first) + +Same two modules, asserted through the real harness instead of poking privates. + +- **Backbone / setup test** — set up the integration via `setup_integration`; assert entities land in the state machine and registry. This exercises `entity.py`'s `__init__` / `unique_id` / `device_info` wiring as a side effect, with no direct instantiation. +- **Coordinator behavior:** + - Successful refresh → entities have expected states after `coordinator.async_refresh()`. + - Failed refresh (mock client raises) → `coordinator.last_update_success` False → entities report `unavailable` via `hass.states.get()`. + - Poll scheduling (`trigger_poll`, charging-interval switch) → driven via public methods and the harness's time control (`freezer` / `async_fire_time_changed`), not a hand-attached loop. +- **Entity behavior:** + - Value present → `hass.states.get("sensor.…").state` equals the expected transformed value (covers `_get_zaptec_value`, dotted keys, lowercasing through a real entity). + - Key missing / non-mapping object → assert actual reported availability + no crash propagation. + - Availability transition logging — kept, asserted on observable state where possible. + +**Assertion style:** move from `entity._attr_available is False # noqa: SLF001` to `hass.states.get("sensor.x").state == "unavailable"`. A **small residue** of white-box tests is acceptable for pure-logging helpers (e.g. `_log_value` dedup) that have no observable state effect — kept to a minimum. + +**Coverage target:** match or beat current 100% / 98% on `coordinator.py` / `entity.py`, achieved via behavior. + +### 3. The mocked Zaptec client & shared test data + +**Patch at the `Zaptec` client boundary (Layer 2), not the HTTP/SignalR wire (Layer 1).** + +The integration builds the client in `__init__.py` (`zaptec = Zaptec(...)`, then `.login()`, `ZaptecManager.first_time_setup(zaptec=...)`, `ZaptecManager(..., zaptec=...)`). We patch the `Zaptec` symbol where `__init__.py` uses it so construction returns `mock_zaptec`: + +```python +with patch("custom_components.zaptec.Zaptec", return_value=mock_zaptec): + await hass.config_entries.async_setup(entry.entry_id) +``` + +Everything *above* the client — `ZaptecManager`, `ZaptecUpdateCoordinator`, `ZaptecBaseEntity`, all platforms — runs as real code against the mock's data. + +- **Layer 1 (HTTP wire) rejected:** would additionally exercise `api.py`, but couples every setup test to the cloud's JSON/SignalR format (login, installations, chargers, constants, state polls, SignalR handshake) — large and brittle. `api.py` already has dedicated tests in `tests/zaptec/test_api.py`, so re-testing it through the integration adds fragility for no coverage gain. +- **Layer 3 (mock the manager) rejected:** too high; would stop exercising the coordinator/entity code under test. + +**`spec=` discipline:** `MagicMock(spec=Zaptec)` / `spec=Charger` / `spec=Installation` so a typo'd or renamed client method fails loudly instead of returning a fresh mock. Carried over as a deliberate strength of the current tests. + +**Test-data source:** a small hand-authored dict suffices for #394 (coordinator/entity base behavior needs only a couple of keys). The **fuller** payload needed by the #395 replacement will be seeded from a **redacted real diagnostics dump** (the repo already has `diagnostics.py` + `redact.py`), stored as a JSON fixture, so snapshots reflect real-world data rather than invented values. + +### 4. Bug #410 handling — test-only, deferred fix + +Filed as custom-components/zaptec#410: `ZaptecBaseEntity` sets `_attr_available = False` on `KeyUnavailableError` but never overrides `available`, so the flag has no effect on reported availability. + +Investigation showed the fix is **not obvious** and needs maintainer input: + +1. **Mechanical gap:** `available` isn't overridden (trivial to add). +2. **Latent sticky-flag bug:** `_handle_coordinator_update` never resets `_attr_available = True` on the success path, so a naive override would leave recovered entities unavailable forever. Any fix must override `available` *and* reset the flag on success. +3. **Semantic design question:** many keys are legitimately absent for some charger models / installation types / roles (the code already has a skip-set for such keys in `_log_unavailable`). Making *any* `KeyUnavailableError` flip an entity to `unavailable` could make entities disappear for real users. Which keys are "required" vs. optional is a maintainer decision. + +**Decision:** this PR stays **test-only**. The availability case is asserted as today's real behavior with an `xfail(reason="#410")` documenting the gap through the real harness. The fix is deferred to a separate PR after the semantics are decided. #410 has been updated with findings (2) and (3) and a request for input from @sveinse / @steinmn. + +## Success criteria (this PR) + +- Coverage on `coordinator.py` / `entity.py` ≥ current (100% / 98%), achieved via behavior. +- `pytest tests` green in native-Windows py314 (via shim) **and** Linux CI. +- `ruff format` + `ruff check` clean. +- hassfest / HACS unaffected (`requirements_test.txt` is not shipped in the component; the root `conftest.py` and pytest config are dev-only). + +## PR / branch strategy + +- #394 and #395 held as draft (done 2026-07-25); reply posted on #394 explaining the direction. +- New branch off `master` for this replacement PR (per repo convention: dedicated branch per unit of work). +- The #395 replacement is a **separate, later** PR that reuses this infrastructure. + +## Open items carried into planning + +- Confirm exact pinned `homeassistant` version and whether the repo has existing pytest config to reconcile. +- Confirm exact patch target (`custom_components.zaptec.Zaptec` import site) and the minimal `mock_zaptec` object-graph shape for #394. +- Feasibility probe (plan step 1) before writing real tests. From 1143bf137d061ce614e9c25e46a2610e991078e5 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:52:54 +0200 Subject: [PATCH 02/29] test: adopt pytest-homeassistant-custom-component harness with Windows shim --- conftest.py | 56 +++++++++++++++++++++++++++++++++++++ pyproject.toml | 1 + requirements_test.txt | 4 ++- tests/test_harness_smoke.py | 11 ++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 conftest.py create mode 100644 tests/test_harness_smoke.py diff --git a/conftest.py b/conftest.py new file mode 100644 index 00000000..46c3bf76 --- /dev/null +++ b/conftest.py @@ -0,0 +1,56 @@ +"""Repo-root conftest: load pytest-homeassistant-custom-component explicitly. + +The plugin autoloads via a pytest11 entry point, but importing it on Windows +fails immediately (`homeassistant.runner` imports `fcntl`, Unix-only) before any +test collects. `-p no:homeassistant` in pyproject.toml blocks that autoload; +this file loads the plugin back explicitly, with Windows compatibility shims +applied first. pytest only honors `pytest_plugins` in the rootdir conftest, so +this cannot live in tests/conftest.py. The shim is a no-op on Linux (CI), where +fcntl/resource exist and the plugin imports natively. +""" + +import socket +import sys +import types +from typing import Any + +if sys.platform == "win32": + import pytest_socket + + if "fcntl" not in sys.modules: + fake_fcntl = types.ModuleType("fcntl") + fake_fcntl.LOCK_SH = 1 + fake_fcntl.LOCK_EX = 2 + fake_fcntl.LOCK_NB = 4 + fake_fcntl.LOCK_UN = 8 + fake_fcntl.flock = lambda *_args: None + fake_fcntl.lockf = lambda *_args: None + fake_fcntl.fcntl = lambda *_args: 0 + fake_fcntl.ioctl = lambda *_args: 0 + sys.modules["fcntl"] = fake_fcntl + + if "resource" not in sys.modules: + fake_resource = types.ModuleType("resource") + fake_resource.RLIMIT_NOFILE = 7 + fake_resource.RLIM_INFINITY = -1 + fake_resource.getrlimit = lambda *_args: (8192, 8192) + fake_resource.setrlimit = lambda *_args: None + sys.modules["resource"] = fake_resource + + _orig_socketpair = socket.socketpair + + def _shimmed_socketpair(*args: Any, **kwargs: Any) -> tuple[socket.socket, socket.socket]: + blocked = getattr(socket.socket, "__module__", "") == "pytest_socket" + if not blocked: + return _orig_socketpair(*args, **kwargs) + + pytest_socket.enable_socket() + try: + return _orig_socketpair(*args, **kwargs) + finally: + pytest_socket.socket_allow_hosts(["127.0.0.1"]) + pytest_socket.disable_socket(allow_unix_socket=True) + + socket.socketpair = _shimmed_socketpair + +pytest_plugins = "pytest_homeassistant_custom_component.plugins" diff --git a/pyproject.toml b/pyproject.toml index d3c71a4f..a2aadd6c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,6 +8,7 @@ pythonpath = [ testpaths = [ "tests", ] +addopts = "-p no:homeassistant" log_format = "%(asctime)s.%(msecs)03d %(levelname)-8s %(threadName)s %(name)s:%(filename)s:%(lineno)s %(message)s" log_date_format = "%Y-%m-%d %H:%M:%S" filterwarnings = [ diff --git a/requirements_test.txt b/requirements_test.txt index cf21f7bc..bfa83032 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,4 +1,6 @@ pytest pytest-asyncio pytest-mock -pytest-cov \ No newline at end of file +pytest-cov +pytest-homeassistant-custom-component==0.13.324 +homeassistant==2026.4.3 diff --git a/tests/test_harness_smoke.py b/tests/test_harness_smoke.py new file mode 100644 index 00000000..de6b0491 --- /dev/null +++ b/tests/test_harness_smoke.py @@ -0,0 +1,11 @@ +"""Smoke test: the real HA `hass` fixture spins up under the shim. Removed in Task 5.""" + +from homeassistant.core import HomeAssistant + + +async def test_hass_fixture_starts(hass: HomeAssistant) -> None: + """The harness's real hass fixture is a live HomeAssistant with a working state machine.""" + assert isinstance(hass, HomeAssistant) + hass.states.async_set("probe.entity", "on") + await hass.async_block_till_done() + assert hass.states.get("probe.entity").state == "on" From 8cd0c0c0e2b9c05a62991cbb8d2a0bb570dfcbcc Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:58:53 +0200 Subject: [PATCH 03/29] test: prevent zaptec_constants fixture from clobbering HA event-loop policy --- tests/conftest.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index ba07a5c5..cbac5b7d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -56,7 +56,18 @@ def zaptec_password(skip_if_user_disabled_api_tests, skip_if_in_github_actions) @pytest.fixture(scope="session") def zaptec_constants() -> dict: - """Get latest constants from Zaptec API.""" + """Get latest constants from Zaptec API. + + Uses a self-contained event loop instead of `asyncio.run()`. Under + pytest-homeassistant-custom-component's `HassEventLoopPolicy`, + `asyncio.run()` unconditionally resets the thread's registered event + loop to `None` on exit (success or failure) via `asyncio.set_event_loop`. + That policy raises `RuntimeError` from `get_event_loop()` instead of + lazily creating one, so the next bare `asyncio_mode=auto` test in the + session would fail in its autouse loop-setup fixture. Saving/restoring + the previous loop here keeps this fixture from clobbering global + event-loop state for tests that run after it. + """ async def get_zaptec_constants() -> dict: async with Zaptec("N/A", "N/A") as zaptec: @@ -64,4 +75,15 @@ async def get_zaptec_constants() -> dict: const: dict = await zaptec.request("constants") return const - return asyncio.run(get_zaptec_constants()) + try: + previous_loop = asyncio.get_event_loop() + except RuntimeError: + previous_loop = None + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + try: + return loop.run_until_complete(get_zaptec_constants()) + finally: + loop.close() + asyncio.set_event_loop(previous_loop) From 6eb68ace6b3e3b2b14698bc98c85d3af305604a0 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:08:53 +0200 Subject: [PATCH 04/29] test: add real-harness setup fixtures (mock Zaptec client + MockConfigEntry) --- tests/conftest.py | 105 +++++++++++++++++++++++++++++++++++++++++++++ tests/test_init.py | 21 +++++++++ 2 files changed, 126 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index cbac5b7d..efdff1bc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,10 +1,19 @@ """Zaptec testing configuration file.""" import asyncio +from collections.abc import Callable, Iterable import os +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch +from homeassistant.const import CONF_PASSWORD, CONF_USERNAME +from homeassistant.core import HomeAssistant import pytest +from pytest_homeassistant_custom_component.common import MockConfigEntry +from custom_components.zaptec.const import DOMAIN +from custom_components.zaptec.manager import ZaptecManager +from custom_components.zaptec.zaptec import MISSING, Charger, Installation from custom_components.zaptec.zaptec.api import Zaptec @@ -87,3 +96,99 @@ async def get_zaptec_constants() -> dict: finally: loop.close() asyncio.set_event_loop(previous_loop) + + +def _backed_get(data: dict[str, Any]) -> Callable[..., Any]: + """Return a `.get(key, default=MISSING)` implementation backed by `data`.""" + + def _get(key: str, default: Any = MISSING) -> Any: + return data.get(key, default) + + return _get + + +def make_charger( + data: dict[str, Any], *, installation: MagicMock | None = None, charging: bool = False +) -> MagicMock: + """Build a spec'd Charger double backed by `data`.""" + charger = MagicMock(spec=Charger) + charger.id = data["id"] + charger.name = data.get("name", "Mock Charger") + charger.model = "Zaptec Charger" + charger.qual_id = f"Charger[{data['id'][-6:]}]" + charger.get.side_effect = _backed_get(data) + charger.is_charging.return_value = charging + charger.installation = installation + return charger + + +def make_installation(data: dict[str, Any], *, chargers: Iterable[MagicMock] = ()) -> MagicMock: + """Build a spec'd Installation double backed by `data`.""" + install = MagicMock(spec=Installation) + install.id = data["id"] + install.name = data.get("name", "Mock Installation") + install.model = "Zaptec Installation" + install.qual_id = f"Installation[{data['id'][-6:]}]" + install.get.side_effect = _backed_get(data) + install.chargers = list(chargers) + install.stream_main = AsyncMock(return_value=None) + install.stream_close = AsyncMock(return_value=None) + return install + + +@pytest.fixture +def mock_zaptec() -> MagicMock: + """A spec'd Zaptec client seeded with one installation and one charger.""" + installation = make_installation({"id": "inst-mock-1", "name": "Mock Home"}) + charger = make_charger( + { + "id": "chg-mock-1", + "name": "Mock Charger", + # Keys read by entities under test; extend as needed for coverage. + "operating_mode": "Connected", + "charger_operation_mode": "Connected", + }, + installation=installation, + charging=False, + ) + installation.chargers = [charger] + + objects = {installation.id: installation, charger.id: charger} + + zaptec = MagicMock(spec=Zaptec) + zaptec.__getitem__.side_effect = objects.__getitem__ + zaptec.__iter__.side_effect = lambda: iter(objects) + zaptec.__contains__.side_effect = objects.__contains__ + zaptec.__len__.side_effect = lambda: len(objects) + zaptec.objects.return_value = list(objects.values()) + zaptec.installations = [installation] + zaptec.chargers = [charger] + zaptec.login = AsyncMock(return_value=None) + zaptec.build = AsyncMock(return_value=None) + zaptec.poll = AsyncMock(return_value=None) + zaptec.show_all_updates = False + zaptec.redact = MagicMock() + zaptec.redact.dumps.return_value = "" + return zaptec + + +@pytest.fixture +def mock_config_entry() -> MockConfigEntry: + """A MockConfigEntry for the zaptec domain.""" + return MockConfigEntry( + domain=DOMAIN, + title="Mock Zaptec", + data={CONF_USERNAME: "user", CONF_PASSWORD: "pass"}, + entry_id="mock_entry_1", + ) + + +async def setup_integration( + hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_zaptec: MagicMock +) -> ZaptecManager: + """Set the integration up through the real async_setup, with a mocked client.""" + mock_config_entry.add_to_hass(hass) + with patch("custom_components.zaptec.Zaptec", return_value=mock_zaptec): + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + return mock_config_entry.runtime_data diff --git a/tests/test_init.py b/tests/test_init.py index 4082b177..6bd66daa 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -1,17 +1,22 @@ """Tests for custom_components.zaptec.__init__.""" from http import HTTPStatus +from unittest.mock import MagicMock +from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryError, ConfigEntryNotReady import pytest +from pytest_homeassistant_custom_component.common import MockConfigEntry from custom_components.zaptec import _config_entry_error +from custom_components.zaptec.manager import ZaptecManager from custom_components.zaptec.zaptec.exceptions import ( AuthenticationError, RequestConnectionError, RequestError, RequestTimeoutError, ) +from tests.conftest import setup_integration @pytest.mark.parametrize( @@ -33,3 +38,19 @@ def test_config_entry_error_mapping(err: Exception, expected: type[Exception]) -> None: """Setup login errors map to the right Home Assistant config-entry error.""" assert isinstance(_config_entry_error(err), expected) + + +async def test_setup_entry_creates_manager_and_entities( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A full setup wires up the manager and registers at least one entity.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + + assert isinstance(manager, ZaptecManager) + assert mock_config_entry.runtime_data is manager + # At least one entity from the seeded charger reached the state machine. + states = [s for s in hass.states.async_all() if s.entity_id.split(".")[1].startswith("mock")] + assert states, "expected at least one zaptec entity to be created" From bcdb186829f9842c965d359b4a0387a9abaaa6fc Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:55:48 +0200 Subject: [PATCH 05/29] test: behavior coverage for ZaptecUpdateCoordinator via real harness Adds success-poll, poll-failure, and charging-interval-switch tests driven through the real HA setup path (setup_integration), covering ZaptecUpdateCoordinator.last_update_success and set_update_interval. --- tests/test_coordinator.py | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests/test_coordinator.py diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py new file mode 100644 index 00000000..6c81268e --- /dev/null +++ b/tests/test_coordinator.py @@ -0,0 +1,56 @@ +"""Behavior tests for ZaptecUpdateCoordinator, driven through the real harness.""" + +from unittest.mock import MagicMock + +from homeassistant.core import HomeAssistant +from pytest_homeassistant_custom_component.common import MockConfigEntry + +from custom_components.zaptec.zaptec import ZaptecApiError +from tests.conftest import setup_integration + + +async def test_successful_poll_marks_last_update_success( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A successful poll leaves every coordinator reporting success.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + for coordinator in manager.all_coordinators: + assert coordinator.last_update_success is True + mock_zaptec.poll.assert_awaited() + + +async def test_poll_failure_sets_update_failed( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A ZaptecApiError during poll flips last_update_success to False.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + head = manager.head_coordinator + + mock_zaptec.poll.side_effect = ZaptecApiError("boom") + await head.async_refresh() + + assert head.last_update_success is False + + +async def test_device_coordinator_switches_interval_when_charging( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A charger's coordinator uses the shorter interval once it reports charging.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + charger_coord = manager.device_coordinators["chg-mock-1"] + idle_interval = charger_coord.update_interval + + # Flip the seeded charger to 'charging' and re-run the update-listener path. + mock_zaptec.chargers[0].is_charging.return_value = True + charger_coord.set_update_interval() + + assert charger_coord.update_interval < idle_interval From cedc13b50a478dab5a93b82dac94084242174dfe Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 22:46:56 +0200 Subject: [PATCH 06/29] test: behavior coverage for ZaptecBaseEntity; xfail documents #410 --- tests/test_entity.py | 100 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 tests/test_entity.py diff --git a/tests/test_entity.py b/tests/test_entity.py new file mode 100644 index 00000000..5ffdbfe6 --- /dev/null +++ b/tests/test_entity.py @@ -0,0 +1,100 @@ +"""Behavior tests for ZaptecBaseEntity, driven through the real harness.""" + +import logging +from unittest.mock import MagicMock + +from homeassistant.core import HomeAssistant +import pytest +from pytest_homeassistant_custom_component.common import MockConfigEntry + +from custom_components.zaptec.zaptec import MISSING +from tests.conftest import setup_integration + + +async def _get_zaptec_entity(hass: HomeAssistant) -> str: + """Return one live zaptec entity_id whose value is backed by seeded data. + + Not every zaptec entity reads a key that `mock_zaptec` seeds (e.g. the + 3-to-1-phase-switch-current number entity has no backing value and stays + "unknown"), and platform setup order is not guaranteed to surface a + backed entity first. Skip past unbacked entities to find one that + actually resolved a value, so the test exercises real value surfacing + rather than an incidental "unknown" state. + """ + for state in hass.states.async_all(): + if state.entity_id.startswith( + ("sensor.", "binary_sensor.", "switch.", "number.") + ) and state.state not in ("unavailable", "unknown"): + return state.entity_id + raise AssertionError("no backed zaptec entity found") + + +async def test_entity_reports_value_from_zaptec( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A backed key surfaces as the entity's state (not 'unavailable'/'unknown').""" + await setup_integration(hass, mock_config_entry, mock_zaptec) + entity_id = await _get_zaptec_entity(hass) + state = hass.states.get(entity_id) + assert state.state not in ("unavailable", "unknown") + + +@pytest.mark.xfail( + reason="#410: _attr_available is set on KeyUnavailableError but never affects " + "reported availability (available is not overridden). Documenting current behavior.", + strict=True, +) +async def test_entity_becomes_unavailable_when_key_missing( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A key that disappears SHOULD mark the entity unavailable (currently it does not — #410).""" + await setup_integration(hass, mock_config_entry, mock_zaptec) + entity_id = await _get_zaptec_entity(hass) + + # Make every key lookup miss, then re-run a refresh so entities re-read. + mock_zaptec.chargers[0].get.side_effect = lambda _key, default=MISSING: default + manager = mock_config_entry.runtime_data + for coordinator in manager.all_coordinators: + await coordinator.async_refresh() + await hass.async_block_till_done() + + # This assertion is what SHOULD hold; strict xfail => the test failing here is expected + # and will turn XPASS (alerting us) once #410 is fixed. + assert hass.states.get(entity_id).state == "unavailable" + + +async def test_log_value_logs_on_change_then_skips_when_unchanged( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + caplog: pytest.LogCaptureFixture, + enable_custom_integrations: None, +) -> None: + """_log_value logs when the tracked value changes and stays quiet when it doesn't.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + # Grab a real entity instance from the platform via the coordinator's listeners. + # `_listeners` also holds the coordinator's own `set_update_interval` listener + # (registered in ZaptecUpdateCoordinator.__init__), so filter for a callback + # bound to an actual entity rather than assuming the first one qualifies. + coordinator = manager.device_coordinators["chg-mock-1"] + entity = next( + cb.__self__ + for cb, _context in coordinator._listeners.values() # noqa: SLF001 + if hasattr(cb.__self__, "_log_value") + ) + entity.some_attr = "value1" + + with caplog.at_level(logging.DEBUG): + entity._log_value("some_attr") # noqa: SLF001 + assert "value1" in caplog.text + + caplog.clear() + with caplog.at_level(logging.DEBUG): + entity._log_value("some_attr") # noqa: SLF001 + assert caplog.text == "" From b74943c73ffd4efde19fe08e0f254260c43d7043 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:02:20 +0200 Subject: [PATCH 07/29] test: remove temporary harness smoke test, cover coordinator/entity gaps The `hass` fixture is now exercised by real behavior tests, so the temporary tests/test_harness_smoke.py is no longer needed. Coverage check on coordinator.py/entity.py (the two migrated modules) showed real gaps once measured against just the migrated test files: coordinator.py's trigger_poll()/_trigger_poll() sequence (cancellation, child-coordinator triggering, the no-op-without-zaptec_object path) and the charging-interval-requires-Charger validation were entirely untested, and entity.py had a few uncovered branches in _get_zaptec_value(), _log_zaptec_attribute, and _log_unavailable(). Added targeted behavior tests for both, bringing coordinator.py to 100% and entity.py to 98% (line/branch, matching the pre-migration targets). Co-Authored-By: Claude Opus 4.8 --- tests/test_coordinator.py | 105 ++++++++++++++++++++++++++++- tests/test_entity.py | 130 +++++++++++++++++++++++++++++++++--- tests/test_harness_smoke.py | 11 --- 3 files changed, 224 insertions(+), 22 deletions(-) delete mode 100644 tests/test_harness_smoke.py diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 6c81268e..6bac1ecf 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -1,10 +1,12 @@ """Behavior tests for ZaptecUpdateCoordinator, driven through the real harness.""" -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock from homeassistant.core import HomeAssistant +import pytest from pytest_homeassistant_custom_component.common import MockConfigEntry +from custom_components.zaptec.coordinator import ZaptecUpdateCoordinator, ZaptecUpdateOptions from custom_components.zaptec.zaptec import ZaptecApiError from tests.conftest import setup_integration @@ -54,3 +56,104 @@ async def test_device_coordinator_switches_interval_when_charging( charger_coord.set_update_interval() assert charger_coord.update_interval < idle_interval + + +async def test_charging_update_interval_requires_charger_object( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, +) -> None: + """Constructing a coordinator with a charging interval on a non-Charger object errors.""" + mock_config_entry.add_to_hass(hass) + + with pytest.raises(ValueError, match="Charging update interval requires a Charger object"): + ZaptecUpdateCoordinator( + hass, + entry=mock_config_entry, + manager=MagicMock(), + options=ZaptecUpdateOptions( + name="bad", + update_interval=60, + charging_update_interval=30, + tracked_devices=set(), + poll_args={}, + zaptec_object=object(), # not a Charger instance + ), + ) + + +async def test_trigger_poll_is_noop_without_zaptec_object( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """trigger_poll() on a coordinator with no bound zaptec object does nothing.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + + await manager.head_coordinator.trigger_poll() + + assert manager.head_coordinator._trigger_task is None # noqa: SLF001 + + +async def test_trigger_poll_cancels_in_flight_task_and_reschedules( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A second trigger_poll() call cancels the running poll sequence and starts a new one.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + charger_coord = manager.device_coordinators["chg-mock-1"] + + # Collapse the real multi-second delays to zero so the poll sequence runs fast, while + # still going through real asyncio.sleep(0) checkpoints (needed so the eagerly-started + # background task actually suspends and can be observed/cancelled mid-flight). + monkeypatch.setattr( + "custom_components.zaptec.coordinator.ZAPTEC_POLL_CHARGER_TRIGGER_DELAYS", [0, 0, 0] + ) + + # HA's eager task factory starts the background task running immediately; it + # suspends at the first real `asyncio.sleep(0)` checkpoint and is left pending. + await charger_coord.trigger_poll() + first_task = charger_coord._trigger_task # noqa: SLF001 + assert first_task is not None + + # Second call sees the still-pending first task and cancels it before rescheduling. + await charger_coord.trigger_poll() + assert first_task.cancelled() + + second_task = charger_coord._trigger_task # noqa: SLF001 + assert second_task is not None + assert second_task is not first_task + await second_task + await hass.async_block_till_done() + + assert charger_coord._trigger_task is None # noqa: SLF001 + assert charger_coord.last_update_success is True + + +async def test_trigger_poll_triggers_child_charger_coordinators( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Polling an installation also triggers the poll sequence of its tracked chargers.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + install_coord = manager.device_coordinators["inst-mock-1"] + charger_coord = manager.device_coordinators["chg-mock-1"] + + monkeypatch.setattr( + "custom_components.zaptec.coordinator.asyncio.sleep", AsyncMock(return_value=None) + ) + charger_coord.trigger_poll = AsyncMock() + + await install_coord.trigger_poll() + task = install_coord._trigger_task # noqa: SLF001 + assert task is not None + await task + await hass.async_block_till_done() + + charger_coord.trigger_poll.assert_awaited_once() diff --git a/tests/test_entity.py b/tests/test_entity.py index 5ffdbfe6..e0c23e1c 100644 --- a/tests/test_entity.py +++ b/tests/test_entity.py @@ -1,16 +1,38 @@ """Behavior tests for ZaptecBaseEntity, driven through the real harness.""" import logging -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock from homeassistant.core import HomeAssistant import pytest from pytest_homeassistant_custom_component.common import MockConfigEntry +from custom_components.zaptec.const import KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK +from custom_components.zaptec.coordinator import ZaptecUpdateCoordinator +from custom_components.zaptec.entity import KeyUnavailableError, ZaptecBaseEntity from custom_components.zaptec.zaptec import MISSING from tests.conftest import setup_integration +def _entity_from_coordinator( + coordinator: ZaptecUpdateCoordinator, *, key_not_in_skip_list: bool = False +) -> ZaptecBaseEntity: + """Return a real entity instance bound to `coordinator`. + + `_listeners` also holds the coordinator's own `set_update_interval` listener + (registered in ZaptecUpdateCoordinator.__init__), so filter for a callback + bound to an actual entity rather than assuming the first one qualifies. + """ + for cb, _context in coordinator._listeners.values(): # noqa: SLF001 + candidate = cb.__self__ + if not hasattr(candidate, "_log_value"): + continue + if key_not_in_skip_list and candidate.key in KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK: + continue + return candidate + raise AssertionError("no matching zaptec entity found") + + async def _get_zaptec_entity(hass: HomeAssistant) -> str: """Return one live zaptec entity_id whose value is backed by seeded data. @@ -78,16 +100,8 @@ async def test_log_value_logs_on_change_then_skips_when_unchanged( ) -> None: """_log_value logs when the tracked value changes and stays quiet when it doesn't.""" manager = await setup_integration(hass, mock_config_entry, mock_zaptec) - # Grab a real entity instance from the platform via the coordinator's listeners. - # `_listeners` also holds the coordinator's own `set_update_interval` listener - # (registered in ZaptecUpdateCoordinator.__init__), so filter for a callback - # bound to an actual entity rather than assuming the first one qualifies. coordinator = manager.device_coordinators["chg-mock-1"] - entity = next( - cb.__self__ - for cb, _context in coordinator._listeners.values() # noqa: SLF001 - if hasattr(cb.__self__, "_log_value") - ) + entity = _entity_from_coordinator(coordinator) entity.some_attr = "value1" with caplog.at_level(logging.DEBUG): @@ -98,3 +112,99 @@ async def test_log_value_logs_on_change_then_skips_when_unchanged( with caplog.at_level(logging.DEBUG): entity._log_value("some_attr") # noqa: SLF001 assert caplog.text == "" + + +async def test_get_zaptec_value_returns_default_when_key_missing( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """_get_zaptec_value() returns the caller's default when the key isn't backed.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + + sentinel = object() + assert entity._get_zaptec_value(key="totally_missing_key", default=sentinel) is sentinel # noqa: SLF001 + + +async def test_get_zaptec_value_raises_when_intermediate_value_not_mapping( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """A dotted key whose first segment resolves to a non-Mapping value raises.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + + # "operating_mode" is seeded as a plain string, which has no `.get()`. + with pytest.raises(KeyUnavailableError): + entity._get_zaptec_value(key="operating_mode.sub") # noqa: SLF001 + + +async def test_log_zaptec_attribute_formats_none_str_and_iterable_keys( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """_log_zaptec_attribute formats None, a single key, and an iterable of keys.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + + entity._log_zaptec_key = None # noqa: SLF001 + assert entity._log_zaptec_attribute == "" # noqa: SLF001 + + entity._log_zaptec_key = "foo" # noqa: SLF001 + assert entity._log_zaptec_attribute == ".foo" # noqa: SLF001 + + entity._log_zaptec_key = ["foo", "bar"] # noqa: SLF001 + assert entity._log_zaptec_attribute == ".foo and .bar" # noqa: SLF001 + + +async def test_log_unavailable_logs_error_and_recovery_transitions( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + caplog: pytest.LogCaptureFixture, + enable_custom_integrations: None, +) -> None: + """_log_unavailable logs the real exception on going unavailable, and logs recovery.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator, key_not_in_skip_list=True) + + entity._prev_available = True # noqa: SLF001 + entity._attr_available = False # noqa: SLF001 + with caplog.at_level(logging.DEBUG): + entity._log_unavailable(RuntimeError("boom")) # noqa: SLF001 + assert f"Entity {entity.entity_id} is unavailable" in caplog.text + assert "Getting value failed" in caplog.text + + caplog.clear() + entity._prev_available = False # noqa: SLF001 + entity._attr_available = True # noqa: SLF001 + with caplog.at_level(logging.DEBUG): + entity._log_unavailable() # noqa: SLF001 + assert f"Entity {entity.entity_id} is available" in caplog.text + + +async def test_entity_trigger_poll_delegates_to_coordinator( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """ZaptecBaseEntity.trigger_poll() awaits the bound coordinator's trigger_poll().""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + coordinator = manager.device_coordinators["chg-mock-1"] + entity = _entity_from_coordinator(coordinator) + + coordinator.trigger_poll = AsyncMock() + await entity.trigger_poll() + + coordinator.trigger_poll.assert_awaited_once() diff --git a/tests/test_harness_smoke.py b/tests/test_harness_smoke.py deleted file mode 100644 index de6b0491..00000000 --- a/tests/test_harness_smoke.py +++ /dev/null @@ -1,11 +0,0 @@ -"""Smoke test: the real HA `hass` fixture spins up under the shim. Removed in Task 5.""" - -from homeassistant.core import HomeAssistant - - -async def test_hass_fixture_starts(hass: HomeAssistant) -> None: - """The harness's real hass fixture is a live HomeAssistant with a working state machine.""" - assert isinstance(hass, HomeAssistant) - hass.states.async_set("probe.entity", "on") - await hass.async_block_till_done() - assert hass.states.get("probe.entity").state == "on" From 911d59b8de61e77976931885a6b4b343e26d7c49 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:12:16 +0200 Subject: [PATCH 08/29] test: document _backed_get MISSING-default/no-normalization divergence --- tests/conftest.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index efdff1bc..0b73e964 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -99,7 +99,14 @@ async def get_zaptec_constants() -> dict: def _backed_get(data: dict[str, Any]) -> Callable[..., Any]: - """Return a `.get(key, default=MISSING)` implementation backed by `data`.""" + """Return a `.get(key, default=MISSING)` implementation backed by `data`. + + Intentionally diverges from `ZaptecBase.get` in two ways: (1) defaults to + `MISSING` instead of `None`, and (2) does not normalize keys via `to_under`. + This is sufficient for coordinator/entity code under test (which always passes + `default=MISSING` and uses snake_case keys), but future fixtures like #395's + diagnostics dump should not blindly inherit these assumptions. + """ def _get(key: str, default: Any = MISSING) -> Any: return data.get(key, default) From 0ecebab22edd5d36ae7930eb032c51f09e6172c0 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:38:35 +0200 Subject: [PATCH 09/29] test: assert correct entity-availability behavior (#410 is not a bug) Replace the strict-xfail test_entity_becomes_unavailable_when_key_missing, which asserted incorrect intended behavior, with two passing tests that document the real mechanism: CoordinatorEntity.available is driven solely by coordinator.last_update_success, so a single missing backing key leaves the entity available (it just retains its prior value), while a failed coordinator poll does mark the entity unavailable. --- tests/test_entity.py | 48 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/tests/test_entity.py b/tests/test_entity.py index e0c23e1c..9f854e83 100644 --- a/tests/test_entity.py +++ b/tests/test_entity.py @@ -10,7 +10,7 @@ from custom_components.zaptec.const import KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK from custom_components.zaptec.coordinator import ZaptecUpdateCoordinator from custom_components.zaptec.entity import KeyUnavailableError, ZaptecBaseEntity -from custom_components.zaptec.zaptec import MISSING +from custom_components.zaptec.zaptec import MISSING, ZaptecApiError from tests.conftest import setup_integration @@ -64,21 +64,27 @@ async def test_entity_reports_value_from_zaptec( assert state.state not in ("unavailable", "unknown") -@pytest.mark.xfail( - reason="#410: _attr_available is set on KeyUnavailableError but never affects " - "reported availability (available is not overridden). Documenting current behavior.", - strict=True, -) -async def test_entity_becomes_unavailable_when_key_missing( +async def test_entity_stays_available_when_single_key_missing( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_zaptec: MagicMock, enable_custom_integrations: None, ) -> None: - """A key that disappears SHOULD mark the entity unavailable (currently it does not — #410).""" + """A single missing backing key does NOT mark the entity unavailable (#410 is not a bug). + + `ZaptecBaseEntity` extends `CoordinatorEntity`, whose `available` property is + driven solely by `coordinator.last_update_success` and never reads + `_attr_available`. When `_update_from_zaptec` raises `KeyUnavailableError`, + `_handle_coordinator_update` catches it and the coordinator's poll still + succeeds, so the entity stays available and simply retains its previous + value/state. + """ await setup_integration(hass, mock_config_entry, mock_zaptec) entity_id = await _get_zaptec_entity(hass) + state_before = hass.states.get(entity_id) + assert state_before.state not in ("unavailable", "unknown") + # Make every key lookup miss, then re-run a refresh so entities re-read. mock_zaptec.chargers[0].get.side_effect = lambda _key, default=MISSING: default manager = mock_config_entry.runtime_data @@ -86,8 +92,30 @@ async def test_entity_becomes_unavailable_when_key_missing( await coordinator.async_refresh() await hass.async_block_till_done() - # This assertion is what SHOULD hold; strict xfail => the test failing here is expected - # and will turn XPASS (alerting us) once #410 is fixed. + state_after = hass.states.get(entity_id) + assert state_after.state != "unavailable" + assert state_after.state == state_before.state + + +async def test_entity_unavailable_when_coordinator_poll_fails( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_zaptec: MagicMock, + enable_custom_integrations: None, +) -> None: + """The entity reports 'unavailable' when its coordinator's poll fails. + + This is the actual mechanism behind entity availability: `CoordinatorEntity.available` + reflects `coordinator.last_update_success`, not any per-key state. + """ + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + entity_id = await _get_zaptec_entity(hass) + + mock_zaptec.poll.side_effect = ZaptecApiError("boom") + coordinator = manager.device_coordinators["chg-mock-1"] + await coordinator.async_refresh() + await hass.async_block_till_done() + assert hass.states.get(entity_id).state == "unavailable" From 650e888562ccbd67e61947c6e6ab6d570a8bd32a Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:10:34 +0200 Subject: [PATCH 10/29] test: unpin pytest-hacc so CI 3.13 matrix leg can install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit requirements_test.txt hard-pinned homeassistant==2026.4.3 and pytest-homeassistant-custom-component==0.13.324, both of which require Python >=3.14 — so CI's 3.13 matrix leg failed at "Install requirements". HA is already pinned in requirements.txt (with a 3.13 sed-revert to 2026.2.3 in validate.yaml), and pytest-hacc pins an exact homeassistant itself, so dropping the duplicate HA line and leaving pytest-hacc unpinned lets pip resolve the release matching whichever HA the active Python leg installs. Co-Authored-By: Claude Opus 4.8 --- .../2026-07-25-ha-test-harness-migration.md | 604 ++++++++++++++++++ ...-07-25-ha-test-harness-migration-design.md | 4 +- requirements_test.txt | 9 +- 3 files changed, 613 insertions(+), 4 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md diff --git a/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md b/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md new file mode 100644 index 00000000..b64d8cdd --- /dev/null +++ b/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md @@ -0,0 +1,604 @@ +# HA Test-Harness Migration (coordinator + entity) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace PR #394's hand-rolled coordinator/entity unit tests with behavior-first tests running on the real `pytest-homeassistant-custom-component` (pytest-hacc) harness, plus the reusable infrastructure the later #395 replacement will build on. + +**Architecture:** Adopt pytest-hacc with a repo-root, `win32`-guarded compatibility shim so the harness runs on native-Windows `py314` and on Linux CI. Tests set the integration up through the real `hass` + `MockConfigEntry`, patching only the `Zaptec` client at its construction boundary (`patch("custom_components.zaptec.Zaptec", ...)`) so the manager, coordinators, entities, and platforms all run as real code against canned data. Assertions target public state (`hass.states.get(...)`, registries) instead of private methods. + +**Tech Stack:** Python 3.13/3.14 (CI matrix), Home Assistant 2026.4.3 (3.14) / 2026.2.3 (3.13 revert), `pytest-homeassistant-custom-component` (unpinned, follows HA), pytest 9, `syrupy` (available, used later by #395), `MagicMock(spec=...)` test doubles. + +## Global Constraints + +- Do NOT add `homeassistant` to `requirements_test.txt` (it is pinned in `requirements.txt`, with a 3.13 sed-revert in validate.yaml). Leave `pytest-homeassistant-custom-component` UNPINNED so it transitively resolves to the release matching whichever HA the active Python leg installs. Pinning an exact pytest-hacc version breaks CI's 3.13 leg (newest releases require Python >=3.14). +- The Windows shim MUST be guarded by `if sys.platform == "win32":` — it must be a complete no-op on Linux CI. +- No production code changes in `custom_components/**`. This PR is test-only. Bug #410 is documented via `xfail`, never fixed here. +- Local run command in this env: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest `. Use forward slashes for the python.exe path. +- Ruff (format + check) must be clean on all new/changed files, pinned ruff `0.15.22`. +- Never `git commit` without explicit user approval (project CLAUDE.md). Each task's "Commit" step means: stage, show the diff, and request approval before committing. +- `[tool.pytest.ini_options]` in `pyproject.toml` already sets `asyncio_mode = "auto"` and `filterwarnings = ["ignore::DeprecationWarning"]`. Reuse these; do not duplicate. + +--- + +### Task 1: pytest-hacc harness infrastructure + Windows shim + +**Files:** +- Modify: `requirements_test.txt` +- Create: `conftest.py` (repo root) +- Modify: `pyproject.toml` (add `addopts` under `[tool.pytest.ini_options]`) +- Test: `tests/test_harness_smoke.py` (temporary smoke test, removed in Task 5) + +**Interfaces:** +- Consumes: nothing (first task). +- Produces: a working real `hass` fixture available to every test; the repo-root `conftest.py` re-loads the pytest-hacc plugin after applying the Windows shim. + +- [ ] **Step 1: Add pinned test dependencies** + +Replace the contents of `requirements_test.txt` with: + +``` +pytest +pytest-asyncio +pytest-mock +pytest-cov +pytest-homeassistant-custom-component +``` + +- [ ] **Step 2: Create the repo-root conftest with the Windows shim** + +Create `conftest.py` at the repo root (NOT in `tests/` — `pytest_plugins` is only honored in the rootdir conftest): + +```python +"""Repo-root conftest: load pytest-homeassistant-custom-component explicitly. + +The plugin autoloads via a pytest11 entry point, but importing it on Windows +fails immediately (`homeassistant.runner` imports `fcntl`, Unix-only) before any +test collects. `-p no:homeassistant` in pyproject.toml blocks that autoload; +this file loads the plugin back explicitly, with Windows compatibility shims +applied first. pytest only honors `pytest_plugins` in the rootdir conftest, so +this cannot live in tests/conftest.py. The shim is a no-op on Linux (CI), where +fcntl/resource exist and the plugin imports natively. +""" + +import sys +import types + +if sys.platform == "win32": + if "fcntl" not in sys.modules: + fake_fcntl = types.ModuleType("fcntl") + fake_fcntl.LOCK_SH = 1 + fake_fcntl.LOCK_EX = 2 + fake_fcntl.LOCK_NB = 4 + fake_fcntl.LOCK_UN = 8 + fake_fcntl.flock = lambda *args, **kwargs: None + fake_fcntl.lockf = lambda *args, **kwargs: None + fake_fcntl.fcntl = lambda *args, **kwargs: 0 + fake_fcntl.ioctl = lambda *args, **kwargs: 0 + sys.modules["fcntl"] = fake_fcntl + + if "resource" not in sys.modules: + fake_resource = types.ModuleType("resource") + fake_resource.RLIMIT_NOFILE = 7 + fake_resource.RLIM_INFINITY = -1 + fake_resource.getrlimit = lambda *args, **kwargs: (8192, 8192) + fake_resource.setrlimit = lambda *args, **kwargs: None + sys.modules["resource"] = fake_resource + + import socket as _socket_mod + + _orig_socketpair = _socket_mod.socketpair + + def _shimmed_socketpair(*args, **kwargs): + blocked = getattr(_socket_mod.socket, "__module__", "") == "pytest_socket" + if not blocked: + return _orig_socketpair(*args, **kwargs) + import pytest_socket + + pytest_socket.enable_socket() + try: + return _orig_socketpair(*args, **kwargs) + finally: + pytest_socket.socket_allow_hosts(["127.0.0.1"]) + pytest_socket.disable_socket(allow_unix_socket=True) + + _socket_mod.socketpair = _shimmed_socketpair + +pytest_plugins = "pytest_homeassistant_custom_component.plugins" +``` + +- [ ] **Step 3: Block the broken plugin autoload in pyproject.toml** + +Add an `addopts` line inside the existing `[tool.pytest.ini_options]` table in `pyproject.toml` (leave `asyncio_mode`, `filterwarnings`, `pythonpath`, `testpaths` as-is): + +```toml +addopts = "-p no:homeassistant" +``` + +- [ ] **Step 4: Write the smoke test** + +Create `tests/test_harness_smoke.py`: + +```python +"""Smoke test: the real HA `hass` fixture spins up under the shim. Removed in Task 5.""" + +from homeassistant.core import HomeAssistant + + +async def test_hass_fixture_starts(hass: HomeAssistant) -> None: + """The harness's real hass fixture is a live HomeAssistant with a working state machine.""" + assert isinstance(hass, HomeAssistant) + hass.states.async_set("probe.entity", "on") + await hass.async_block_till_done() + assert hass.states.get("probe.entity").state == "on" +``` + +- [ ] **Step 5: Run the smoke test** + +Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_harness_smoke.py -v` +Expected: PASS (1 passed). If it errors with `No module named 'fcntl'`, the shim/rootdir wiring is wrong — verify `conftest.py` is at repo root and `addopts` was added. + +- [ ] **Step 6: Verify the rest of the suite still collects** + +Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests -q` +Expected: existing tests still pass/skip as before (the known `test_zconst.py`/`test_redact.py` DNS errors may appear — that is pre-existing and unrelated). No new collection errors. + +- [ ] **Step 7: Ruff + commit** + +Run: `"C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m ruff format . --diff` and `-m ruff check`. Fix any issues. +Then stage `requirements_test.txt`, `conftest.py`, `pyproject.toml`, `tests/test_harness_smoke.py`, show the diff, and request approval before: + +```bash +git add requirements_test.txt conftest.py pyproject.toml tests/test_harness_smoke.py +git commit -m "test: adopt pytest-homeassistant-custom-component harness with Windows shim" +``` + +--- + +### Task 2: Shared fixtures — mock Zaptec client, MockConfigEntry, setup helper + +**Files:** +- Modify: `tests/conftest.py` +- Test: `tests/test_init.py` (add an integration-setup test alongside the existing `test_config_entry_error_mapping`) + +**Interfaces:** +- Consumes: the real `hass` fixture (Task 1). +- Produces, in `tests/conftest.py`: + - `make_charger(data: dict, *, installation=None, charging: bool = False) -> MagicMock` — a `MagicMock(spec=Charger)` whose `.get(key, default=MISSING)` is backed by `data`, with `.id`, `.name`, `.model`, `.qual_id`, `.is_charging()`, `.installation` wired. + - `make_installation(data: dict, *, chargers=()) -> MagicMock` — a `MagicMock(spec=Installation)` similarly backed, with `.chargers`, async `.stream_main`/`.stream_close`. + - `mock_zaptec` fixture → `MagicMock(spec=Zaptec)` exposing Mapping access (`__getitem__`/`__iter__`/`__contains__`/`__len__`), `.objects()`, `.installations`, `.chargers`, async `.login`/`.build`/`.poll`, and `.redact`, seeded with one installation + one charger. + - `mock_config_entry` fixture → `MockConfigEntry` for domain `zaptec`. + - `setup_integration(hass, mock_config_entry, mock_zaptec) -> ZaptecManager` async helper that patches the client and runs full `async_setup`. + +- [ ] **Step 1: Write the failing integration-setup test** + +Add to `tests/test_init.py`: + +```python +from unittest.mock import patch + +from homeassistant.core import HomeAssistant + +from custom_components.zaptec.manager import ZaptecManager + + +async def test_setup_entry_creates_manager_and_entities( + hass: HomeAssistant, mock_config_entry, mock_zaptec +) -> None: + """A full setup wires up the manager and registers at least one entity.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + + assert isinstance(manager, ZaptecManager) + assert mock_config_entry.runtime_data is manager + # At least one entity from the seeded charger reached the state machine. + states = [s for s in hass.states.async_all() if s.entity_id.split(".")[1].startswith("mock")] + assert states, "expected at least one zaptec entity to be created" +``` + +(Note: `setup_integration`, `mock_config_entry`, `mock_zaptec` come from `tests/conftest.py`, added next.) + +- [ ] **Step 2: Run it to verify it fails** + +Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_init.py::test_setup_entry_creates_manager_and_entities -v` +Expected: FAIL — `fixture 'mock_config_entry' not found` (or `NameError: setup_integration`). + +- [ ] **Step 3: Add the fixtures and helper to tests/conftest.py** + +Append to `tests/conftest.py` (keep the existing api-login fixtures): + +```python +from unittest.mock import AsyncMock, MagicMock, patch + +from pytest_homeassistant_custom_component.common import MockConfigEntry + +from custom_components.zaptec.const import CONF_PASSWORD, CONF_USERNAME, DOMAIN +from custom_components.zaptec.manager import ZaptecManager +from custom_components.zaptec.zaptec import MISSING, Charger, Installation + + +def _backed_get(data: dict): + """Return a `.get(key, default=MISSING)` implementation backed by `data`.""" + + def _get(key, default=MISSING): + return data.get(key, default) + + return _get + + +def make_charger(data: dict, *, installation=None, charging: bool = False) -> MagicMock: + """Build a spec'd Charger double backed by `data`.""" + charger = MagicMock(spec=Charger) + charger.id = data["id"] + charger.name = data.get("name", "Mock Charger") + charger.model = "Zaptec Charger" + charger.qual_id = f"Charger[{data['id'][-6:]}]" + charger.get.side_effect = _backed_get(data) + charger.is_charging.return_value = charging + charger.installation = installation + return charger + + +def make_installation(data: dict, *, chargers=()) -> MagicMock: + """Build a spec'd Installation double backed by `data`.""" + install = MagicMock(spec=Installation) + install.id = data["id"] + install.name = data.get("name", "Mock Installation") + install.model = "Zaptec Installation" + install.qual_id = f"Installation[{data['id'][-6:]}]" + install.get.side_effect = _backed_get(data) + install.chargers = list(chargers) + install.stream_main = AsyncMock(return_value=None) + install.stream_close = AsyncMock(return_value=None) + return install + + +@pytest.fixture +def mock_zaptec() -> MagicMock: + """A spec'd Zaptec client seeded with one installation and one charger.""" + installation = make_installation({"id": "inst-mock-1", "name": "Mock Home"}) + charger = make_charger( + { + "id": "chg-mock-1", + "name": "Mock Charger", + # Keys read by entities under test; extend as needed for coverage. + "operating_mode": "Connected", + "charger_operation_mode": "Connected", + }, + installation=installation, + charging=False, + ) + installation.chargers = [charger] + + objects = {installation.id: installation, charger.id: charger} + + zaptec = MagicMock(spec=Zaptec) + zaptec.__getitem__.side_effect = objects.__getitem__ + zaptec.__iter__.side_effect = lambda: iter(objects) + zaptec.__contains__.side_effect = objects.__contains__ + zaptec.__len__.side_effect = lambda: len(objects) + zaptec.objects.return_value = list(objects.values()) + zaptec.installations = [installation] + zaptec.chargers = [charger] + zaptec.login = AsyncMock(return_value=None) + zaptec.build = AsyncMock(return_value=None) + zaptec.poll = AsyncMock(return_value=None) + zaptec.show_all_updates = False + zaptec.redact = MagicMock() + zaptec.redact.dumps.return_value = "" + return zaptec + + +@pytest.fixture +def mock_config_entry() -> MockConfigEntry: + """A MockConfigEntry for the zaptec domain.""" + return MockConfigEntry( + domain=DOMAIN, + title="Mock Zaptec", + data={CONF_USERNAME: "user", CONF_PASSWORD: "pass"}, + entry_id="mock_entry_1", + ) + + +async def setup_integration(hass, mock_config_entry, mock_zaptec) -> ZaptecManager: + """Set the integration up through the real async_setup, with a mocked client.""" + mock_config_entry.add_to_hass(hass) + with patch("custom_components.zaptec.Zaptec", return_value=mock_zaptec): + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + return mock_config_entry.runtime_data +``` + +Also add these imports at the top of `tests/test_init.py` so the test can call the helper: + +```python +from tests.conftest import setup_integration +``` + +- [ ] **Step 4: Run and iterate to green** + +Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_init.py -v` +Expected: PASS. Likely iteration points (fix against the real code if they surface): +- `CONF_USERNAME`/`CONF_PASSWORD`/`DOMAIN` import path — confirm they live in `custom_components/zaptec/const.py`; adjust if re-exported elsewhere. +- If setup calls a `Zaptec` member not wired above (e.g. an attribute read during `async_setup_entry`), add it to `mock_zaptec` as a `MagicMock`/`AsyncMock`. Check the traceback for the exact missing member. +- `enable_custom_integrations` — pytest-hacc's autouse fixture should load `custom_components.zaptec`; if the domain isn't found, add the `enable_custom_integrations` fixture arg to the test. + +- [ ] **Step 5: Ruff + commit** + +Run ruff format/check. Then stage `tests/conftest.py`, `tests/test_init.py`, show diff, request approval: + +```bash +git add tests/conftest.py tests/test_init.py +git commit -m "test: add real-harness setup fixtures (mock Zaptec client + MockConfigEntry)" +``` + +--- + +### Task 3: Coordinator behavior tests + +**Files:** +- Test: `tests/test_coordinator.py` (create) + +**Interfaces:** +- Consumes: `mock_zaptec`, `mock_config_entry`, `setup_integration` (Task 2); `hass` (Task 1). +- Produces: behavior coverage of `coordinator.py` via public coordinator API. + +- [ ] **Step 1: Write the failing tests** + +Create `tests/test_coordinator.py`: + +```python +"""Behavior tests for ZaptecUpdateCoordinator, driven through the real harness.""" + +from unittest.mock import patch + +from homeassistant.core import HomeAssistant +from homeassistant.helpers.update_coordinator import UpdateFailed +import pytest + +from custom_components.zaptec.zaptec import ZaptecApiError +from tests.conftest import setup_integration + + +async def test_successful_poll_marks_last_update_success( + hass: HomeAssistant, mock_config_entry, mock_zaptec +) -> None: + """A successful poll leaves every coordinator reporting success.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + for coordinator in manager.all_coordinators: + assert coordinator.last_update_success is True + mock_zaptec.poll.assert_awaited() + + +async def test_poll_failure_sets_update_failed( + hass: HomeAssistant, mock_config_entry, mock_zaptec +) -> None: + """A ZaptecApiError during poll flips last_update_success to False.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + head = manager.head_coordinator + + mock_zaptec.poll.side_effect = ZaptecApiError("boom") + await head.async_refresh() + + assert head.last_update_success is False + + +async def test_device_coordinator_switches_interval_when_charging( + hass: HomeAssistant, mock_config_entry, mock_zaptec +) -> None: + """A charger's coordinator uses the shorter interval once it reports charging.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + charger_coord = manager.device_coordinators["chg-mock-1"] + idle_interval = charger_coord.update_interval + + # Flip the seeded charger to 'charging' and re-run the update-listener path. + mock_zaptec.chargers[0].is_charging.return_value = True + charger_coord.set_update_interval() + + assert charger_coord.update_interval < idle_interval +``` + +- [ ] **Step 2: Run to verify they fail** + +Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_coordinator.py -v` +Expected: FAIL only if something is wired wrong — these use already-built fixtures, so a failure here signals a fixture gap (e.g. `set_update_interval` needs `options.zaptec_object` to be the same charger mock; confirm `mock_zaptec.chargers[0]` is the object stored as `zaptec["chg-mock-1"]`). Fix in `tests/conftest.py` if needed. + +- [ ] **Step 3: Make them pass** + +Iterate on fixtures/assertions until green. The charging-interval test depends on `ZAPTEC_POLL_INTERVAL_CHARGING < ZAPTEC_POLL_INTERVAL_IDLE` (true in `const.py`) and on the device coordinator's `options.zaptec_object` being a `Charger` — verify `make_charger` returns a `spec=Charger` instance so `isinstance(..., Charger)` in the coordinator passes. If `isinstance` fails against `MagicMock(spec=Charger)`, switch that check by constructing a real `Charger` (see Task 2 iteration note) or confirm `spec=Charger` satisfies `isinstance` (it does for `MagicMock(spec=Cls)`). + +- [ ] **Step 4: Run to verify pass** + +Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_coordinator.py -v` +Expected: PASS (3 passed). + +- [ ] **Step 5: Ruff + commit** + +```bash +git add tests/test_coordinator.py +git commit -m "test: behavior coverage for ZaptecUpdateCoordinator via real harness" +``` + +--- + +### Task 4: Entity behavior tests (incl. #410 xfail) + +**Files:** +- Test: `tests/test_entity.py` (create) + +**Interfaces:** +- Consumes: `mock_zaptec`, `mock_config_entry`, `setup_integration`, `hass`. +- Produces: behavior coverage of `entity.py`; documents #410 via `xfail`. + +- [ ] **Step 1: Write the failing tests** + +Create `tests/test_entity.py`. The value/availability tests read a real entity's public state; the two logging-dedup assertions are the deliberately-allowed small white-box residue. + +```python +"""Behavior tests for ZaptecBaseEntity, driven through the real harness.""" + +import logging + +from homeassistant.core import HomeAssistant +import pytest + +from tests.conftest import setup_integration + + +async def _get_zaptec_entity(hass: HomeAssistant): + """Return one live zaptec entity_id whose value is backed by seeded data.""" + for state in hass.states.async_all(): + if state.entity_id.startswith(("sensor.", "binary_sensor.", "switch.", "number.")): + return state.entity_id + raise AssertionError("no zaptec entity found") + + +async def test_entity_reports_value_from_zaptec( + hass: HomeAssistant, mock_config_entry, mock_zaptec +) -> None: + """A backed key surfaces as the entity's state (not 'unavailable'/'unknown').""" + await setup_integration(hass, mock_config_entry, mock_zaptec) + entity_id = await _get_zaptec_entity(hass) + state = hass.states.get(entity_id) + assert state.state not in ("unavailable", "unknown") + + +@pytest.mark.xfail( + reason="#410: _attr_available is set on KeyUnavailableError but never affects " + "reported availability (available is not overridden). Documenting current behavior.", + strict=True, +) +async def test_entity_becomes_unavailable_when_key_missing( + hass: HomeAssistant, mock_config_entry, mock_zaptec +) -> None: + """A key that disappears SHOULD mark the entity unavailable (currently it does not — #410).""" + await setup_integration(hass, mock_config_entry, mock_zaptec) + entity_id = await _get_zaptec_entity(hass) + + # Make every key lookup miss, then re-run a refresh so entities re-read. + from custom_components.zaptec.zaptec import MISSING + + mock_zaptec.chargers[0].get.side_effect = lambda key, default=MISSING: default + manager = mock_config_entry.runtime_data + for coordinator in manager.all_coordinators: + await coordinator.async_refresh() + await hass.async_block_till_done() + + # This assertion is what SHOULD hold; strict xfail => the test failing here is expected + # and will turn XPASS (alerting us) once #410 is fixed. + assert hass.states.get(entity_id).state == "unavailable" +``` + +- [ ] **Step 2: Run to verify status** + +Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_entity.py -v` +Expected: `test_entity_reports_value_from_zaptec` PASS; `test_entity_becomes_unavailable_when_key_missing` XFAIL (not FAIL, not XPASS). If it XPASSes, #410 is somehow already satisfied — stop and re-examine before proceeding. + +- [ ] **Step 3: Add the logging-dedup residue tests** + +These cover `_log_value`'s change-detection, which has no observable state effect, so a small white-box test is justified per the spec. Append to `tests/test_entity.py`: + +```python +async def test_log_value_logs_on_change_then_skips_when_unchanged( + hass: HomeAssistant, mock_config_entry, mock_zaptec, caplog +) -> None: + """_log_value logs when the tracked value changes and stays quiet when it doesn't.""" + manager = await setup_integration(hass, mock_config_entry, mock_zaptec) + # Grab a real entity instance from the platform via the coordinator's listeners. + coordinator = manager.device_coordinators["chg-mock-1"] + entity = next(iter(coordinator._listeners.values()))[0].__self__ # noqa: SLF001 + entity.some_attr = "value1" + + with caplog.at_level(logging.DEBUG): + entity._log_value("some_attr") # noqa: SLF001 + assert "value1" in caplog.text + + caplog.clear() + with caplog.at_level(logging.DEBUG): + entity._log_value("some_attr") # noqa: SLF001 + assert caplog.text == "" +``` + +Note: retrieving the entity instance from `coordinator._listeners` is fragile; if it doesn't resolve cleanly, instead import a concrete entity class (e.g. from `sensor.py`) and instantiate it directly with the `mock_zaptec` charger + the real coordinator — a minimal, contained white-box construction. Confirm the exact listener structure during implementation. + +- [ ] **Step 4: Run to verify pass** + +Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_entity.py -v` +Expected: 2 passed, 1 xfailed. + +- [ ] **Step 5: Ruff + commit** + +```bash +git add tests/test_entity.py +git commit -m "test: behavior coverage for ZaptecBaseEntity; xfail documents #410" +``` + +--- + +### Task 5: Coverage verification, smoke-test cleanup, final gate + +**Files:** +- Delete: `tests/test_harness_smoke.py` +- Verify only: coverage on `coordinator.py` / `entity.py` + +**Interfaces:** +- Consumes: everything from Tasks 1–4. +- Produces: the final, CI-ready state. + +- [ ] **Step 1: Remove the temporary smoke test** + +The `hass` fixture is now exercised by real tests; delete `tests/test_harness_smoke.py`. + +- [ ] **Step 2: Coverage check on the two target modules** + +Run: +```bash +SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest \ + tests/test_coordinator.py tests/test_entity.py tests/test_init.py \ + --cov=custom_components/zaptec/coordinator --cov=custom_components/zaptec/entity \ + --cov-branch --cov-report=term-missing +``` +Expected: `coordinator.py` and `entity.py` at or above the pre-migration numbers (100% / 98%). If below, add targeted behavior tests for the uncovered lines (name them in the gap and add a test in the appropriate file); do not pad with white-box tests where a behavior test is possible. + +- [ ] **Step 3: Full suite + lint gate** + +Run all three, expect clean (bar the pre-existing `test_zconst.py`/`test_redact.py` DNS errors): +```bash +SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests -q +"C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m ruff format . --diff +"C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m ruff check +``` + +- [ ] **Step 4: hassfest/HACS sanity (manual)** + +Confirm no shipped-component files changed: `git diff --stat master -- custom_components/` must be empty. `requirements_test.txt`, root `conftest.py`, and `pyproject.toml` are dev-only and not shipped. (Use the `hassfest-hacs-check` skill for the checklist.) + +- [ ] **Step 5: Commit + push** + +```bash +git add -A +git commit -m "test: remove temporary harness smoke test after migration" +git push -u origin test/ha-test-harness-migration +``` +Then (with user approval) open the replacement PR against `custom-components/zaptec:master`, noting in the body that it replaces #394, is stacked-independent of the upstream PR queue (see upstream-pr-stack), and that #410 is deferred/xfail pending maintainer input. + +--- + +## Self-Review + +**Spec coverage:** +- Infra (spec §1): Task 1 — requirements pin, root conftest shim, `-p no:homeassistant`. ✓ +- `mock_zaptec`/`setup_integration` (spec §3, Layer-2 patch): Task 2. ✓ +- Coordinator + entity behavior (spec §2): Tasks 3–4, asserting public state. ✓ +- Small white-box residue allowed (spec §2): Task 4 Step 3, explicitly bounded. ✓ +- #410 test-only via xfail (spec §4): Task 4 Step 1, `strict=True`. ✓ +- Success criteria (spec): coverage + native-Windows + CI + ruff + hassfest — Task 5. ✓ +- Non-goals (platforms/snapshots/#410 fix): correctly excluded; snapshots + full platform coverage left to the #395 replacement. ✓ + +**Placeholder scan:** No "TBD"/"handle edge cases" — each code step has concrete code. The two acknowledged fragile spots (entity retrieval from `_listeners`; `isinstance` vs `spec=`) carry explicit fallbacks, not vague hand-waves. + +**Type consistency:** `setup_integration(hass, mock_config_entry, mock_zaptec) -> ZaptecManager`, `make_charger`/`make_installation`, and the charger id `"chg-mock-1"` are used identically across Tasks 2–4. `mock_zaptec.chargers[0]` is the same object as `zaptec["chg-mock-1"]` (seeded from one `objects` dict), which Task 3's interval test relies on. + +## Known risks carried into execution + +1. `MagicMock(spec=Charger)` must satisfy `isinstance(obj, Charger)` in `coordinator.py:84` — true for `spec=`, but if a real `Charger` is needed, Task 2's iteration note covers constructing one with canned `_attrs`. +2. Full `async_setup` pulls in services + all six platforms + streams; the mock must satisfy whatever they touch. Task 2 Step 4 is the iteration point; add missing mock members from tracebacks. +3. Entity-instance retrieval for the logging-residue test is implementation-coupled; Task 4 Step 3 gives a direct-construction fallback. diff --git a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md index c73955ce..d161d476 100644 --- a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md +++ b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md @@ -38,7 +38,7 @@ PRs #394 and #395 have been converted to **draft** and will be replaced by PRs b ### 1. Test infrastructure (the foundation) -- **`requirements_test.txt`** — add `pytest-homeassistant-custom-component` and a **pinned** `homeassistant` version matching what CI already resolves (validate workflow tests Python 3.13/3.14). Pin rather than float, so a new HA/plugin release can't silently break the harness. +- **`requirements_test.txt`** — add `pytest-homeassistant-custom-component` **unpinned**. Do NOT add `homeassistant` here: it is already pinned in `requirements.txt`, and validate.yaml sed-reverts it to the last 3.13-compatible release (`2026.2.3`) on the 3.13 matrix leg. pytest-hacc pins an exact `homeassistant==` itself, so leaving it unpinned makes pip resolve the release matching whichever HA the active Python installs — stable (transitively pinned via HA) yet 3.13/3.14-portable. Pinning an exact pytest-hacc version breaks 3.13 (newest releases require Python >=3.14). - **`conftest.py` (repo root, new)** — port luxtronik's OS-guarded shim: under `sys.platform == "win32"`, stub `fcntl` / `resource` and wrap `socket.socketpair`, then `pytest_plugins = "pytest_homeassistant_custom_component.plugins"`. Completely no-op on Linux, so CI is unaffected. `pytest_plugins` is only honored in the rootdir conftest, so this cannot live in `tests/conftest.py`. - **pytest config** (`pyproject.toml` or `pytest.ini`) — add `-p no:homeassistant` to block the broken plugin autoload; the root conftest re-loads it explicitly *after* shimming. Confirm during planning that the repo has no conflicting existing pytest config. - **`tests/conftest.py`** — replace the hand-rolled `hass` / `FakeConfigEntry` with: @@ -115,6 +115,6 @@ Investigation showed the fix is **not obvious** and needs maintainer input: ## Open items carried into planning -- Confirm exact pinned `homeassistant` version and whether the repo has existing pytest config to reconcile. +- Confirm whether the repo has existing pytest config to reconcile (it does: `[tool.pytest.ini_options]` in `pyproject.toml`). HA version is managed by `requirements.txt` + validate.yaml's 3.13 sed-revert, not by `requirements_test.txt`. - Confirm exact patch target (`custom_components.zaptec.Zaptec` import site) and the minimal `mock_zaptec` object-graph shape for #394. - Feasibility probe (plan step 1) before writing real tests. diff --git a/requirements_test.txt b/requirements_test.txt index bfa83032..cef85c1b 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -2,5 +2,10 @@ pytest pytest-asyncio pytest-mock pytest-cov -pytest-homeassistant-custom-component==0.13.324 -homeassistant==2026.4.3 +# HA version is pinned in requirements.txt (with a 3.13 sed-revert to the last +# 3.13-compatible release in validate.yaml). pytest-homeassistant-custom-component +# pins an exact `homeassistant==` itself, so leaving it unpinned lets pip resolve +# the release matching whichever HA the active Python leg installs. Do NOT pin an +# exact version here: the newest releases require Python >=3.14 and break the 3.13 +# CI leg. +pytest-homeassistant-custom-component From ffa2f78848ac0745b13c8ebbbd3ce73e1363164a Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:22:48 +0200 Subject: [PATCH 11/29] test: pin pytest-hacc per-Python via markers to match reverted HA on 3.13 pytest-homeassistant-custom-component pins an exact homeassistant version, so it must match the HA each CI Python leg installs (requirements.txt pins HA; validate.yaml sed-reverts it to 2026.2.3 on the 3.13 leg). Leaving pytest-hacc unpinned made pip backtrack to an ancient 0.2.1 release (dragging in pytest 6.2.2, which crashes Python 3.13's assertion rewriter). Select the matching release per Python version via environment markers: 0.13.324 (HA 2026.4.3) on py>=3.14, 0.13.316 (HA 2026.2.3) on py<3.14. Co-Authored-By: Claude Opus 4.8 --- .../2026-07-25-ha-test-harness-migration.md | 9 ++++++--- ...6-07-25-ha-test-harness-migration-design.md | 2 +- requirements_test.txt | 18 +++++++++++------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md b/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md index b64d8cdd..1358b989 100644 --- a/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md +++ b/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md @@ -6,11 +6,11 @@ **Architecture:** Adopt pytest-hacc with a repo-root, `win32`-guarded compatibility shim so the harness runs on native-Windows `py314` and on Linux CI. Tests set the integration up through the real `hass` + `MockConfigEntry`, patching only the `Zaptec` client at its construction boundary (`patch("custom_components.zaptec.Zaptec", ...)`) so the manager, coordinators, entities, and platforms all run as real code against canned data. Assertions target public state (`hass.states.get(...)`, registries) instead of private methods. -**Tech Stack:** Python 3.13/3.14 (CI matrix), Home Assistant 2026.4.3 (3.14) / 2026.2.3 (3.13 revert), `pytest-homeassistant-custom-component` (unpinned, follows HA), pytest 9, `syrupy` (available, used later by #395), `MagicMock(spec=...)` test doubles. +**Tech Stack:** Python 3.13/3.14 (CI matrix), Home Assistant 2026.4.3 (3.14) / 2026.2.3 (3.13 revert), `pytest-homeassistant-custom-component` pinned per-Python via markers (0.13.324 / 0.13.316), pytest 9.0.0 (pinned by pytest-hacc), `syrupy` (available, used later by #395), `MagicMock(spec=...)` test doubles. ## Global Constraints -- Do NOT add `homeassistant` to `requirements_test.txt` (it is pinned in `requirements.txt`, with a 3.13 sed-revert in validate.yaml). Leave `pytest-homeassistant-custom-component` UNPINNED so it transitively resolves to the release matching whichever HA the active Python leg installs. Pinning an exact pytest-hacc version breaks CI's 3.13 leg (newest releases require Python >=3.14). +- Do NOT add `homeassistant` to `requirements_test.txt` (it is pinned in `requirements.txt`, with a 3.13 sed-revert to `2026.2.3` in validate.yaml). `pytest-homeassistant-custom-component` pins an EXACT `homeassistant==`, so it must match the HA of each CI Python leg — pin it per-Python via environment markers: `==0.13.324` (HA 2026.4.3) for `python_version >= "3.14"`, `==0.13.316` (HA 2026.2.3, last 3.13-compatible) for `< "3.14"`. Do NOT leave it unpinned (pip backtracks to an ancient release + pytest 6.2.2 → crashes on 3.13) and do NOT single-pin the newest (uninstallable on 3.13, which requires py>=3.14). - The Windows shim MUST be guarded by `if sys.platform == "win32":` — it must be a complete no-op on Linux CI. - No production code changes in `custom_components/**`. This PR is test-only. Bug #410 is documented via `xfail`, never fixed here. - Local run command in this env: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest `. Use forward slashes for the python.exe path. @@ -41,9 +41,12 @@ pytest pytest-asyncio pytest-mock pytest-cov -pytest-homeassistant-custom-component +pytest-homeassistant-custom-component==0.13.324; python_version >= "3.14" +pytest-homeassistant-custom-component==0.13.316; python_version < "3.14" ``` +(pytest-hacc pins an exact HA version, so it must match the HA each CI Python leg installs; markers select the release matching the reverted HA on 3.13.) + - [ ] **Step 2: Create the repo-root conftest with the Windows shim** Create `conftest.py` at the repo root (NOT in `tests/` — `pytest_plugins` is only honored in the rootdir conftest): diff --git a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md index d161d476..d8dc49c3 100644 --- a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md +++ b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md @@ -38,7 +38,7 @@ PRs #394 and #395 have been converted to **draft** and will be replaced by PRs b ### 1. Test infrastructure (the foundation) -- **`requirements_test.txt`** — add `pytest-homeassistant-custom-component` **unpinned**. Do NOT add `homeassistant` here: it is already pinned in `requirements.txt`, and validate.yaml sed-reverts it to the last 3.13-compatible release (`2026.2.3`) on the 3.13 matrix leg. pytest-hacc pins an exact `homeassistant==` itself, so leaving it unpinned makes pip resolve the release matching whichever HA the active Python installs — stable (transitively pinned via HA) yet 3.13/3.14-portable. Pinning an exact pytest-hacc version breaks 3.13 (newest releases require Python >=3.14). +- **`requirements_test.txt`** — add `pytest-homeassistant-custom-component` pinned **per-Python via environment markers** (`==0.13.324` for `python_version >= "3.14"`, `==0.13.316` for `< "3.14"`). Do NOT add `homeassistant` here: it is already pinned in `requirements.txt`, and validate.yaml sed-reverts it to `2026.2.3` on the 3.13 leg. pytest-hacc pins an exact `homeassistant==`, so its version MUST match the HA of each Python leg — 0.13.324↔2026.4.3 (py≥3.14), 0.13.316↔2026.2.3 (py≥3.13). Leaving it unpinned makes pip backtrack to an ancient release (pytest 6.2.2 → crashes on 3.13); single-pinning the newest is uninstallable on 3.13. - **`conftest.py` (repo root, new)** — port luxtronik's OS-guarded shim: under `sys.platform == "win32"`, stub `fcntl` / `resource` and wrap `socket.socketpair`, then `pytest_plugins = "pytest_homeassistant_custom_component.plugins"`. Completely no-op on Linux, so CI is unaffected. `pytest_plugins` is only honored in the rootdir conftest, so this cannot live in `tests/conftest.py`. - **pytest config** (`pyproject.toml` or `pytest.ini`) — add `-p no:homeassistant` to block the broken plugin autoload; the root conftest re-loads it explicitly *after* shimming. Confirm during planning that the repo has no conflicting existing pytest config. - **`tests/conftest.py`** — replace the hand-rolled `hass` / `FakeConfigEntry` with: diff --git a/requirements_test.txt b/requirements_test.txt index cef85c1b..92504590 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -2,10 +2,14 @@ pytest pytest-asyncio pytest-mock pytest-cov -# HA version is pinned in requirements.txt (with a 3.13 sed-revert to the last -# 3.13-compatible release in validate.yaml). pytest-homeassistant-custom-component -# pins an exact `homeassistant==` itself, so leaving it unpinned lets pip resolve -# the release matching whichever HA the active Python leg installs. Do NOT pin an -# exact version here: the newest releases require Python >=3.14 and break the 3.13 -# CI leg. -pytest-homeassistant-custom-component +# pytest-homeassistant-custom-component pins an EXACT `homeassistant==` version, +# so it must match the HA that requirements.txt installs on each CI Python leg +# (validate.yaml sed-reverts HA to 2026.2.3 on the 3.13 leg). Select the matching +# release per Python version via environment markers: the newest releases require +# Python >=3.14, so the 3.13 leg needs the last 3.13-compatible one (0.13.316, +# which pins homeassistant==2026.2.3); 3.14 uses 0.13.324 (pins ==2026.4.3). +# Leaving this unpinned makes pip backtrack to an ancient release (dragging in +# pytest 6.2.2, which crashes on Python 3.13). When bumping the HA pin in +# requirements.txt, bump these two to the matching pytest-hacc releases. +pytest-homeassistant-custom-component==0.13.324; python_version >= "3.14" +pytest-homeassistant-custom-component==0.13.316; python_version < "3.14" From 5475a3be4fe330dd82a3d7c46c7a96047c1eb1d8 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:36:39 +0200 Subject: [PATCH 12/29] test: decouple test-job install from requirements.txt (pytest-hacc owns HA closure) Installing requirements.txt (dev-container pins) alongside pytest-hacc's HA dependency closure caused irreconcilable conflicts (e.g. pydantic 2.13.1 vs pytest-hacc's 2.12.2). pytest-hacc is designed to own the HA + test dependency set, so the test job now installs only requirements_test.txt: pytest-hacc brings HA + the pytest stack, and requirements_test.txt adds just the integration's non-HA manifest deps (azure-servicebus, aiolimiter). HA version per Python leg comes from the pytest-hacc marker, so the 3.13 sed-revert is no longer needed. requirements.txt (dev container) is left untouched. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/validate.yaml | 9 +-------- requirements_test.txt | 27 +++++++++++++++------------ 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/.github/workflows/validate.yaml b/.github/workflows/validate.yaml index 20dc7526..b1c3c1e2 100644 --- a/.github/workflows/validate.yaml +++ b/.github/workflows/validate.yaml @@ -93,16 +93,9 @@ jobs: with: python-version: ${{ matrix.python-version }} - - name: Revert HA requirement to last version supporting Python ${{ matrix.python-version }} - if: matrix.python-version == '3.13' - run: | - sed -i 's/^homeassistant==.*/homeassistant==2026.2.3/' requirements.txt - - name: Install requirements run: | - pip install \ - -r requirements.txt \ - -r requirements_test.txt + pip install -r requirements_test.txt - name: Tests suite run: | diff --git a/requirements_test.txt b/requirements_test.txt index 92504590..6031957f 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,15 +1,18 @@ -pytest -pytest-asyncio +# The CI test job installs ONLY this file (see .github/workflows/validate.yaml). +# pytest-homeassistant-custom-component brings Home Assistant plus the full, +# version-matched pytest stack (pytest, pytest-asyncio, pytest-cov, ...), so those +# are not listed separately. Add here only the integration's own non-HA runtime +# deps (from manifest.json) that HA's closure does not provide, plus test tools +# pytest-hacc doesn't pin (pytest-mock). pytest-mock -pytest-cov -# pytest-homeassistant-custom-component pins an EXACT `homeassistant==` version, -# so it must match the HA that requirements.txt installs on each CI Python leg -# (validate.yaml sed-reverts HA to 2026.2.3 on the 3.13 leg). Select the matching -# release per Python version via environment markers: the newest releases require -# Python >=3.14, so the 3.13 leg needs the last 3.13-compatible one (0.13.316, -# which pins homeassistant==2026.2.3); 3.14 uses 0.13.324 (pins ==2026.4.3). -# Leaving this unpinned makes pip backtrack to an ancient release (dragging in -# pytest 6.2.2, which crashes on Python 3.13). When bumping the HA pin in -# requirements.txt, bump these two to the matching pytest-hacc releases. +azure-servicebus==7.14.3 +aiolimiter==1.2.1 +# pydantic is intentionally NOT pinned here: pytest-hacc pins it to HA's version +# (2.12.2), which satisfies the manifest range (>=2.11.7,<2.14). +# +# pytest-hacc pins an EXACT homeassistant version, so the release must match the +# Python leg: newest releases require Python >=3.14, so 3.13 uses the last +# 3.13-compatible one. 0.13.324 -> HA 2026.4.3 (py>=3.14); 0.13.316 -> HA +# 2026.2.3 (py>=3.13). Bump both together when raising the HA target. pytest-homeassistant-custom-component==0.13.324; python_version >= "3.14" pytest-homeassistant-custom-component==0.13.316; python_version < "3.14" From 221406a2b61af20a5cc6c1347bc39d006408093f Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 00:58:46 +0200 Subject: [PATCH 13/29] test: make requirements.txt + pytest-hacc coexist (relax pydantic pin) Revert the validate.yaml decoupling: the devcontainer (scripts/setup) also installs requirements.txt + requirements_test.txt together, so decoupling only CI would leave the devcontainer broken by the same conflict. Instead, relax requirements.txt's pydantic from ==2.13.1 to the manifest range (>=2.11.7,<2.14). pytest-hacc pins pydantic to HA's version (2.12.2), which the range allows, so both files now install together in CI and the devcontainer. A dry run confirmed pydantic was the only dependency conflict. Remaining CI failures are the pre-existing live-network tests (test_zconst.py / test_redact.py) which pytest-hacc's socket blocking rejects; those are addressed separately by removing the zaptec_constants live call (supersedes #398). Co-Authored-By: Claude Opus 4.8 --- .github/workflows/validate.yaml | 9 ++++++++- requirements.txt | 2 +- requirements_test.txt | 26 +++++++++++--------------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/.github/workflows/validate.yaml b/.github/workflows/validate.yaml index b1c3c1e2..20dc7526 100644 --- a/.github/workflows/validate.yaml +++ b/.github/workflows/validate.yaml @@ -93,9 +93,16 @@ jobs: with: python-version: ${{ matrix.python-version }} + - name: Revert HA requirement to last version supporting Python ${{ matrix.python-version }} + if: matrix.python-version == '3.13' + run: | + sed -i 's/^homeassistant==.*/homeassistant==2026.2.3/' requirements.txt + - name: Install requirements run: | - pip install -r requirements_test.txt + pip install \ + -r requirements.txt \ + -r requirements_test.txt - name: Tests suite run: | diff --git a/requirements.txt b/requirements.txt index 1adb323f..be7d8a98 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,5 +6,5 @@ ruff==0.15.22 # Copy from manifest.json to get this into the dev container # without needing to start HA azure-servicebus==7.14.3 -pydantic==2.13.1 +pydantic>=2.11.7,<2.14 aiolimiter==1.2.1 diff --git a/requirements_test.txt b/requirements_test.txt index 6031957f..b1cc0088 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,18 +1,14 @@ -# The CI test job installs ONLY this file (see .github/workflows/validate.yaml). -# pytest-homeassistant-custom-component brings Home Assistant plus the full, -# version-matched pytest stack (pytest, pytest-asyncio, pytest-cov, ...), so those -# are not listed separately. Add here only the integration's own non-HA runtime -# deps (from manifest.json) that HA's closure does not provide, plus test tools -# pytest-hacc doesn't pin (pytest-mock). +pytest +pytest-asyncio pytest-mock -azure-servicebus==7.14.3 -aiolimiter==1.2.1 -# pydantic is intentionally NOT pinned here: pytest-hacc pins it to HA's version -# (2.12.2), which satisfies the manifest range (>=2.11.7,<2.14). -# -# pytest-hacc pins an EXACT homeassistant version, so the release must match the -# Python leg: newest releases require Python >=3.14, so 3.13 uses the last -# 3.13-compatible one. 0.13.324 -> HA 2026.4.3 (py>=3.14); 0.13.316 -> HA -# 2026.2.3 (py>=3.13). Bump both together when raising the HA target. +pytest-cov +# pytest-homeassistant-custom-component pins an EXACT `homeassistant==`, so the +# release must match the HA that requirements.txt installs on each CI Python leg +# (validate.yaml sed-reverts HA to 2026.2.3 on 3.13). Select the matching release +# per Python version: newest releases require Python >=3.14, so 3.13 uses the last +# 3.13-compatible one. 0.13.324 -> HA 2026.4.3 (py>=3.14); 0.13.316 -> HA 2026.2.3. +# It also brings the full version-matched pytest stack, and pins pydantic to HA's +# version (2.12.2), which is why requirements.txt uses a pydantic RANGE (matching +# manifest.json) rather than an exact pin that would conflict. pytest-homeassistant-custom-component==0.13.324; python_version >= "3.14" pytest-homeassistant-custom-component==0.13.316; python_version < "3.14" From 06c3eeb66c45d78f84625c62c8c8746fa62a4f7d Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 03:15:24 +0200 Subject: [PATCH 14/29] docs: rework HA-harness migration to Linux-native + Option C (#257) Pivot the migration design/plan away from the committed native-Windows shim to Linux-native infra: on Linux pytest-hacc autoloads, so the harness is scoped to HA-integration tests via two pytest invocations (pytest tests --ignore=tests/zaptec + pytest tests/zaptec -p no:homeassistant), combining coverage with --cov-append. Keeps the tests/zaptec/* API-client tests (future standalone PyPI lib, #257) as plain pytest with their live constants call intact. Plan is a delta over the implemented branch and is meant to be executed in the devcontainer. Co-Authored-By: Claude Opus 4.8 --- .../2026-07-25-ha-test-harness-migration.md | 664 +++++------------- ...-07-25-ha-test-harness-migration-design.md | 108 ++- 2 files changed, 264 insertions(+), 508 deletions(-) diff --git a/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md b/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md index 1358b989..6baac826 100644 --- a/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md +++ b/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md @@ -1,607 +1,313 @@ -# HA Test-Harness Migration (coordinator + entity) Implementation Plan +# HA Test-Harness Migration — Linux-native + Option C rework Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Replace PR #394's hand-rolled coordinator/entity unit tests with behavior-first tests running on the real `pytest-homeassistant-custom-component` (pytest-hacc) harness, plus the reusable infrastructure the later #395 replacement will build on. +**Goal:** Pivot the already-implemented pytest-hacc migration on branch `test/ha-test-harness-migration` from its committed native-Windows shim to **Linux-native** test infrastructure, and scope the harness to the HA-integration tests only (**Option C**, per issue #257) via two pytest invocations — so `tests/zaptec/*` (the future-standalone API client) run as plain pytest with their live constants call intact. -**Architecture:** Adopt pytest-hacc with a repo-root, `win32`-guarded compatibility shim so the harness runs on native-Windows `py314` and on Linux CI. Tests set the integration up through the real `hass` + `MockConfigEntry`, patching only the `Zaptec` client at its construction boundary (`patch("custom_components.zaptec.Zaptec", ...)`) so the manager, coordinators, entities, and platforms all run as real code against canned data. Assertions target public state (`hass.states.get(...)`, registries) instead of private methods. +**This is a delta plan, not a from-scratch migration.** The branch (head `221406a`, off `master`) already contains the full harness migration: `requirements_test.txt` (per-Python pytest-hacc pins), the relaxed pydantic pin in `requirements.txt`, `tests/conftest.py` (mock_zaptec/setup_integration/zaptec_constants), and the behavior tests (`test_coordinator.py`, `test_entity.py` with correct #410 assertions, `test_init.py`, `test_diagnostics.py`). What this plan changes is **only** the harness activation mechanism and the test runners. Do NOT rewrite the tests or fixtures. -**Tech Stack:** Python 3.13/3.14 (CI matrix), Home Assistant 2026.4.3 (3.14) / 2026.2.3 (3.13 revert), `pytest-homeassistant-custom-component` pinned per-Python via markers (0.13.324 / 0.13.316), pytest 9.0.0 (pinned by pytest-hacc), `syrupy` (available, used later by #395), `MagicMock(spec=...)` test doubles. +**Architecture:** On Linux (CI + devcontainer) pytest-hacc autoloads via its `pytest11` entry point, so activating the harness needs no conftest machinery. Remove the committed native-Windows shim (root `conftest.py`) and the global `-p no:homeassistant` (pyproject `addopts`). Run the suite as **two invocations**: `pytest tests --ignore=tests/zaptec` (harness autoloads; integration tests use mocked client, no network) and `pytest tests/zaptec -p no:homeassistant` (harness disabled → plain pytest → live `api.zaptec.com/api/constants` call works exactly as on `master`), combining coverage with `--cov-append`. + +**Tech Stack:** Python 3.13/3.14 (CI matrix), Home Assistant 2026.4.3 (3.14) / 2026.2.3 (3.13 revert), `pytest-homeassistant-custom-component` (0.13.324 / 0.13.316 per-Python markers — already pinned, do not touch), pytest 9.x (pinned by pytest-hacc), `ruff` 0.15.22. + +## Execution environment + +**Run this plan inside the project's VS Code Dev Container (Linux).** That is where the harness autoloads natively, both invocations run, and the live constants call reaches the network. Ensure deps are installed first (`scripts/setup`, or `pip install -r requirements.txt -r requirements_test.txt`). Do **not** attempt the integration-test invocation on native Windows — that environment is intentionally no longer supported by tracked files (see spec §3). Commands below use plain `pytest` / `ruff` as available in the devcontainer. ## Global Constraints -- Do NOT add `homeassistant` to `requirements_test.txt` (it is pinned in `requirements.txt`, with a 3.13 sed-revert to `2026.2.3` in validate.yaml). `pytest-homeassistant-custom-component` pins an EXACT `homeassistant==`, so it must match the HA of each CI Python leg — pin it per-Python via environment markers: `==0.13.324` (HA 2026.4.3) for `python_version >= "3.14"`, `==0.13.316` (HA 2026.2.3, last 3.13-compatible) for `< "3.14"`. Do NOT leave it unpinned (pip backtracks to an ancient release + pytest 6.2.2 → crashes on 3.13) and do NOT single-pin the newest (uninstallable on 3.13, which requires py>=3.14). -- The Windows shim MUST be guarded by `if sys.platform == "win32":` — it must be a complete no-op on Linux CI. -- No production code changes in `custom_components/**`. This PR is test-only. Bug #410 is documented via `xfail`, never fixed here. -- Local run command in this env: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest `. Use forward slashes for the python.exe path. -- Ruff (format + check) must be clean on all new/changed files, pinned ruff `0.15.22`. -- Never `git commit` without explicit user approval (project CLAUDE.md). Each task's "Commit" step means: stage, show the diff, and request approval before committing. -- `[tool.pytest.ini_options]` in `pyproject.toml` already sets `asyncio_mode = "auto"` and `filterwarnings = ["ignore::DeprecationWarning"]`. Reuse these; do not duplicate. +- **Linux-native only.** No native-Windows accommodation may be (re)introduced into tracked files: no root `conftest.py` shim, no `pytest_plugins` force-load, no global `-p no:homeassistant`. Native-Windows local runs are handled outside the repo (uncommitted shim or devcontainer) and are out of scope here. +- **Two-invocation contract.** The harness must be active for `tests/test_*.py` and inactive for `tests/zaptec/*`. This is a per-process choice, so the suite always runs as the two invocations below. A bare `pytest` (which would collect `tests/zaptec/*` under the autoloaded harness and re-trip the socket block) is intentionally no longer the entry point. + - Integration: `pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch` + - API client: `pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append` +- **Do NOT modify** `requirements_test.txt` (per-Python pytest-hacc markers are correct), the `pydantic` range in `requirements.txt`, `tests/conftest.py` fixtures, or any `tests/test_*.py` / `tests/zaptec/test_*.py` content. This plan changes activation + runners only. +- No production code changes in `custom_components/**`. This branch is test/infra-only. Bug #410 is already handled (tests assert correct behavior, no xfail — do not reintroduce one). +- Ruff (format + check) must be clean on all changed files, pinned ruff `0.15.22`, scoped to the whole repo (`src: "."`). +- **Commit policy:** committing per task locally is pre-approved for this plan's execution (SDD auto-commit). **Pushing to any remote and opening/altering any PR requires explicit user approval** (project CLAUDE.md) — Task 5 stops for it. +- `[tool.pytest.ini_options]` also sets `pythonpath`, `testpaths=["tests"]`, `log_format`, `log_date_format`, `filterwarnings`, `asyncio_mode="auto"`, `asyncio_default_fixture_loop_scope="function"`. Preserve all of these; only the `addopts` line is removed. --- -### Task 1: pytest-hacc harness infrastructure + Windows shim +### Task 1: Go Linux-native — remove the committed shim and global plugin-disable **Files:** -- Modify: `requirements_test.txt` -- Create: `conftest.py` (repo root) -- Modify: `pyproject.toml` (add `addopts` under `[tool.pytest.ini_options]`) -- Test: `tests/test_harness_smoke.py` (temporary smoke test, removed in Task 5) +- Delete: `conftest.py` (repo root) +- Modify: `pyproject.toml` (remove one line from `[tool.pytest.ini_options]`) **Interfaces:** - Consumes: nothing (first task). -- Produces: a working real `hass` fixture available to every test; the repo-root `conftest.py` re-loads the pytest-hacc plugin after applying the Windows shim. +- Produces: a repo where, on Linux, pytest-hacc autoloads for `pytest tests --ignore=tests/zaptec` and is disabled by `-p no:homeassistant` for `pytest tests/zaptec`. No tracked Windows shim remains. -- [ ] **Step 1: Add pinned test dependencies** +- [ ] **Step 1: Delete the repo-root conftest (the Windows shim + explicit plugin load)** -Replace the contents of `requirements_test.txt` with: +Remove the file entirely: -``` -pytest -pytest-asyncio -pytest-mock -pytest-cov -pytest-homeassistant-custom-component==0.13.324; python_version >= "3.14" -pytest-homeassistant-custom-component==0.13.316; python_version < "3.14" +```bash +git rm conftest.py ``` -(pytest-hacc pins an exact HA version, so it must match the HA each CI Python leg installs; markers select the release matching the reverted HA on 3.13.) - -- [ ] **Step 2: Create the repo-root conftest with the Windows shim** - -Create `conftest.py` at the repo root (NOT in `tests/` — `pytest_plugins` is only honored in the rootdir conftest): - -```python -"""Repo-root conftest: load pytest-homeassistant-custom-component explicitly. - -The plugin autoloads via a pytest11 entry point, but importing it on Windows -fails immediately (`homeassistant.runner` imports `fcntl`, Unix-only) before any -test collects. `-p no:homeassistant` in pyproject.toml blocks that autoload; -this file loads the plugin back explicitly, with Windows compatibility shims -applied first. pytest only honors `pytest_plugins` in the rootdir conftest, so -this cannot live in tests/conftest.py. The shim is a no-op on Linux (CI), where -fcntl/resource exist and the plugin imports natively. -""" - -import sys -import types - -if sys.platform == "win32": - if "fcntl" not in sys.modules: - fake_fcntl = types.ModuleType("fcntl") - fake_fcntl.LOCK_SH = 1 - fake_fcntl.LOCK_EX = 2 - fake_fcntl.LOCK_NB = 4 - fake_fcntl.LOCK_UN = 8 - fake_fcntl.flock = lambda *args, **kwargs: None - fake_fcntl.lockf = lambda *args, **kwargs: None - fake_fcntl.fcntl = lambda *args, **kwargs: 0 - fake_fcntl.ioctl = lambda *args, **kwargs: 0 - sys.modules["fcntl"] = fake_fcntl - - if "resource" not in sys.modules: - fake_resource = types.ModuleType("resource") - fake_resource.RLIMIT_NOFILE = 7 - fake_resource.RLIM_INFINITY = -1 - fake_resource.getrlimit = lambda *args, **kwargs: (8192, 8192) - fake_resource.setrlimit = lambda *args, **kwargs: None - sys.modules["resource"] = fake_resource - - import socket as _socket_mod - - _orig_socketpair = _socket_mod.socketpair - - def _shimmed_socketpair(*args, **kwargs): - blocked = getattr(_socket_mod.socket, "__module__", "") == "pytest_socket" - if not blocked: - return _orig_socketpair(*args, **kwargs) - import pytest_socket - - pytest_socket.enable_socket() - try: - return _orig_socketpair(*args, **kwargs) - finally: - pytest_socket.socket_allow_hosts(["127.0.0.1"]) - pytest_socket.disable_socket(allow_unix_socket=True) - - _socket_mod.socketpair = _shimmed_socketpair - -pytest_plugins = "pytest_homeassistant_custom_component.plugins" -``` +Rationale: on Linux the plugin autoloads; this file existed only to load it after a `win32` fcntl/resource/socketpair shim. It is the root cause of the session-wide harness load that Option C must avoid. -- [ ] **Step 3: Block the broken plugin autoload in pyproject.toml** +- [ ] **Step 2: Remove the global plugin-disable from pyproject.toml** -Add an `addopts` line inside the existing `[tool.pytest.ini_options]` table in `pyproject.toml` (leave `asyncio_mode`, `filterwarnings`, `pythonpath`, `testpaths` as-is): +In `pyproject.toml`, inside `[tool.pytest.ini_options]`, delete exactly this line: ```toml addopts = "-p no:homeassistant" ``` -- [ ] **Step 4: Write the smoke test** - -Create `tests/test_harness_smoke.py`: - -```python -"""Smoke test: the real HA `hass` fixture spins up under the shim. Removed in Task 5.""" +Leave every other key in that table unchanged (`pythonpath`, `testpaths`, `log_format`, `log_date_format`, `filterwarnings`, `asyncio_mode`, `asyncio_default_fixture_loop_scope`). Do not add a replacement `addopts`. -from homeassistant.core import HomeAssistant +- [ ] **Step 3: Verify the integration invocation (harness autoloads)** - -async def test_hass_fixture_starts(hass: HomeAssistant) -> None: - """The harness's real hass fixture is a live HomeAssistant with a working state machine.""" - assert isinstance(hass, HomeAssistant) - hass.states.async_set("probe.entity", "on") - await hass.async_block_till_done() - assert hass.states.get("probe.entity").state == "on" +Run: +```bash +pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch ``` +Expected: the real `hass` fixture works (harness autoloaded), and `test_coordinator.py`, `test_entity.py`, `test_init.py`, `test_diagnostics.py` all pass. If you see `fixture 'hass' not found` or `No module named 'pytest_homeassistant_custom_component'`, the harness didn't autoload — confirm `requirements_test.txt` is installed in this environment (`pip show pytest-homeassistant-custom-component`). -- [ ] **Step 5: Run the smoke test** - -Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_harness_smoke.py -v` -Expected: PASS (1 passed). If it errors with `No module named 'fcntl'`, the shim/rootdir wiring is wrong — verify `conftest.py` is at repo root and `addopts` was added. +- [ ] **Step 4: Verify the API-client invocation (harness disabled, plain pytest)** -- [ ] **Step 6: Verify the rest of the suite still collects** +Run: +```bash +pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append +``` +Expected: `test_zconst.py` / `test_redact.py` make the live `api.zaptec.com/api/constants` call and pass (devcontainer has network); `test_utils.py` / `test_validate.py` pass; `test_api.py` login tests behave exactly as on `master` (skipped without creds, or set `SKIP_ZAPTEC_API_TEST=true` to skip them). Crucially: **no `SocketBlockedError`** — `-p no:homeassistant` turned the harness off for this run. If you see `SocketBlockedError`, the harness is still active — confirm Step 2 removed the global `-p no:homeassistant` and that you passed `-p no:homeassistant` on this command. -Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests -q` -Expected: existing tests still pass/skip as before (the known `test_zconst.py`/`test_redact.py` DNS errors may appear — that is pre-existing and unrelated). No new collection errors. +- [ ] **Step 5: Confirm combined coverage did not regress** -- [ ] **Step 7: Ruff + commit** +Run: +```bash +coverage report --include="*/coordinator.py,*/entity.py" +``` +Expected: `coordinator.py` ≥ 100%, `entity.py` ≥ 98% (the combined figure from Steps 3+4's `--cov-append`). If lower, do NOT add tests here — stop and report; a regression means the two-invocation split dropped coverage the single run had, which is a wiring problem to diagnose, not a test gap. -Run: `"C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m ruff format . --diff` and `-m ruff check`. Fix any issues. -Then stage `requirements_test.txt`, `conftest.py`, `pyproject.toml`, `tests/test_harness_smoke.py`, show the diff, and request approval before: +- [ ] **Step 6: Ruff + commit** ```bash -git add requirements_test.txt conftest.py pyproject.toml tests/test_harness_smoke.py -git commit -m "test: adopt pytest-homeassistant-custom-component harness with Windows shim" +ruff format . --diff +ruff check +git add -A +git commit -m "test: drop committed Windows shim; rely on Linux pytest-hacc autoload" ``` +(If `ruff format .` reports diffs, apply `ruff format .` and re-stage.) --- -### Task 2: Shared fixtures — mock Zaptec client, MockConfigEntry, setup helper +### Task 2: Wire the two-invocation structure into CI (validate.yaml) **Files:** -- Modify: `tests/conftest.py` -- Test: `tests/test_init.py` (add an integration-setup test alongside the existing `test_config_entry_error_mapping`) +- Modify: `.github/workflows/validate.yaml` (the `tests` job's "Tests suite" step, ~lines 107-109) **Interfaces:** -- Consumes: the real `hass` fixture (Task 1). -- Produces, in `tests/conftest.py`: - - `make_charger(data: dict, *, installation=None, charging: bool = False) -> MagicMock` — a `MagicMock(spec=Charger)` whose `.get(key, default=MISSING)` is backed by `data`, with `.id`, `.name`, `.model`, `.qual_id`, `.is_charging()`, `.installation` wired. - - `make_installation(data: dict, *, chargers=()) -> MagicMock` — a `MagicMock(spec=Installation)` similarly backed, with `.chargers`, async `.stream_main`/`.stream_close`. - - `mock_zaptec` fixture → `MagicMock(spec=Zaptec)` exposing Mapping access (`__getitem__`/`__iter__`/`__contains__`/`__len__`), `.objects()`, `.installations`, `.chargers`, async `.login`/`.build`/`.poll`, and `.redact`, seeded with one installation + one charger. - - `mock_config_entry` fixture → `MockConfigEntry` for domain `zaptec`. - - `setup_integration(hass, mock_config_entry, mock_zaptec) -> ZaptecManager` async helper that patches the client and runs full `async_setup`. +- Consumes: the Linux-native repo from Task 1. +- Produces: a CI `tests` job that runs both invocations with combined coverage, on both the 3.13 and 3.14 matrix legs. -- [ ] **Step 1: Write the failing integration-setup test** +- [ ] **Step 1: Replace the single test step with the two invocations** -Add to `tests/test_init.py`: +In `.github/workflows/validate.yaml`, replace the existing step: -```python -from unittest.mock import patch - -from homeassistant.core import HomeAssistant - -from custom_components.zaptec.manager import ZaptecManager - - -async def test_setup_entry_creates_manager_and_entities( - hass: HomeAssistant, mock_config_entry, mock_zaptec -) -> None: - """A full setup wires up the manager and registers at least one entity.""" - manager = await setup_integration(hass, mock_config_entry, mock_zaptec) - - assert isinstance(manager, ZaptecManager) - assert mock_config_entry.runtime_data is manager - # At least one entity from the seeded charger reached the state machine. - states = [s for s in hass.states.async_all() if s.entity_id.split(".")[1].startswith("mock")] - assert states, "expected at least one zaptec entity to be created" +```yaml + - name: Tests suite + run: | + pytest --cov=./custom_components/zaptec --cov-branch ``` -(Note: `setup_integration`, `mock_config_entry`, `mock_zaptec` come from `tests/conftest.py`, added next.) - -- [ ] **Step 2: Run it to verify it fails** - -Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_init.py::test_setup_entry_creates_manager_and_entities -v` -Expected: FAIL — `fixture 'mock_config_entry' not found` (or `NameError: setup_integration`). - -- [ ] **Step 3: Add the fixtures and helper to tests/conftest.py** - -Append to `tests/conftest.py` (keep the existing api-login fixtures): - -```python -from unittest.mock import AsyncMock, MagicMock, patch - -from pytest_homeassistant_custom_component.common import MockConfigEntry - -from custom_components.zaptec.const import CONF_PASSWORD, CONF_USERNAME, DOMAIN -from custom_components.zaptec.manager import ZaptecManager -from custom_components.zaptec.zaptec import MISSING, Charger, Installation - - -def _backed_get(data: dict): - """Return a `.get(key, default=MISSING)` implementation backed by `data`.""" - - def _get(key, default=MISSING): - return data.get(key, default) - - return _get - - -def make_charger(data: dict, *, installation=None, charging: bool = False) -> MagicMock: - """Build a spec'd Charger double backed by `data`.""" - charger = MagicMock(spec=Charger) - charger.id = data["id"] - charger.name = data.get("name", "Mock Charger") - charger.model = "Zaptec Charger" - charger.qual_id = f"Charger[{data['id'][-6:]}]" - charger.get.side_effect = _backed_get(data) - charger.is_charging.return_value = charging - charger.installation = installation - return charger - - -def make_installation(data: dict, *, chargers=()) -> MagicMock: - """Build a spec'd Installation double backed by `data`.""" - install = MagicMock(spec=Installation) - install.id = data["id"] - install.name = data.get("name", "Mock Installation") - install.model = "Zaptec Installation" - install.qual_id = f"Installation[{data['id'][-6:]}]" - install.get.side_effect = _backed_get(data) - install.chargers = list(chargers) - install.stream_main = AsyncMock(return_value=None) - install.stream_close = AsyncMock(return_value=None) - return install - - -@pytest.fixture -def mock_zaptec() -> MagicMock: - """A spec'd Zaptec client seeded with one installation and one charger.""" - installation = make_installation({"id": "inst-mock-1", "name": "Mock Home"}) - charger = make_charger( - { - "id": "chg-mock-1", - "name": "Mock Charger", - # Keys read by entities under test; extend as needed for coverage. - "operating_mode": "Connected", - "charger_operation_mode": "Connected", - }, - installation=installation, - charging=False, - ) - installation.chargers = [charger] - - objects = {installation.id: installation, charger.id: charger} - - zaptec = MagicMock(spec=Zaptec) - zaptec.__getitem__.side_effect = objects.__getitem__ - zaptec.__iter__.side_effect = lambda: iter(objects) - zaptec.__contains__.side_effect = objects.__contains__ - zaptec.__len__.side_effect = lambda: len(objects) - zaptec.objects.return_value = list(objects.values()) - zaptec.installations = [installation] - zaptec.chargers = [charger] - zaptec.login = AsyncMock(return_value=None) - zaptec.build = AsyncMock(return_value=None) - zaptec.poll = AsyncMock(return_value=None) - zaptec.show_all_updates = False - zaptec.redact = MagicMock() - zaptec.redact.dumps.return_value = "" - return zaptec - - -@pytest.fixture -def mock_config_entry() -> MockConfigEntry: - """A MockConfigEntry for the zaptec domain.""" - return MockConfigEntry( - domain=DOMAIN, - title="Mock Zaptec", - data={CONF_USERNAME: "user", CONF_PASSWORD: "pass"}, - entry_id="mock_entry_1", - ) - - -async def setup_integration(hass, mock_config_entry, mock_zaptec) -> ZaptecManager: - """Set the integration up through the real async_setup, with a mocked client.""" - mock_config_entry.add_to_hass(hass) - with patch("custom_components.zaptec.Zaptec", return_value=mock_zaptec): - assert await hass.config_entries.async_setup(mock_config_entry.entry_id) - await hass.async_block_till_done() - return mock_config_entry.runtime_data -``` +with: -Also add these imports at the top of `tests/test_init.py` so the test can call the helper: +```yaml + - name: Tests suite (HA integration — harness) + run: | + pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch -```python -from tests.conftest import setup_integration + - name: Tests suite (API client — plain pytest, no harness) + run: | + pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append ``` -- [ ] **Step 4: Run and iterate to green** +Leave the rest of the `tests` job untouched: the matrix (`["3.13", "3.14"]`), the 3.13 HA sed-revert, and the `pip install -r requirements.txt -r requirements_test.txt` step all stay. -Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_init.py -v` -Expected: PASS. Likely iteration points (fix against the real code if they surface): -- `CONF_USERNAME`/`CONF_PASSWORD`/`DOMAIN` import path — confirm they live in `custom_components/zaptec/const.py`; adjust if re-exported elsewhere. -- If setup calls a `Zaptec` member not wired above (e.g. an attribute read during `async_setup_entry`), add it to `mock_zaptec` as a `MagicMock`/`AsyncMock`. Check the traceback for the exact missing member. -- `enable_custom_integrations` — pytest-hacc's autouse fixture should load `custom_components.zaptec`; if the domain isn't found, add the `enable_custom_integrations` fixture arg to the test. +- [ ] **Step 2: Sanity-check the YAML** -- [ ] **Step 5: Ruff + commit** +Run (in the devcontainer, if `python` + `pyyaml` are present): +```bash +python -c "import yaml; yaml.safe_load(open('.github/workflows/validate.yaml')); print('yaml ok')" +``` +Expected: `yaml ok`. (If pyyaml isn't available, visually confirm indentation matches the surrounding steps — two spaces under `steps:` items.) -Run ruff format/check. Then stage `tests/conftest.py`, `tests/test_init.py`, show diff, request approval: +- [ ] **Step 3: Commit** ```bash -git add tests/conftest.py tests/test_init.py -git commit -m "test: add real-harness setup fixtures (mock Zaptec client + MockConfigEntry)" +git add .github/workflows/validate.yaml +git commit -m "ci: run harness + API-client tests as two scoped pytest invocations" ``` --- -### Task 3: Coordinator behavior tests +### Task 3: Wire scripts/test to the two-invocation structure **Files:** -- Test: `tests/test_coordinator.py` (create) +- Modify: `scripts/test` **Interfaces:** -- Consumes: `mock_zaptec`, `mock_config_entry`, `setup_integration` (Task 2); `hass` (Task 1). -- Produces: behavior coverage of `coordinator.py` via public coordinator API. - -- [ ] **Step 1: Write the failing tests** - -Create `tests/test_coordinator.py`: +- Consumes: the Linux-native repo from Task 1. +- Produces: a `./scripts/test` that mirrors CI (two invocations, combined coverage) and still supports `--skip-api` and the html/xml coverage reports. -```python -"""Behavior tests for ZaptecUpdateCoordinator, driven through the real harness.""" +- [ ] **Step 1: Update scripts/test** -from unittest.mock import patch +Replace the single `pytest ...` line in `scripts/test` so the file reads: -from homeassistant.core import HomeAssistant -from homeassistant.helpers.update_coordinator import UpdateFailed -import pytest - -from custom_components.zaptec.zaptec import ZaptecApiError -from tests.conftest import setup_integration - - -async def test_successful_poll_marks_last_update_success( - hass: HomeAssistant, mock_config_entry, mock_zaptec -) -> None: - """A successful poll leaves every coordinator reporting success.""" - manager = await setup_integration(hass, mock_config_entry, mock_zaptec) - for coordinator in manager.all_coordinators: - assert coordinator.last_update_success is True - mock_zaptec.poll.assert_awaited() - - -async def test_poll_failure_sets_update_failed( - hass: HomeAssistant, mock_config_entry, mock_zaptec -) -> None: - """A ZaptecApiError during poll flips last_update_success to False.""" - manager = await setup_integration(hass, mock_config_entry, mock_zaptec) - head = manager.head_coordinator - - mock_zaptec.poll.side_effect = ZaptecApiError("boom") - await head.async_refresh() - - assert head.last_update_success is False +```bash +#!/usr/bin/env bash +set -e -async def test_device_coordinator_switches_interval_when_charging( - hass: HomeAssistant, mock_config_entry, mock_zaptec -) -> None: - """A charger's coordinator uses the shorter interval once it reports charging.""" - manager = await setup_integration(hass, mock_config_entry, mock_zaptec) - charger_coord = manager.device_coordinators["chg-mock-1"] - idle_interval = charger_coord.update_interval +if [ "$1" == "--skip-api" ]; then + export SKIP_ZAPTEC_API_TEST="true" +fi - # Flip the seeded charger to 'charging' and re-run the update-listener path. - mock_zaptec.chargers[0].is_charging.return_value = True - charger_coord.set_update_interval() +# HA-integration tests run under the pytest-hacc harness (autoloads on Linux). +# API-client tests (tests/zaptec/*, the future-standalone client per #257) run +# as plain pytest with the harness disabled, so their live constants call is not +# socket-blocked. Coverage from both is combined via --cov-append. +# run tests with -s to display printouts and --log-cli-level to get logger output +pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch --log-cli-level=INFO -s +pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append --log-cli-level=INFO -s - assert charger_coord.update_interval < idle_interval +# generate coverage report in html and xml +coverage html +coverage xml ``` -- [ ] **Step 2: Run to verify they fail** - -Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_coordinator.py -v` -Expected: FAIL only if something is wired wrong — these use already-built fixtures, so a failure here signals a fixture gap (e.g. `set_update_interval` needs `options.zaptec_object` to be the same charger mock; confirm `mock_zaptec.chargers[0]` is the object stored as `zaptec["chg-mock-1"]`). Fix in `tests/conftest.py` if needed. +Keep the file executable (`git` preserves the mode; if needed `chmod +x scripts/test`). -- [ ] **Step 3: Make them pass** +- [ ] **Step 2: Run it end-to-end** -Iterate on fixtures/assertions until green. The charging-interval test depends on `ZAPTEC_POLL_INTERVAL_CHARGING < ZAPTEC_POLL_INTERVAL_IDLE` (true in `const.py`) and on the device coordinator's `options.zaptec_object` being a `Charger` — verify `make_charger` returns a `spec=Charger` instance so `isinstance(..., Charger)` in the coordinator passes. If `isinstance` fails against `MagicMock(spec=Charger)`, switch that check by constructing a real `Charger` (see Task 2 iteration note) or confirm `spec=Charger` satisfies `isinstance` (it does for `MagicMock(spec=Cls)`). - -- [ ] **Step 4: Run to verify pass** - -Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_coordinator.py -v` -Expected: PASS (3 passed). +Run: +```bash +./scripts/test --skip-api +``` +Expected: both invocations run and pass (with API-login tests skipped), then `htmlcov/` and `coverage.xml` are generated. Confirm the terminal shows both invocations executing (two pytest runs), not one. -- [ ] **Step 5: Ruff + commit** +- [ ] **Step 3: Commit** ```bash -git add tests/test_coordinator.py -git commit -m "test: behavior coverage for ZaptecUpdateCoordinator via real harness" +git add scripts/test +git commit -m "test: scripts/test runs harness + API-client invocations, combined coverage" ``` --- -### Task 4: Entity behavior tests (incl. #410 xfail) +### Task 4: Document the split in DEVELOPMENT.md **Files:** -- Test: `tests/test_entity.py` (create) +- Modify: `DEVELOPMENT.md` (the "## Running tests" section, ~line 139) **Interfaces:** -- Consumes: `mock_zaptec`, `mock_config_entry`, `setup_integration`, `hass`. -- Produces: behavior coverage of `entity.py`; documents #410 via `xfail`. - -- [ ] **Step 1: Write the failing tests** - -Create `tests/test_entity.py`. The value/availability tests read a real entity's public state; the two logging-dedup assertions are the deliberately-allowed small white-box residue. - -```python -"""Behavior tests for ZaptecBaseEntity, driven through the real harness.""" - -import logging - -from homeassistant.core import HomeAssistant -import pytest - -from tests.conftest import setup_integration - - -async def _get_zaptec_entity(hass: HomeAssistant): - """Return one live zaptec entity_id whose value is backed by seeded data.""" - for state in hass.states.async_all(): - if state.entity_id.startswith(("sensor.", "binary_sensor.", "switch.", "number.")): - return state.entity_id - raise AssertionError("no zaptec entity found") - - -async def test_entity_reports_value_from_zaptec( - hass: HomeAssistant, mock_config_entry, mock_zaptec -) -> None: - """A backed key surfaces as the entity's state (not 'unavailable'/'unknown').""" - await setup_integration(hass, mock_config_entry, mock_zaptec) - entity_id = await _get_zaptec_entity(hass) - state = hass.states.get(entity_id) - assert state.state not in ("unavailable", "unknown") - - -@pytest.mark.xfail( - reason="#410: _attr_available is set on KeyUnavailableError but never affects " - "reported availability (available is not overridden). Documenting current behavior.", - strict=True, -) -async def test_entity_becomes_unavailable_when_key_missing( - hass: HomeAssistant, mock_config_entry, mock_zaptec -) -> None: - """A key that disappears SHOULD mark the entity unavailable (currently it does not — #410).""" - await setup_integration(hass, mock_config_entry, mock_zaptec) - entity_id = await _get_zaptec_entity(hass) - - # Make every key lookup miss, then re-run a refresh so entities re-read. - from custom_components.zaptec.zaptec import MISSING - - mock_zaptec.chargers[0].get.side_effect = lambda key, default=MISSING: default - manager = mock_config_entry.runtime_data - for coordinator in manager.all_coordinators: - await coordinator.async_refresh() - await hass.async_block_till_done() - - # This assertion is what SHOULD hold; strict xfail => the test failing here is expected - # and will turn XPASS (alerting us) once #410 is fixed. - assert hass.states.get(entity_id).state == "unavailable" -``` - -- [ ] **Step 2: Run to verify status** - -Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_entity.py -v` -Expected: `test_entity_reports_value_from_zaptec` PASS; `test_entity_becomes_unavailable_when_key_missing` XFAIL (not FAIL, not XPASS). If it XPASSes, #410 is somehow already satisfied — stop and re-examine before proceeding. - -- [ ] **Step 3: Add the logging-dedup residue tests** - -These cover `_log_value`'s change-detection, which has no observable state effect, so a small white-box test is justified per the spec. Append to `tests/test_entity.py`: - -```python -async def test_log_value_logs_on_change_then_skips_when_unchanged( - hass: HomeAssistant, mock_config_entry, mock_zaptec, caplog -) -> None: - """_log_value logs when the tracked value changes and stays quiet when it doesn't.""" - manager = await setup_integration(hass, mock_config_entry, mock_zaptec) - # Grab a real entity instance from the platform via the coordinator's listeners. - coordinator = manager.device_coordinators["chg-mock-1"] - entity = next(iter(coordinator._listeners.values()))[0].__self__ # noqa: SLF001 - entity.some_attr = "value1" - - with caplog.at_level(logging.DEBUG): - entity._log_value("some_attr") # noqa: SLF001 - assert "value1" in caplog.text - - caplog.clear() - with caplog.at_level(logging.DEBUG): - entity._log_value("some_attr") # noqa: SLF001 - assert caplog.text == "" +- Consumes: the runners from Tasks 2-3. +- Produces: contributor docs that explain the two-invocation split and why `tests/zaptec/*` are separate. + +- [ ] **Step 1: Expand the "Running tests" section** + +In `DEVELOPMENT.md`, under `## Running tests`, after the existing `./scripts/test` bullet, add an explanatory paragraph (adjust wording to match the file's voice): + +```markdown +The suite runs as **two pytest invocations**, and `./scripts/test` runs both: + +- **HA-integration tests** (`tests/test_*.py`) run under the + `pytest-homeassistant-custom-component` harness, which autoloads on Linux. + Run directly with: + `pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch` +- **API-client tests** (`tests/zaptec/*`) test the vendored `zaptec/` client, + which is destined to become a standalone PyPI library (issue #257) and has no + Home Assistant dependency. They run as plain pytest with the harness disabled + (the harness blocks non-localhost sockets, which would break their live + `api.zaptec.com/api/constants` call): + `pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append` + +Because the harness (and its socket block) is process-wide, a bare `pytest` +is not the entry point — use `./scripts/test` or the two commands above. The +HA-integration tests require Linux; run them in the Dev Container (native +Windows is not supported for that half). `tests/zaptec/*` run anywhere. ``` -Note: retrieving the entity instance from `coordinator._listeners` is fragile; if it doesn't resolve cleanly, instead import a concrete entity class (e.g. from `sensor.py`) and instantiate it directly with the `mock_zaptec` charger + the real coordinator — a minimal, contained white-box construction. Confirm the exact listener structure during implementation. - -- [ ] **Step 4: Run to verify pass** - -Run: `SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests/test_entity.py -v` -Expected: 2 passed, 1 xfailed. - -- [ ] **Step 5: Ruff + commit** +- [ ] **Step 2: Commit** ```bash -git add tests/test_entity.py -git commit -m "test: behavior coverage for ZaptecBaseEntity; xfail documents #410" +git add DEVELOPMENT.md +git commit -m "docs: explain two-invocation test split (harness vs API-client, #257)" ``` --- -### Task 5: Coverage verification, smoke-test cleanup, final gate +### Task 5: Final gate, then push + verify CI on the fork (approval required) **Files:** -- Delete: `tests/test_harness_smoke.py` -- Verify only: coverage on `coordinator.py` / `entity.py` +- Verify only (no new edits unless a gate fails). **Interfaces:** -- Consumes: everything from Tasks 1–4. -- Produces: the final, CI-ready state. - -- [ ] **Step 1: Remove the temporary smoke test** +- Consumes: everything from Tasks 1-4. +- Produces: a pushed, CI-green branch on the fork, ready for PR packaging (PR itself deferred to the user). -The `hass` fixture is now exercised by real tests; delete `tests/test_harness_smoke.py`. - -- [ ] **Step 2: Coverage check on the two target modules** +- [ ] **Step 1: Guard — nothing outside tests/zaptec triggers the live call** Run: ```bash -SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest \ - tests/test_coordinator.py tests/test_entity.py tests/test_init.py \ - --cov=custom_components/zaptec/coordinator --cov=custom_components/zaptec/entity \ - --cov-branch --cov-report=term-missing +grep -rln "zaptec_constants" tests --include="*.py" ``` -Expected: `coordinator.py` and `entity.py` at or above the pre-migration numbers (100% / 98%). If below, add targeted behavior tests for the uncovered lines (name them in the gap and add a test in the appropriate file); do not pad with white-box tests where a behavior test is possible. +Expected: only `tests/conftest.py`, `tests/zaptec/test_zconst.py`, `tests/zaptec/test_redact.py`. If any `tests/test_*.py` (integration) requests `zaptec_constants`, it would hit the live call under the harness in invocation 1 → stop and report (the fixture would need moving to `tests/zaptec/conftest.py`). -- [ ] **Step 3: Full suite + lint gate** +- [ ] **Step 2: Full local (devcontainer) gate** -Run all three, expect clean (bar the pre-existing `test_zconst.py`/`test_redact.py` DNS errors): +Run both invocations fresh and the linters: ```bash -SKIP_ZAPTEC_API_TEST=true "C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m pytest tests -q -"C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m ruff format . --diff -"C:/Users/rhamm/anaconda3/envs/py314/python.exe" -m ruff check +pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch +pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append +coverage report --include="*/coordinator.py,*/entity.py" +ruff format . --diff +ruff check ``` +Expected: both green; coordinator.py ≥ 100%, entity.py ≥ 98%; ruff clean. -- [ ] **Step 4: hassfest/HACS sanity (manual)** +- [ ] **Step 3: hassfest/HACS sanity (manual)** -Confirm no shipped-component files changed: `git diff --stat master -- custom_components/` must be empty. `requirements_test.txt`, root `conftest.py`, and `pyproject.toml` are dev-only and not shipped. (Use the `hassfest-hacs-check` skill for the checklist.) +Confirm no shipped-component files changed on this branch: +```bash +git diff --stat master -- custom_components/ +``` +Expected: empty (the migration is test/infra-only). `requirements_test.txt`, `pyproject.toml`, `scripts/test`, `.github/`, `DEVELOPMENT.md` are dev-only and not shipped in the component. (Use the `hassfest-hacs-check` skill for the checklist; note `requirements.txt`'s pydantic-range change is a range within `manifest.json`'s supported bounds, not a manifest change.) -- [ ] **Step 5: Commit + push** +- [ ] **Step 4: Push to the fork — STOP for explicit user approval first** +Do not run this until the user approves the push (Global Constraints): ```bash -git add -A -git commit -m "test: remove temporary harness smoke test after migration" -git push -u origin test/ha-test-harness-migration +git push origin test/ha-test-harness-migration ``` -Then (with user approval) open the replacement PR against `custom-components/zaptec:master`, noting in the body that it replaces #394, is stacked-independent of the upstream PR queue (see upstream-pr-stack), and that #410 is deferred/xfail pending maintainer input. +Then watch the fork's Actions run and confirm BOTH matrix legs (3.13 and 3.14) are green on both invocations. If a leg fails, diagnose against that leg's HA/pytest-hacc pairing (0.13.316↔2026.2.3 on 3.13; 0.13.324↔2026.4.3 on 3.14) before any further change. + +- [ ] **Step 5: PR packaging — user-driven, not automatic** + +Leave PR creation to the user. When they ask, the replacement PR targets `custom-components/zaptec:master`, supersedes draft #394, references #257 as the rationale for the `tests/zaptec/*` split, and notes the upstream-PR-stack dependency. No autonomous PR/issue/comment submission (AI-policy provisional compliance). --- ## Self-Review **Spec coverage:** -- Infra (spec §1): Task 1 — requirements pin, root conftest shim, `-p no:homeassistant`. ✓ -- `mock_zaptec`/`setup_integration` (spec §3, Layer-2 patch): Task 2. ✓ -- Coordinator + entity behavior (spec §2): Tasks 3–4, asserting public state. ✓ -- Small white-box residue allowed (spec §2): Task 4 Step 3, explicitly bounded. ✓ -- #410 test-only via xfail (spec §4): Task 4 Step 1, `strict=True`. ✓ -- Success criteria (spec): coverage + native-Windows + CI + ruff + hassfest — Task 5. ✓ -- Non-goals (platforms/snapshots/#410 fix): correctly excluded; snapshots + full platform coverage left to the #395 replacement. ✓ +- Linux-native infra, no committed shim (spec §3): Task 1 (delete root conftest + global `-p no:homeassistant`). ✓ +- Two-invocation harness scoping (spec §1): Task 1 Steps 3-4 verify; Tasks 2-3 wire CI + scripts. ✓ +- `tests/zaptec/*` unchanged, live call intact, per #257 (spec "two-audience", §2): Task 1 Step 4, Task 5 Step 1 guard. ✓ +- Combined coverage via `--cov-append` (spec §1): Tasks 1-3, verified Task 1 Step 5 / Task 5 Step 2. ✓ +- validate.yaml + scripts/test + DEVELOPMENT.md (spec §7): Tasks 2, 3, 4. ✓ +- #410 already correct, no xfail (spec §6): Global Constraints forbid reintroducing one; no task touches the tests. ✓ +- Success criteria (spec): both invocations green on Linux CI, no native-Windows in tracked files, ruff clean, hassfest/HACS unaffected — Task 5. ✓ -**Placeholder scan:** No "TBD"/"handle edge cases" — each code step has concrete code. The two acknowledged fragile spots (entity retrieval from `_listeners`; `isinstance` vs `spec=`) carry explicit fallbacks, not vague hand-waves. +**Placeholder scan:** No "TBD"/"handle later" — every step is a concrete file op, command, or exact snippet. -**Type consistency:** `setup_integration(hass, mock_config_entry, mock_zaptec) -> ZaptecManager`, `make_charger`/`make_installation`, and the charger id `"chg-mock-1"` are used identically across Tasks 2–4. `mock_zaptec.chargers[0]` is the same object as `zaptec["chg-mock-1"]` (seeded from one `objects` dict), which Task 3's interval test relies on. +**Type/consistency:** The two invocation commands are byte-identical everywhere they appear (Global Constraints, Tasks 1-3, Task 5), so CI, `scripts/test`, and the docs cannot drift. `--cov-append` is present on the second invocation and absent on the first in every occurrence. ## Known risks carried into execution -1. `MagicMock(spec=Charger)` must satisfy `isinstance(obj, Charger)` in `coordinator.py:84` — true for `spec=`, but if a real `Charger` is needed, Task 2's iteration note covers constructing one with canned `_attrs`. -2. Full `async_setup` pulls in services + all six platforms + streams; the mock must satisfy whatever they touch. Task 2 Step 4 is the iteration point; add missing mock members from tracebacks. -3. Entity-instance retrieval for the logging-residue test is implementation-coupled; Task 4 Step 3 gives a direct-construction fallback. +1. **Coverage combine.** If `coverage report` after the split shows less than the pre-split single-run numbers, the cause is almost always a missing `--cov-append` on run 2 (erasing run 1's data) or a stray `.coverage` from a prior run — check both before treating it as a real coverage gap (Task 1 Step 5). +2. **Autoload assumption.** The whole design rests on pytest-hacc autoloading on Linux. If it does not (e.g. deps not installed), Task 1 Step 3 fails fast with a clear fixture/import error — install `requirements_test.txt` and retry; do not add back a conftest force-load. +3. **`test_api.py` without creds.** Behavior must match `master` (skip without creds). If it errors instead, that is pre-existing to how `tests/zaptec` runs on `master`, not introduced here — note it, don't fix it in this plan. diff --git a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md index d8dc49c3..ea44db00 100644 --- a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md +++ b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md @@ -1,6 +1,6 @@ # Design: Migrate coordinator/entity tests to the HA test harness -**Date:** 2026-07-25 +**Date:** 2026-07-25 (revised 2026-07-26: Linux-native infra + harness scoping per #257) **Status:** Approved (brainstorming complete) **Scope of this spec:** the replacement for PR #394 (coordinator + entity tests). Establishes the reusable infrastructure that a later, separate PR (replacing #395, the platform-entity tests) will build on. @@ -12,45 +12,91 @@ The current test suite is hand-rolled: it instantiates `ZaptecUpdateCoordinator` Gold/platinum HA integrations instead use `pytest-homeassistant-custom-component` (pytest-hacc): a real `hass`, `MockConfigEntry`, and tests that set the integration up through the normal `async_setup` path with the cloud API mocked, then assert on **public state** (`hass.states.get(...)`, entity/device registries), often via `syrupy` snapshot tests. -The original reason for the hand-rolled mocks was that pytest-hacc did not run on the maintainer's native-Windows dev environment (`homeassistant` imports `fcntl`, which is Unix-only). That is a local-dev constraint, not a project one: CI runs on Linux where pytest-hacc works, and the Windows issue is solvable with a small, OS-guarded compatibility shim (the sibling `luxtronik` integration already does exactly this). - PRs #394 and #395 have been converted to **draft** and will be replaced by PRs built on this approach. +## The two-audience problem this spec must solve + +The test suite has two structurally different halves, and pytest-hacc changes the rules for one of them: + +1. **HA-integration tests** (`tests/test_*.py`: coordinator, entity, init, diagnostics) — these want the real `hass` harness. pytest-hacc is exactly right here. +2. **API-client tests** (`tests/zaptec/*`: `test_zconst`, `test_redact`, `test_api`, `test_utils`, `test_validate`) — these test the vendored `zaptec/` client, which per **issue #257** is a **standalone-PyPI-library-in-waiting** (sveinse: *"the API access parts will have to be a separate library on pypi… namespace `zaptec`, such as `from zaptec import Zaptec`"*). They have **no HA dependency** and must not acquire one. Two of them (`test_zconst`, `test_redact`) make a **live** call to `api.zaptec.com/api/constants`. + +pytest-hacc blocks non-localhost sockets **unconditionally** on every test in any process where its plugin is active (verified empirically: `disable_socket()` + a `127.0.0.1` allow-list run in `pytest_runtest_setup` before every test; neither the `enable_socket` marker nor a manual `pytest_socket.enable_socket()` defeats the host allow-list). So the moment the harness is active in a process, the live constants call raises `SocketBlockedError`. + +Interweaving the two — running `tests/zaptec/*` under the harness — both couples the future-standalone library to HA (against #257) and breaks its live call. The design keeps them **separated**: the harness governs only the HA-integration tests; the API-client tests run as plain pytest, exactly as today. + ## Goals - Bring the coordinator + entity tests to gold/platinum shape: behavior-first, through the real HA harness. -- Establish reusable test infrastructure (`mock_zaptec` + `setup_integration` + Windows shim) that the #395 replacement reuses without re-solving anything. +- Establish reusable test infrastructure (`mock_zaptec` + `setup_integration`) that the #395 replacement reuses without re-solving anything. - Match or beat current coverage on `coordinator.py` / `entity.py` (100% / 98%) — but via observable behavior, not private-method assertions. -- Tests must run green in **native-Windows py314** (via the shim) *and* Linux CI. +- Keep the `tests/zaptec/*` API-client tests running exactly as today (plain pytest, live constants call intact), per #257. +- Shipped test infra is **Linux-native**: it matches CI and the maintainers' devcontainer, and carries **no native-Windows accommodation in tracked files**. ## Non-goals (out of scope for this spec) - The six platform files (`sensor/switch/number/button/binary_sensor/update`) — that is the #395 replacement, a separate PR. - Config-flow / `__init__` coverage. - Snapshot tests (deferred to the #395 replacement, where full-state snapshots pay off). -- Fixing bug #410 (see "Bug #410" below — this PR stays test-only). +- Fixing bug #410 (this PR stays test-only; see "Bug #410" below). +- An offline constants snapshot. The shelved `fix/constants-snapshot-fixture` design existed only to dodge the socket block; Option C avoids the block entirely by never running those tests under the harness, so the snapshot is unnecessary. + +## Approach (selected): Linux-native harness, scoped to the integration tests -## Approach (selected) +Adopt pytest-hacc, patch the integration at the `Zaptec` client boundary, and assert on public state — but **scope the harness to the HA-integration tests only** ("Option C"), so `tests/zaptec/*` stays plain pytest. -**Real harness, behavior-first.** Adopt pytest-hacc, patch the integration at the `Zaptec` client boundary, and assert on public state. Chosen over (B) a like-for-like fixture swap and (C) staying hand-rolled, because it is the only option that reaches the target standard and it turns the #410 gap into a real, self-catching test. +The scoping mechanism is deliberately minimal and standard: on Linux, pytest-hacc autoloads via its normal `pytest11` entry point, so the HA-integration run needs **no conftest machinery** to activate it. The API-client run disables it with a single, per-invocation `-p no:homeassistant`. Two pytest invocations, nothing more. + +Rejected alternatives: +- **(B) like-for-like fixture swap / (C-stay) hand-rolled mocks** — don't reach the target standard; keep the white-box coupling the review flagged. +- **Snapshot-under-harness** — runs `tests/zaptec/*` under the harness and dodges the socket block with a committed offline snapshot; keeps the two concerns interwoven (against #257) and adds a fixture to maintain. +- **Per-test socket opt-out** — empirically does not defeat pytest-hacc's `127.0.0.1` allow-list. +- **Committed native-Windows shim** — see "Why the shim is not committed" below. ## Design -### 1. Test infrastructure (the foundation) +### 1. Harness scope: two pytest invocations + +The harness must be active for `tests/test_*.py` and inactive for `tests/zaptec/*`. Because pytest-hacc's plugin (and its socket block) is process-wide, this is a **per-process** choice — one pytest run cannot have the harness on for some tests and off for others. So the suite runs as **two invocations**: + +```bash +# 1. HA-integration tests — harness autoloads (Linux pytest11 entry point) +pytest tests --ignore=tests/zaptec --cov=custom_components/zaptec --cov-branch + +# 2. API-client tests — harness disabled → plain pytest → live constants works +pytest tests/zaptec -p no:homeassistant --cov=custom_components/zaptec --cov-branch --cov-append +``` + +- `-p no:homeassistant` disables pytest-hacc's autoloaded plugin for run 2 only, so there is no socket block and the live `api.zaptec.com/api/constants` call behaves exactly as today. +- `--cov-append` on run 2 merges the two runs' coverage into one report, preserving the combined `coordinator.py`/`entity.py` numbers. +- No global `-p no:homeassistant` and no root `conftest.py` `pytest_plugins` line — committing either would disable autoload for run 1 and defeat the harness. Scoping lives entirely in the two commands. + +### 2. Test infrastructure (the foundation) - **`requirements_test.txt`** — add `pytest-homeassistant-custom-component` pinned **per-Python via environment markers** (`==0.13.324` for `python_version >= "3.14"`, `==0.13.316` for `< "3.14"`). Do NOT add `homeassistant` here: it is already pinned in `requirements.txt`, and validate.yaml sed-reverts it to `2026.2.3` on the 3.13 leg. pytest-hacc pins an exact `homeassistant==`, so its version MUST match the HA of each Python leg — 0.13.324↔2026.4.3 (py≥3.14), 0.13.316↔2026.2.3 (py≥3.13). Leaving it unpinned makes pip backtrack to an ancient release (pytest 6.2.2 → crashes on 3.13); single-pinning the newest is uninstallable on 3.13. -- **`conftest.py` (repo root, new)** — port luxtronik's OS-guarded shim: under `sys.platform == "win32"`, stub `fcntl` / `resource` and wrap `socket.socketpair`, then `pytest_plugins = "pytest_homeassistant_custom_component.plugins"`. Completely no-op on Linux, so CI is unaffected. `pytest_plugins` is only honored in the rootdir conftest, so this cannot live in `tests/conftest.py`. -- **pytest config** (`pyproject.toml` or `pytest.ini`) — add `-p no:homeassistant` to block the broken plugin autoload; the root conftest re-loads it explicitly *after* shimming. Confirm during planning that the repo has no conflicting existing pytest config. +- **`requirements.txt`** — relax the `pydantic` pin from an exact `==` to the manifest's supported range (`>=2.11.7,<2.14`) so pytest-hacc's transitive `pydantic==2.12.2` resolves alongside it. The devcontainer's `scripts/setup` installs both `requirements.txt` and `requirements_test.txt`, so they must co-resolve. +- **No committed root `conftest.py` for the harness.** On Linux (CI + devcontainer) the plugin autoloads; no shim, no `pytest_plugins`, no global `-p no:homeassistant`. (See "Why the shim is not committed.") - **`tests/conftest.py`** — replace the hand-rolled `hass` / `FakeConfigEntry` with: - - the harness's real `hass` fixture, + - the harness's real `hass` fixture (available via autoload; no import needed in the conftest), - a **`mock_zaptec`** fixture: `MagicMock(spec=Zaptec)` pre-populated with a representative installation + charger object graph (and, because `Zaptec` is a `Mapping[str, ZaptecBase]`, implementing `__getitem__` / `__iter__` / `values()` to yield the fake `Charger` / `Installation` objects the platforms enumerate), - a **`mock_config_entry`** (`MockConfigEntry`) and a **`setup_integration(hass, mock_zaptec)`** helper that patches the client into the setup path and awaits `async_setup`. + - the existing **`zaptec_constants`** fixture stays as-is (live call). It is only requested by `tests/zaptec/test_zconst.py` / `test_redact.py`, which run in invocation 2 (no harness → no socket block). It is never triggered in invocation 1 (that run `--ignore`s `tests/zaptec`), so it needs no socket guard. The event-loop save/restore added earlier stays (it protects the async fetch regardless of harness). -**Why this shim is justified now (and #403 was not):** a standalone shim PR (#403) was closed because pytest-hacc was not a real dependency, so CI didn't install it and the unconditional plugin import broke Linux CI. Here pytest-hacc becomes a genuine `requirements_test.txt` dependency (CI installs it, Linux import works natively) and the shim is `win32`-guarded (never runs on Linux). Both failure modes are avoided. +### 3. Why the shim is not committed (native-Windows is a local-only concern) -**Key risk & mitigation:** the whole approach hinges on the shim making pytest-hacc run in native-Windows py314. **Plan step 1 is a throwaway feasibility probe** (a trivial `async def test_hass(hass)` under the shim) before any real test is written. Fallback if it fails: run tests in a devcontainer on the user's Raspberry Pi (HA-in-Docker, separate port). Rated low-risk because luxtronik already runs pytest-hacc in this exact py314 env. +Home Assistant imports `fcntl` (Unix-only), so pytest-hacc's plugin cannot autoload on native Windows. Earlier iterations of this migration carried a `win32`-guarded shim (fcntl/resource/socketpair stubs) in a root `conftest.py`, plus a `pytest_plugins` line and a global `-p no:homeassistant`, purely so the maintainer's — and this assistant's — native-Windows environment could run the integration tests. -### 2. The #394-replacement tests (coordinator + entity, behavior-first) +That machinery is **not committed**, for three reasons: + +1. **It contradicts the maintainers' stated workflow.** They promote the devcontainer and have pushed back on native-Windows accommodation (steinmn on #398); a standalone committed Windows shim (PR #403) was already **closed**. +2. **It is the root cause of the scoping complexity.** The shim must run before pytest-hacc imports `fcntl`, which forces a root-conftest `pytest_plugins` + global `-p no:homeassistant` and thus a **session-wide** harness load — which is exactly what makes scoping `tests/zaptec/*` away from the harness hard. Dropping the shim lets Linux autoload the plugin, so scoping collapses to one per-invocation `-p no:homeassistant` (§1). +3. **CI and the devcontainer are both Linux**, so nothing shipped needs the shim. + +**Local native-Windows runs** (this assistant's environment, and any contributor on native Windows) are handled outside tracked files: +- `tests/zaptec/*` already run natively today: `pytest tests/zaptec -p no:homeassistant` (this is the existing convention; the harness is off, `fcntl` is never imported). +- The **HA-integration tests** need Linux: run them in the **devcontainer** (what the maintainers promote), or, for a quick local check, under an **uncommitted, untracked** local shim conftest. Neither path ships. + +### 4. The #394-replacement tests (coordinator + entity, behavior-first) Same two modules, asserted through the real harness instead of poking privates. @@ -68,7 +114,7 @@ Same two modules, asserted through the real harness instead of poking privates. **Coverage target:** match or beat current 100% / 98% on `coordinator.py` / `entity.py`, achieved via behavior. -### 3. The mocked Zaptec client & shared test data +### 5. The mocked Zaptec client & shared test data **Patch at the `Zaptec` client boundary (Layer 2), not the HTTP/SignalR wire (Layer 1).** @@ -88,33 +134,37 @@ Everything *above* the client — `ZaptecManager`, `ZaptecUpdateCoordinator`, `Z **Test-data source:** a small hand-authored dict suffices for #394 (coordinator/entity base behavior needs only a couple of keys). The **fuller** payload needed by the #395 replacement will be seeded from a **redacted real diagnostics dump** (the repo already has `diagnostics.py` + `redact.py`), stored as a JSON fixture, so snapshots reflect real-world data rather than invented values. -### 4. Bug #410 handling — test-only, deferred fix +### 6. Bug #410 handling — test-only, deferred fix + +Filed as custom-components/zaptec#410: `ZaptecBaseEntity` sets `_attr_available = False` on `KeyUnavailableError` but never overrides `available`. Contributor steinmn responded that this is **not a bug**, and the exact mechanism was confirmed against installed HA (2026.4.3): `ZaptecBaseEntity` subclasses `CoordinatorEntity`, whose `available` property is first in the MRO and returns `self.coordinator.last_update_success` — it never reads `_attr_available`. So a single missing key does not (and by design should not) take an entity `unavailable`; only a failed coordinator poll does. (steinmn's comment cited the *base* `Entity.available` reading `_attr_available`, but that base property is overridden by `CoordinatorEntity.available`, so it doesn't govern these entities — the conclusion holds via the override.) steinmn also noted the HA entity-vs-device distinction: one entity going unavailable would not make its charger/installation *device* unavailable. -Filed as custom-components/zaptec#410: `ZaptecBaseEntity` sets `_attr_available = False` on `KeyUnavailableError` but never overrides `available`, so the flag has no effect on reported availability. +**Status:** the reporter (rhammen) has concluded #410 is not a bug and intends to close it; it is currently still open. The earlier concern that `_attr_available` is "never reset to True on success" is also refuted: each derived platform's `_update_from_zaptec` sets it back to `True` on a good update (e.g. `binary_sensor.py:34`). -Investigation showed the fix is **not obvious** and needs maintainer input: +**Decision:** this PR stays **test-only** and asserts the **observed, correct** behavior (no xfail — the observed behavior is definitive regardless of the still-open semantic discussion): an entity with a single missing key stays available; an entity is `unavailable` only when the coordinator poll fails. (Reworked from the earlier xfail-encoded premise; see commit 0ecebab.) -1. **Mechanical gap:** `available` isn't overridden (trivial to add). -2. **Latent sticky-flag bug:** `_handle_coordinator_update` never resets `_attr_available = True` on the success path, so a naive override would leave recovered entities unavailable forever. Any fix must override `available` *and* reset the flag on success. -3. **Semantic design question:** many keys are legitimately absent for some charger models / installation types / roles (the code already has a skip-set for such keys in `_log_unavailable`). Making *any* `KeyUnavailableError` flip an entity to `unavailable` could make entities disappear for real users. Which keys are "required" vs. optional is a maintainer decision. +### 7. CI + scripts wiring -**Decision:** this PR stays **test-only**. The availability case is asserted as today's real behavior with an `xfail(reason="#410")` documenting the gap through the real harness. The fix is deferred to a separate PR after the semantics are decided. #410 has been updated with findings (2) and (3) and a request for input from @sveinse / @steinmn. +- **`.github/workflows/validate.yaml`** — the test job installs `requirements.txt` + `requirements_test.txt` (co-resolving per §2) and runs the **two** pytest invocations (§1), preserving the existing 3.13 HA sed-revert. Coverage combines via `--cov-append`. +- **`scripts/test`** — mirror the two-invocation structure so local (Linux/devcontainer) runs match CI. Keep the `--skip-api` path (`SKIP_ZAPTEC_API_TEST=true`) working for invocation 2's login-gated tests. +- **`DEVELOPMENT.md`** — document the two-invocation split and that the HA-integration tests require Linux (devcontainer); `tests/zaptec/*` run natively with `-p no:homeassistant`. ## Success criteria (this PR) - Coverage on `coordinator.py` / `entity.py` ≥ current (100% / 98%), achieved via behavior. -- `pytest tests` green in native-Windows py314 (via shim) **and** Linux CI. +- `pytest tests --ignore=tests/zaptec` (harness) **and** `pytest tests/zaptec -p no:homeassistant` (plain) both green on Linux CI; combined coverage via `--cov-append`. +- `tests/zaptec/*` behavior unchanged — live constants call still runs, no socket block, no HA import. +- No native-Windows accommodation in tracked files (Linux-native infra). - `ruff format` + `ruff check` clean. -- hassfest / HACS unaffected (`requirements_test.txt` is not shipped in the component; the root `conftest.py` and pytest config are dev-only). +- hassfest / HACS unaffected (`requirements_test.txt` is not shipped in the component; no root `conftest.py` / pytest-config changes are shipped for the harness). ## PR / branch strategy - #394 and #395 held as draft (done 2026-07-25); reply posted on #394 explaining the direction. -- New branch off `master` for this replacement PR (per repo convention: dedicated branch per unit of work). -- The #395 replacement is a **separate, later** PR that reuses this infrastructure. +- Migration lands on `test/ha-test-harness-migration` (already off `master`); it replaces #394. The #395 replacement is a **separate, later** PR reusing this infrastructure. +- Reference #257 in the PR body as the rationale for keeping `tests/zaptec/*` out of the harness. Note the upstream-PR-stack dependency (see [[upstream-pr-stack]]). ## Open items carried into planning -- Confirm whether the repo has existing pytest config to reconcile (it does: `[tool.pytest.ini_options]` in `pyproject.toml`). HA version is managed by `requirements.txt` + validate.yaml's 3.13 sed-revert, not by `requirements_test.txt`. +- Confirm the two-invocation coverage numbers combine correctly under `--cov-append` (branch coverage merges). +- Confirm `pyproject.toml`'s existing `[tool.pytest.ini_options]` has no global option that conflicts with the per-invocation `-p no:homeassistant` (e.g. no committed `addopts` that force-loads or force-disables the plugin). - Confirm exact patch target (`custom_components.zaptec.Zaptec` import site) and the minimal `mock_zaptec` object-graph shape for #394. -- Feasibility probe (plan step 1) before writing real tests. From 95b080044589b23a46c23d5e2f577ecc3e2d4caa Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 03:30:04 +0200 Subject: [PATCH 15/29] test: drop committed Windows shim; rely on Linux pytest-hacc autoload On Linux (CI + devcontainer) pytest-hacc autoloads via its pytest11 entry point, so the win32-guarded shim (fcntl/resource/socketpair stubs + explicit pytest_plugins load) and the global -p no:homeassistant are no longer needed. This also removes the root cause of the harness's session-wide load, which is what made scoping tests/zaptec/* away from it hard (see Option C in the migration spec). Verified (native Windows, py314): tests/zaptec -p no:homeassistant runs as before (80 passed, 1 skipped, 22 pre-existing DNS errors, no SocketBlockedError); pytest tests --ignore=tests/zaptec now fails fast on ModuleNotFoundError: fcntl, confirming the harness autoload takes effect and that half now requires Linux. ruff format/check clean. Co-Authored-By: Claude Opus 4.8 --- conftest.py | 56 -------------------------------------------------- pyproject.toml | 1 - 2 files changed, 57 deletions(-) delete mode 100644 conftest.py diff --git a/conftest.py b/conftest.py deleted file mode 100644 index 46c3bf76..00000000 --- a/conftest.py +++ /dev/null @@ -1,56 +0,0 @@ -"""Repo-root conftest: load pytest-homeassistant-custom-component explicitly. - -The plugin autoloads via a pytest11 entry point, but importing it on Windows -fails immediately (`homeassistant.runner` imports `fcntl`, Unix-only) before any -test collects. `-p no:homeassistant` in pyproject.toml blocks that autoload; -this file loads the plugin back explicitly, with Windows compatibility shims -applied first. pytest only honors `pytest_plugins` in the rootdir conftest, so -this cannot live in tests/conftest.py. The shim is a no-op on Linux (CI), where -fcntl/resource exist and the plugin imports natively. -""" - -import socket -import sys -import types -from typing import Any - -if sys.platform == "win32": - import pytest_socket - - if "fcntl" not in sys.modules: - fake_fcntl = types.ModuleType("fcntl") - fake_fcntl.LOCK_SH = 1 - fake_fcntl.LOCK_EX = 2 - fake_fcntl.LOCK_NB = 4 - fake_fcntl.LOCK_UN = 8 - fake_fcntl.flock = lambda *_args: None - fake_fcntl.lockf = lambda *_args: None - fake_fcntl.fcntl = lambda *_args: 0 - fake_fcntl.ioctl = lambda *_args: 0 - sys.modules["fcntl"] = fake_fcntl - - if "resource" not in sys.modules: - fake_resource = types.ModuleType("resource") - fake_resource.RLIMIT_NOFILE = 7 - fake_resource.RLIM_INFINITY = -1 - fake_resource.getrlimit = lambda *_args: (8192, 8192) - fake_resource.setrlimit = lambda *_args: None - sys.modules["resource"] = fake_resource - - _orig_socketpair = socket.socketpair - - def _shimmed_socketpair(*args: Any, **kwargs: Any) -> tuple[socket.socket, socket.socket]: - blocked = getattr(socket.socket, "__module__", "") == "pytest_socket" - if not blocked: - return _orig_socketpair(*args, **kwargs) - - pytest_socket.enable_socket() - try: - return _orig_socketpair(*args, **kwargs) - finally: - pytest_socket.socket_allow_hosts(["127.0.0.1"]) - pytest_socket.disable_socket(allow_unix_socket=True) - - socket.socketpair = _shimmed_socketpair - -pytest_plugins = "pytest_homeassistant_custom_component.plugins" diff --git a/pyproject.toml b/pyproject.toml index a2aadd6c..d3c71a4f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,6 @@ pythonpath = [ testpaths = [ "tests", ] -addopts = "-p no:homeassistant" log_format = "%(asctime)s.%(msecs)03d %(levelname)-8s %(threadName)s %(name)s:%(filename)s:%(lineno)s %(message)s" log_date_format = "%Y-%m-%d %H:%M:%S" filterwarnings = [ From 70ed43f447cc3427e36fa186ed6be77d29d36a70 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 03:30:24 +0200 Subject: [PATCH 16/29] ci: run harness + API-client tests as two scoped pytest invocations Co-Authored-By: Claude Opus 4.8 --- .github/workflows/validate.yaml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/validate.yaml b/.github/workflows/validate.yaml index 20dc7526..07b5dce2 100644 --- a/.github/workflows/validate.yaml +++ b/.github/workflows/validate.yaml @@ -104,7 +104,11 @@ jobs: -r requirements.txt \ -r requirements_test.txt - - name: Tests suite + - name: Tests suite (HA integration — harness) run: | - pytest --cov=./custom_components/zaptec --cov-branch + pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch + + - name: Tests suite (API client — plain pytest, no harness) + run: | + pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append From a77a9b750b920d416557c43e7cb205dd8426904b Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 03:30:51 +0200 Subject: [PATCH 17/29] test: scripts/test runs harness + API-client invocations, combined coverage Co-Authored-By: Claude Opus 4.8 --- scripts/test | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/test b/scripts/test index b3d15862..8017ebd1 100755 --- a/scripts/test +++ b/scripts/test @@ -5,8 +5,14 @@ set -e if [ "$1" == "--skip-api" ]; then export SKIP_ZAPTEC_API_TEST="true" fi + +# HA-integration tests run under the pytest-hacc harness (autoloads on Linux). +# API-client tests (tests/zaptec/*, the future-standalone client per #257) run +# as plain pytest with the harness disabled, so their live constants call is not +# socket-blocked. Coverage from both is combined via --cov-append. # run tests with -s to display printouts and --log-cli-level to get logger output -pytest --cov=./custom_components/zaptec --cov-branch --log-cli-level=INFO -s +pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch --log-cli-level=INFO -s +pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append --log-cli-level=INFO -s # generate coverage report in html and xml coverage html From 3ba66e8fc0d8254fa655fb84dd3d284fc7a8688a Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 03:31:05 +0200 Subject: [PATCH 18/29] docs: explain two-invocation test split (harness vs API-client, #257) Co-Authored-By: Claude Opus 4.8 --- DEVELOPMENT.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 02ec9413..78f57cdf 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -147,6 +147,24 @@ To run tests and check test coverage: report, or enable the "Coverage Gutters" extension to view the coverage directly in VSCode. +The suite runs as **two pytest invocations**, and `./scripts/test` runs both: + +- **HA-integration tests** (`tests/test_*.py`) run under the + `pytest-homeassistant-custom-component` harness, which autoloads on Linux. + Run directly with: + `pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch` +- **API-client tests** (`tests/zaptec/*`) test the vendored `zaptec/` client, + which is destined to become a standalone PyPI library (issue #257) and has no + Home Assistant dependency. They run as plain pytest with the harness disabled + (the harness blocks non-localhost sockets, which would break their live + `api.zaptec.com/api/constants` call): + `pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append` + +Because the harness (and its socket block) is process-wide, a bare `pytest` +is not the entry point — use `./scripts/test` or the two commands above. The +HA-integration tests require Linux; run them in the Dev Container (native +Windows is not supported for that half). `tests/zaptec/*` run anywhere. + HA requires [95% coverage](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/test-coverage/) for all core integration modules, and while HACS doesn't have the same requirements, reaching this level is still a goal for this integration. From 6550f734b522c3988ca49a15b6f04664bbdd40f1 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:27:34 +0200 Subject: [PATCH 19/29] test: normalize keys in _backed_get via to_under, matching ZaptecBase.__getitem__ _backed_get's mock .get() did a bare dict lookup, diverging from the real ZaptecBase (which normalizes camelCase API keys to snake_case symmetrically on both read and write). Harmless today since all seed data is hand-authored snake_case, but would have silently broken a future fixture seeded from a raw diagnostics dump (e.g. #395) without the normalization. to_under is idempotent on already-normalized keys, so this has no effect on current tests. Co-Authored-By: Claude Opus 4.8 --- tests/conftest.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0b73e964..5b2f7a7c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,6 +15,7 @@ from custom_components.zaptec.manager import ZaptecManager from custom_components.zaptec.zaptec import MISSING, Charger, Installation from custom_components.zaptec.zaptec.api import Zaptec +from custom_components.zaptec.zaptec.utils import to_under @pytest.fixture(scope="session") @@ -101,15 +102,16 @@ async def get_zaptec_constants() -> dict: def _backed_get(data: dict[str, Any]) -> Callable[..., Any]: """Return a `.get(key, default=MISSING)` implementation backed by `data`. - Intentionally diverges from `ZaptecBase.get` in two ways: (1) defaults to - `MISSING` instead of `None`, and (2) does not normalize keys via `to_under`. - This is sufficient for coordinator/entity code under test (which always passes - `default=MISSING` and uses snake_case keys), but future fixtures like #395's - diagnostics dump should not blindly inherit these assumptions. + Mirrors `ZaptecBase.__getitem__`'s key normalization (`to_under`) so lookups + behave the same whether `data` is hand-authored snake_case or seeded from a + raw API payload. Still diverges from `ZaptecBase.get` (inherited from + `Mapping.get`) in its own default: `MISSING` instead of `None`. Harmless in + practice, since every real call site (`entity.py`'s `_get_zaptec_value`) + always passes `default=MISSING` explicitly. """ def _get(key: str, default: Any = MISSING) -> Any: - return data.get(key, default) + return data.get(to_under(key), default) return _get From b8f212a42f55e298efa67138f4c6b882d5bd9c32 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:49:27 +0200 Subject: [PATCH 20/29] test: pin charging-interval test to named constants, keep the relation check The interval-switch test only asserted charger_coord.update_interval < idle_interval, which verifies ordering but not the actual values selected. Add equality assertions against ZAPTEC_POLL_INTERVAL_IDLE/CHARGING (matching the pattern in HA core's own coordinator tests, e.g. tests/components/jvc_projector/test_coordinator.py's assert coordinator.update_interval == INTERVAL_SLOW/FAST), which catches a coordinator wiring bug the relation alone would miss. Kept the relation assertion too: it catches a const.py regression (charging >= idle) that the equality checks alone would miss, since the coordinator would still be 'correctly' wired to whatever (wrong) constants are defined. Co-Authored-By: Claude Opus 4.8 --- tests/test_coordinator.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 6bac1ecf..5dc5a0f9 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -1,11 +1,16 @@ """Behavior tests for ZaptecUpdateCoordinator, driven through the real harness.""" +from datetime import timedelta from unittest.mock import AsyncMock, MagicMock from homeassistant.core import HomeAssistant import pytest from pytest_homeassistant_custom_component.common import MockConfigEntry +from custom_components.zaptec.const import ( + ZAPTEC_POLL_INTERVAL_CHARGING, + ZAPTEC_POLL_INTERVAL_IDLE, +) from custom_components.zaptec.coordinator import ZaptecUpdateCoordinator, ZaptecUpdateOptions from custom_components.zaptec.zaptec import ZaptecApiError from tests.conftest import setup_integration @@ -50,11 +55,17 @@ async def test_device_coordinator_switches_interval_when_charging( manager = await setup_integration(hass, mock_config_entry, mock_zaptec) charger_coord = manager.device_coordinators["chg-mock-1"] idle_interval = charger_coord.update_interval + assert idle_interval == timedelta(seconds=ZAPTEC_POLL_INTERVAL_IDLE) # Flip the seeded charger to 'charging' and re-run the update-listener path. mock_zaptec.chargers[0].is_charging.return_value = True charger_coord.set_update_interval() + assert charger_coord.update_interval == timedelta(seconds=ZAPTEC_POLL_INTERVAL_CHARGING) + # Also assert the relation directly: charging must poll faster than idle. This + # catches a regression that equality checks alone would miss, e.g. const.py + # setting ZAPTEC_POLL_INTERVAL_CHARGING >= ZAPTEC_POLL_INTERVAL_IDLE, which would + # still pass both equality asserts above despite breaking the whole feature. assert charger_coord.update_interval < idle_interval From 057af4bc77881bef200e5eb5189e986ffad2827c Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:24:06 +0200 Subject: [PATCH 21/29] test: explain non-obvious white-box test rationale, cover 4th log-format branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audited every test docstring in test_entity.py for whether a maintainer could tell WHY each private-attribute/method access is needed without re-deriving it themselves. Added the missing 'why' to six spots: - _entity_from_coordinator: why it reaches into _listeners at all (no public API for 'entities subscribed to this coordinator'), why hasattr(_log_value) is the discriminator against the coordinator's own listener, and why key_not_in_skip_list exists (gates one specific _log_unavailable log line). - test_log_value_*: _log_value has no public-state effect, so there's no hass.states equivalent to test through. - test_get_zaptec_value_returns_default_when_key_missing: distinguishes the MISSING-sentinel default (triggers unavailability) from sensor.py's one explicit-default call site (opts out, for a genuinely optional key). - test_get_zaptec_value_raises_when_intermediate_value_not_mapping: no shipped entity currently uses a dotted key, so this guards the helper's documented contract ahead of any real caller. - test_log_zaptec_attribute_*: clarifies which of the four formatting branches are live in production (str, Iterable) vs. unused-but-documented (None) vs. genuinely dead (the scalar fallback) — and adds a 4th assertion covering that previously-untested fallback branch (confirmed missing via direct query against a coverage.py SQLite db from a prior run). - test_log_unavailable_*: explains why it pokes _attr_available/_prev_available directly instead of driving both transitions through a real coordinator refresh — ties directly to the #410 finding that those attributes are decoupled from the entity's actual HA-reported availability. Co-Authored-By: Claude Opus 4.8 --- tests/test_entity.py | 76 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 9 deletions(-) diff --git a/tests/test_entity.py b/tests/test_entity.py index 9f854e83..dcf47ed5 100644 --- a/tests/test_entity.py +++ b/tests/test_entity.py @@ -19,9 +19,24 @@ def _entity_from_coordinator( ) -> ZaptecBaseEntity: """Return a real entity instance bound to `coordinator`. - `_listeners` also holds the coordinator's own `set_update_interval` listener - (registered in ZaptecUpdateCoordinator.__init__), so filter for a callback - bound to an actual entity rather than assuming the first one qualifies. + There's no public API to list "the entities subscribed to this coordinator" — + entities are owned by the entity platform/registry, not the coordinator. So + this reaches into the coordinator's private `_listeners` dict ({id: (callback, + context)}, populated by every `async_add_listener` call) and reads `cb.__self__` + off each bound-method callback to get back the object it belongs to. + + `_listeners` isn't only entities: `ZaptecUpdateCoordinator.__init__` also + registers its own `set_update_interval` as a listener for charger coordinators + (coordinator.py). `hasattr(candidate, "_log_value")` filters that out — + `_log_value` is defined only on `ZaptecBaseEntity`, never on the coordinator, + so it reliably distinguishes "an entity" from "the coordinator itself." + + `key_not_in_skip_list=True` additionally skips any entity whose `.key` is in + `KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK`. That set gates one specific log line + in `ZaptecBaseEntity._log_unavailable` (`"Getting value failed"`, suppressed + for skip-listed keys) — a test asserting that line appears needs an entity + NOT on the skip list, since which entity this function returns otherwise + depends on `_listeners`' iteration order, not anything the test controls. """ for cb, _context in coordinator._listeners.values(): # noqa: SLF001 candidate = cb.__self__ @@ -126,7 +141,14 @@ async def test_log_value_logs_on_change_then_skips_when_unchanged( caplog: pytest.LogCaptureFixture, enable_custom_integrations: None, ) -> None: - """_log_value logs when the tracked value changes and stays quiet when it doesn't.""" + """_log_value logs when the tracked value changes and stays quiet when it doesn't. + + `_log_value(attribute)` reads an arbitrary instance attribute via + `getattr(self, attribute, MISSING)` and dedups against `self._prev_value`, + purely to feed a debug log line — no public state changes either way, so + there's no `hass.states` equivalent to test through. `entity.some_attr` is + set here as the attribute the method is told to read by name. + """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] entity = _entity_from_coordinator(coordinator) @@ -148,7 +170,14 @@ async def test_get_zaptec_value_returns_default_when_key_missing( mock_zaptec: MagicMock, enable_custom_integrations: None, ) -> None: - """_get_zaptec_value() returns the caller's default when the key isn't backed.""" + """_get_zaptec_value() returns the caller's default when the key isn't backed. + + Most call sites rely on the `MISSING` sentinel default to trigger + `KeyUnavailableError` when a key is absent. Exactly one production call + site opts out of that (`sensor.py`'s `default={}` for the optional + `completed_session` key) — this covers that explicit-default path, not + just generic `.get()` plumbing. + """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] entity = _entity_from_coordinator(coordinator) @@ -163,7 +192,15 @@ async def test_get_zaptec_value_raises_when_intermediate_value_not_mapping( mock_zaptec: MagicMock, enable_custom_integrations: None, ) -> None: - """A dotted key whose first segment resolves to a non-Mapping value raises.""" + """A dotted key whose first segment resolves to a non-Mapping value raises. + + No shipped entity description currently uses a dotted key (`sensor.py`, + `binary_sensor.py`, `number.py`, `switch.py`, `update.py` all pass a single + flat key, e.g. "signed_meter_value"), so this exercises the "obj isn't + Mapping-like" half of `_get_zaptec_value`'s documented `Raises:` contract + directly rather than through a real entity, guarding it for whenever a + future entity does use one. + """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] entity = _entity_from_coordinator(coordinator) @@ -173,13 +210,23 @@ async def test_get_zaptec_value_raises_when_intermediate_value_not_mapping( entity._get_zaptec_value(key="operating_mode.sub") # noqa: SLF001 -async def test_log_zaptec_attribute_formats_none_str_and_iterable_keys( +async def test_log_zaptec_attribute_formats_none_str_iterable_and_scalar_keys( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_zaptec: MagicMock, enable_custom_integrations: None, ) -> None: - """_log_zaptec_attribute formats None, a single key, and an iterable of keys.""" + """_log_zaptec_attribute formats None, a single key, an iterable, and a scalar. + + `str` is the live default (every entity's `description.key`) and `Iterable` + is also live (sensor.py/update.py override it with a list for multi-key + logging). `None` is a documented-but-currently-unused hook, and the final + scalar case (anything that's not None/str/Iterable, e.g. an int) is the + property's fallback branch — also currently unreachable in production, but + covered here since a test can exercise it even though no shipped entity + does. Pokes all four directly since the property only ever feeds a debug + log line, so no real entity's state exposes it. + """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] entity = _entity_from_coordinator(coordinator) @@ -193,6 +240,9 @@ async def test_log_zaptec_attribute_formats_none_str_and_iterable_keys( entity._log_zaptec_key = ["foo", "bar"] # noqa: SLF001 assert entity._log_zaptec_attribute == ".foo and .bar" # noqa: SLF001 + entity._log_zaptec_key = 42 # noqa: SLF001 + assert entity._log_zaptec_attribute == ".42" # noqa: SLF001 + async def test_log_unavailable_logs_error_and_recovery_transitions( hass: HomeAssistant, @@ -201,7 +251,15 @@ async def test_log_unavailable_logs_error_and_recovery_transitions( caplog: pytest.LogCaptureFixture, enable_custom_integrations: None, ) -> None: - """_log_unavailable logs the real exception on going unavailable, and logs recovery.""" + """_log_unavailable logs the real exception on going unavailable, and logs recovery. + + Its transition logging is driven purely by `_attr_available`/`_prev_available` + — which, per #410, are decoupled from the entity's actual HA-reported + availability (`CoordinatorEntity.available` reads `coordinator.last_update_success`, + never these). So there's no realistic way to drive both log transitions + through a real coordinator refresh; setting the attributes directly is the + only way to exercise this logging branch in isolation. + """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] entity = _entity_from_coordinator(coordinator, key_not_in_skip_list=True) From 0b7941071bd4d192a8e45df66a53877da12491b8 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:26:14 +0200 Subject: [PATCH 22/29] test: explain the mock-name coupling behind test_init.py's entity filter The entity_id.startswith('mock') filter only works because conftest.py's mock_zaptec seeds 'Mock Charger'/'Mock Home' and HA slugifies entity names into entity_id's object_id half. That dependency lived silently in a different file; note it here so a future rename doesn't produce a confusing 'expected at least one zaptec entity' failure with no pointer to the cause. Co-Authored-By: Claude Opus 4.8 --- tests/test_init.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_init.py b/tests/test_init.py index 6bd66daa..1e3a7260 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -51,6 +51,9 @@ async def test_setup_entry_creates_manager_and_entities( assert isinstance(manager, ZaptecManager) assert mock_config_entry.runtime_data is manager - # At least one entity from the seeded charger reached the state machine. + # HA slugifies each entity's name into its entity_id's object_id half; this + # only matches because conftest.py's mock_zaptec seeds "Mock Charger"/"Mock + # Home" (make_charger/make_installation), so every zaptec-created entity_id + # starts with "mock". If that seed naming ever changes, update this filter. states = [s for s in hass.states.async_all() if s.entity_id.split(".")[1].startswith("mock")] assert states, "expected at least one zaptec entity to be created" From 7611e674a7f7d07dcd05a1369392270fc7dae90b Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:30:15 +0200 Subject: [PATCH 23/29] test: explain three non-obvious setup choices in trigger_poll/constructor tests Audited the remaining tests in test_coordinator.py for the same 'why' gap: - test_charging_update_interval_requires_charger_object: why a bare MagicMock() manager suffices (accessed via manager.zaptec, auto-vivifies, before the guard fires) and why setup_integration isn't used (deliberately bypassed to hit the constructor guard in isolation). - test_trigger_poll_is_noop_without_zaptec_object: why head_coordinator specifically (the one coordinator built with zaptec_object=None, unlike every device coordinator). - test_trigger_poll_triggers_child_charger_coordinators: why asyncio.sleep is patched globally instead of a delays-list constant (installations use a different constant than the sibling cancel/reschedule test patches), and why the child coordinator's trigger_poll is mocked rather than left real (isolates parent-calls-child from the child's own mechanics). Co-Authored-By: Claude Opus 4.8 --- tests/test_coordinator.py | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 5dc5a0f9..17a9b9cb 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -73,7 +73,15 @@ async def test_charging_update_interval_requires_charger_object( hass: HomeAssistant, mock_config_entry: MockConfigEntry, ) -> None: - """Constructing a coordinator with a charging interval on a non-Charger object errors.""" + """Constructing a coordinator with a charging interval on a non-Charger object errors. + + Constructs `ZaptecUpdateCoordinator` directly (skipping `setup_integration`) + to hit this constructor-time guard in isolation. A bare, unconfigured + `manager=MagicMock()` is enough: `__init__` does `self.zaptec = manager.zaptec` + (auto-vivifies on a MagicMock, no error) before the `isinstance(zaptec_object, + Charger)` check runs, so nothing about the manager's real behavior is + exercised before the ValueError fires. + """ mock_config_entry.add_to_hass(hass) with pytest.raises(ValueError, match="Charging update interval requires a Charger object"): @@ -98,7 +106,13 @@ async def test_trigger_poll_is_noop_without_zaptec_object( mock_zaptec: MagicMock, enable_custom_integrations: None, ) -> None: - """trigger_poll() on a coordinator with no bound zaptec object does nothing.""" + """trigger_poll() on a coordinator with no bound zaptec object does nothing. + + `manager.head_coordinator` specifically: it's the one coordinator built with + `zaptec_object=None` (the account-wide coordinator; __init__.py), unlike every + device coordinator, which always gets a real Charger/Installation. That's what + satisfies trigger_poll()'s `if zaptec_obj is None: return` no-op guard. + """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) await manager.head_coordinator.trigger_poll() @@ -151,7 +165,16 @@ async def test_trigger_poll_triggers_child_charger_coordinators( enable_custom_integrations: None, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Polling an installation also triggers the poll sequence of its tracked chargers.""" + """Polling an installation also triggers the poll sequence of its tracked chargers. + + Patches `asyncio.sleep` globally rather than a delays-list constant (as the + cancel/reschedule test does) because installations use a different constant + (ZAPTEC_POLL_INSTALLATION_TRIGGER_DELAYS) whose values this test doesn't care + about — it only needs to reach the loop's first iteration fast, since + children are triggered at `i == 1` (coordinator.py). `charger_coord.trigger_poll` + is replaced with a mock rather than left real, to isolate "did the parent call + the child" from "does the child's own trigger_poll work" (covered elsewhere). + """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) install_coord = manager.device_coordinators["inst-mock-1"] charger_coord = manager.device_coordinators["chg-mock-1"] From 6858e82b62ba2f64363e24abf4ce4aac163ed1c6 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:34:36 +0200 Subject: [PATCH 24/29] test: explain four non-obvious fixture/mock choices in conftest.py Audited conftest.py for the same 'why' gap as the other test files: - make_charger: model is hardcoded to ZaptecBase's base default format, not Charger.model's real device-ID-prefix lookup override -- a known simplification, same category as _backed_get's already-documented ones. - mock_zaptec: the __getitem__/__iter__/__contains__/__len__ wiring isn't arbitrary scaffolding -- Zaptec is itself Mapping[str, ZaptecBase] in production, and real code indexes into it directly. - mock_zaptec: redact.dumps.return_value = '' is load-bearing, not incidental -- __init__.py's startup debug-dump path concatenates its result into a string, which would TypeError on an unconfigured MagicMock. - setup_integration: notes the unittest.mock 'patch where it's used, not where it's defined' rule behind the patch target, since __init__.py holds its own local Zaptec reference via 'from .zaptec import Zaptec'. Co-Authored-By: Claude Opus 4.8 --- tests/conftest.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5b2f7a7c..a47dcd6f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -119,7 +119,13 @@ def _get(key: str, default: Any = MISSING) -> Any: def make_charger( data: dict[str, Any], *, installation: MagicMock | None = None, charging: bool = False ) -> MagicMock: - """Build a spec'd Charger double backed by `data`.""" + """Build a spec'd Charger double backed by `data`. + + `model` is hardcoded to the base `ZaptecBase.model`'s default format + (`f"Zaptec {qualname}"`, api.py) — it does NOT model `Charger.model`'s real + override, which looks up a device-ID-prefix in `ZCONST.serial_to_model`. A + known, deliberate simplification, same category as `_backed_get`'s divergences. + """ charger = MagicMock(spec=Charger) charger.id = data["id"] charger.name = data.get("name", "Mock Charger") @@ -147,7 +153,13 @@ def make_installation(data: dict[str, Any], *, chargers: Iterable[MagicMock] = ( @pytest.fixture def mock_zaptec() -> MagicMock: - """A spec'd Zaptec client seeded with one installation and one charger.""" + """A spec'd Zaptec client seeded with one installation and one charger. + + The `__getitem__`/`__iter__`/`__contains__`/`__len__` wiring isn't arbitrary + mock scaffolding: `Zaptec` is itself `Mapping[str, ZaptecBase]` in production + (api.py), and real code indexes into it directly (e.g. `zaptec[deviceid]` in + `__init__.py`/`coordinator.py`). + """ installation = make_installation({"id": "inst-mock-1", "name": "Mock Home"}) charger = make_charger( { @@ -177,6 +189,9 @@ def mock_zaptec() -> MagicMock: zaptec.poll = AsyncMock(return_value=None) zaptec.show_all_updates = False zaptec.redact = MagicMock() + # Load-bearing, not incidental: __init__.py's startup debug-dump path does + # `message += manager.zaptec.redact.dumps()`, which setup_integration actually + # exercises. An unconfigured MagicMock here would raise TypeError on the +=. zaptec.redact.dumps.return_value = "" return zaptec @@ -195,7 +210,14 @@ def mock_config_entry() -> MockConfigEntry: async def setup_integration( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_zaptec: MagicMock ) -> ZaptecManager: - """Set the integration up through the real async_setup, with a mocked client.""" + """Set the integration up through the real async_setup, with a mocked client. + + Patches `custom_components.zaptec.Zaptec`, not `custom_components.zaptec.zaptec. + api.Zaptec` where the class is defined — the standard unittest.mock rule is to + patch where a name is *looked up*, not where it's *defined*. `__init__.py` does + `from .zaptec import Zaptec`, so it holds its own local reference; patching the + original definition would silently do nothing here. + """ mock_config_entry.add_to_hass(hass) with patch("custom_components.zaptec.Zaptec", return_value=mock_zaptec): assert await hass.config_entries.async_setup(mock_config_entry.entry_id) From 4361b0aa3a31512706a35b12324816bf2600f0fb Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:53:00 +0200 Subject: [PATCH 25/29] docs: remove planning docs (archived to docs/ai-planning-archive) Moved to a dedicated fork-only branch per maintainer preference not to carry AI-generated planning docs in the upstream repo. --- .../2026-07-25-ha-test-harness-migration.md | 313 ------------------ ...-07-25-ha-test-harness-migration-design.md | 170 ---------- 2 files changed, 483 deletions(-) delete mode 100644 docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md delete mode 100644 docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md diff --git a/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md b/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md deleted file mode 100644 index 6baac826..00000000 --- a/docs/superpowers/plans/2026-07-25-ha-test-harness-migration.md +++ /dev/null @@ -1,313 +0,0 @@ -# HA Test-Harness Migration — Linux-native + Option C rework Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Pivot the already-implemented pytest-hacc migration on branch `test/ha-test-harness-migration` from its committed native-Windows shim to **Linux-native** test infrastructure, and scope the harness to the HA-integration tests only (**Option C**, per issue #257) via two pytest invocations — so `tests/zaptec/*` (the future-standalone API client) run as plain pytest with their live constants call intact. - -**This is a delta plan, not a from-scratch migration.** The branch (head `221406a`, off `master`) already contains the full harness migration: `requirements_test.txt` (per-Python pytest-hacc pins), the relaxed pydantic pin in `requirements.txt`, `tests/conftest.py` (mock_zaptec/setup_integration/zaptec_constants), and the behavior tests (`test_coordinator.py`, `test_entity.py` with correct #410 assertions, `test_init.py`, `test_diagnostics.py`). What this plan changes is **only** the harness activation mechanism and the test runners. Do NOT rewrite the tests or fixtures. - -**Architecture:** On Linux (CI + devcontainer) pytest-hacc autoloads via its `pytest11` entry point, so activating the harness needs no conftest machinery. Remove the committed native-Windows shim (root `conftest.py`) and the global `-p no:homeassistant` (pyproject `addopts`). Run the suite as **two invocations**: `pytest tests --ignore=tests/zaptec` (harness autoloads; integration tests use mocked client, no network) and `pytest tests/zaptec -p no:homeassistant` (harness disabled → plain pytest → live `api.zaptec.com/api/constants` call works exactly as on `master`), combining coverage with `--cov-append`. - -**Tech Stack:** Python 3.13/3.14 (CI matrix), Home Assistant 2026.4.3 (3.14) / 2026.2.3 (3.13 revert), `pytest-homeassistant-custom-component` (0.13.324 / 0.13.316 per-Python markers — already pinned, do not touch), pytest 9.x (pinned by pytest-hacc), `ruff` 0.15.22. - -## Execution environment - -**Run this plan inside the project's VS Code Dev Container (Linux).** That is where the harness autoloads natively, both invocations run, and the live constants call reaches the network. Ensure deps are installed first (`scripts/setup`, or `pip install -r requirements.txt -r requirements_test.txt`). Do **not** attempt the integration-test invocation on native Windows — that environment is intentionally no longer supported by tracked files (see spec §3). Commands below use plain `pytest` / `ruff` as available in the devcontainer. - -## Global Constraints - -- **Linux-native only.** No native-Windows accommodation may be (re)introduced into tracked files: no root `conftest.py` shim, no `pytest_plugins` force-load, no global `-p no:homeassistant`. Native-Windows local runs are handled outside the repo (uncommitted shim or devcontainer) and are out of scope here. -- **Two-invocation contract.** The harness must be active for `tests/test_*.py` and inactive for `tests/zaptec/*`. This is a per-process choice, so the suite always runs as the two invocations below. A bare `pytest` (which would collect `tests/zaptec/*` under the autoloaded harness and re-trip the socket block) is intentionally no longer the entry point. - - Integration: `pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch` - - API client: `pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append` -- **Do NOT modify** `requirements_test.txt` (per-Python pytest-hacc markers are correct), the `pydantic` range in `requirements.txt`, `tests/conftest.py` fixtures, or any `tests/test_*.py` / `tests/zaptec/test_*.py` content. This plan changes activation + runners only. -- No production code changes in `custom_components/**`. This branch is test/infra-only. Bug #410 is already handled (tests assert correct behavior, no xfail — do not reintroduce one). -- Ruff (format + check) must be clean on all changed files, pinned ruff `0.15.22`, scoped to the whole repo (`src: "."`). -- **Commit policy:** committing per task locally is pre-approved for this plan's execution (SDD auto-commit). **Pushing to any remote and opening/altering any PR requires explicit user approval** (project CLAUDE.md) — Task 5 stops for it. -- `[tool.pytest.ini_options]` also sets `pythonpath`, `testpaths=["tests"]`, `log_format`, `log_date_format`, `filterwarnings`, `asyncio_mode="auto"`, `asyncio_default_fixture_loop_scope="function"`. Preserve all of these; only the `addopts` line is removed. - ---- - -### Task 1: Go Linux-native — remove the committed shim and global plugin-disable - -**Files:** -- Delete: `conftest.py` (repo root) -- Modify: `pyproject.toml` (remove one line from `[tool.pytest.ini_options]`) - -**Interfaces:** -- Consumes: nothing (first task). -- Produces: a repo where, on Linux, pytest-hacc autoloads for `pytest tests --ignore=tests/zaptec` and is disabled by `-p no:homeassistant` for `pytest tests/zaptec`. No tracked Windows shim remains. - -- [ ] **Step 1: Delete the repo-root conftest (the Windows shim + explicit plugin load)** - -Remove the file entirely: - -```bash -git rm conftest.py -``` - -Rationale: on Linux the plugin autoloads; this file existed only to load it after a `win32` fcntl/resource/socketpair shim. It is the root cause of the session-wide harness load that Option C must avoid. - -- [ ] **Step 2: Remove the global plugin-disable from pyproject.toml** - -In `pyproject.toml`, inside `[tool.pytest.ini_options]`, delete exactly this line: - -```toml -addopts = "-p no:homeassistant" -``` - -Leave every other key in that table unchanged (`pythonpath`, `testpaths`, `log_format`, `log_date_format`, `filterwarnings`, `asyncio_mode`, `asyncio_default_fixture_loop_scope`). Do not add a replacement `addopts`. - -- [ ] **Step 3: Verify the integration invocation (harness autoloads)** - -Run: -```bash -pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch -``` -Expected: the real `hass` fixture works (harness autoloaded), and `test_coordinator.py`, `test_entity.py`, `test_init.py`, `test_diagnostics.py` all pass. If you see `fixture 'hass' not found` or `No module named 'pytest_homeassistant_custom_component'`, the harness didn't autoload — confirm `requirements_test.txt` is installed in this environment (`pip show pytest-homeassistant-custom-component`). - -- [ ] **Step 4: Verify the API-client invocation (harness disabled, plain pytest)** - -Run: -```bash -pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append -``` -Expected: `test_zconst.py` / `test_redact.py` make the live `api.zaptec.com/api/constants` call and pass (devcontainer has network); `test_utils.py` / `test_validate.py` pass; `test_api.py` login tests behave exactly as on `master` (skipped without creds, or set `SKIP_ZAPTEC_API_TEST=true` to skip them). Crucially: **no `SocketBlockedError`** — `-p no:homeassistant` turned the harness off for this run. If you see `SocketBlockedError`, the harness is still active — confirm Step 2 removed the global `-p no:homeassistant` and that you passed `-p no:homeassistant` on this command. - -- [ ] **Step 5: Confirm combined coverage did not regress** - -Run: -```bash -coverage report --include="*/coordinator.py,*/entity.py" -``` -Expected: `coordinator.py` ≥ 100%, `entity.py` ≥ 98% (the combined figure from Steps 3+4's `--cov-append`). If lower, do NOT add tests here — stop and report; a regression means the two-invocation split dropped coverage the single run had, which is a wiring problem to diagnose, not a test gap. - -- [ ] **Step 6: Ruff + commit** - -```bash -ruff format . --diff -ruff check -git add -A -git commit -m "test: drop committed Windows shim; rely on Linux pytest-hacc autoload" -``` -(If `ruff format .` reports diffs, apply `ruff format .` and re-stage.) - ---- - -### Task 2: Wire the two-invocation structure into CI (validate.yaml) - -**Files:** -- Modify: `.github/workflows/validate.yaml` (the `tests` job's "Tests suite" step, ~lines 107-109) - -**Interfaces:** -- Consumes: the Linux-native repo from Task 1. -- Produces: a CI `tests` job that runs both invocations with combined coverage, on both the 3.13 and 3.14 matrix legs. - -- [ ] **Step 1: Replace the single test step with the two invocations** - -In `.github/workflows/validate.yaml`, replace the existing step: - -```yaml - - name: Tests suite - run: | - pytest --cov=./custom_components/zaptec --cov-branch -``` - -with: - -```yaml - - name: Tests suite (HA integration — harness) - run: | - pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch - - - name: Tests suite (API client — plain pytest, no harness) - run: | - pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append -``` - -Leave the rest of the `tests` job untouched: the matrix (`["3.13", "3.14"]`), the 3.13 HA sed-revert, and the `pip install -r requirements.txt -r requirements_test.txt` step all stay. - -- [ ] **Step 2: Sanity-check the YAML** - -Run (in the devcontainer, if `python` + `pyyaml` are present): -```bash -python -c "import yaml; yaml.safe_load(open('.github/workflows/validate.yaml')); print('yaml ok')" -``` -Expected: `yaml ok`. (If pyyaml isn't available, visually confirm indentation matches the surrounding steps — two spaces under `steps:` items.) - -- [ ] **Step 3: Commit** - -```bash -git add .github/workflows/validate.yaml -git commit -m "ci: run harness + API-client tests as two scoped pytest invocations" -``` - ---- - -### Task 3: Wire scripts/test to the two-invocation structure - -**Files:** -- Modify: `scripts/test` - -**Interfaces:** -- Consumes: the Linux-native repo from Task 1. -- Produces: a `./scripts/test` that mirrors CI (two invocations, combined coverage) and still supports `--skip-api` and the html/xml coverage reports. - -- [ ] **Step 1: Update scripts/test** - -Replace the single `pytest ...` line in `scripts/test` so the file reads: - -```bash -#!/usr/bin/env bash - -set -e - -if [ "$1" == "--skip-api" ]; then - export SKIP_ZAPTEC_API_TEST="true" -fi - -# HA-integration tests run under the pytest-hacc harness (autoloads on Linux). -# API-client tests (tests/zaptec/*, the future-standalone client per #257) run -# as plain pytest with the harness disabled, so their live constants call is not -# socket-blocked. Coverage from both is combined via --cov-append. -# run tests with -s to display printouts and --log-cli-level to get logger output -pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch --log-cli-level=INFO -s -pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append --log-cli-level=INFO -s - -# generate coverage report in html and xml -coverage html -coverage xml -``` - -Keep the file executable (`git` preserves the mode; if needed `chmod +x scripts/test`). - -- [ ] **Step 2: Run it end-to-end** - -Run: -```bash -./scripts/test --skip-api -``` -Expected: both invocations run and pass (with API-login tests skipped), then `htmlcov/` and `coverage.xml` are generated. Confirm the terminal shows both invocations executing (two pytest runs), not one. - -- [ ] **Step 3: Commit** - -```bash -git add scripts/test -git commit -m "test: scripts/test runs harness + API-client invocations, combined coverage" -``` - ---- - -### Task 4: Document the split in DEVELOPMENT.md - -**Files:** -- Modify: `DEVELOPMENT.md` (the "## Running tests" section, ~line 139) - -**Interfaces:** -- Consumes: the runners from Tasks 2-3. -- Produces: contributor docs that explain the two-invocation split and why `tests/zaptec/*` are separate. - -- [ ] **Step 1: Expand the "Running tests" section** - -In `DEVELOPMENT.md`, under `## Running tests`, after the existing `./scripts/test` bullet, add an explanatory paragraph (adjust wording to match the file's voice): - -```markdown -The suite runs as **two pytest invocations**, and `./scripts/test` runs both: - -- **HA-integration tests** (`tests/test_*.py`) run under the - `pytest-homeassistant-custom-component` harness, which autoloads on Linux. - Run directly with: - `pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch` -- **API-client tests** (`tests/zaptec/*`) test the vendored `zaptec/` client, - which is destined to become a standalone PyPI library (issue #257) and has no - Home Assistant dependency. They run as plain pytest with the harness disabled - (the harness blocks non-localhost sockets, which would break their live - `api.zaptec.com/api/constants` call): - `pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append` - -Because the harness (and its socket block) is process-wide, a bare `pytest` -is not the entry point — use `./scripts/test` or the two commands above. The -HA-integration tests require Linux; run them in the Dev Container (native -Windows is not supported for that half). `tests/zaptec/*` run anywhere. -``` - -- [ ] **Step 2: Commit** - -```bash -git add DEVELOPMENT.md -git commit -m "docs: explain two-invocation test split (harness vs API-client, #257)" -``` - ---- - -### Task 5: Final gate, then push + verify CI on the fork (approval required) - -**Files:** -- Verify only (no new edits unless a gate fails). - -**Interfaces:** -- Consumes: everything from Tasks 1-4. -- Produces: a pushed, CI-green branch on the fork, ready for PR packaging (PR itself deferred to the user). - -- [ ] **Step 1: Guard — nothing outside tests/zaptec triggers the live call** - -Run: -```bash -grep -rln "zaptec_constants" tests --include="*.py" -``` -Expected: only `tests/conftest.py`, `tests/zaptec/test_zconst.py`, `tests/zaptec/test_redact.py`. If any `tests/test_*.py` (integration) requests `zaptec_constants`, it would hit the live call under the harness in invocation 1 → stop and report (the fixture would need moving to `tests/zaptec/conftest.py`). - -- [ ] **Step 2: Full local (devcontainer) gate** - -Run both invocations fresh and the linters: -```bash -pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch -pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append -coverage report --include="*/coordinator.py,*/entity.py" -ruff format . --diff -ruff check -``` -Expected: both green; coordinator.py ≥ 100%, entity.py ≥ 98%; ruff clean. - -- [ ] **Step 3: hassfest/HACS sanity (manual)** - -Confirm no shipped-component files changed on this branch: -```bash -git diff --stat master -- custom_components/ -``` -Expected: empty (the migration is test/infra-only). `requirements_test.txt`, `pyproject.toml`, `scripts/test`, `.github/`, `DEVELOPMENT.md` are dev-only and not shipped in the component. (Use the `hassfest-hacs-check` skill for the checklist; note `requirements.txt`'s pydantic-range change is a range within `manifest.json`'s supported bounds, not a manifest change.) - -- [ ] **Step 4: Push to the fork — STOP for explicit user approval first** - -Do not run this until the user approves the push (Global Constraints): -```bash -git push origin test/ha-test-harness-migration -``` -Then watch the fork's Actions run and confirm BOTH matrix legs (3.13 and 3.14) are green on both invocations. If a leg fails, diagnose against that leg's HA/pytest-hacc pairing (0.13.316↔2026.2.3 on 3.13; 0.13.324↔2026.4.3 on 3.14) before any further change. - -- [ ] **Step 5: PR packaging — user-driven, not automatic** - -Leave PR creation to the user. When they ask, the replacement PR targets `custom-components/zaptec:master`, supersedes draft #394, references #257 as the rationale for the `tests/zaptec/*` split, and notes the upstream-PR-stack dependency. No autonomous PR/issue/comment submission (AI-policy provisional compliance). - ---- - -## Self-Review - -**Spec coverage:** -- Linux-native infra, no committed shim (spec §3): Task 1 (delete root conftest + global `-p no:homeassistant`). ✓ -- Two-invocation harness scoping (spec §1): Task 1 Steps 3-4 verify; Tasks 2-3 wire CI + scripts. ✓ -- `tests/zaptec/*` unchanged, live call intact, per #257 (spec "two-audience", §2): Task 1 Step 4, Task 5 Step 1 guard. ✓ -- Combined coverage via `--cov-append` (spec §1): Tasks 1-3, verified Task 1 Step 5 / Task 5 Step 2. ✓ -- validate.yaml + scripts/test + DEVELOPMENT.md (spec §7): Tasks 2, 3, 4. ✓ -- #410 already correct, no xfail (spec §6): Global Constraints forbid reintroducing one; no task touches the tests. ✓ -- Success criteria (spec): both invocations green on Linux CI, no native-Windows in tracked files, ruff clean, hassfest/HACS unaffected — Task 5. ✓ - -**Placeholder scan:** No "TBD"/"handle later" — every step is a concrete file op, command, or exact snippet. - -**Type/consistency:** The two invocation commands are byte-identical everywhere they appear (Global Constraints, Tasks 1-3, Task 5), so CI, `scripts/test`, and the docs cannot drift. `--cov-append` is present on the second invocation and absent on the first in every occurrence. - -## Known risks carried into execution - -1. **Coverage combine.** If `coverage report` after the split shows less than the pre-split single-run numbers, the cause is almost always a missing `--cov-append` on run 2 (erasing run 1's data) or a stray `.coverage` from a prior run — check both before treating it as a real coverage gap (Task 1 Step 5). -2. **Autoload assumption.** The whole design rests on pytest-hacc autoloading on Linux. If it does not (e.g. deps not installed), Task 1 Step 3 fails fast with a clear fixture/import error — install `requirements_test.txt` and retry; do not add back a conftest force-load. -3. **`test_api.py` without creds.** Behavior must match `master` (skip without creds). If it errors instead, that is pre-existing to how `tests/zaptec` runs on `master`, not introduced here — note it, don't fix it in this plan. diff --git a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md b/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md deleted file mode 100644 index ea44db00..00000000 --- a/docs/superpowers/specs/2026-07-25-ha-test-harness-migration-design.md +++ /dev/null @@ -1,170 +0,0 @@ -# Design: Migrate coordinator/entity tests to the HA test harness - -**Date:** 2026-07-25 (revised 2026-07-26: Linux-native infra + harness scoping per #257) -**Status:** Approved (brainstorming complete) -**Scope of this spec:** the replacement for PR #394 (coordinator + entity tests). Establishes the reusable infrastructure that a later, separate PR (replacing #395, the platform-entity tests) will build on. - -## Background & motivation - -Maintainer review on PR #394 (CHANGES_REQUESTED, 2026-07-25) asked how gold/platinum HA integrations test coordinators and entities, aiming at a high-quality standard. - -The current test suite is hand-rolled: it instantiates `ZaptecUpdateCoordinator` and `ZaptecBaseEntity` directly and asserts against private methods (`# noqa: SLF001` throughout `tests/test_entity.py`), with a `MagicMock`-based fake `hass` and a `FakeConfigEntry` in `tests/conftest.py`. This is white-box, implementation-coupled testing. - -Gold/platinum HA integrations instead use `pytest-homeassistant-custom-component` (pytest-hacc): a real `hass`, `MockConfigEntry`, and tests that set the integration up through the normal `async_setup` path with the cloud API mocked, then assert on **public state** (`hass.states.get(...)`, entity/device registries), often via `syrupy` snapshot tests. - -PRs #394 and #395 have been converted to **draft** and will be replaced by PRs built on this approach. - -## The two-audience problem this spec must solve - -The test suite has two structurally different halves, and pytest-hacc changes the rules for one of them: - -1. **HA-integration tests** (`tests/test_*.py`: coordinator, entity, init, diagnostics) — these want the real `hass` harness. pytest-hacc is exactly right here. -2. **API-client tests** (`tests/zaptec/*`: `test_zconst`, `test_redact`, `test_api`, `test_utils`, `test_validate`) — these test the vendored `zaptec/` client, which per **issue #257** is a **standalone-PyPI-library-in-waiting** (sveinse: *"the API access parts will have to be a separate library on pypi… namespace `zaptec`, such as `from zaptec import Zaptec`"*). They have **no HA dependency** and must not acquire one. Two of them (`test_zconst`, `test_redact`) make a **live** call to `api.zaptec.com/api/constants`. - -pytest-hacc blocks non-localhost sockets **unconditionally** on every test in any process where its plugin is active (verified empirically: `disable_socket()` + a `127.0.0.1` allow-list run in `pytest_runtest_setup` before every test; neither the `enable_socket` marker nor a manual `pytest_socket.enable_socket()` defeats the host allow-list). So the moment the harness is active in a process, the live constants call raises `SocketBlockedError`. - -Interweaving the two — running `tests/zaptec/*` under the harness — both couples the future-standalone library to HA (against #257) and breaks its live call. The design keeps them **separated**: the harness governs only the HA-integration tests; the API-client tests run as plain pytest, exactly as today. - -## Goals - -- Bring the coordinator + entity tests to gold/platinum shape: behavior-first, through the real HA harness. -- Establish reusable test infrastructure (`mock_zaptec` + `setup_integration`) that the #395 replacement reuses without re-solving anything. -- Match or beat current coverage on `coordinator.py` / `entity.py` (100% / 98%) — but via observable behavior, not private-method assertions. -- Keep the `tests/zaptec/*` API-client tests running exactly as today (plain pytest, live constants call intact), per #257. -- Shipped test infra is **Linux-native**: it matches CI and the maintainers' devcontainer, and carries **no native-Windows accommodation in tracked files**. - -## Non-goals (out of scope for this spec) - -- The six platform files (`sensor/switch/number/button/binary_sensor/update`) — that is the #395 replacement, a separate PR. -- Config-flow / `__init__` coverage. -- Snapshot tests (deferred to the #395 replacement, where full-state snapshots pay off). -- Fixing bug #410 (this PR stays test-only; see "Bug #410" below). -- An offline constants snapshot. The shelved `fix/constants-snapshot-fixture` design existed only to dodge the socket block; Option C avoids the block entirely by never running those tests under the harness, so the snapshot is unnecessary. - -## Approach (selected): Linux-native harness, scoped to the integration tests - -Adopt pytest-hacc, patch the integration at the `Zaptec` client boundary, and assert on public state — but **scope the harness to the HA-integration tests only** ("Option C"), so `tests/zaptec/*` stays plain pytest. - -The scoping mechanism is deliberately minimal and standard: on Linux, pytest-hacc autoloads via its normal `pytest11` entry point, so the HA-integration run needs **no conftest machinery** to activate it. The API-client run disables it with a single, per-invocation `-p no:homeassistant`. Two pytest invocations, nothing more. - -Rejected alternatives: -- **(B) like-for-like fixture swap / (C-stay) hand-rolled mocks** — don't reach the target standard; keep the white-box coupling the review flagged. -- **Snapshot-under-harness** — runs `tests/zaptec/*` under the harness and dodges the socket block with a committed offline snapshot; keeps the two concerns interwoven (against #257) and adds a fixture to maintain. -- **Per-test socket opt-out** — empirically does not defeat pytest-hacc's `127.0.0.1` allow-list. -- **Committed native-Windows shim** — see "Why the shim is not committed" below. - -## Design - -### 1. Harness scope: two pytest invocations - -The harness must be active for `tests/test_*.py` and inactive for `tests/zaptec/*`. Because pytest-hacc's plugin (and its socket block) is process-wide, this is a **per-process** choice — one pytest run cannot have the harness on for some tests and off for others. So the suite runs as **two invocations**: - -```bash -# 1. HA-integration tests — harness autoloads (Linux pytest11 entry point) -pytest tests --ignore=tests/zaptec --cov=custom_components/zaptec --cov-branch - -# 2. API-client tests — harness disabled → plain pytest → live constants works -pytest tests/zaptec -p no:homeassistant --cov=custom_components/zaptec --cov-branch --cov-append -``` - -- `-p no:homeassistant` disables pytest-hacc's autoloaded plugin for run 2 only, so there is no socket block and the live `api.zaptec.com/api/constants` call behaves exactly as today. -- `--cov-append` on run 2 merges the two runs' coverage into one report, preserving the combined `coordinator.py`/`entity.py` numbers. -- No global `-p no:homeassistant` and no root `conftest.py` `pytest_plugins` line — committing either would disable autoload for run 1 and defeat the harness. Scoping lives entirely in the two commands. - -### 2. Test infrastructure (the foundation) - -- **`requirements_test.txt`** — add `pytest-homeassistant-custom-component` pinned **per-Python via environment markers** (`==0.13.324` for `python_version >= "3.14"`, `==0.13.316` for `< "3.14"`). Do NOT add `homeassistant` here: it is already pinned in `requirements.txt`, and validate.yaml sed-reverts it to `2026.2.3` on the 3.13 leg. pytest-hacc pins an exact `homeassistant==`, so its version MUST match the HA of each Python leg — 0.13.324↔2026.4.3 (py≥3.14), 0.13.316↔2026.2.3 (py≥3.13). Leaving it unpinned makes pip backtrack to an ancient release (pytest 6.2.2 → crashes on 3.13); single-pinning the newest is uninstallable on 3.13. -- **`requirements.txt`** — relax the `pydantic` pin from an exact `==` to the manifest's supported range (`>=2.11.7,<2.14`) so pytest-hacc's transitive `pydantic==2.12.2` resolves alongside it. The devcontainer's `scripts/setup` installs both `requirements.txt` and `requirements_test.txt`, so they must co-resolve. -- **No committed root `conftest.py` for the harness.** On Linux (CI + devcontainer) the plugin autoloads; no shim, no `pytest_plugins`, no global `-p no:homeassistant`. (See "Why the shim is not committed.") -- **`tests/conftest.py`** — replace the hand-rolled `hass` / `FakeConfigEntry` with: - - the harness's real `hass` fixture (available via autoload; no import needed in the conftest), - - a **`mock_zaptec`** fixture: `MagicMock(spec=Zaptec)` pre-populated with a representative installation + charger object graph (and, because `Zaptec` is a `Mapping[str, ZaptecBase]`, implementing `__getitem__` / `__iter__` / `values()` to yield the fake `Charger` / `Installation` objects the platforms enumerate), - - a **`mock_config_entry`** (`MockConfigEntry`) and a **`setup_integration(hass, mock_zaptec)`** helper that patches the client into the setup path and awaits `async_setup`. - - the existing **`zaptec_constants`** fixture stays as-is (live call). It is only requested by `tests/zaptec/test_zconst.py` / `test_redact.py`, which run in invocation 2 (no harness → no socket block). It is never triggered in invocation 1 (that run `--ignore`s `tests/zaptec`), so it needs no socket guard. The event-loop save/restore added earlier stays (it protects the async fetch regardless of harness). - -### 3. Why the shim is not committed (native-Windows is a local-only concern) - -Home Assistant imports `fcntl` (Unix-only), so pytest-hacc's plugin cannot autoload on native Windows. Earlier iterations of this migration carried a `win32`-guarded shim (fcntl/resource/socketpair stubs) in a root `conftest.py`, plus a `pytest_plugins` line and a global `-p no:homeassistant`, purely so the maintainer's — and this assistant's — native-Windows environment could run the integration tests. - -That machinery is **not committed**, for three reasons: - -1. **It contradicts the maintainers' stated workflow.** They promote the devcontainer and have pushed back on native-Windows accommodation (steinmn on #398); a standalone committed Windows shim (PR #403) was already **closed**. -2. **It is the root cause of the scoping complexity.** The shim must run before pytest-hacc imports `fcntl`, which forces a root-conftest `pytest_plugins` + global `-p no:homeassistant` and thus a **session-wide** harness load — which is exactly what makes scoping `tests/zaptec/*` away from the harness hard. Dropping the shim lets Linux autoload the plugin, so scoping collapses to one per-invocation `-p no:homeassistant` (§1). -3. **CI and the devcontainer are both Linux**, so nothing shipped needs the shim. - -**Local native-Windows runs** (this assistant's environment, and any contributor on native Windows) are handled outside tracked files: -- `tests/zaptec/*` already run natively today: `pytest tests/zaptec -p no:homeassistant` (this is the existing convention; the harness is off, `fcntl` is never imported). -- The **HA-integration tests** need Linux: run them in the **devcontainer** (what the maintainers promote), or, for a quick local check, under an **uncommitted, untracked** local shim conftest. Neither path ships. - -### 4. The #394-replacement tests (coordinator + entity, behavior-first) - -Same two modules, asserted through the real harness instead of poking privates. - -- **Backbone / setup test** — set up the integration via `setup_integration`; assert entities land in the state machine and registry. This exercises `entity.py`'s `__init__` / `unique_id` / `device_info` wiring as a side effect, with no direct instantiation. -- **Coordinator behavior:** - - Successful refresh → entities have expected states after `coordinator.async_refresh()`. - - Failed refresh (mock client raises) → `coordinator.last_update_success` False → entities report `unavailable` via `hass.states.get()`. - - Poll scheduling (`trigger_poll`, charging-interval switch) → driven via public methods and the harness's time control (`freezer` / `async_fire_time_changed`), not a hand-attached loop. -- **Entity behavior:** - - Value present → `hass.states.get("sensor.…").state` equals the expected transformed value (covers `_get_zaptec_value`, dotted keys, lowercasing through a real entity). - - Key missing / non-mapping object → assert actual reported availability + no crash propagation. - - Availability transition logging — kept, asserted on observable state where possible. - -**Assertion style:** move from `entity._attr_available is False # noqa: SLF001` to `hass.states.get("sensor.x").state == "unavailable"`. A **small residue** of white-box tests is acceptable for pure-logging helpers (e.g. `_log_value` dedup) that have no observable state effect — kept to a minimum. - -**Coverage target:** match or beat current 100% / 98% on `coordinator.py` / `entity.py`, achieved via behavior. - -### 5. The mocked Zaptec client & shared test data - -**Patch at the `Zaptec` client boundary (Layer 2), not the HTTP/SignalR wire (Layer 1).** - -The integration builds the client in `__init__.py` (`zaptec = Zaptec(...)`, then `.login()`, `ZaptecManager.first_time_setup(zaptec=...)`, `ZaptecManager(..., zaptec=...)`). We patch the `Zaptec` symbol where `__init__.py` uses it so construction returns `mock_zaptec`: - -```python -with patch("custom_components.zaptec.Zaptec", return_value=mock_zaptec): - await hass.config_entries.async_setup(entry.entry_id) -``` - -Everything *above* the client — `ZaptecManager`, `ZaptecUpdateCoordinator`, `ZaptecBaseEntity`, all platforms — runs as real code against the mock's data. - -- **Layer 1 (HTTP wire) rejected:** would additionally exercise `api.py`, but couples every setup test to the cloud's JSON/SignalR format (login, installations, chargers, constants, state polls, SignalR handshake) — large and brittle. `api.py` already has dedicated tests in `tests/zaptec/test_api.py`, so re-testing it through the integration adds fragility for no coverage gain. -- **Layer 3 (mock the manager) rejected:** too high; would stop exercising the coordinator/entity code under test. - -**`spec=` discipline:** `MagicMock(spec=Zaptec)` / `spec=Charger` / `spec=Installation` so a typo'd or renamed client method fails loudly instead of returning a fresh mock. Carried over as a deliberate strength of the current tests. - -**Test-data source:** a small hand-authored dict suffices for #394 (coordinator/entity base behavior needs only a couple of keys). The **fuller** payload needed by the #395 replacement will be seeded from a **redacted real diagnostics dump** (the repo already has `diagnostics.py` + `redact.py`), stored as a JSON fixture, so snapshots reflect real-world data rather than invented values. - -### 6. Bug #410 handling — test-only, deferred fix - -Filed as custom-components/zaptec#410: `ZaptecBaseEntity` sets `_attr_available = False` on `KeyUnavailableError` but never overrides `available`. Contributor steinmn responded that this is **not a bug**, and the exact mechanism was confirmed against installed HA (2026.4.3): `ZaptecBaseEntity` subclasses `CoordinatorEntity`, whose `available` property is first in the MRO and returns `self.coordinator.last_update_success` — it never reads `_attr_available`. So a single missing key does not (and by design should not) take an entity `unavailable`; only a failed coordinator poll does. (steinmn's comment cited the *base* `Entity.available` reading `_attr_available`, but that base property is overridden by `CoordinatorEntity.available`, so it doesn't govern these entities — the conclusion holds via the override.) steinmn also noted the HA entity-vs-device distinction: one entity going unavailable would not make its charger/installation *device* unavailable. - -**Status:** the reporter (rhammen) has concluded #410 is not a bug and intends to close it; it is currently still open. The earlier concern that `_attr_available` is "never reset to True on success" is also refuted: each derived platform's `_update_from_zaptec` sets it back to `True` on a good update (e.g. `binary_sensor.py:34`). - -**Decision:** this PR stays **test-only** and asserts the **observed, correct** behavior (no xfail — the observed behavior is definitive regardless of the still-open semantic discussion): an entity with a single missing key stays available; an entity is `unavailable` only when the coordinator poll fails. (Reworked from the earlier xfail-encoded premise; see commit 0ecebab.) - -### 7. CI + scripts wiring - -- **`.github/workflows/validate.yaml`** — the test job installs `requirements.txt` + `requirements_test.txt` (co-resolving per §2) and runs the **two** pytest invocations (§1), preserving the existing 3.13 HA sed-revert. Coverage combines via `--cov-append`. -- **`scripts/test`** — mirror the two-invocation structure so local (Linux/devcontainer) runs match CI. Keep the `--skip-api` path (`SKIP_ZAPTEC_API_TEST=true`) working for invocation 2's login-gated tests. -- **`DEVELOPMENT.md`** — document the two-invocation split and that the HA-integration tests require Linux (devcontainer); `tests/zaptec/*` run natively with `-p no:homeassistant`. - -## Success criteria (this PR) - -- Coverage on `coordinator.py` / `entity.py` ≥ current (100% / 98%), achieved via behavior. -- `pytest tests --ignore=tests/zaptec` (harness) **and** `pytest tests/zaptec -p no:homeassistant` (plain) both green on Linux CI; combined coverage via `--cov-append`. -- `tests/zaptec/*` behavior unchanged — live constants call still runs, no socket block, no HA import. -- No native-Windows accommodation in tracked files (Linux-native infra). -- `ruff format` + `ruff check` clean. -- hassfest / HACS unaffected (`requirements_test.txt` is not shipped in the component; no root `conftest.py` / pytest-config changes are shipped for the harness). - -## PR / branch strategy - -- #394 and #395 held as draft (done 2026-07-25); reply posted on #394 explaining the direction. -- Migration lands on `test/ha-test-harness-migration` (already off `master`); it replaces #394. The #395 replacement is a **separate, later** PR reusing this infrastructure. -- Reference #257 in the PR body as the rationale for keeping `tests/zaptec/*` out of the harness. Note the upstream-PR-stack dependency (see [[upstream-pr-stack]]). - -## Open items carried into planning - -- Confirm the two-invocation coverage numbers combine correctly under `--cov-append` (branch coverage merges). -- Confirm `pyproject.toml`'s existing `[tool.pytest.ini_options]` has no global option that conflicts with the per-invocation `-p no:homeassistant` (e.g. no committed `addopts` that force-loads or force-disables the plugin). -- Confirm exact patch target (`custom_components.zaptec.Zaptec` import site) and the minimal `mock_zaptec` object-graph shape for #394. From 946490868e4f384ac827b5366e0cb550c3b4bfc6 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:05:59 +0200 Subject: [PATCH 26/29] test: drop in-code issue references flagged in PR #414 review sveinse noted this code may move to other repos where the referenced issue numbers become meaningless; drop them from comments/docstrings while keeping the technical explanation. Co-Authored-By: Claude Sonnet 5 --- scripts/test | 6 +++--- tests/test_entity.py | 6 +++--- tests/test_init.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/test b/scripts/test index 8017ebd1..58923e90 100755 --- a/scripts/test +++ b/scripts/test @@ -7,9 +7,9 @@ if [ "$1" == "--skip-api" ]; then fi # HA-integration tests run under the pytest-hacc harness (autoloads on Linux). -# API-client tests (tests/zaptec/*, the future-standalone client per #257) run -# as plain pytest with the harness disabled, so their live constants call is not -# socket-blocked. Coverage from both is combined via --cov-append. +# API-client tests (tests/zaptec/*) run as plain pytest with the harness +# disabled, so their live constants call is not socket-blocked. Coverage +# from both is combined via --cov-append. # run tests with -s to display printouts and --log-cli-level to get logger output pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch --log-cli-level=INFO -s pytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append --log-cli-level=INFO -s diff --git a/tests/test_entity.py b/tests/test_entity.py index dcf47ed5..223d6879 100644 --- a/tests/test_entity.py +++ b/tests/test_entity.py @@ -85,7 +85,7 @@ async def test_entity_stays_available_when_single_key_missing( mock_zaptec: MagicMock, enable_custom_integrations: None, ) -> None: - """A single missing backing key does NOT mark the entity unavailable (#410 is not a bug). + """A single missing backing key does NOT mark the entity unavailable. `ZaptecBaseEntity` extends `CoordinatorEntity`, whose `available` property is driven solely by `coordinator.last_update_success` and never reads @@ -254,8 +254,8 @@ async def test_log_unavailable_logs_error_and_recovery_transitions( """_log_unavailable logs the real exception on going unavailable, and logs recovery. Its transition logging is driven purely by `_attr_available`/`_prev_available` - — which, per #410, are decoupled from the entity's actual HA-reported - availability (`CoordinatorEntity.available` reads `coordinator.last_update_success`, + — which are decoupled from the entity's actual HA-reported availability + (`CoordinatorEntity.available` reads `coordinator.last_update_success`, never these). So there's no realistic way to drive both log transitions through a real coordinator refresh; setting the attributes directly is the only way to exercise this logging branch in isolation. diff --git a/tests/test_init.py b/tests/test_init.py index 1e3a7260..968d671f 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -27,7 +27,7 @@ # Connection/timeout are recoverable -> HA retries setup. (RequestTimeoutError("slow"), ConfigEntryNotReady), (RequestConnectionError("down"), ConfigEntryNotReady), - # Transient server statuses are recoverable -> HA retries setup (issue #392). + # Transient server statuses are recoverable -> HA retries setup. (RequestError("unavailable", HTTPStatus.SERVICE_UNAVAILABLE), ConfigEntryNotReady), (RequestError("too many", HTTPStatus.TOO_MANY_REQUESTS), ConfigEntryNotReady), # Other HTTP errors stay permanent. From e3a7c6872918157ebc7f1a9d70d09855bc531950 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:52:47 +0200 Subject: [PATCH 27/29] test: split zaptec_constants into tests/zaptec/conftest.py sveinse asked for setup specific to the future-standalone API client (#414 review) to live under tests/zaptec/, separate from the HA integration test setup. zaptec_username/zaptec_password stay in the root conftest since tests/test_diagnostics.py (outside tests/zaptec, not yet converted to the new pattern) still needs them. Co-Authored-By: Claude Sonnet 5 --- tests/conftest.py | 36 ---------------------------------- tests/zaptec/conftest.py | 42 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 36 deletions(-) create mode 100644 tests/zaptec/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py index a47dcd6f..1aba5761 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,5 @@ """Zaptec testing configuration file.""" -import asyncio from collections.abc import Callable, Iterable import os from typing import Any @@ -64,41 +63,6 @@ def zaptec_password(skip_if_user_disabled_api_tests, skip_if_in_github_actions) return password -@pytest.fixture(scope="session") -def zaptec_constants() -> dict: - """Get latest constants from Zaptec API. - - Uses a self-contained event loop instead of `asyncio.run()`. Under - pytest-homeassistant-custom-component's `HassEventLoopPolicy`, - `asyncio.run()` unconditionally resets the thread's registered event - loop to `None` on exit (success or failure) via `asyncio.set_event_loop`. - That policy raises `RuntimeError` from `get_event_loop()` instead of - lazily creating one, so the next bare `asyncio_mode=auto` test in the - session would fail in its autouse loop-setup fixture. Saving/restoring - the previous loop here keeps this fixture from clobbering global - event-loop state for tests that run after it. - """ - - async def get_zaptec_constants() -> dict: - async with Zaptec("N/A", "N/A") as zaptec: - # the constants API endpoint does not require login - const: dict = await zaptec.request("constants") - return const - - try: - previous_loop = asyncio.get_event_loop() - except RuntimeError: - previous_loop = None - - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) - try: - return loop.run_until_complete(get_zaptec_constants()) - finally: - loop.close() - asyncio.set_event_loop(previous_loop) - - def _backed_get(data: dict[str, Any]) -> Callable[..., Any]: """Return a `.get(key, default=MISSING)` implementation backed by `data`. diff --git a/tests/zaptec/conftest.py b/tests/zaptec/conftest.py new file mode 100644 index 00000000..595de04a --- /dev/null +++ b/tests/zaptec/conftest.py @@ -0,0 +1,42 @@ +"""Test configuration for the vendored Zaptec API client (tests/zaptec/*).""" + +import asyncio + +import pytest + +from custom_components.zaptec.zaptec.api import Zaptec + + +@pytest.fixture(scope="session") +def zaptec_constants() -> dict: + """Get latest constants from Zaptec API. + + Uses a self-contained event loop instead of `asyncio.run()`. Under + pytest-homeassistant-custom-component's `HassEventLoopPolicy`, + `asyncio.run()` unconditionally resets the thread's registered event + loop to `None` on exit (success or failure) via `asyncio.set_event_loop`. + That policy raises `RuntimeError` from `get_event_loop()` instead of + lazily creating one, so the next bare `asyncio_mode=auto` test in the + session would fail in its autouse loop-setup fixture. Saving/restoring + the previous loop here keeps this fixture from clobbering global + event-loop state for tests that run after it. + """ + + async def get_zaptec_constants() -> dict: + async with Zaptec("N/A", "N/A") as zaptec: + # the constants API endpoint does not require login + const: dict = await zaptec.request("constants") + return const + + try: + previous_loop = asyncio.get_event_loop() + except RuntimeError: + previous_loop = None + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + try: + return loop.run_until_complete(get_zaptec_constants()) + finally: + loop.close() + asyncio.set_event_loop(previous_loop) From 01d997d9edebd3e4b5b2a14b5d2af1bf9014f6c9 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:55:34 +0200 Subject: [PATCH 28/29] test: trim redundant comment block in requirements_test.txt steinmn noted the per-Python pytest-hacc pins speak for themselves (#414 review). Co-Authored-By: Claude Sonnet 5 --- requirements_test.txt | 8 -------- 1 file changed, 8 deletions(-) diff --git a/requirements_test.txt b/requirements_test.txt index b1cc0088..be62f2c4 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -2,13 +2,5 @@ pytest pytest-asyncio pytest-mock pytest-cov -# pytest-homeassistant-custom-component pins an EXACT `homeassistant==`, so the -# release must match the HA that requirements.txt installs on each CI Python leg -# (validate.yaml sed-reverts HA to 2026.2.3 on 3.13). Select the matching release -# per Python version: newest releases require Python >=3.14, so 3.13 uses the last -# 3.13-compatible one. 0.13.324 -> HA 2026.4.3 (py>=3.14); 0.13.316 -> HA 2026.2.3. -# It also brings the full version-matched pytest stack, and pins pydantic to HA's -# version (2.12.2), which is why requirements.txt uses a pydantic RANGE (matching -# manifest.json) rather than an exact pin that would conflict. pytest-homeassistant-custom-component==0.13.324; python_version >= "3.14" pytest-homeassistant-custom-component==0.13.316; python_version < "3.14" From 55a5ed1b6d6bcb9ad949909159c88f00a6227349 Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:08:40 +0200 Subject: [PATCH 29/29] test: trim wordy docstrings/comments flagged in PR #414 review sveinse noted the AI-assisted docstrings were much wordier than a developer would write. Cut each to the non-obvious "why", dropping restated "what" and secondary asides. Co-Authored-By: Claude Sonnet 5 --- tests/conftest.py | 44 ++++++------------- tests/test_coordinator.py | 43 ++++++++---------- tests/test_entity.py | 92 +++++++++++++-------------------------- tests/test_init.py | 6 +-- tests/zaptec/conftest.py | 13 ++---- 5 files changed, 66 insertions(+), 132 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 1aba5761..26d41b8b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -33,12 +33,7 @@ def skip_if_user_disabled_api_tests() -> None: @pytest.fixture(scope="session") def zaptec_username(skip_if_user_disabled_api_tests, skip_if_in_github_actions) -> str: # noqa: ANN001 (the inputs are purely to create dependencies to the env-flags above) - """ - Get the zaptec username stored in env. - - Any test relying on this fixture will be skipped if the test is running - in Gihub Actions, or the user has disabled tests requiring API login. - """ + """Get the zaptec username from env, skipping if API-login tests are disabled.""" username = os.environ.get("ZAPTEC_USERNAME") assert username, ( "Missing username, either set it with \"export ZAPTEC_USERNAME='username'\" " @@ -49,12 +44,7 @@ def zaptec_username(skip_if_user_disabled_api_tests, skip_if_in_github_actions) @pytest.fixture(scope="session") def zaptec_password(skip_if_user_disabled_api_tests, skip_if_in_github_actions) -> str: # noqa: ANN001 - """ - Get the zaptec password stored in env. - - Any test relying on this fixture will be skipped if the test is running - in Gihub Actions, or the user has disabled tests requiring API login. - """ + """Get the zaptec password from env, skipping if API-login tests are disabled.""" password = os.environ.get("ZAPTEC_PASSWORD") assert password, ( "Missing password, either set it with \"export ZAPTEC_PASSWORD='password'\" " @@ -66,12 +56,9 @@ def zaptec_password(skip_if_user_disabled_api_tests, skip_if_in_github_actions) def _backed_get(data: dict[str, Any]) -> Callable[..., Any]: """Return a `.get(key, default=MISSING)` implementation backed by `data`. - Mirrors `ZaptecBase.__getitem__`'s key normalization (`to_under`) so lookups - behave the same whether `data` is hand-authored snake_case or seeded from a - raw API payload. Still diverges from `ZaptecBase.get` (inherited from - `Mapping.get`) in its own default: `MISSING` instead of `None`. Harmless in - practice, since every real call site (`entity.py`'s `_get_zaptec_value`) - always passes `default=MISSING` explicitly. + Mirrors `ZaptecBase.__getitem__`'s key normalization (`to_under`); defaults + to `MISSING` rather than `Mapping.get`'s `None` since every real call site + (`entity.py`'s `_get_zaptec_value`) passes `default=MISSING` explicitly. """ def _get(key: str, default: Any = MISSING) -> Any: @@ -85,10 +72,8 @@ def make_charger( ) -> MagicMock: """Build a spec'd Charger double backed by `data`. - `model` is hardcoded to the base `ZaptecBase.model`'s default format - (`f"Zaptec {qualname}"`, api.py) — it does NOT model `Charger.model`'s real - override, which looks up a device-ID-prefix in `ZCONST.serial_to_model`. A - known, deliberate simplification, same category as `_backed_get`'s divergences. + `model` is hardcoded rather than modeling `Charger.model`'s real + `ZCONST.serial_to_model` lookup — a deliberate simplification. """ charger = MagicMock(spec=Charger) charger.id = data["id"] @@ -119,10 +104,9 @@ def make_installation(data: dict[str, Any], *, chargers: Iterable[MagicMock] = ( def mock_zaptec() -> MagicMock: """A spec'd Zaptec client seeded with one installation and one charger. - The `__getitem__`/`__iter__`/`__contains__`/`__len__` wiring isn't arbitrary - mock scaffolding: `Zaptec` is itself `Mapping[str, ZaptecBase]` in production - (api.py), and real code indexes into it directly (e.g. `zaptec[deviceid]` in - `__init__.py`/`coordinator.py`). + `__getitem__`/`__iter__`/`__contains__`/`__len__` are wired because `Zaptec` + is itself `Mapping[str, ZaptecBase]` in production, and real code (e.g. + `zaptec[deviceid]` in `__init__.py`/`coordinator.py`) indexes into it directly. """ installation = make_installation({"id": "inst-mock-1", "name": "Mock Home"}) charger = make_charger( @@ -176,11 +160,9 @@ async def setup_integration( ) -> ZaptecManager: """Set the integration up through the real async_setup, with a mocked client. - Patches `custom_components.zaptec.Zaptec`, not `custom_components.zaptec.zaptec. - api.Zaptec` where the class is defined — the standard unittest.mock rule is to - patch where a name is *looked up*, not where it's *defined*. `__init__.py` does - `from .zaptec import Zaptec`, so it holds its own local reference; patching the - original definition would silently do nothing here. + Patches `custom_components.zaptec.Zaptec` — where `__init__.py` looks the name + up, per unittest.mock's patch-at-the-lookup rule — not the original definition + in `zaptec/api.py`, which `__init__.py`'s own import wouldn't see patched. """ mock_config_entry.add_to_hass(hass) with patch("custom_components.zaptec.Zaptec", return_value=mock_zaptec): diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 17a9b9cb..fdb74b39 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -62,10 +62,8 @@ async def test_device_coordinator_switches_interval_when_charging( charger_coord.set_update_interval() assert charger_coord.update_interval == timedelta(seconds=ZAPTEC_POLL_INTERVAL_CHARGING) - # Also assert the relation directly: charging must poll faster than idle. This - # catches a regression that equality checks alone would miss, e.g. const.py - # setting ZAPTEC_POLL_INTERVAL_CHARGING >= ZAPTEC_POLL_INTERVAL_IDLE, which would - # still pass both equality asserts above despite breaking the whole feature. + # Also assert the relation directly, catching e.g. const.py setting + # CHARGING >= IDLE, which the equality asserts alone would miss. assert charger_coord.update_interval < idle_interval @@ -75,12 +73,9 @@ async def test_charging_update_interval_requires_charger_object( ) -> None: """Constructing a coordinator with a charging interval on a non-Charger object errors. - Constructs `ZaptecUpdateCoordinator` directly (skipping `setup_integration`) - to hit this constructor-time guard in isolation. A bare, unconfigured - `manager=MagicMock()` is enough: `__init__` does `self.zaptec = manager.zaptec` - (auto-vivifies on a MagicMock, no error) before the `isinstance(zaptec_object, - Charger)` check runs, so nothing about the manager's real behavior is - exercised before the ValueError fires. + Skips `setup_integration` to hit the constructor-time guard directly: a + bare `manager=MagicMock()` auto-vivifies `self.zaptec` with no error before + the `isinstance(zaptec_object, Charger)` check runs. """ mock_config_entry.add_to_hass(hass) @@ -108,10 +103,9 @@ async def test_trigger_poll_is_noop_without_zaptec_object( ) -> None: """trigger_poll() on a coordinator with no bound zaptec object does nothing. - `manager.head_coordinator` specifically: it's the one coordinator built with - `zaptec_object=None` (the account-wide coordinator; __init__.py), unlike every - device coordinator, which always gets a real Charger/Installation. That's what - satisfies trigger_poll()'s `if zaptec_obj is None: return` no-op guard. + `head_coordinator` is the one coordinator built with `zaptec_object=None` + (device coordinators always get a real Charger/Installation), satisfying + trigger_poll()'s no-op guard. """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) @@ -131,15 +125,14 @@ async def test_trigger_poll_cancels_in_flight_task_and_reschedules( manager = await setup_integration(hass, mock_config_entry, mock_zaptec) charger_coord = manager.device_coordinators["chg-mock-1"] - # Collapse the real multi-second delays to zero so the poll sequence runs fast, while - # still going through real asyncio.sleep(0) checkpoints (needed so the eagerly-started - # background task actually suspends and can be observed/cancelled mid-flight). + # Collapse the real delays to zero, keeping real asyncio.sleep(0) checkpoints so + # the eagerly-started background task actually suspends and can be cancelled mid-flight. monkeypatch.setattr( "custom_components.zaptec.coordinator.ZAPTEC_POLL_CHARGER_TRIGGER_DELAYS", [0, 0, 0] ) - # HA's eager task factory starts the background task running immediately; it - # suspends at the first real `asyncio.sleep(0)` checkpoint and is left pending. + # HA's eager task factory runs the task immediately; it suspends at the + # first sleep(0) checkpoint and is left pending. await charger_coord.trigger_poll() first_task = charger_coord._trigger_task # noqa: SLF001 assert first_task is not None @@ -167,13 +160,11 @@ async def test_trigger_poll_triggers_child_charger_coordinators( ) -> None: """Polling an installation also triggers the poll sequence of its tracked chargers. - Patches `asyncio.sleep` globally rather than a delays-list constant (as the - cancel/reschedule test does) because installations use a different constant - (ZAPTEC_POLL_INSTALLATION_TRIGGER_DELAYS) whose values this test doesn't care - about — it only needs to reach the loop's first iteration fast, since - children are triggered at `i == 1` (coordinator.py). `charger_coord.trigger_poll` - is replaced with a mock rather than left real, to isolate "did the parent call - the child" from "does the child's own trigger_poll work" (covered elsewhere). + Patches `asyncio.sleep` globally, rather than the delays-list constant (as the + cancel/reschedule test does), just to reach the loop's first iteration fast — + installations use their own delay constant this test doesn't care about. + `charger_coord.trigger_poll` is mocked to isolate "parent calls child" from + the child's own trigger_poll logic (covered elsewhere). """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) install_coord = manager.device_coordinators["inst-mock-1"] diff --git a/tests/test_entity.py b/tests/test_entity.py index 223d6879..cf7da4e9 100644 --- a/tests/test_entity.py +++ b/tests/test_entity.py @@ -19,24 +19,14 @@ def _entity_from_coordinator( ) -> ZaptecBaseEntity: """Return a real entity instance bound to `coordinator`. - There's no public API to list "the entities subscribed to this coordinator" — - entities are owned by the entity platform/registry, not the coordinator. So - this reaches into the coordinator's private `_listeners` dict ({id: (callback, - context)}, populated by every `async_add_listener` call) and reads `cb.__self__` - off each bound-method callback to get back the object it belongs to. - - `_listeners` isn't only entities: `ZaptecUpdateCoordinator.__init__` also - registers its own `set_update_interval` as a listener for charger coordinators - (coordinator.py). `hasattr(candidate, "_log_value")` filters that out — - `_log_value` is defined only on `ZaptecBaseEntity`, never on the coordinator, - so it reliably distinguishes "an entity" from "the coordinator itself." - - `key_not_in_skip_list=True` additionally skips any entity whose `.key` is in - `KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK`. That set gates one specific log line - in `ZaptecBaseEntity._log_unavailable` (`"Getting value failed"`, suppressed - for skip-listed keys) — a test asserting that line appears needs an entity - NOT on the skip list, since which entity this function returns otherwise - depends on `_listeners`' iteration order, not anything the test controls. + Reaches into the private `_listeners` dict since there's no public way to + list entities subscribed to a coordinator. `hasattr(candidate, "_log_value")` + filters out non-entity listeners (e.g. the coordinator's own + `set_update_interval`, also registered as a listener). + + `key_not_in_skip_list=True` skips entities whose `.key` is in + `KEYS_TO_SKIP_ENTITY_AVAILABILITY_CHECK`, needed when asserting on the + "Getting value failed" log line those keys suppress. """ for cb, _context in coordinator._listeners.values(): # noqa: SLF001 candidate = cb.__self__ @@ -51,12 +41,8 @@ def _entity_from_coordinator( async def _get_zaptec_entity(hass: HomeAssistant) -> str: """Return one live zaptec entity_id whose value is backed by seeded data. - Not every zaptec entity reads a key that `mock_zaptec` seeds (e.g. the - 3-to-1-phase-switch-current number entity has no backing value and stays - "unknown"), and platform setup order is not guaranteed to surface a - backed entity first. Skip past unbacked entities to find one that - actually resolved a value, so the test exercises real value surfacing - rather than an incidental "unknown" state. + Not every zaptec entity reads a key `mock_zaptec` seeds, and setup order + isn't guaranteed to surface a backed one first — skip unbacked entities. """ for state in hass.states.async_all(): if state.entity_id.startswith( @@ -87,12 +73,10 @@ async def test_entity_stays_available_when_single_key_missing( ) -> None: """A single missing backing key does NOT mark the entity unavailable. - `ZaptecBaseEntity` extends `CoordinatorEntity`, whose `available` property is - driven solely by `coordinator.last_update_success` and never reads - `_attr_available`. When `_update_from_zaptec` raises `KeyUnavailableError`, - `_handle_coordinator_update` catches it and the coordinator's poll still - succeeds, so the entity stays available and simply retains its previous - value/state. + `CoordinatorEntity.available` is driven by `coordinator.last_update_success`, + not `_attr_available` — `_handle_coordinator_update` catches the + `KeyUnavailableError` from `_update_from_zaptec`, so the poll still succeeds + and the entity keeps its previous value/state. """ await setup_integration(hass, mock_config_entry, mock_zaptec) entity_id = await _get_zaptec_entity(hass) @@ -120,8 +104,8 @@ async def test_entity_unavailable_when_coordinator_poll_fails( ) -> None: """The entity reports 'unavailable' when its coordinator's poll fails. - This is the actual mechanism behind entity availability: `CoordinatorEntity.available` - reflects `coordinator.last_update_success`, not any per-key state. + `CoordinatorEntity.available` reflects `coordinator.last_update_success`, + not any per-key state. """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) entity_id = await _get_zaptec_entity(hass) @@ -143,11 +127,8 @@ async def test_log_value_logs_on_change_then_skips_when_unchanged( ) -> None: """_log_value logs when the tracked value changes and stays quiet when it doesn't. - `_log_value(attribute)` reads an arbitrary instance attribute via - `getattr(self, attribute, MISSING)` and dedups against `self._prev_value`, - purely to feed a debug log line — no public state changes either way, so - there's no `hass.states` equivalent to test through. `entity.some_attr` is - set here as the attribute the method is told to read by name. + `_log_value` reads an arbitrary attribute via `getattr` and dedups against + `_prev_value`, purely to feed a debug log line — no `hass.states` to assert on. """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] @@ -172,11 +153,8 @@ async def test_get_zaptec_value_returns_default_when_key_missing( ) -> None: """_get_zaptec_value() returns the caller's default when the key isn't backed. - Most call sites rely on the `MISSING` sentinel default to trigger - `KeyUnavailableError` when a key is absent. Exactly one production call - site opts out of that (`sensor.py`'s `default={}` for the optional - `completed_session` key) — this covers that explicit-default path, not - just generic `.get()` plumbing. + Covers the one production call site that opts out of the `MISSING`-triggers- + `KeyUnavailableError` default (`sensor.py`'s `default={}` for `completed_session`). """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] @@ -194,12 +172,9 @@ async def test_get_zaptec_value_raises_when_intermediate_value_not_mapping( ) -> None: """A dotted key whose first segment resolves to a non-Mapping value raises. - No shipped entity description currently uses a dotted key (`sensor.py`, - `binary_sensor.py`, `number.py`, `switch.py`, `update.py` all pass a single - flat key, e.g. "signed_meter_value"), so this exercises the "obj isn't - Mapping-like" half of `_get_zaptec_value`'s documented `Raises:` contract - directly rather than through a real entity, guarding it for whenever a - future entity does use one. + No shipped entity uses a dotted key today, so this exercises + `_get_zaptec_value`'s "obj isn't Mapping-like" branch directly, guarding it + for whenever one does. """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] @@ -218,14 +193,10 @@ async def test_log_zaptec_attribute_formats_none_str_iterable_and_scalar_keys( ) -> None: """_log_zaptec_attribute formats None, a single key, an iterable, and a scalar. - `str` is the live default (every entity's `description.key`) and `Iterable` - is also live (sensor.py/update.py override it with a list for multi-key - logging). `None` is a documented-but-currently-unused hook, and the final - scalar case (anything that's not None/str/Iterable, e.g. an int) is the - property's fallback branch — also currently unreachable in production, but - covered here since a test can exercise it even though no shipped entity - does. Pokes all four directly since the property only ever feeds a debug - log line, so no real entity's state exposes it. + Pokes all four branches directly since the property only feeds a debug log + line. `str` (default `description.key`) and `Iterable` (sensor.py/update.py's + multi-key logging) are live; `None` and the scalar fallback are currently + unreachable in production but worth guarding. """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] @@ -253,12 +224,9 @@ async def test_log_unavailable_logs_error_and_recovery_transitions( ) -> None: """_log_unavailable logs the real exception on going unavailable, and logs recovery. - Its transition logging is driven purely by `_attr_available`/`_prev_available` - — which are decoupled from the entity's actual HA-reported availability - (`CoordinatorEntity.available` reads `coordinator.last_update_success`, - never these). So there's no realistic way to drive both log transitions - through a real coordinator refresh; setting the attributes directly is the - only way to exercise this logging branch in isolation. + Sets `_attr_available`/`_prev_available` directly since they're decoupled + from `CoordinatorEntity.available` (which reads `last_update_success`) — no + real coordinator refresh can drive both log transitions. """ manager = await setup_integration(hass, mock_config_entry, mock_zaptec) coordinator = manager.device_coordinators["chg-mock-1"] diff --git a/tests/test_init.py b/tests/test_init.py index 968d671f..6a00fb20 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -51,9 +51,7 @@ async def test_setup_entry_creates_manager_and_entities( assert isinstance(manager, ZaptecManager) assert mock_config_entry.runtime_data is manager - # HA slugifies each entity's name into its entity_id's object_id half; this - # only matches because conftest.py's mock_zaptec seeds "Mock Charger"/"Mock - # Home" (make_charger/make_installation), so every zaptec-created entity_id - # starts with "mock". If that seed naming ever changes, update this filter. + # Matches because mock_zaptec seeds "Mock Charger"/"Mock Home", which HA + # slugifies into "mock..." entity_ids. Update if that seed naming changes. states = [s for s in hass.states.async_all() if s.entity_id.split(".")[1].startswith("mock")] assert states, "expected at least one zaptec entity to be created" diff --git a/tests/zaptec/conftest.py b/tests/zaptec/conftest.py index 595de04a..2b86570b 100644 --- a/tests/zaptec/conftest.py +++ b/tests/zaptec/conftest.py @@ -11,15 +11,10 @@ def zaptec_constants() -> dict: """Get latest constants from Zaptec API. - Uses a self-contained event loop instead of `asyncio.run()`. Under - pytest-homeassistant-custom-component's `HassEventLoopPolicy`, - `asyncio.run()` unconditionally resets the thread's registered event - loop to `None` on exit (success or failure) via `asyncio.set_event_loop`. - That policy raises `RuntimeError` from `get_event_loop()` instead of - lazily creating one, so the next bare `asyncio_mode=auto` test in the - session would fail in its autouse loop-setup fixture. Saving/restoring - the previous loop here keeps this fixture from clobbering global - event-loop state for tests that run after it. + Uses a self-contained event loop instead of `asyncio.run()`: under + pytest-hacc's `HassEventLoopPolicy`, `asyncio.run()` resets the thread's + event loop to `None` on exit, breaking later `asyncio_mode=auto` tests. + Saving/restoring the previous loop avoids clobbering that global state. """ async def get_zaptec_constants() -> dict: