From 200f29563aec16b6dee2d401c5071acf91e5b27c Mon Sep 17 00:00:00 2001
From: epenet <6771947+epenet@users.noreply.github.com>
Date: Wed, 30 Nov 2022 10:13:14 +0100
Subject: [PATCH] Enforce MediaPlayerState in group (#78465)

* Enforce MediaPlayerState in group

* Adjust

* Use self._attr_state

* Add warning

* Add group name to logger warning

* Remove logger warning

* Remove duplicate code

* Add type hints to tests

* Improve coverage

* Improve coverage

* suppress ValueError
---
 .../components/group/media_player.py          | 19 ++++++++-----------
 tests/components/group/test_media_player.py   | 19 +++++++++++++------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/homeassistant/components/group/media_player.py b/homeassistant/components/group/media_player.py
index e966016d63d..a349a628004 100644
--- a/homeassistant/components/group/media_player.py
+++ b/homeassistant/components/group/media_player.py
@@ -1,6 +1,7 @@
 """This platform allows several media players to be grouped into one media player."""
 from __future__ import annotations
 
+from contextlib import suppress
 from typing import Any
 
 import voluptuous as vol
@@ -108,7 +109,6 @@ class MediaPlayerGroup(MediaPlayerEntity):
     def __init__(self, unique_id: str | None, name: str, entities: list[str]) -> None:
         """Initialize a Media Group entity."""
         self._name = name
-        self._state: str | None = None
         self._attr_unique_id = unique_id
 
         self._entities = entities
@@ -206,11 +206,6 @@ class MediaPlayerGroup(MediaPlayerEntity):
         """Return the name of the entity."""
         return self._name
 
-    @property
-    def state(self) -> str | None:
-        """Return the state of the media group."""
-        return self._state
-
     @property
     def extra_state_attributes(self) -> dict:
         """Return the state attributes for the media group."""
@@ -395,15 +390,17 @@ class MediaPlayerGroup(MediaPlayerEntity):
         )
         if not valid_state:
             # Set as unknown if all members are unknown or unavailable
-            self._state = None
+            self._attr_state = None
         else:
             off_values = {MediaPlayerState.OFF, STATE_UNAVAILABLE, STATE_UNKNOWN}
-            if states.count(states[0]) == len(states):
-                self._state = states[0]
+            if states.count(single_state := states[0]) == len(states):
+                self._attr_state = None
+                with suppress(ValueError):
+                    self._attr_state = MediaPlayerState(single_state)
             elif any(state for state in states if state not in off_values):
-                self._state = MediaPlayerState.ON
+                self._attr_state = MediaPlayerState.ON
             else:
-                self._state = MediaPlayerState.OFF
+                self._attr_state = MediaPlayerState.OFF
 
         supported_features = MediaPlayerEntityFeature(0)
         if self._features[KEY_CLEAR_PLAYLIST]:
diff --git a/tests/components/group/test_media_player.py b/tests/components/group/test_media_player.py
index 42bc96dbc6e..4549a7f5fec 100644
--- a/tests/components/group/test_media_player.py
+++ b/tests/components/group/test_media_player.py
@@ -1,5 +1,5 @@
 """The tests for the Media group platform."""
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 
 import async_timeout
 import pytest
@@ -43,6 +43,7 @@ from homeassistant.const import (
     STATE_UNAVAILABLE,
     STATE_UNKNOWN,
 )
+from homeassistant.core import HomeAssistant
 from homeassistant.helpers import entity_registry as er
 from homeassistant.setup import async_setup_component
 
@@ -57,7 +58,7 @@ def media_player_media_seek_fixture():
         yield seek
 
 
-async def test_default_state(hass):
+async def test_default_state(hass: HomeAssistant) -> None:
     """Test media group default state."""
     hass.states.async_set("media_player.player_1", "on")
     await async_setup_component(
@@ -91,7 +92,7 @@ async def test_default_state(hass):
     assert entry.unique_id == "unique_identifier"
 
 
-async def test_state_reporting(hass):
+async def test_state_reporting(hass: HomeAssistant) -> None:
     """Test the state reporting.
 
     The group state is unavailable if all group members are unavailable.
@@ -170,6 +171,12 @@ async def test_state_reporting(hass):
         await hass.async_block_till_done()
         assert hass.states.get("media_player.media_group").state == STATE_OFF
 
+    # All group members in same invalid state -> unknown
+    hass.states.async_set("media_player.player_1", "invalid_state")
+    hass.states.async_set("media_player.player_2", "invalid_state")
+    await hass.async_block_till_done()
+    assert hass.states.get("media_player.media_group").state == STATE_UNKNOWN
+
     # All group members removed from the state machine -> unavailable
     hass.states.async_remove("media_player.player_1")
     hass.states.async_remove("media_player.player_2")
@@ -177,7 +184,7 @@ async def test_state_reporting(hass):
     assert hass.states.get("media_player.media_group").state == STATE_UNAVAILABLE
 
 
-async def test_supported_features(hass):
+async def test_supported_features(hass: HomeAssistant) -> None:
     """Test supported features reporting."""
     pause_play_stop = (
         MediaPlayerEntityFeature.PAUSE
@@ -241,7 +248,7 @@ async def test_supported_features(hass):
     assert state.attributes[ATTR_SUPPORTED_FEATURES] == pause_play_stop | play_media
 
 
-async def test_service_calls(hass, mock_media_seek):
+async def test_service_calls(hass: HomeAssistant, mock_media_seek: Mock) -> None:
     """Test service calls."""
     await async_setup_component(
         hass,
@@ -533,7 +540,7 @@ async def test_service_calls(hass, mock_media_seek):
     assert hass.states.get("media_player.living_room").state == STATE_OFF
 
 
-async def test_nested_group(hass):
+async def test_nested_group(hass: HomeAssistant) -> None:
     """Test nested media group."""
     await async_setup_component(
         hass,