diff --git a/tests/ha_api_drift.py b/tests/ha_api_drift.py index e958b0a..d28bb31 100644 --- a/tests/ha_api_drift.py +++ b/tests/ha_api_drift.py @@ -30,7 +30,14 @@ dependency against an INSTALLED homeassistant: must exist in the real signature — the ``options_updates`` class of bug; 3. every ``self.()`` 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.``) — 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 diff --git a/tests/test_entity_contract.py b/tests/test_entity_contract.py new file mode 100644 index 0000000..05a0176 --- /dev/null +++ b/tests/test_entity_contract.py @@ -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")) == [] diff --git a/tests/test_ha_api_drift.py b/tests/test_ha_api_drift.py index d8c0fee..befefdf 100644 --- a/tests/test_ha_api_drift.py +++ b/tests/test_ha_api_drift.py @@ -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" + )