test: migrate coordinator/entity tests to pytest-homeassistant-custom-component harness - #414
test: migrate coordinator/entity tests to pytest-homeassistant-custom-component harness#414rhammen wants to merge 30 commits into
Conversation
Design for replacing PR custom-components#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 custom-components#395 replacement. Bug custom-components#410 kept test-only (xfail) pending maintainer input on availability semantics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
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 <noreply@anthropic.com>
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.
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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
…ns 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 <noreply@anthropic.com>
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 custom-components#398). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…components#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, custom-components#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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…verage Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…m-components#257) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
….__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. custom-components#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 <noreply@anthropic.com>
…n 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 <noreply@anthropic.com>
…mat branch 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 custom-components#410 finding that those attributes are decoupled from the entity's actual HA-reported availability. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…ctor 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Moved to a dedicated fork-only branch per maintainer preference not to carry AI-generated planning docs in the upstream repo.
…dep)
The energy-statistics feature declares a hard `recorder` dependency in
manifest.json, so `async_setup` now fails ("Setup failed for dependencies:
['recorder']") unless a recorder is available. custom-components#414's setup_integration
helper is a plain function and can't request fixtures, so make the
`mock_config_entry` fixture depend on `recorder_mock` — every HA-integration
test pulls it transitively, while the tests/zaptec (no-harness) leg, which
never uses this fixture, is unaffected.
Fixes the 16 setup_integration-based failures in test_coordinator/test_entity/
test_init that appear once custom-components#414's harness meets energy's recorder dependency.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sveinse
left a comment
sveinse
left a comment
There was a problem hiding this comment.
The introduction of two runs of pytest was unexpected. What is the main reason for not running them all in one? I.e. why can't we run the API tests under the HA test bench? -- Since the split is imminent, its not necessarily a bad thing to do it like this. But it does create more code and more steps, so we need a clear reason why not run it in one.
Another observation is that the AI assistance creates a lot of comments in the code. While it is good to be informed what the full intent is in the different functionds, thay are very wordy. A lot more wordy than any developer would do. And at some point the blocks of text disrupts the flow of the code. Is it possible to instruct the agent to reduce the words?
pytest-hacc's hass fixture blocks all non-localhost sockets. tests/zaptec/test_zconst.py/test_redact.py need a real call to api.zaptec.com/api/constants (deliberate — catches drift against the live API), so they can't run under the harness at all. I did look at running everything in one invocation via a snapshot fixture instead of the live call, but that just trades a real API-drift check for a fake one to avoid the split, so I dropped it. Given #257 already commits zaptec/ to becoming a standalone package tested independently of HA, and splitting themm allows one test to run under the harness and the other one to do the live api call, I judged that the better, cleaner solution.
Yes of course. I'm still a bit searching for the optimum here. |
… 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 <noreply@anthropic.com>
That is a very good approach for gaining the necessary understanding to be the "developer in the loop" before submitting the PR. As with the superpower docs you're using, I think they can help you to gain the required understanding of the problem and the fix. Perhaps a good way would be to generate the code in its full comment verbosity first so you can understand it, and then instruct the agent to reduce and compact the comments? I think that could work. In case you encounter something you don't understand, ask the agent to clarify. This is one of its great strengths. 👍 |
sveinse asked for setup specific to the future-standalone API client (custom-components#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 <noreply@anthropic.com>
steinmn noted the per-Python pytest-hacc pins speak for themselves (custom-components#414 review). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Trimmed the code comments across the branch — cut every docstring/comment down to the non-obvious "why", dropped the restated "what" and secondary asides. Net -66 lines in the test files, same coverage/behavior. |
… resolution Fixes custom-components#253 - moves service registration from async_setup_entry (once per config entry) to async_setup (once at HA startup), per HA's action-setup quality-scale rule. async_unload_services is removed - nothing else called it, and services now live for HA's lifetime, not any one config entry's. Also fixes a real bug this refactor uncovered: iter_objects used to search only the one ZaptecManager captured in a closure at registration time. Because registration was guarded by hass.services.has_service, a user with two Zaptec accounts could never reach the second account's chargers via zaptec.* services - only whichever entry set up first ever got wired in. iter_objects now resolves service_call.hass and searches every currently loaded config entry's manager. Rebuilt on top of the pytest-hacc harness migration (PR custom-components#414): tests use the real hass/MockConfigEntry harness throughout, including a full test_services.py rewrite exercising real HA device/entity registries and real service dispatch through async_setup_services.
3 participants
Summary
Replaces #394. Maintainer review on #394 (CHANGES_REQUESTED) asked how gold/platinum HA integrations test coordinators and entities — the answer is the real
pytest-homeassistant-custom-componentharness (realhass,MockConfigEntry, behavior asserted viahass.states), not hand-rolledFakeConfigEntry/mock-hassdoubles poking private attributes.tests/test_*.py(coordinator/entity/init), pinned per-Python via environment markers to match each CI leg's HA version.pytest tests --ignore=tests/zaptec), leavingtests/zaptec/*— the vendoredzaptec/API client, a standalone-PyPI-library-in-waiting per Cleanup directory structure and split between API library and HA integration #257 — running as plain pytest (-p no:homeassistant), so its liveapi.zaptec.com/api/constantscall keeps working and the two concerns stay separated as Cleanup directory structure and split between API library and HA integration #257 intends. Two pytest invocations, combined via--cov-append.pytest_pluginsforce-load). On Linux, pytest-hacc autoloads on its own — CI and the devcontainer both just work.coordinator.pyandentity.pycoverage: 100% / 100% (up from ~27-29% pre-migration).Sequencing note
#402, #401, #399, #397 each carry their own copy of
tests/conftest.py/test_coordinator.py/test_entity.py/test_init.pyusing the oldFakeConfigEntrypattern, built independently of this branch. I've marked all four as draft for now. Once this merges, each will either be updated in place to the new test conventions and re-marked ready, or - if that turns out easier - replaced with a new PR off a fresh branch, the same way #396 was replaced by thefeature/energy-statistics-standalonebranch.Test plan
pytest tests --ignore=tests/zaptec --cov=./custom_components/zaptec --cov-branch- 24 passed, 1 skippedpytest tests/zaptec -p no:homeassistant --cov=./custom_components/zaptec --cov-branch --cov-append- 102 passed, 1 skipped (--skip-api)coordinator.py100%,entity.py100%ruff format --diff/ruff checkcleancustom_components/**production changes - test/infra-onlyAI policy compliance
I personally reviewed this code together with Claude to the best of my knowledge. That review resulted in several updated docstrings for things that weren't clear or obvious to me on first read. Based on that process, I believe this complies with the AI policy to the best of my ability.
🤖 Generated with Claude Code