fix(media_player): D1 — browse builds real BrowseMedia nodes, not BrowseMediaSource (t37)
`homeassistant.components.media_player` has not exported `BrowseMediaSource` for years: the class lives in `homeassistant.components.media_source.models`, where it exists to build `media_source://<domain>` ids and is the only place a `domain` parameter belongs (t30 G18's note was about THAT class). The local import therefore raised ImportError, and with it every real browse failed — while the suite stayed green because the harness stubbed a `BrowseMediaSource` that real HA does not expose from there. `async_browse_media` now builds the standard tree exactly as core's own androidtv_remote/media_player.py does: `BrowseMedia` imported from homeassistant.components.media_player at module scope, every node carrying an explicit media_content_id/media_content_type (MediaType.APP/APPS), `children_media_class=MediaClass.APP` on the directory, and no `domain`. Verified against a real install: the old import raises ImportError, the old `domain=` kwarg raises TypeError, and the new node constructs. The harness is now faithful so this cannot hide again: * `entity_stublib` exposes a `BrowseMedia` with the real keyword-only signature, `as_dict()` and `calculate_children_class()`, plus `MediaType`, and deliberately NOT `BrowseMediaSource` — so both the drifted import and the drifted kwarg fail in-suite; * the app-directory test asserts the standard shape (classes, explicit ids and types, children_media_class, the serialised payload) instead of the removed `domain` attribute; * TestFaithfulHaStubs pins the harness contract on the AST (no production module imports BrowseMediaSource) rather than on prose. Verified: 284 passed, 2 skipped (D1 stage); ruff clean. Full gate re-run on the completed branch: 289 passed / 2 skipped and 290 passed / 1 skipped.
This commit is contained in:
1 parent
5ede4fe722
commit
37b2accfda
3 files changed
+160
-28
No files matched your search
@@ -26,10 +26,13 @@ import time
|
|||||||
|
|
||||||
import voluptuous as vol
|
import voluptuous as vol
|
||||||
from homeassistant.components.media_player import (
|
from homeassistant.components.media_player import (
|
||||||
|
BrowseMedia,
|
||||||
|
MediaClass,
|
||||||
MediaPlayerDeviceClass,
|
MediaPlayerDeviceClass,
|
||||||
MediaPlayerEntity,
|
MediaPlayerEntity,
|
||||||
MediaPlayerEntityFeature,
|
MediaPlayerEntityFeature,
|
||||||
MediaPlayerState,
|
MediaPlayerState,
|
||||||
|
MediaType,
|
||||||
)
|
)
|
||||||
from homeassistant.config_entries import ConfigEntry
|
from homeassistant.config_entries import ConfigEntry
|
||||||
from homeassistant.const import STATE_PAUSED, STATE_PLAYING
|
from homeassistant.const import STATE_PAUSED, STATE_PLAYING
|
||||||
@@ -961,36 +964,40 @@ class NvidiaShieldMediaPlayer(MediaPlayerEntity):
|
|||||||
# ── BrowseMedia app directory (P4-2c) ──
|
# ── BrowseMedia app directory (P4-2c) ──
|
||||||
|
|
||||||
async def async_browse_media(self, media_content_type=None, media_content_id=None):
|
async def async_browse_media(self, media_content_type=None, media_content_id=None):
|
||||||
"""Root lists the apps map (or the curated default) → play_media(app)."""
|
"""Root lists the apps map (or the curated default) → play_media(app).
|
||||||
from homeassistant.components.media_player import (
|
|
||||||
BrowseMediaSource,
|
|
||||||
MediaClass,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
t37 D1: the standard media_player tree, exactly as core's own
|
||||||
|
androidtv_remote/media_player.py builds it — ``BrowseMedia`` from
|
||||||
|
homeassistant.components.media_player, every node carrying an explicit
|
||||||
|
media_content_id/media_content_type, and the directory naming the
|
||||||
|
class of its children. ``BrowseMediaSource`` only ever existed in
|
||||||
|
homeassistant.components.media_source.models (t30 G18's domain
|
||||||
|
parameter is ITS contract: it builds ``media_source://<domain>`` ids),
|
||||||
|
so an app-directory node must never claim one — and the import above
|
||||||
|
removed a name media_player has not exported for a long time, which
|
||||||
|
made every real browse raise.
|
||||||
|
"""
|
||||||
app_map = self._entry.options.get(CONF_APP_NAME_MAP) or DEFAULT_APP_DIRECTORY
|
app_map = self._entry.options.get(CONF_APP_NAME_MAP) or DEFAULT_APP_DIRECTORY
|
||||||
# G18 (2026-05-20 blog): BrowseMediaSource must carry the domain
|
|
||||||
# parameter so the frontend can attribute the media tree.
|
|
||||||
children = [
|
children = [
|
||||||
BrowseMediaSource(
|
BrowseMedia(
|
||||||
title=name,
|
title=name,
|
||||||
media_class=MediaClass.APP,
|
media_class=MediaClass.APP,
|
||||||
media_content_type="app",
|
media_content_type=MediaType.APP,
|
||||||
media_content_id=pkg,
|
media_content_id=pkg,
|
||||||
can_play=True,
|
can_play=True,
|
||||||
can_expand=False,
|
can_expand=False,
|
||||||
domain=DOMAIN,
|
|
||||||
)
|
)
|
||||||
for pkg, name in app_map.items()
|
for pkg, name in app_map.items()
|
||||||
]
|
]
|
||||||
return BrowseMediaSource(
|
return BrowseMedia(
|
||||||
title="Apps",
|
title="Apps",
|
||||||
media_class=MediaClass.DIRECTORY,
|
media_class=MediaClass.DIRECTORY,
|
||||||
media_content_type="apps",
|
media_content_type=MediaType.APPS,
|
||||||
media_content_id="apps",
|
media_content_id="apps",
|
||||||
|
children_media_class=MediaClass.APP,
|
||||||
can_play=False,
|
can_play=False,
|
||||||
can_expand=True,
|
can_expand=True,
|
||||||
children=children,
|
children=children,
|
||||||
domain=DOMAIN,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
# ── play_media (§7): channel digits, app/url launch ──
|
# ── play_media (§7): channel digits, app/url launch ──
|
||||||
|
|||||||
+74
-11
@@ -64,35 +64,94 @@ class MediaPlayerEntityFeature(enum.IntFlag):
|
|||||||
|
|
||||||
|
|
||||||
class MediaClass:
|
class MediaClass:
|
||||||
|
"""homeassistant.components.media_player.MediaClass (values we use)."""
|
||||||
|
|
||||||
APP = "app"
|
APP = "app"
|
||||||
DIRECTORY = "directory"
|
DIRECTORY = "directory"
|
||||||
|
|
||||||
|
|
||||||
class BrowseMediaSource:
|
class MediaType:
|
||||||
"""HA BrowseMedia shape used by the app directory tests."""
|
"""homeassistant.components.media_player.MediaType (values we use)."""
|
||||||
|
|
||||||
|
APP = "app"
|
||||||
|
APPS = "apps"
|
||||||
|
|
||||||
|
|
||||||
|
class BrowseMedia:
|
||||||
|
"""The REAL media_player.BrowseMedia shape (t37 D1).
|
||||||
|
|
||||||
|
Keyword-only with the six arguments real HA requires, the optional ones it
|
||||||
|
accepts, a faithful ``as_dict`` and — crucially — NO ``domain`` parameter:
|
||||||
|
``BrowseMediaSource`` lives in
|
||||||
|
``homeassistant.components.media_source.models`` (its contract is the
|
||||||
|
``media_source://<domain>`` id) and is NOT importable from
|
||||||
|
``homeassistant.components.media_player`` at all. The old import and the
|
||||||
|
old ``domain=`` kwarg therefore fail IN-SUITE instead of hiding behind a
|
||||||
|
permissive stub.
|
||||||
|
"""
|
||||||
|
|
||||||
def __init__(
|
def __init__(
|
||||||
self,
|
self,
|
||||||
*,
|
*,
|
||||||
title,
|
|
||||||
media_class,
|
media_class,
|
||||||
media_content_type,
|
|
||||||
media_content_id,
|
media_content_id,
|
||||||
|
media_content_type,
|
||||||
|
title,
|
||||||
can_play,
|
can_play,
|
||||||
can_expand,
|
can_expand,
|
||||||
domain=None, # t30 G18 (2026-05-20 blog contract)
|
|
||||||
children=None,
|
children=None,
|
||||||
|
children_media_class=None,
|
||||||
thumbnail=None,
|
thumbnail=None,
|
||||||
|
not_shown=0,
|
||||||
|
can_search=False,
|
||||||
|
search_media_classes=None,
|
||||||
):
|
):
|
||||||
self.title = title
|
|
||||||
self.media_class = media_class
|
self.media_class = media_class
|
||||||
self.media_content_type = media_content_type
|
|
||||||
self.media_content_id = media_content_id
|
self.media_content_id = media_content_id
|
||||||
|
self.media_content_type = media_content_type
|
||||||
|
self.title = title
|
||||||
self.can_play = can_play
|
self.can_play = can_play
|
||||||
self.can_expand = can_expand
|
self.can_expand = can_expand
|
||||||
self.domain = domain
|
self.children = children
|
||||||
self.children = children or []
|
self.children_media_class = children_media_class
|
||||||
self.thumbnail = thumbnail
|
self.thumbnail = thumbnail
|
||||||
|
self.not_shown = not_shown
|
||||||
|
self.can_search = can_search
|
||||||
|
self.search_media_classes = search_media_classes
|
||||||
|
|
||||||
|
def as_dict(self, *, parent=True):
|
||||||
|
"""Real payload conversion — what the frontend actually consumes."""
|
||||||
|
if self.children_media_class is None and self.children:
|
||||||
|
self.calculate_children_class()
|
||||||
|
response = {
|
||||||
|
"title": self.title,
|
||||||
|
"media_class": self.media_class,
|
||||||
|
"media_content_type": self.media_content_type,
|
||||||
|
"media_content_id": self.media_content_id,
|
||||||
|
"children_media_class": self.children_media_class,
|
||||||
|
"can_play": self.can_play,
|
||||||
|
"can_expand": self.can_expand,
|
||||||
|
"can_search": self.can_search,
|
||||||
|
"search_media_classes": self.search_media_classes,
|
||||||
|
"thumbnail": self.thumbnail,
|
||||||
|
}
|
||||||
|
if not parent:
|
||||||
|
return response
|
||||||
|
response["not_shown"] = self.not_shown
|
||||||
|
response["children"] = (
|
||||||
|
[child.as_dict(parent=False) for child in self.children]
|
||||||
|
if self.children
|
||||||
|
else []
|
||||||
|
)
|
||||||
|
return response
|
||||||
|
|
||||||
|
def calculate_children_class(self):
|
||||||
|
"""Real helper: the children's common class, else DIRECTORY."""
|
||||||
|
self.children_media_class = MediaClass.DIRECTORY
|
||||||
|
assert self.children is not None
|
||||||
|
proposed_class = self.children[0].media_class
|
||||||
|
if all(child.media_class == proposed_class for child in self.children):
|
||||||
|
self.children_media_class = proposed_class
|
||||||
|
|
||||||
|
|
||||||
class MediaEntityManager:
|
class MediaEntityManager:
|
||||||
@@ -278,9 +337,12 @@ def install() -> None:
|
|||||||
mp.MediaPlayerEntity = MediaPlayerEntity
|
mp.MediaPlayerEntity = MediaPlayerEntity
|
||||||
mp.MediaPlayerDeviceClass = types.SimpleNamespace(TV="tv")
|
mp.MediaPlayerDeviceClass = types.SimpleNamespace(TV="tv")
|
||||||
mp.MediaPlayerEntityFeature = MediaPlayerEntityFeature
|
mp.MediaPlayerEntityFeature = MediaPlayerEntityFeature
|
||||||
mp.BrowseMedia = BrowseMediaSource # factory used as a constructor
|
mp.BrowseMedia = BrowseMedia
|
||||||
mp.BrowseMediaSource = BrowseMediaSource
|
|
||||||
mp.MediaClass = MediaClass
|
mp.MediaClass = MediaClass
|
||||||
|
mp.MediaType = MediaType
|
||||||
|
# NOTE (t37 D1): homeassistant.components.media_player exposes NO
|
||||||
|
# BrowseMediaSource — deliberately not stubbed, so the drifted import
|
||||||
|
# fails here exactly as it does in a real install.
|
||||||
mp.MediaPlayerState = MediaPlayerState
|
mp.MediaPlayerState = MediaPlayerState
|
||||||
sys.modules["homeassistant.components.media_player"] = mp
|
sys.modules["homeassistant.components.media_player"] = mp
|
||||||
|
|
||||||
@@ -347,6 +409,7 @@ def install() -> None:
|
|||||||
entity_platform = types.ModuleType("homeassistant.helpers.entity_platform")
|
entity_platform = types.ModuleType("homeassistant.helpers.entity_platform")
|
||||||
entity_platform.AddEntitiesCallback = object
|
entity_platform.AddEntitiesCallback = object
|
||||||
helpers.entity_platform = entity_platform
|
helpers.entity_platform = entity_platform
|
||||||
|
|
||||||
sys.modules["homeassistant.helpers.event"] = event
|
sys.modules["homeassistant.helpers.event"] = event
|
||||||
sys.modules["homeassistant.helpers.entity"] = entity
|
sys.modules["homeassistant.helpers.entity"] = entity
|
||||||
sys.modules["homeassistant.helpers.entity_platform"] = entity_platform
|
sys.modules["homeassistant.helpers.entity_platform"] = entity_platform
|
||||||
|
|||||||
@@ -6,11 +6,13 @@ legacy external entity, plus the §7 channel digit sequence (protocol digits,
|
|||||||
layer marks which tier served each command.
|
layer marks which tier served each command.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import ast
|
||||||
import asyncio
|
import asyncio
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
import entity_stublib
|
import entity_stublib
|
||||||
import pytest
|
import pytest
|
||||||
|
from conftest import _SHIELD_DIR
|
||||||
from entity_stublib import ENTITIES_STUB_NAMES, install, load_entity_modules
|
from entity_stublib import ENTITIES_STUB_NAMES, install, load_entity_modules
|
||||||
from nsh.connection import ShieldConnectionManager
|
from nsh.connection import ShieldConnectionManager
|
||||||
from nsh.const import TIVIMATE_PACKAGE
|
from nsh.const import TIVIMATE_PACKAGE
|
||||||
@@ -1014,6 +1016,52 @@ class TestEntryDiagnostics:
|
|||||||
assert payload["transport"]["protocol"]["active"] is not None
|
assert payload["transport"]["protocol"]["active"] is not None
|
||||||
|
|
||||||
|
|
||||||
|
class TestFaithfulHaStubs:
|
||||||
|
"""t37: the harness mirrors WHERE real HA exposes its API.
|
||||||
|
|
||||||
|
A regression of the exact drifts this task repairs must fail IN-SUITE, not
|
||||||
|
hide behind a stub that exposes anything anywhere.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def test_browse_media_source_is_not_importable_from_media_player(self):
|
||||||
|
import homeassistant.components.media_player as mp
|
||||||
|
|
||||||
|
assert isinstance(mp.BrowseMedia, type) # the real node class
|
||||||
|
assert not hasattr(mp, "BrowseMediaSource")
|
||||||
|
with pytest.raises(ImportError):
|
||||||
|
# the drifted import must fail here exactly as in a real install
|
||||||
|
from homeassistant.components.media_player import BrowseMediaSource # noqa: F401
|
||||||
|
|
||||||
|
def test_no_production_module_imports_browse_media_source(self):
|
||||||
|
"""t37 D1: the drift is an IMPORT — checked on the AST, not on prose."""
|
||||||
|
offenders = []
|
||||||
|
for path in sorted(_SHIELD_DIR.glob("*.py")):
|
||||||
|
tree = ast.parse(path.read_text(encoding="utf-8"))
|
||||||
|
for node in ast.walk(tree):
|
||||||
|
if isinstance(node, ast.ImportFrom) and any(
|
||||||
|
alias.name == "BrowseMediaSource" for alias in node.names
|
||||||
|
):
|
||||||
|
offenders.append(f"{path.name}:{node.lineno} imports it")
|
||||||
|
if isinstance(node, ast.Import) and any(
|
||||||
|
alias.name.endswith("BrowseMediaSource") for alias in node.names
|
||||||
|
):
|
||||||
|
offenders.append(f"{path.name}:{node.lineno} imports it")
|
||||||
|
assert offenders == []
|
||||||
|
|
||||||
|
def test_the_browse_nodes_refuse_the_media_source_domain_kwarg(self):
|
||||||
|
import homeassistant.components.media_player as mp
|
||||||
|
|
||||||
|
with pytest.raises(TypeError):
|
||||||
|
mp.BrowseMedia(
|
||||||
|
title="x",
|
||||||
|
media_class=mp.MediaClass.APP,
|
||||||
|
media_content_type=mp.MediaType.APP,
|
||||||
|
media_content_id="pkg",
|
||||||
|
can_play=True,
|
||||||
|
can_expand=False,
|
||||||
|
domain="nvidia_shield", # G18 keyword of BrowseMediaSource only
|
||||||
|
)
|
||||||
|
|
||||||
class TestBrowseMediaDirectory:
|
class TestBrowseMediaDirectory:
|
||||||
"""P4-2c: the app directory root and its launch path."""
|
"""P4-2c: the app directory root and its launch path."""
|
||||||
|
|
||||||
@@ -1034,10 +1082,24 @@ class TestBrowseMediaDirectory:
|
|||||||
("com.netflix.ninja", "Netflix"),
|
("com.netflix.ninja", "Netflix"),
|
||||||
]
|
]
|
||||||
assert all(c.media_content_type == "app" for c in root.children)
|
assert all(c.media_content_type == "app" for c in root.children)
|
||||||
# t30 G18 (2026-05-20 blog): the domain parameter is present at
|
# t37 D1: a standard media_player.BrowseMedia tree — explicit
|
||||||
# every level of the tree
|
# media_content_id/-type at every level, the directory naming the class
|
||||||
assert root.domain == "nvidia_shield"
|
# of its children, and NO media_source `domain` (that parameter belongs
|
||||||
assert all(c.domain == "nvidia_shield" for c in root.children)
|
# to media_source.models.BrowseMediaSource, which also builds the
|
||||||
|
# media_source:// ids).
|
||||||
|
assert root.media_class == "directory"
|
||||||
|
assert root.media_content_id == "apps"
|
||||||
|
assert root.media_content_type == "apps"
|
||||||
|
assert root.children_media_class == "app"
|
||||||
|
assert not hasattr(root, "domain")
|
||||||
|
assert [c.media_class for c in root.children] == ["app", "app"]
|
||||||
|
# ...and it serialises into the payload the frontend consumes
|
||||||
|
payload = root.as_dict()
|
||||||
|
assert payload["media_content_id"] == "apps"
|
||||||
|
assert payload["children_media_class"] == "app"
|
||||||
|
assert payload["children"][0]["media_content_id"] == "ar.tvplayer.tv"
|
||||||
|
assert payload["children"][0]["media_content_type"] == "app"
|
||||||
|
assert payload["children"][0]["can_play"] is True
|
||||||
|
|
||||||
async def test_root_falls_back_to_the_curated_default(self, modules):
|
async def test_root_falls_back_to_the_curated_default(self, modules):
|
||||||
player, _s, _i = make_player(modules) # no apps map configured
|
player, _s, _i = make_player(modules) # no apps map configured
|
||||||
|
|||||||
Reference in new issue
Block a user