Skip to content

Commit 4fd2852

Browse files
committed
refactor: address PR #1398 KP13 second-pass review (3 items)
1. Narrow cleanup-loop exception in test_poll_cadence_measurement.py from `except Exception` to `except _POLLING_TRANSIENT_ERRORS` (the canonical tuple from wait_helpers.py). Programmer bugs (TypeError, AttributeError, KeyError, AssertionError) now propagate with their stack trace instead of being downgraded to a `logger.warning` line across 10 iterations. Aligns per-entity cleanup with the polling-helper pattern established for AGENTS.md "Exception handling in polling helpers" (#1266). 2. Lift the soft-failure warning prefix out of tools_config_automations.py as module-level `NOT_VERIFIED_WARNING_PREFIX` and import it in the test. The literal coupling becomes a compile-time edge instead of a runtime puzzle — rewording the source warning now breaks the test import, replacing the previous "9 samples + 0 not-verified != 10 attempts" indirect signal with a clear ModuleNotFoundError-style failure. 3. Replace the stale `session-scoped` label in the cleanup-block comment with `logging-only` (matches the commit-message framing for the ``cleanup_tracker`` fixture, which is function-scoped per its `@pytest.fixture` decorator in tests/src/e2e/conftest.py:1951).
1 parent f34ea9a commit 4fd2852

2 files changed

Lines changed: 22 additions & 15 deletions

File tree

src/ha_mcp/tools/tools_config_automations.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@
5353

5454
logger = logging.getLogger(__name__)
5555

56+
# Distinctive prefix of the soft-failure warning emitted by
57+
# ``ha_config_set_automation`` when ``_poll_for_automation_entity``
58+
# exhausts ``_POLL_CADENCE`` without matching the new automation.
59+
# Exported so tests (e.g. ``test_poll_cadence_measurement.py``) can
60+
# detect a missed registration without hard-coding the literal —
61+
# rewording the warning becomes a compile-time coupling rather than
62+
# a silent test drift.
63+
NOT_VERIFIED_WARNING_PREFIX = (
64+
"Automation was submitted to Home Assistant but the entity was not found"
65+
)
66+
5667

5768
def _normalize_automation_config(
5869
config: Any,
@@ -767,7 +778,7 @@ async def ha_config_set_automation(
767778
# If the client could not verify the entity was registered, warn but don't hard-fail.
768779
if result.get("entity_not_verified"):
769780
result.setdefault("warnings", []).append(
770-
"Automation was submitted to Home Assistant but the entity was not found "
781+
f"{NOT_VERIFIED_WARNING_PREFIX} "
771782
"after polling. The automation may still have been created -- check Home "
772783
"Assistant logs and try reloading automations. Common causes: "
773784
"automations.yaml vs automation.yaml filename mismatch, invalid config "

tests/src/e2e/workflows/automation/test_poll_cadence_measurement.py

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,18 @@
2525

2626
import pytest
2727

28+
from ha_mcp.tools.tools_config_automations import NOT_VERIFIED_WARNING_PREFIX
29+
2830
from ...conftest import record_poll_cadence_measurement
2931
from ...utilities.assertions import safe_call_tool
32+
from ...utilities.wait_helpers import _POLLING_TRANSIENT_ERRORS
3033

3134
logger = logging.getLogger(__name__)
3235

3336
# Captures the DEBUG line emitted by rest_client._poll_for_automation_entity
3437
# on every successful registration. Format owned by that function.
3538
_ELAPSED_RE = re.compile(r"entity-registration-elapsed:\s*([\d.]+)ms")
3639

37-
# Distinctive substring of the soft-failure warning ``ha_config_set_automation``
38-
# emits when ``_poll_for_automation_entity`` exhausts ``_POLL_CADENCE`` without
39-
# matching the new automation. The tool pops ``entity_not_verified`` from the
40-
# response, so this warning is the only reliable signal that a registration
41-
# missed. Mirrors the literal in ``tools_config_automations.py``.
42-
_NOT_VERIFIED_WARNING_PREFIX = (
43-
"Automation was submitted to Home Assistant but the entity was not found"
44-
)
45-
4640

4741
@pytest.mark.automation
4842
@pytest.mark.cleanup
@@ -100,7 +94,7 @@ async def test_poll_cadence_p50_worst(
10094
warnings = create_data.get("warnings") or []
10195
if any(
10296
isinstance(w, str)
103-
and w.startswith(_NOT_VERIFIED_WARNING_PREFIX)
97+
and w.startswith(NOT_VERIFIED_WARNING_PREFIX)
10498
for w in warnings
10599
):
106100
not_verified_count += 1
@@ -194,19 +188,21 @@ async def test_poll_cadence_p50_worst(
194188
logger.info("VERDICT: %s", verdict)
195189
logger.info(sep)
196190
finally:
197-
# The session-scoped ``cleanup_tracker`` fixture only logs what
191+
# The logging-only ``cleanup_tracker`` fixture only logs what
198192
# it tracked; without an explicit delete here this test would
199193
# leak 10 automations into the next worker run on a loadscope
200-
# split. Best-effort: per-entity remove, swallow individual
201-
# failures so cleanup doesn't mask the real assertion outcome.
194+
# split. Best-effort: per-entity remove, swallow only transient
195+
# transport/HA errors so programmer bugs (TypeError, KeyError,
196+
# AttributeError, AssertionError) propagate with their stack
197+
# trace instead of being downgraded to a warning line.
202198
for ent_id in created_entity_ids:
203199
try:
204200
await safe_call_tool(
205201
mcp_client,
206202
"ha_config_remove_automation",
207203
{"identifier": ent_id},
208204
)
209-
except Exception as cleanup_err:
205+
except _POLLING_TRANSIENT_ERRORS as cleanup_err:
210206
logger.warning(
211207
"cleanup: failed to remove %s: %s", ent_id, cleanup_err
212208
)

0 commit comments

Comments
 (0)