From f0363ac221bef5fc07af8291e24ac10643b5cb99 Mon Sep 17 00:00:00 2001 From: Joost Lekkerkerker Date: Mon, 7 Oct 2024 17:36:39 +0200 Subject: [PATCH] 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 | 14 +- tests/components/spotify/conftest.py | 128 ++++++------------ .../spotify/fixtures/current_user.json | 4 + .../fixtures/current_user_playlist.json | 18 +++ .../spotify/snapshots/test_media_browser.ambr | 25 ---- tests/components/spotify/test_config_flow.py | 112 ++++++--------- .../components/spotify/test_media_browser.py | 71 +++++++--- 7 files changed, 174 insertions(+), 198 deletions(-) create mode 100644 tests/components/spotify/fixtures/current_user.json create mode 100644 tests/components/spotify/fixtures/current_user_playlist.json diff --git a/tests/components/spotify/__init__.py b/tests/components/spotify/__init__.py index 51e3404d3ad..4730530b4f3 100644 --- a/tests/components/spotify/__init__.py +++ b/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() diff --git a/tests/components/spotify/conftest.py b/tests/components/spotify/conftest.py index 722851d097c..58100ee676f 100644 --- a/tests/components/spotify/conftest.py +++ b/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: - """Mock a config entry with an upper case entry id.""" +def mock_config_entry(expires_at: int) -> MockConfigEntry: + """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", - "token_type": "Bearer", - "expires_in": 3600, - "refresh_token": "RefreshToken", + "access_token": "mock-access-token", + "refresh_token": "mock-refresh-token", + "expires_at": expires_at, "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: - """Mock a config entry with a lower case entry id.""" - return MockConfigEntry( - 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", +async def setup_credentials(hass: HomeAssistant) -> None: + """Fixture to setup credentials.""" + assert await async_setup_component(hass, "application_credentials", {}) + await async_import_client_credential( + hass, + DOMAIN, + ClientCredential("CLIENT_ID", "CLIENT_SECRET"), + DOMAIN, ) @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]: +def mock_spotify() -> 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" + with ( + patch( + "homeassistant.components.spotify.Spotify", + autospec=True, + ) as spotify_mock, + patch( + "homeassistant.components.spotify.config_flow.Spotify", + new=spotify_mock, + ), ): - 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", + client = spotify_mock.return_value + client.current_user_playlists.return_value = load_json_value_fixture( + "current_user_playlist.json", 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) - mock_config_entry_2.add_to_hass(hass) - await hass.config_entries.async_setup(mock_config_entry_2.entry_id) - await hass.async_block_till_done(wait_background_tasks=True) - yield + client.current_user.return_value = load_json_value_fixture( + "current_user.json", DOMAIN + ) + yield spotify_mock diff --git a/tests/components/spotify/fixtures/current_user.json b/tests/components/spotify/fixtures/current_user.json new file mode 100644 index 00000000000..4684af40f58 --- /dev/null +++ b/tests/components/spotify/fixtures/current_user.json @@ -0,0 +1,4 @@ +{ + "id": "fake_id", + "display_name": "frenck" +} diff --git a/tests/components/spotify/fixtures/current_user_playlist.json b/tests/components/spotify/fixtures/current_user_playlist.json new file mode 100644 index 00000000000..e9f01cbbc31 --- /dev/null +++ b/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" + } + ] +} diff --git a/tests/components/spotify/snapshots/test_media_browser.ambr b/tests/components/spotify/snapshots/test_media_browser.ambr index 4236fcb2e79..7457d31e2ca 100644 --- a/tests/components/spotify/snapshots/test_media_browser.ambr +++ b/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': , - 'media_class': , - 'media_content_id': 'spotify://01j5tx5a0ff6g5v0qjx6hbc94t/spotify:playlist:unique_identifier_00', - 'media_content_type': 'spotify://playlist', - 'thumbnail': None, - 'title': 'Playlist1', - }), - ]), - 'children_media_class': , - 'media_class': , - '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, diff --git a/tests/components/spotify/test_config_flow.py b/tests/components/spotify/test_config_flow.py index dd662d12681..68b1f0583d6 100644 --- a/tests/components/spotify/test_config_flow.py +++ b/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,38 +157,34 @@ async def test_abort_if_spotify_error( }, ) - with patch( - "homeassistant.components.spotify.config_flow.Spotify.current_user", - side_effect=SpotifyException(400, -1, "message"), - ): - result = await hass.config_entries.flow.async_configure(result["flow_id"]) + mock_spotify.return_value.current_user.side_effect = SpotifyException( + 400, -1, "message" + ) + + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] is FlowResultType.ABORT assert result["reason"] == "connection_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( - domain=DOMAIN, - unique_id=123, - version=1, - data={"id": "frenck", "auth_implementation": "cred"}, - ) - old_entry.add_to_hass(hass) + mock_config_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 len(flows) == 1 + assert result["type"] is FlowResultType.FORM + 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", - "access_token": "mock-access-token", + "refresh_token": "new-refresh-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 updated_data["auth_implementation"] == "cred" - updated_data["token"].pop("expires_at") - assert updated_data["token"] == { - "refresh_token": "mock-refresh-token", - "access_token": "mock-access-token", + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "reauth_successful" + + mock_config_entry.data["token"].pop("expires_at") + assert mock_config_entry.data["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( - domain=DOMAIN, - unique_id=123, - version=1, - data={"id": "frenck", "auth_implementation": "cred"}, - ) - old_entry.add_to_hass(hass) + mock_config_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() - result = await hass.config_entries.flow.async_configure(flows[0]["flow_id"], {}) + assert result["type"] is FlowResultType.FORM + 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,9 +262,8 @@ async def test_reauth_account_mismatch( }, ) - with patch("homeassistant.components.spotify.config_flow.Spotify") as spotify_mock: - spotify_mock.return_value.current_user.return_value = {"id": "fake_id"} - result = await hass.config_entries.flow.async_configure(result["flow_id"]) + mock_spotify.return_value.current_user.return_value["id"] = "new_user_id" + result = await hass.config_entries.flow.async_configure(result["flow_id"]) assert result["type"] is FlowResultType.ABORT assert result["reason"] == "reauth_account_mismatch" diff --git a/tests/components/spotify/test_media_browser.py b/tests/components/spotify/test_media_browser.py index 2b47aed9ee3..1b17da74d4a 100644 --- a/tests/components/spotify/test_media_browser.py +++ b/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: - """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) - - +@pytest.mark.usefixtures("setup_credentials") 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",