Improve Spotify mock (#127825)

* Improve Spotify mock

* Fix comments

* Fix comments

* Fix comments

* Fix comments

* Fix comments

* Fix comments

* Fix comments

* Fix comments

tests/components/spotify/__init__.py

@@ -1 +1,13 @@
-"""Tests for the Spotify integration."""
+"""Tests for the Spotify component."""
+
+from homeassistant.core import HomeAssistant
+
+from tests.common import MockConfigEntry
+
+
+async def setup_integration(hass: HomeAssistant, config_entry: MockConfigEntry) -> None:
+    """Set up the component."""
+    config_entry.add_to_hass(hass)
+
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    await hass.async_block_till_done()

tests/components/spotify/conftest.py

@@ -1,7 +1,7 @@
 """Common test fixtures."""
 
 from collections.abc import Generator
-from typing import Any
+import time
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -14,115 +14,69 @@ from homeassistant.components.spotify.const import DOMAIN, SPOTIFY_SCOPES
 from homeassistant.core import HomeAssistant
 from homeassistant.setup import async_setup_component
 
-from tests.common import MockConfigEntry
+from tests.common import MockConfigEntry, load_json_value_fixture
 
 SCOPES = " ".join(SPOTIFY_SCOPES)
 
 
+@pytest.fixture(name="expires_at")
+def mock_expires_at() -> int:
+    """Fixture to set the oauth token expiration time."""
+    return time.time() + 3600
+
+
 @pytest.fixture
-def mock_config_entry_1() -> MockConfigEntry:
+def mock_config_entry(expires_at: int) -> MockConfigEntry:
-    """Mock a config entry with an upper case entry id."""
+    """Create Spotify entry in Home Assistant."""
     return MockConfigEntry(
         domain=DOMAIN,
         title="spotify_1",
+        unique_id="fake_id",
         data={
-            "auth_implementation": "spotify_c95e4090d4d3438b922331e7428f8171",
+            "auth_implementation": DOMAIN,
             "token": {
-                "access_token": "AccessToken",
+                "access_token": "mock-access-token",
-                "token_type": "Bearer",
+                "refresh_token": "mock-refresh-token",
-                "expires_in": 3600,
+                "expires_at": expires_at,
-                "refresh_token": "RefreshToken",
                 "scope": SCOPES,
-                "expires_at": 1724198975.8829377,
             },
-            "id": "32oesphrnacjcf7vw5bf6odx3oiu",
+            "id": "fake_id",
             "name": "spotify_account_1",
         },
-        unique_id="84fce612f5b8",
         entry_id="01J5TX5A0FF6G5V0QJX6HBC94T",
     )
 
 
 @pytest.fixture
-def mock_config_entry_2() -> MockConfigEntry:
+async def setup_credentials(hass: HomeAssistant) -> None:
-    """Mock a config entry with a lower case entry id."""
+    """Fixture to setup credentials."""
-    return MockConfigEntry(
+    assert await async_setup_component(hass, "application_credentials", {})
-        domain=DOMAIN,
-        title="spotify_2",
-        data={
-            "auth_implementation": "spotify_c95e4090d4d3438b922331e7428f8171",
-            "token": {
-                "access_token": "AccessToken",
-                "token_type": "Bearer",
-                "expires_in": 3600,
-                "refresh_token": "RefreshToken",
-                "scope": SCOPES,
-                "expires_at": 1724198975.8829377,
-            },
-            "id": "55oesphrnacjcf7vw5bf6odx3oiu",
-            "name": "spotify_account_2",
-        },
-        unique_id="99fce612f5b8",
-        entry_id="32oesphrnacjcf7vw5bf6odx3",
-    )
-
-
-@pytest.fixture
-def spotify_playlists() -> dict[str, Any]:
-    """Mock the return from getting a list of playlists."""
-    return {
-        "href": "https://api.spotify.com/v1/users/31oesphrnacjcf7vw5bf6odx3oiu/playlists?offset=0&limit=48",
-        "limit": 48,
-        "next": None,
-        "offset": 0,
-        "previous": None,
-        "total": 1,
-        "items": [
-            {
-                "collaborative": False,
-                "description": "",
-                "id": "unique_identifier_00",
-                "name": "Playlist1",
-                "type": "playlist",
-                "uri": "spotify:playlist:unique_identifier_00",
-            }
-        ],
-    }
-
-
-@pytest.fixture
-def spotify_mock(spotify_playlists: dict[str, Any]) -> Generator[MagicMock]:
-    """Mock the Spotify API."""
-    with patch("homeassistant.components.spotify.Spotify") as spotify_mock:
-        mock = MagicMock()
-        mock.current_user_playlists.return_value = spotify_playlists
-        spotify_mock.return_value = mock
-        yield spotify_mock
-
-
-@pytest.fixture
-async def spotify_setup(
-    hass: HomeAssistant,
-    spotify_mock: MagicMock,
-    mock_config_entry_1: MockConfigEntry,
-    mock_config_entry_2: MockConfigEntry,
-):
-    """Set up the spotify integration."""
-    with patch(
-        "homeassistant.components.spotify.OAuth2Session.async_ensure_token_valid"
-    ):
-        await async_setup_component(hass, "application_credentials", {})
-        await hass.async_block_till_done()
     await async_import_client_credential(
         hass,
         DOMAIN,
         ClientCredential("CLIENT_ID", "CLIENT_SECRET"),
-            "spotify_c95e4090d4d3438b922331e7428f8171",
+        DOMAIN,
     )
-        await hass.async_block_till_done()
+
-        mock_config_entry_1.add_to_hass(hass)
+
-        await hass.config_entries.async_setup(mock_config_entry_1.entry_id)
+@pytest.fixture
-        mock_config_entry_2.add_to_hass(hass)
+def mock_spotify() -> Generator[MagicMock]:
-        await hass.config_entries.async_setup(mock_config_entry_2.entry_id)
+    """Mock the Spotify API."""
-        await hass.async_block_till_done(wait_background_tasks=True)
+    with (
-        yield
+        patch(
+            "homeassistant.components.spotify.Spotify",
+            autospec=True,
+        ) as spotify_mock,
+        patch(
+            "homeassistant.components.spotify.config_flow.Spotify",
+            new=spotify_mock,
+        ),
+    ):
+        client = spotify_mock.return_value
+        client.current_user_playlists.return_value = load_json_value_fixture(
+            "current_user_playlist.json", DOMAIN
+        )
+        client.current_user.return_value = load_json_value_fixture(
+            "current_user.json", DOMAIN
+        )
+        yield spotify_mock

tests/components/spotify/fixtures/current_user.json

@@ -0,0 +1,4 @@
+{
+  "id": "fake_id",
+  "display_name": "frenck"
+}

tests/components/spotify/fixtures/current_user_playlist.json

@@ -0,0 +1,18 @@
+{
+  "href": "https://api.spotify.com/v1/users/31oesphrnacjcf7vw5bf6odx3oiu/playlists?offset=0&limit=48",
+  "limit": 48,
+  "next": null,
+  "offset": 0,
+  "previous": null,
+  "total": 1,
+  "items": [
+    {
+      "collaborative": null,
+      "description": "",
+      "id": "unique_identifier_00",
+      "name": "Playlist1",
+      "type": "playlist",
+      "uri": "spotify:playlist:unique_identifier_00"
+    }
+  ]
+}

tests/components/spotify/snapshots/test_media_browser.ambr

@@ -124,31 +124,6 @@
     'title': 'Media Library',
   })
 # ---
-# name: test_browse_media_playlists
-  dict({
-    'can_expand': True,
-    'can_play': False,
-    'children': list([
-      dict({
-        'can_expand': True,
-        'can_play': True,
-        'children_media_class': <MediaClass.TRACK: 'track'>,
-        'media_class': <MediaClass.PLAYLIST: 'playlist'>,
-        'media_content_id': 'spotify://01j5tx5a0ff6g5v0qjx6hbc94t/spotify:playlist:unique_identifier_00',
-        'media_content_type': 'spotify://playlist',
-        'thumbnail': None,
-        'title': 'Playlist1',
-      }),
-    ]),
-    'children_media_class': <MediaClass.PLAYLIST: 'playlist'>,
-    'media_class': <MediaClass.DIRECTORY: 'directory'>,
-    'media_content_id': 'spotify://01j5tx5a0ff6g5v0qjx6hbc94t/current_user_playlists',
-    'media_content_type': 'spotify://current_user_playlists',
-    'not_shown': 0,
-    'thumbnail': None,
-    'title': 'Playlists',
-  })
-# ---
 # name: test_browse_media_playlists[01J5TX5A0FF6G5V0QJX6HBC94T]
   dict({
     'can_expand': True,

tests/components/spotify/test_config_flow.py

@@ -2,22 +2,17 @@
 
 from http import HTTPStatus
 from ipaddress import ip_address
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 import pytest
 from spotipy import SpotifyException
 
 from homeassistant.components import zeroconf
-from homeassistant.components.application_credentials import (
-    ClientCredential,
-    async_import_client_credential,
-)
 from homeassistant.components.spotify.const import DOMAIN
 from homeassistant.config_entries import SOURCE_USER, SOURCE_ZEROCONF
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
 from homeassistant.helpers import config_entry_oauth2_flow
-from homeassistant.setup import async_setup_component
 
 from tests.common import MockConfigEntry
 from tests.test_util.aiohttp import AiohttpClientMocker
@@ -34,19 +29,6 @@ BLANK_ZEROCONF_INFO = zeroconf.ZeroconfServiceInfo(
 )
 
 
-@pytest.fixture
-async def component_setup(hass: HomeAssistant) -> None:
-    """Fixture for setting up the integration."""
-    result = await async_setup_component(hass, DOMAIN, {})
-    await hass.async_block_till_done()
-
-    await async_import_client_credential(
-        hass, DOMAIN, ClientCredential("client", "secret"), "cred"
-    )
-
-    assert result
-
-
 async def test_abort_if_no_configuration(hass: HomeAssistant) -> None:
     """Check flow aborts when no configuration is present."""
     result = await hass.config_entries.flow.async_init(
@@ -77,11 +59,12 @@ async def test_zeroconf_abort_if_existing_entry(hass: HomeAssistant) -> None:
 
 
 @pytest.mark.usefixtures("current_request_with_host")
+@pytest.mark.usefixtures("setup_credentials")
 async def test_full_flow(
     hass: HomeAssistant,
-    component_setup,
     hass_client_no_auth: ClientSessionGenerator,
     aioclient_mock: AiohttpClientMocker,
+    mock_spotify: MagicMock,
 ) -> None:
     """Check a full flow."""
     result = await hass.config_entries.flow.async_init(
@@ -99,7 +82,7 @@ async def test_full_flow(
     assert result["type"] is FlowResultType.EXTERNAL_STEP
     assert result["url"] == (
         "https://accounts.spotify.com/authorize"
-        "?response_type=code&client_id=client"
+        "?response_type=code&client_id=CLIENT_ID"
         "&redirect_uri=https://example.com/auth/external/callback"
         f"&state={state}"
         "&scope=user-modify-playback-state,user-read-playback-state,user-read-private,"
@@ -112,6 +95,7 @@ async def test_full_flow(
     assert resp.status == HTTPStatus.OK
     assert resp.headers["content-type"] == "text/html; charset=utf-8"
 
+    aioclient_mock.clear_requests()
     aioclient_mock.post(
         "https://accounts.spotify.com/api/token",
         json={
@@ -124,15 +108,12 @@ async def test_full_flow(
 
     with (
         patch("homeassistant.components.spotify.async_setup_entry", return_value=True),
-        patch("homeassistant.components.spotify.config_flow.Spotify") as spotify_mock,
     ):
-        spotify_mock.return_value.current_user.return_value = {
-            "id": "fake_id",
-            "display_name": "frenck",
-        }
         result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
-    assert result["data"]["auth_implementation"] == "cred"
+    assert len(hass.config_entries.async_entries(DOMAIN)) == 1, result
+
+    assert result["type"] is FlowResultType.CREATE_ENTRY
     result["data"]["token"].pop("expires_at")
     assert result["data"]["name"] == "frenck"
     assert result["data"]["token"] == {
@@ -144,11 +125,12 @@ async def test_full_flow(
 
 
 @pytest.mark.usefixtures("current_request_with_host")
+@pytest.mark.usefixtures("setup_credentials")
 async def test_abort_if_spotify_error(
     hass: HomeAssistant,
-    component_setup,
     hass_client_no_auth: ClientSessionGenerator,
     aioclient_mock: AiohttpClientMocker,
+    mock_spotify: MagicMock,
 ) -> None:
     """Check Spotify errors causes flow to abort."""
     result = await hass.config_entries.flow.async_init(
@@ -175,10 +157,10 @@ async def test_abort_if_spotify_error(
         },
     )
 
-    with patch(
+    mock_spotify.return_value.current_user.side_effect = SpotifyException(
-        "homeassistant.components.spotify.config_flow.Spotify.current_user",
+        400, -1, "message"
-        side_effect=SpotifyException(400, -1, "message"),
+    )
-    ):
+
     result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
     assert result["type"] is FlowResultType.ABORT
@@ -186,27 +168,23 @@ async def test_abort_if_spotify_error(
 
 
 @pytest.mark.usefixtures("current_request_with_host")
+@pytest.mark.usefixtures("setup_credentials")
 async def test_reauthentication(
     hass: HomeAssistant,
-    component_setup,
     hass_client_no_auth: ClientSessionGenerator,
     aioclient_mock: AiohttpClientMocker,
+    mock_spotify: MagicMock,
+    mock_config_entry: MockConfigEntry,
 ) -> None:
     """Test Spotify reauthentication."""
-    old_entry = MockConfigEntry(
+    mock_config_entry.add_to_hass(hass)
-        domain=DOMAIN,
-        unique_id=123,
-        version=1,
-        data={"id": "frenck", "auth_implementation": "cred"},
-    )
-    old_entry.add_to_hass(hass)
 
-    result = await old_entry.start_reauth_flow(hass)
+    result = await mock_config_entry.start_reauth_flow(hass)
 
-    flows = hass.config_entries.flow.async_progress()
+    assert result["type"] is FlowResultType.FORM
-    assert len(flows) == 1
+    assert result["step_id"] == "reauth_confirm"
 
-    result = await hass.config_entries.flow.async_configure(flows[0]["flow_id"], {})
+    result = await hass.config_entries.flow.async_configure(result["flow_id"], {})
 
     state = config_entry_oauth2_flow._encode_jwt(
         hass,
@@ -221,8 +199,8 @@ async def test_reauthentication(
     aioclient_mock.post(
         "https://accounts.spotify.com/api/token",
         json={
-            "refresh_token": "mock-refresh-token",
+            "refresh_token": "new-refresh-token",
-            "access_token": "mock-access-token",
+            "access_token": "mew-access-token",
             "type": "Bearer",
             "expires_in": 60,
         },
@@ -230,42 +208,39 @@ async def test_reauthentication(
 
     with (
         patch("homeassistant.components.spotify.async_setup_entry", return_value=True),
-        patch("homeassistant.components.spotify.config_flow.Spotify") as spotify_mock,
     ):
-        spotify_mock.return_value.current_user.return_value = {"id": "frenck"}
         result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
-    updated_data = old_entry.data.copy()
+    assert result["type"] is FlowResultType.ABORT
-    assert updated_data["auth_implementation"] == "cred"
+    assert result["reason"] == "reauth_successful"
-    updated_data["token"].pop("expires_at")
+
-    assert updated_data["token"] == {
+    mock_config_entry.data["token"].pop("expires_at")
-        "refresh_token": "mock-refresh-token",
+    assert mock_config_entry.data["token"] == {
-        "access_token": "mock-access-token",
+        "refresh_token": "new-refresh-token",
+        "access_token": "mew-access-token",
         "type": "Bearer",
         "expires_in": 60,
     }
 
 
 @pytest.mark.usefixtures("current_request_with_host")
+@pytest.mark.usefixtures("setup_credentials")
 async def test_reauth_account_mismatch(
     hass: HomeAssistant,
-    component_setup,
     hass_client_no_auth: ClientSessionGenerator,
     aioclient_mock: AiohttpClientMocker,
+    mock_spotify: MagicMock,
+    mock_config_entry: MockConfigEntry,
 ) -> None:
     """Test Spotify reauthentication with different account."""
-    old_entry = MockConfigEntry(
+    mock_config_entry.add_to_hass(hass)
-        domain=DOMAIN,
-        unique_id=123,
-        version=1,
-        data={"id": "frenck", "auth_implementation": "cred"},
-    )
-    old_entry.add_to_hass(hass)
 
-    result = await old_entry.start_reauth_flow(hass)
+    result = await mock_config_entry.start_reauth_flow(hass)
 
-    flows = hass.config_entries.flow.async_progress()
+    assert result["type"] is FlowResultType.FORM
-    result = await hass.config_entries.flow.async_configure(flows[0]["flow_id"], {})
+    assert result["step_id"] == "reauth_confirm"
+
+    result = await hass.config_entries.flow.async_configure(result["flow_id"], {})
 
     state = config_entry_oauth2_flow._encode_jwt(
         hass,
@@ -287,8 +262,7 @@ async def test_reauth_account_mismatch(
         },
     )
 
-    with patch("homeassistant.components.spotify.config_flow.Spotify") as spotify_mock:
+    mock_spotify.return_value.current_user.return_value["id"] = "new_user_id"
-        spotify_mock.return_value.current_user.return_value = {"id": "fake_id"}
     result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
     assert result["type"] is FlowResultType.ABORT

tests/components/spotify/test_media_browser.py

@@ -1,44 +1,65 @@
 """Test the media browser interface."""
 
+from unittest.mock import MagicMock
+
 import pytest
 from syrupy import SnapshotAssertion
 
 from homeassistant.components.spotify import DOMAIN
 from homeassistant.components.spotify.browse_media import async_browse_media
+from homeassistant.const import CONF_ID
 from homeassistant.core import HomeAssistant
-from homeassistant.setup import async_setup_component
+
+from . import setup_integration
+from .conftest import SCOPES
 
 from tests.common import MockConfigEntry
 
 
-async def setup_integration(hass: HomeAssistant, config_entry: MockConfigEntry) -> None:
+@pytest.mark.usefixtures("setup_credentials")
-    """Fixture for setting up the component."""
-    config_entry.add_to_hass(hass)
-
-    await hass.config_entries.async_setup(config_entry.entry_id)
-    await hass.async_block_till_done(wait_background_tasks=True)
-    await async_setup_component(hass, DOMAIN, {})
-    await hass.async_block_till_done(wait_background_tasks=True)
-
-
 async def test_browse_media_root(
     hass: HomeAssistant,
+    mock_spotify: MagicMock,
+    mock_config_entry: MockConfigEntry,
     snapshot: SnapshotAssertion,
-    spotify_setup,
+    expires_at: int,
 ) -> None:
     """Test browsing the root."""
+    await setup_integration(hass, mock_config_entry)
+    # We add a second config entry to test that lowercase entry_ids also work
+    config_entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="spotify_2",
+        unique_id="second_fake_id",
+        data={
+            CONF_ID: "second_fake_id",
+            "name": "spotify_account_2",
+            "auth_implementation": DOMAIN,
+            "token": {
+                "access_token": "mock-access-token",
+                "refresh_token": "mock-refresh-token",
+                "expires_at": expires_at,
+                "scope": SCOPES,
+            },
+        },
+        entry_id="32oesphrnacjcf7vw5bf6odx3",
+    )
+    await setup_integration(hass, config_entry)
     response = await async_browse_media(hass, None, None)
     assert response.as_dict() == snapshot
 
 
+@pytest.mark.usefixtures("setup_credentials")
 async def test_browse_media_categories(
     hass: HomeAssistant,
+    mock_spotify: MagicMock,
+    mock_config_entry: MockConfigEntry,
     snapshot: SnapshotAssertion,
-    spotify_setup,
 ) -> None:
     """Test browsing categories."""
+    await setup_integration(hass, mock_config_entry)
     response = await async_browse_media(
-        hass, "spotify://library", "spotify://01J5TX5A0FF6G5V0QJX6HBC94T"
+        hass, "spotify://library", f"spotify://{mock_config_entry.entry_id}"
     )
     assert response.as_dict() == snapshot
 
@@ -46,13 +67,31 @@ async def test_browse_media_categories(
 @pytest.mark.parametrize(
     ("config_entry_id"), [("01J5TX5A0FF6G5V0QJX6HBC94T"), ("32oesphrnacjcf7vw5bf6odx3")]
 )
+@pytest.mark.usefixtures("setup_credentials")
 async def test_browse_media_playlists(
     hass: HomeAssistant,
-    snapshot: SnapshotAssertion,
     config_entry_id: str,
-    spotify_setup,
+    mock_spotify: MagicMock,
+    snapshot: SnapshotAssertion,
+    expires_at: int,
 ) -> None:
     """Test browsing playlists for the two config entries."""
+    mock_config_entry = MockConfigEntry(
+        domain=DOMAIN,
+        title="Spotify",
+        unique_id="1112264649",
+        data={
+            "auth_implementation": DOMAIN,
+            "token": {
+                "access_token": "mock-access-token",
+                "refresh_token": "mock-refresh-token",
+                "expires_at": expires_at,
+                "scope": SCOPES,
+            },
+        },
+        entry_id=config_entry_id,
+    )
+    await setup_integration(hass, mock_config_entry)
     response = await async_browse_media(
         hass,
         "spotify://current_user_playlists",