From 6d542613226def75fc90f9b9b6f9f900d660fad0 Mon Sep 17 00:00:00 2001 From: jjlawren Date: Fri, 10 Dec 2021 15:28:47 -0600 Subject: [PATCH] Remove external library discovery call in Sonos (#61461) Co-authored-by: J. Nick Koston --- homeassistant/components/sonos/config_flow.py | 11 +-- tests/components/sonos/test_config_flow.py | 90 +++++++++++++++++-- tests/components/sonos/test_init.py | 26 ++++-- 3 files changed, 107 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/sonos/config_flow.py b/homeassistant/components/sonos/config_flow.py index e6d2bb337b8..07add8e6d7c 100644 --- a/homeassistant/components/sonos/config_flow.py +++ b/homeassistant/components/sonos/config_flow.py @@ -1,22 +1,19 @@ """Config flow for SONOS.""" import dataclasses -import soco - from homeassistant import config_entries -from homeassistant.components import zeroconf +from homeassistant.components import ssdp, zeroconf from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResult from homeassistant.helpers.config_entry_flow import DiscoveryFlowHandler -from .const import DATA_SONOS_DISCOVERY_MANAGER, DOMAIN +from .const import DATA_SONOS_DISCOVERY_MANAGER, DOMAIN, UPNP_ST from .helpers import hostname_to_uid async def _async_has_devices(hass: HomeAssistant) -> bool: - """Return if there are devices that can be discovered.""" - result = await hass.async_add_executor_job(soco.discover) - return bool(result) + """Return if Sonos devices have been seen recently with SSDP.""" + return bool(await ssdp.async_get_discovery_info_by_st(hass, UPNP_ST)) class SonosDiscoveryFlowHandler(DiscoveryFlowHandler): diff --git a/tests/components/sonos/test_config_flow.py b/tests/components/sonos/test_config_flow.py index eee644abeba..aa2ce6cc0be 100644 --- a/tests/components/sonos/test_config_flow.py +++ b/tests/components/sonos/test_config_flow.py @@ -4,14 +4,36 @@ from __future__ import annotations from unittest.mock import MagicMock, patch from homeassistant import config_entries, core -from homeassistant.components import zeroconf +from homeassistant.components import ssdp, zeroconf +from homeassistant.components.media_player import DOMAIN as MP_DOMAIN from homeassistant.components.sonos.const import DATA_SONOS_DISCOVERY_MANAGER, DOMAIN +from homeassistant.const import CONF_HOSTS +from homeassistant.setup import async_setup_component -@patch("homeassistant.components.sonos.config_flow.soco.discover", return_value=True) -async def test_user_form(discover_mock: MagicMock, hass: core.HomeAssistant): +async def test_user_form( + hass: core.HomeAssistant, zeroconf_payload: zeroconf.ZeroconfServiceInfo +): """Test we get the user initiated form.""" + # Ensure config flow will fail if no devices discovered yet + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + + assert result["type"] == "form" + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == "abort" + assert result["reason"] == "no_devices_found" + + # Initiate a discovery to allow config entry creation + await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_ZEROCONF}, + data=zeroconf_payload, + ) + + # Ensure config flow succeeds after discovery result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) @@ -37,7 +59,22 @@ async def test_user_form(discover_mock: MagicMock, hass: core.HomeAssistant): assert len(mock_setup_entry.mock_calls) == 1 -async def test_zeroconf_form(hass: core.HomeAssistant, zeroconf_payload): +async def test_user_form_already_created(hass: core.HomeAssistant): + """Ensure we abort a flow if the entry is already created from config.""" + config = {DOMAIN: {MP_DOMAIN: {CONF_HOSTS: "192.168.4.2"}}} + await async_setup_component(hass, DOMAIN, config) + await hass.async_block_till_done() + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == "abort" + assert result["reason"] == "single_instance_allowed" + + +async def test_zeroconf_form( + hass: core.HomeAssistant, zeroconf_payload: zeroconf.ZeroconfServiceInfo +): """Test we pass Zeroconf discoveries to the manager.""" mock_manager = hass.data[DATA_SONOS_DISCOVERY_MANAGER] = MagicMock() @@ -71,6 +108,47 @@ async def test_zeroconf_form(hass: core.HomeAssistant, zeroconf_payload): assert len(mock_manager.mock_calls) == 2 +async def test_ssdp_discovery(hass: core.HomeAssistant, soco): + """Test that SSDP discoveries create a config flow.""" + + await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_SSDP}, + data=ssdp.SsdpServiceInfo( + ssdp_location=f"http://{soco.ip_address}/", + ssdp_st="urn:schemas-upnp-org:device:ZonePlayer:1", + ssdp_usn=f"uuid:{soco.uid}_MR::urn:schemas-upnp-org:service:GroupRenderingControl:1", + upnp={ + ssdp.ATTR_UPNP_UDN: f"uuid:{soco.uid}", + }, + ), + ) + + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + flow = flows[0] + + with patch( + "homeassistant.components.sonos.async_setup", + return_value=True, + ) as mock_setup, patch( + "homeassistant.components.sonos.async_setup_entry", + return_value=True, + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_configure( + flow["flow_id"], + {}, + ) + await hass.async_block_till_done() + + assert result["type"] == "create_entry" + assert result["title"] == "Sonos" + assert result["data"] == {} + + assert len(mock_setup.mock_calls) == 1 + assert len(mock_setup_entry.mock_calls) == 1 + + async def test_zeroconf_sonos_v1(hass: core.HomeAssistant): """Test we pass sonos devices to the discovery manager with v1 firmware devices.""" @@ -121,7 +199,9 @@ async def test_zeroconf_sonos_v1(hass: core.HomeAssistant): assert len(mock_manager.mock_calls) == 2 -async def test_zeroconf_form_not_sonos(hass: core.HomeAssistant, zeroconf_payload): +async def test_zeroconf_form_not_sonos( + hass: core.HomeAssistant, zeroconf_payload: zeroconf.ZeroconfServiceInfo +): """Test we abort on non-sonos devices.""" mock_manager = hass.data[DATA_SONOS_DISCOVERY_MANAGER] = MagicMock() diff --git a/tests/components/sonos/test_init.py b/tests/components/sonos/test_init.py index 71f89f6d880..02897c523c1 100644 --- a/tests/components/sonos/test_init.py +++ b/tests/components/sonos/test_init.py @@ -1,16 +1,26 @@ """Tests for the Sonos config flow.""" from unittest.mock import patch -from homeassistant import config_entries, data_entry_flow -from homeassistant.components import sonos +from homeassistant import config_entries, core, data_entry_flow +from homeassistant.components import sonos, zeroconf from homeassistant.setup import async_setup_component -async def test_creating_entry_sets_up_media_player(hass): +async def test_creating_entry_sets_up_media_player( + hass: core.HomeAssistant, zeroconf_payload: zeroconf.ZeroconfServiceInfo +): """Test setting up Sonos loads the media player.""" + + # Initiate a discovery to allow a user config flow + await hass.config_entries.flow.async_init( + sonos.DOMAIN, + context={"source": config_entries.SOURCE_ZEROCONF}, + data=zeroconf_payload, + ) + with patch( "homeassistant.components.sonos.media_player.async_setup_entry", - ) as mock_setup, patch("soco.discover", return_value=True): + ) as mock_setup: result = await hass.config_entries.flow.async_init( sonos.DOMAIN, context={"source": config_entries.SOURCE_USER} ) @@ -26,12 +36,12 @@ async def test_creating_entry_sets_up_media_player(hass): assert len(mock_setup.mock_calls) == 1 -async def test_configuring_sonos_creates_entry(hass): +async def test_configuring_sonos_creates_entry(hass: core.HomeAssistant): """Test that specifying config will create an entry.""" with patch( "homeassistant.components.sonos.async_setup_entry", return_value=True, - ) as mock_setup, patch("soco.discover", return_value=True): + ) as mock_setup: await async_setup_component( hass, sonos.DOMAIN, @@ -42,12 +52,12 @@ async def test_configuring_sonos_creates_entry(hass): assert len(mock_setup.mock_calls) == 1 -async def test_not_configuring_sonos_not_creates_entry(hass): +async def test_not_configuring_sonos_not_creates_entry(hass: core.HomeAssistant): """Test that no config will not create an entry.""" with patch( "homeassistant.components.sonos.async_setup_entry", return_value=True, - ) as mock_setup, patch("soco.discover", return_value=True): + ) as mock_setup: await async_setup_component(hass, sonos.DOMAIN, {}) await hass.async_block_till_done()