From 391983a0cf56a226120057390ddd33586019b827 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Mon, 1 Jun 2020 23:25:06 +0200 Subject: [PATCH] Prevent race in pychromecast.start_discovery (#36350) * Workaround for race in pychromecast * Fix tests --- homeassistant/components/cast/discovery.py | 12 +++-- tests/components/cast/test_media_player.py | 56 +++++++++++++--------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/homeassistant/components/cast/discovery.py b/homeassistant/components/cast/discovery.py index b15346417de..4d58fc3383a 100644 --- a/homeassistant/components/cast/discovery.py +++ b/homeassistant/components/cast/discovery.py @@ -3,6 +3,7 @@ import logging import threading import pychromecast +import zeroconf from homeassistant.const import EVENT_HOMEASSISTANT_STOP from homeassistant.core import HomeAssistant @@ -84,10 +85,13 @@ def setup_internal_discovery(hass: HomeAssistant) -> None: ) _LOGGER.debug("Starting internal pychromecast discovery.") - listener, browser = pychromecast.start_discovery( - internal_add_callback, - internal_remove_callback, - ChromeCastZeroconf.get_zeroconf(), + listener = pychromecast.discovery.CastListener( + internal_add_callback, internal_remove_callback + ) + browser = zeroconf.ServiceBrowser( + ChromeCastZeroconf.get_zeroconf() or zeroconf.Zeroconf(), + "_googlecast._tcp.local.", + listener, ) def stop_discovery(event): diff --git a/tests/components/cast/test_media_player.py b/tests/components/cast/test_media_player.py index 2adb8b63052..ce9e4ad7bf8 100644 --- a/tests/components/cast/test_media_player.py +++ b/tests/components/cast/test_media_player.py @@ -80,16 +80,19 @@ async def async_setup_cast_internal_discovery(hass, config=None, discovery_info= browser = MagicMock(zc={}) with patch( - "homeassistant.components.cast.discovery.pychromecast.start_discovery", - return_value=(listener, browser), - ) as start_discovery: + "homeassistant.components.cast.discovery.pychromecast.discovery.CastListener", + return_value=listener, + ) as cast_listener, patch( + "homeassistant.components.cast.discovery.zeroconf.ServiceBrowser", + return_value=browser, + ): add_entities = await async_setup_cast(hass, config, discovery_info) await hass.async_block_till_done() await hass.async_block_till_done() - assert start_discovery.call_count == 1 + assert cast_listener.call_count == 1 - discovery_callback = start_discovery.call_args[0][0] + discovery_callback = cast_listener.call_args[0][0] def discover_chromecast(service_name: str, info: ChromecastInfo) -> None: """Discover a chromecast device.""" @@ -117,9 +120,12 @@ async def async_setup_media_player_cast(hass: HomeAssistantType, info: Chromecas "homeassistant.components.cast.discovery.pychromecast.get_chromecast_from_service", return_value=chromecast, ) as get_chromecast, patch( - "homeassistant.components.cast.discovery.pychromecast.start_discovery", - return_value=(listener, browser), - ) as start_discovery: + "homeassistant.components.cast.discovery.pychromecast.discovery.CastListener", + return_value=listener, + ) as cast_listener, patch( + "homeassistant.components.cast.discovery.zeroconf.ServiceBrowser", + return_value=browser, + ): await async_setup_component( hass, "media_player", @@ -128,7 +134,7 @@ async def async_setup_media_player_cast(hass: HomeAssistantType, info: Chromecas await hass.async_block_till_done() - discovery_callback = start_discovery.call_args[0][0] + discovery_callback = cast_listener.call_args[0][0] def discover_chromecast(service_name: str, info: ChromecastInfo) -> None: """Discover a chromecast device.""" @@ -153,15 +159,17 @@ async def async_setup_media_player_cast(hass: HomeAssistantType, info: Chromecas async def test_start_discovery_called_once(hass): """Test pychromecast.start_discovery called exactly once.""" with patch( - "homeassistant.components.cast.discovery.pychromecast.start_discovery", - return_value=(None, Mock()), - ) as start_discovery: + "homeassistant.components.cast.discovery.pychromecast.discovery.CastListener", + ) as cast_listener, patch( + "homeassistant.components.cast.discovery.zeroconf.ServiceBrowser", + return_value=Mock(), + ): await async_setup_cast(hass) - assert start_discovery.call_count == 1 + assert cast_listener.call_count == 1 await async_setup_cast(hass) - assert start_discovery.call_count == 1 + assert cast_listener.call_count == 1 async def test_stop_discovery_called_on_stop(hass): @@ -169,13 +177,15 @@ async def test_stop_discovery_called_on_stop(hass): browser = MagicMock(zc={}) with patch( - "homeassistant.components.cast.discovery.pychromecast.start_discovery", - return_value=(None, browser), - ) as start_discovery: + "homeassistant.components.cast.discovery.pychromecast.discovery.CastListener", + ) as cast_listener, patch( + "homeassistant.components.cast.discovery.zeroconf.ServiceBrowser", + return_value=browser, + ): # start_discovery should be called with empty config await async_setup_cast(hass, {}) - assert start_discovery.call_count == 1 + assert cast_listener.call_count == 1 with patch( "homeassistant.components.cast.discovery.pychromecast.stop_discovery" @@ -187,13 +197,15 @@ async def test_stop_discovery_called_on_stop(hass): stop_discovery.assert_called_once_with(browser) with patch( - "homeassistant.components.cast.discovery.pychromecast.start_discovery", - return_value=(None, browser), - ) as start_discovery: + "homeassistant.components.cast.discovery.pychromecast.discovery.CastListener", + ) as cast_listener, patch( + "homeassistant.components.cast.discovery.zeroconf.ServiceBrowser", + return_value=browser, + ): # start_discovery should be called again on re-startup await async_setup_cast(hass) - assert start_discovery.call_count == 1 + assert cast_listener.call_count == 1 async def test_create_cast_device_without_uuid(hass):