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.
… 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>
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>
…-components#311) installation/update, chargers/{id}/update and chargers/{id}/SendCommand all require the Owner or Service role per docs.zaptec.com. A shared ZaptecBase._require_write_role() now raises InsufficientRoleError naming the action and the role needed, before the call is made, which the existing exception handling in number.py/services.py/button.py already surfaces as a HomeAssistantError. authorizecharge and localSettings stay ungated: neither is documented anywhere, and guessing risks blocking calls that work today. The per-installation coordinator also raises a non-fixable Repair issue when it observes an insufficient role, so the limitation is visible before anyone tries a control. async_create_issue() runs on every poll rather than only on first observation: HA's issue registry replaces the entry via dataclasses.replace() without touching dismissed_version, so an "Ignore" survives; only a real role change deletes and recreates the issue, which resets it deliberately. Charger roles come from poll_info() (chargers/{id}, or the /chargers list on 403, which carries the field too) - verified live. The hierarchy used at build time omits CurrentUserRoles, so charger-level gating falls through to "let the API decide" only before the first poll. Ported from custom-components#401 onto custom-components#414's pytest-homeassistant-custom-component test conventions: the Repair-issue tests now drive real coordinators through setup_integration() and assert against the real issue registry, including ignoring an issue and proving the dismissal survives further polls. The role is varied within "still insufficient" between those polls so the test also proves the check re-ran, rather than passing because nothing touched the issue. conftest's _backed_get() now defaults to None like the Mapping.get that ZaptecBase inherits, instead of the MISSING sentinel: coordinator.py reads current_user_roles with no default, and MISSING made that "Owner" in MISSING -> TypeError. entity.py passes default=MISSING explicitly and is unaffected. The fixture seed dicts move to module level so reseed() calls extend them instead of silently replacing the seeded keys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Roles are granted per object, so an account can hold Owner on an installation and only User on one of its chargers - the installation-level Repair issue said nothing about that, leaving charger settings and commands to fail only when someone pressed the control. The installation coordinator now also inspects the installation's chargers and raises a single issue naming the restricted ones, rather than one issue per charger, which would nag people who deliberately manage per-charger access. The two issues are mutually exclusive: an insufficient installation role supersedes the charger one, since it carries the same remedy. A charger whose role hasn't been observed yet is left out rather than assumed restricted, matching how the gate itself defers to the API's own 403. The Owner/Maintainer test moves into zaptec.has_write_role() so the client and the coordinator share one definition, with None distinguishing "not observed yet" from "no write access". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
type_user_roles() joined a set, so the rendered role string varied between runs - two consecutive live calls returned "Maintainer, User, Owner" and "User, Maintainer, Owner" for the same bitmask. That reached the role Repair issue's text and the diagnostics dump, and test_user_roles had to assert multi-role values with substring checks instead of equality. Ordering by bit value keeps roles ascending by privilege rather than alphabetical, and lets those assertions be exact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #401. Same feature, ported onto #414's test conventions, plus a
per-charger warning that #401 didn't have.
What it does
installation/update,chargers/{id}/updateandchargers/{id}/SendCommand/{id}all require the Owner or Service role per docs.zaptec.com.
_require_write_role()now raises
InsufficientRoleErrorbefore the call, which the existing handling innumber.py/services.py/button.pysurfaces as aHomeAssistantErrorinstead ofa raw HTTP 403.
The installation coordinator also raises a Repair issue naming the installation
and the role it needs. Roles are per object, so an account can be Owner on an
installation and User on one of its chargers: in that case one issue per
installation names the restricted chargers instead. The two are mutually
exclusive.
Deliberate limits
authorizechargeandlocalSettingsstay ungated - undocumented endpoints,no evidence what role they need, so gating them risks blocking calls that work.
CurrentUserRolesafterpoll_info()(the hierarchy usedat build time omits it, verified live), so charger gating defers to the API's
own 403 during the first poll.
replaces the entry without touching
dismissed_version, so an "Ignore" holdsuntil the role actually changes.
type_user_roles()joined a set, so the rendered role string varied between runsand reached both the issue text and diagnostics; it is now ordered by bit value.
Translations
nb/nn/pl/svare machine-translated - corrections welcome.de/fraremissing: this branch predates them, and adding them here conflicts with master.
They will follow in a separate PR once this lands.
Test plan
pytest tests --ignore=tests/zaptec- 34 passed, 1 skippedpytest tests/zaptec -p no:homeassistant- 117 passed, 1 skippedcoordinator.py100% line+branchruff format --diff/ruff checkcleanAI policy compliance
I reviewed this code together with Claude, walking through each part of the
change. That review resulted in stronger tests for the dismissal behaviour and
the per-charger warning above. Based on that process, I believe this complies
with the AI policy to the best of my ability.
🤖 Generated with Claude Code