Skip to content

fix: small quirks surfaced while writing the test suite#493

Merged
shbatm merged 1 commit into
v3.x.xfrom
fix/test-suite-quirks
May 4, 2026
Merged

fix: small quirks surfaced while writing the test suite#493
shbatm merged 1 commit into
v3.x.xfrom
fix/test-suite-quirks

Conversation

@shbatm

@shbatm shbatm commented May 4, 2026

Copy link
Copy Markdown
Collaborator

Three independent issues observed while building out the offline pytest suite (#484), grouped here because each is one or two lines and none warrants its own ticket.

1. Nodes.parse() re-update path silently dropped

pyisy/nodes/__init__.py line 415 (pre-fix) called the async Node.update(xmldoc=feature) from a synchronous parsing loop without await. The returned coroutine was never run, so when parse() re-encountered an existing address (the re-update branch) the node's status was never refreshed from the new XML. The test suite caught this as RuntimeWarning: coroutine 'Node.update' was never awaited.

Fix inlines the synchronous portion of Node.update(xmldoc=...) directly in the parser: capture last_update, merge aux_properties, and call update_state(state). No new public method on Node; the async Node.update is unchanged for its other callers.

2. NodeIterator missing __iter__

NodeIterator implemented __next__ and __len__ but not __iter__. Python's for protocol calls iter() on the iterable, which requires __iter__ on the iterator object itself — without it, for x in iter(nodes) and for x in reversed(nodes) raise TypeError. Added the conventional __iter__ returning self.

Programs reuses the same iterator class (aliased as ProgramIterator), so this fix covers both managers.

3. Programs.__getitem__ raised KeyError on unrecognized keys

The inner try around fun(val) returns None on KeyError/IndexError/ValueError, and the signature is -> Program | Folder | None — but the path where val isn't an address, isn't a known name, and can't be int()-converted raised KeyError("Unrecognized Key: ...") instead of returning None. Inconsistent with the other miss paths.

Fix returns None from that branch too, matching the rest of the method.

Test plan

Regression coverage lands in PR #484 (the test-suite PR), which already has the failing-warning case for #1 and tightens the Programs miss assertions for #3. No new tests in this PR — the fixes are small enough to review on their own.

HA Core impact

None. homeassistant/components/isy994/ doesn't iterate NodeIterator directly, doesn't call Programs.__getitem__("missing-key") with the expectation of a KeyError, and the dropped re-parse manifested as a stale cache the existing event-stream handlers immediately re-corrected.

Three independent issues observed during the test-suite PR (#484):

- Nodes.parse() called the async ``Node.update(xmldoc=...)`` from a
  synchronous parsing loop without awaiting, so the returned coroutine
  was silently dropped and the re-parse path never updated the node's
  state. Inline the work (apply ``last_update``, aux-property merge,
  and ``update_state``) directly in the sync parser.

- NodeIterator implemented ``__next__`` and ``__len__`` but no
  ``__iter__``, so ``for x in iter(nodes)`` and ``for x in
  reversed(nodes)`` raised ``TypeError`` because Python's ``for``
  protocol calls ``iter()`` on the iterable, which requires
  ``__iter__`` to exist on the iterator itself. Add the conventional
  ``__iter__`` returning ``self``.

- Programs.__getitem__ raised ``KeyError`` on an unrecognized key
  (one that wasn't an address, name, or int-convertible index) even
  though the rest of the method's miss paths return ``None`` through
  the inner ``try`` and the signature is ``-> ... | None``. Return
  ``None`` for that branch too so behavior is consistent across all
  miss cases.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@shbatm shbatm mentioned this pull request May 4, 2026
5 tasks
@shbatm
shbatm merged commit b4040bc into v3.x.x May 4, 2026
2 checks passed
@shbatm
shbatm deleted the fix/test-suite-quirks branch May 4, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant