test(ci): F4 — entity-contract pass in the drift sweep + real-HA state-write proof (t40)

The F1 blocker was invisible to every existing test: the stub harnesses call the
entity and replace ``async_write_ha_state``, so a non-property
``extra_state_attributes`` looked perfectly healthy. Two new gates close that
hole.

1. ``ha_api_drift.scan_entity_contracts()`` — a pure-AST pass (so it needs no
   homeassistant, unlike passes 1-3) that walks every class inheriting from an
   HA entity base, including our own intermediate ``NvidiaShieldSensorBase``, and
   flags any HA-READ attribute (``extra_state_attributes``, ``available``,
   ``state``, ``native_value``, ``media_*``, …) declared as a plain method
   instead of ``@property``. It is wired into ``scan()`` before the
   homeassistant gate and exported for the fast suite, so ONE implementation
   backs the sweep, the CLI red proof and tests/test_entity_contract.py (the t39
   F1 lesson).
   Red proof on the pre-fix tree:
     media_player.py:828: entity-attribute-is-a-method:
     NvidiaShieldMediaPlayer.extra_state_attributes is a plain method, but HA
     reads it as an attribute (``attr |= self.<name>``) — declare it @property
   …and HEAD is clean (13 modules, 60 imports, 35 call arguments, 7 awaited).

2. tests/test_ha_api_drift.py now builds each entity under REAL homeassistant
   (real ``PlatformData``, real ``async_write_ha_state``) and asserts HA's own
   ``__async_calculate_state`` succeeds and that the media player's app_id/
   app_name survive the merge — for the media player, both sensors, the remote
   and the pairing button. A coverage test compares that matrix against the
   sweep's classification, so a new entity class cannot quietly skip it.

Verified: 296 passed / 2 skipped (fast suite), 16 passed (drift suite under
Python 3.14 + homeassistant 2026.10.1), ruff clean; the entity-contract red
proof above reproduces exactly the reported line.
This commit is contained in:
James Mackay committed 2026-10-11 06:02:22 +01:00
1 parent acc7bc3d01
commit be3abae3e7
3 files changed
+460 -1

No files matched your search

+188 -1
View File
@@ -30,7 +30,14 @@ dependency against an INSTALLED homeassistant:
must exist in the real signature — the ``options_updates`` class of bug;
3. every ``self.<method>()`` call inside a class that inherits from an HA class
is compared against the real definition: a coroutine must be awaited, and an
awaited call must actually be a coroutine.
awaited call must actually be a coroutine;
4. every class inheriting from an HA entity base has its HA-READ attributes
(``extra_state_attributes``, ``available``, …) declared as properties — HA
merges them with ``attr |= self.extra_state_attributes``, so a plain method
raises ``TypeError: 'method' object is not iterable`` on every state write and
the entity can never render (t40 live install: ``media_player.py:828``). The
stub suites call the method happily; this pass is pure AST and, unlike 1-3,
needs no homeassistant at all.
Known limits (deliberate, and why the sweep still catches the real classes):
``**kwargs`` splats cannot be checked statically, calls on objects other than
@@ -439,6 +446,183 @@ def parse_requires_python(spec: str | None) -> tuple[int, ...] | None:
return tuple(int(part) for part in match.group(1).split("."))
# ----------------------------------------------------------- entity contracts
#: Attributes Home Assistant reads as ATTRIBUTES — it never calls them. A plain
#: method with one of these names is invisible to the stub suites (permissive
#: stubs, and the harness replaces ``async_write_ha_state``) and fatal under real
#: HA: ``Entity.__async_calculate_state`` merges ``extra_state_attributes`` with
#: ``attr |= self.extra_state_attributes``, which raises
#: ``TypeError: 'method' object is not iterable`` on every state write. That was
#: the t40 live-install blocker (``media_player.py:828``).
ENTITY_ATTRIBUTES = frozenset(
{
"activity",
"activity_list",
"app_id",
"app_name",
"assumed_state",
"attribution",
"available",
"capability_attributes",
"current_activity",
"device_class",
"device_info",
"entity_category",
"entity_picture",
"extra_state_attributes",
"icon",
"is_volume_muted",
"last_reset",
"media_album_name",
"media_artist",
"media_content_id",
"media_content_type",
"media_duration",
"media_image_url",
"media_position",
"media_title",
"name",
"native_unit_of_measurement",
"native_value",
"options",
"should_poll",
"sound_mode",
"source",
"source_list",
"state",
"state_attributes",
"supported_features",
"translation_key",
"unique_id",
"unit_of_measurement",
"volume_level",
}
)
#: Entity base classes Home Assistant ships. A class counts as an entity when its
#: transitive bases include one of these, so our own intermediate bases
#: (``NvidiaShieldSensorBase``) are covered as well.
HA_ENTITY_BASES = frozenset(
{
"AlarmControlPanelEntity",
"BinarySensorEntity",
"ButtonEntity",
"CalendarEntity",
"CameraEntity",
"ClimateEntity",
"CoverEntity",
"DeviceTrackerEntity",
"Entity",
"EventEntity",
"FanEntity",
"HumidifierEntity",
"ImageEntity",
"LawnMowerEntity",
"LightEntity",
"LockEntity",
"MediaPlayerEntity",
"NotifyEntity",
"NumberEntity",
"RemoteEntity",
"SelectEntity",
"SensorEntity",
"SirenEntity",
"SwitchEntity",
"TextEntity",
"TodoListEntity",
"UpdateEntity",
"VacuumEntity",
"ValveEntity",
"WaterHeaterEntity",
"WeatherEntity",
}
)
def _is_attribute_decorated(node: ast.FunctionDef | ast.AsyncFunctionDef) -> bool:
"""True when the method is declared ``@property`` / ``@cached_property``."""
for decorator in node.decorator_list:
name = decorator.id if isinstance(decorator, ast.Name) else getattr(decorator, "attr", "")
if name in {"property", "cached_property"}:
return True
return False
def _class_bases(node: ast.ClassDef) -> set[str]:
"""The names in a class's base list (``**kwargs``-style bases are ignored)."""
return {
base.id if isinstance(base, ast.Name) else getattr(base, "attr", "")
for base in node.bases
}
def entity_class_names(tree: ast.Module) -> set[str]:
"""Names of the classes in one module that inherit from an HA entity base."""
bases = {
node.name: _class_bases(node) for node in ast.walk(tree) if isinstance(node, ast.ClassDef)
}
entities = {name for name, parents in bases.items() if parents & HA_ENTITY_BASES}
# Transitive: our own intermediate bases (NvidiaShieldSensorBase → SensorEntity)
changed = True
while changed:
changed = False
for name, parents in bases.items():
if name not in entities and parents & entities:
entities.add(name)
changed = True
return entities
def _scan_entity_contracts(paths, report: Report) -> None:
"""t40: flag HA-read attributes declared as plain methods.
The stub suites cannot see this class of defect — they call the method
successfully, because the harnesses replace ``async_write_ha_state`` — and
only real HA turns it into a TypeError. Pure AST, so it also runs when
homeassistant is not installed.
"""
for root in paths:
root = Path(root)
if not root.exists():
continue # scan() reports bad-path for the same target
files = sorted(root.glob("*.py")) if root.is_dir() else [root]
for file in files:
try:
tree = ast.parse(file.read_text(encoding="utf-8"), filename=str(file))
except (OSError, SyntaxError):
continue
entity_classes = entity_class_names(tree)
for node in ast.walk(tree):
if not isinstance(node, ast.ClassDef) or node.name not in entity_classes:
continue
for item in node.body:
if not isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)):
continue
if item.name not in ENTITY_ATTRIBUTES or _is_attribute_decorated(item):
continue
report.findings.append(
Finding(
str(file),
item.lineno,
"entity-attribute-is-a-method",
f"{node.name}.{item.name} is a plain method, but HA reads it as an "
"attribute (``attr |= self.<name>``) — declare it @property",
)
)
def scan_entity_contracts(paths=None) -> list[Finding]:
"""The entity-contract pass on its own (t40) — the fast suite's entry point."""
if paths is None:
paths = [SHIELD_DIR]
elif isinstance(paths, (str, Path)):
paths = [paths]
report = Report()
_scan_entity_contracts(paths, report)
return report.findings
# ----------------------------------------------------------------------- scan
@@ -455,6 +639,9 @@ def scan(paths=None) -> Report:
paths = [paths]
report = Report()
# t40: pure-AST shape check, so it runs even without homeassistant — that is
# the whole point of a contract stubs cannot enforce.
_scan_entity_contracts(paths, report)
if not homeassistant_available():
# Neither a false green nor a wall of phantom findings: with no package
# to resolve against, every import would "fail", so the sweep reports
+120
View File
@@ -0,0 +1,120 @@
"""Entity-contract shape checks (t40) — the defect class stubs cannot see.
The stub suites drive entities through hand-written fakes and replace
``async_write_ha_state``, so a non-property ``extra_state_attributes`` looks
perfectly healthy there while a live install raises
``TypeError: 'method' object is not iterable`` on every state write and the
entity never renders.
The check lives in ``tests/ha_api_drift.py`` (``scan_entity_contracts``) so this
suite and the drift sweep share ONE implementation — and the CI drift job, which
runs against a real Home Assistant, gates the same code path.
"""
import ast
from pathlib import Path
import ha_api_drift
from conftest import _SHIELD_DIR
_PLAIN_DEF = " def extra_state_attributes(self) -> dict:"
#: The t40 shape: HA reads ``extra_state_attributes`` as an attribute
#: (``attr |= self.extra_state_attributes``), so a method here is a crash.
_PRE_FIX = f'''
from homeassistant.components.media_player import MediaPlayerEntity
class Player(MediaPlayerEntity):
{_PLAIN_DEF}
return {{"app_id": "x"}}
'''
_FIXED = f'''
from homeassistant.components.media_player import MediaPlayerEntity
class Player(MediaPlayerEntity):
@property
{_PLAIN_DEF}
return {{"app_id": "x"}}
'''
_INHERITED_BASE = '''
from homeassistant.components.sensor import SensorEntity
class Base(SensorEntity):
"""Our own intermediate base — subclasses are entities too."""
class Child(Base):
def native_value(self):
return "x"
'''
def _findings(tmp_path, source: str):
(tmp_path / "module.py").write_text(source, encoding="utf-8")
return ha_api_drift.scan_entity_contracts(tmp_path)
def test_no_entity_attribute_in_the_tree_is_a_plain_method():
"""t40: this is the assertion that fails on the pre-fix media_player.py."""
assert ha_api_drift.scan_entity_contracts() == []
def test_the_check_flags_the_pre_fix_shape(tmp_path):
expected_line = _PRE_FIX.splitlines().index(_PLAIN_DEF) + 1
findings = _findings(tmp_path, _PRE_FIX)
assert [finding.kind for finding in findings] == ["entity-attribute-is-a-method"]
assert findings[0].lineno == expected_line
assert "Player.extra_state_attributes" in findings[0].detail
assert "@property" in findings[0].detail
def test_the_check_accepts_the_property_form(tmp_path):
assert _findings(tmp_path, _FIXED) == []
def test_the_check_follows_our_own_intermediate_bases(tmp_path):
"""NvidiaShieldSensorBase-style bases must not hide their subclasses."""
findings = _findings(tmp_path, _INHERITED_BASE)
assert [finding.detail.split()[0] for finding in findings] == ["Child.native_value"]
def test_the_tree_classification_is_not_vacuous():
"""A classifier that sees no entities would pass every check above."""
classified: set[str] = set()
for path in sorted(_SHIELD_DIR.glob("*.py")):
classified |= ha_api_drift.entity_class_names(ast.parse(path.read_text(encoding="utf-8")))
assert {
"NvidiaShieldMediaPlayer",
"NvidiaShieldCurrentAppSensor",
"NvidiaShieldNowPlayingSensor",
"NvidiaShieldRemoteEntity",
"NvidiaShieldPairAdbButton",
} <= classified
def test_the_sweep_reports_the_same_findings_as_this_check(tmp_path):
"""One implementation: the sweep's report carries the contract pass too."""
(tmp_path / "module.py").write_text(_PRE_FIX, encoding="utf-8")
report = ha_api_drift.scan(tmp_path)
kinds = [finding.kind for finding in report.findings]
# The contract pass runs FIRST (pure AST). Without homeassistant installed the
# sweep then adds "cannot judge the API surface" — that path is covered in
# tests/test_ha_api_drift.py.
assert kinds[0] == "entity-attribute-is-a-method"
assert "no-homeassistant" in kinds
def test_the_check_ignores_missing_paths_instead_of_raising():
"""scan() owns the bad-path finding; the pass just skips."""
assert ha_api_drift.scan_entity_contracts(Path("/nonexistent-does-not-exist")) == []
+152
View File
@@ -12,9 +12,12 @@ The sweep itself lives in ``tests/ha_api_drift.py`` (documented there).
from __future__ import annotations
import ast
import importlib
import subprocess
import sys
import threading
from pathlib import Path
from types import SimpleNamespace
import ha_api_drift
import pytest
@@ -206,3 +209,152 @@ def test_the_declared_python_floor_never_lags_homeassistant():
f"but the installed homeassistant requires {declared} — raise the constant "
f"and the setup-python version in .gitea/workflows/test.yml"
)
# ------------------------------------------------- entity contract (t40, real HA)
#
# The stub suites cannot see this class of defect: their harness replaces
# ``async_write_ha_state``, so a non-property ``extra_state_attributes`` looks
# fine there. Only HA's own ``Entity.__async_calculate_state`` — which does
# ``attr |= self.extra_state_attributes`` — turns it into
# ``TypeError: 'method' object is not iterable`` and stops the entity from ever
# rendering. These tests drive that real path.
_REPO_ROOT = Path(__file__).resolve().parent.parent
class _RecordingStates:
"""Minimal ``hass.states``: keeps what HA's entity writes."""
def __init__(self):
self.registry: dict = {}
self.written: list = []
def get(self, entity_id):
return self.registry.get(entity_id)
def async_set_internal(self, entity_id, state, attr, *args):
self.written.append((entity_id, state, dict(attr)))
def _real_ha_entity(module_name, class_name, *, options=None, entity_id=None):
"""Build a production entity under real homeassistant (no platform)."""
if str(_REPO_ROOT) not in sys.path:
sys.path.insert(0, str(_REPO_ROOT))
module = importlib.import_module(f"custom_components.nvidia_shield.{module_name}")
states = _RecordingStates()
hass = SimpleNamespace(
loop_thread_id=threading.get_ident(),
states=states,
data={},
config=SimpleNamespace(components=set()),
)
entry = SimpleNamespace(
entry_id="e1", data={}, options=dict(options or {}), title="Shield"
)
entity = getattr(module, class_name)(hass, entry)
entity.entity_id = entity_id
# HA's real translation-key paths (``unit_of_measurement``, device-class
# names) need the platform's translation data; without it a translation-keyed
# entity raises "cannot have a translation key ... before being added to the
# entity platform". PlatformData is the real container, so the entity is
# exercised with the same shape it sees in production.
from homeassistant.helpers.entity_platform import PlatformData
entity.platform_data = PlatformData(
hass, domain=module_name, platform_name="nvidia_shield"
)
# Not added through an EntityComponent: HA would warn once via
# _suggest_report_issue (which needs hass.data["integrations"]) — set the
# flag it uses to report that once instead of faking a whole platform.
entity._no_platform_reported = True
return entity, states
def _write_ha_state(entity, states):
"""Run HA's real state write and return what reached the state machine."""
entity.async_write_ha_state()
assert states.written, f"{type(entity).__name__} wrote no state"
return states.written[-1]
#: Every concrete entity class in the integration, exercised above against the
#: REAL state machine. t40: the media player's defect was a state-write crash, so
#: "the whole tree renders" is the contract under test, not a single entity.
_REAL_HA_MATRIX = (
("media_player", "NvidiaShieldMediaPlayer", None, "media_player.shield"),
("sensor", "NvidiaShieldCurrentAppSensor", None, "sensor.shield_current_app"),
(
"sensor",
"NvidiaShieldNowPlayingSensor",
{"jellyfin_entity": "media_player.jellyfin"},
"sensor.shield_now_playing",
),
("remote", "NvidiaShieldRemoteEntity", None, "remote.shield"),
("button", "NvidiaShieldPairAdbButton", None, "button.shield_pair_adb"),
)
#: Classified as an entity by the sweep, but an abstract intermediate base HA
#: never instantiates — its concrete subclasses are in the matrix.
_INTERMEDIATE_BASES = frozenset({"NvidiaShieldSensorBase"})
@pytest.mark.parametrize(
("module_name", "class_name", "options", "entity_id"), _REAL_HA_MATRIX
)
def test_entities_calculate_state_under_real_homeassistant(
module_name, class_name, options, entity_id
):
"""t40 blocker: HA's own state write must not raise for ANY entity.
``Entity.async_write_ha_state`` → ``__async_calculate_state`` merges the
extra attributes with ``attr |= ...``; the pre-fix media player raised
``TypeError: 'method' object is not iterable`` here, so it could never render.
"""
_require_installed_homeassistant()
entity, states = _real_ha_entity(
module_name, class_name, options=options, entity_id=entity_id
)
written_id, state, attrs = _write_ha_state(entity, states)
assert written_id == entity_id
assert isinstance(state, str)
assert isinstance(attrs, dict)
def test_the_media_player_extra_attributes_reach_the_state_machine():
"""The property's payload must survive HA's merge, not merely not raise."""
_require_installed_homeassistant()
entity, states = _real_ha_entity(
"media_player", "NvidiaShieldMediaPlayer", entity_id="media_player.shield"
)
_entity_id, _state, attrs = _write_ha_state(entity, states)
assert {"app_id", "app_name"} <= set(attrs), sorted(attrs)
def test_the_real_ha_matrix_covers_every_entity_class_in_the_tree():
"""A new entity class must join the matrix above, or this test fails.
The sweep classifies entities from the AST; this keeps the real-HA coverage
in step with that classification instead of silently shrinking.
"""
_require_installed_homeassistant()
classified: set[str] = set()
for path in sorted(ha_api_drift.SHIELD_DIR.glob("*.py")):
classified |= ha_api_drift.entity_class_names(
ast.parse(path.read_text(encoding="utf-8"))
)
covered = {class_name for _module, class_name, _options, _entity_id in _REAL_HA_MATRIX}
uncovered = classified - covered - _INTERMEDIATE_BASES
assert uncovered == set(), (
f"entity classes never exercised against real HA: {sorted(uncovered)} — "
f"add them to _REAL_HA_MATRIX"
)