From 071f6d7099d1e755f0800b4145d042a47b64f7d2 Mon Sep 17 00:00:00 2001 From: mkmer Date: Mon, 23 May 2022 08:13:21 -0400 Subject: [PATCH] Aladdin connect unload cleanup (#71948) --- .../components/aladdin_connect/__init__.py | 15 ++-- .../components/aladdin_connect/config_flow.py | 14 +-- .../components/aladdin_connect/cover.py | 11 +-- .../aladdin_connect/test_config_flow.py | 90 +------------------ .../components/aladdin_connect/test_cover.py | 82 +++++++---------- tests/components/aladdin_connect/test_init.py | 30 +------ 6 files changed, 51 insertions(+), 191 deletions(-) diff --git a/homeassistant/components/aladdin_connect/__init__.py b/homeassistant/components/aladdin_connect/__init__.py index cbd4a195a3a..048624641bd 100644 --- a/homeassistant/components/aladdin_connect/__init__.py +++ b/homeassistant/components/aladdin_connect/__init__.py @@ -7,7 +7,7 @@ from aladdin_connect import AladdinConnectClient from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_PASSWORD, CONF_USERNAME, Platform from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady +from homeassistant.exceptions import ConfigEntryAuthFailed from .const import DOMAIN @@ -21,12 +21,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: username = entry.data[CONF_USERNAME] password = entry.data[CONF_PASSWORD] acc = AladdinConnectClient(username, password) - try: - if not await hass.async_add_executor_job(acc.login): - raise ConfigEntryAuthFailed("Incorrect Password") - except (TypeError, KeyError, NameError, ValueError) as ex: - _LOGGER.error("%s", ex) - raise ConfigEntryNotReady from ex + if not await hass.async_add_executor_job(acc.login): + raise ConfigEntryAuthFailed("Incorrect Password") hass.data.setdefault(DOMAIN, {})[entry.entry_id] = acc hass.config_entries.async_setup_platforms(entry, PLATFORMS) @@ -35,4 +31,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" - return await hass.config_entries.async_unload_platforms(entry, PLATFORMS) + if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS): + hass.data[DOMAIN].pop(entry.entry_id) + + return unload_ok diff --git a/homeassistant/components/aladdin_connect/config_flow.py b/homeassistant/components/aladdin_connect/config_flow.py index f912d36a3f0..153a63ffb06 100644 --- a/homeassistant/components/aladdin_connect/config_flow.py +++ b/homeassistant/components/aladdin_connect/config_flow.py @@ -33,13 +33,9 @@ async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> None: Data has the keys from STEP_USER_DATA_SCHEMA with values provided by the user. """ acc = AladdinConnectClient(data[CONF_USERNAME], data[CONF_PASSWORD]) - try: - login = await hass.async_add_executor_job(acc.login) - except (TypeError, KeyError, NameError, ValueError) as ex: - raise ConnectionError from ex - else: - if not login: - raise InvalidAuth + login = await hass.async_add_executor_job(acc.login) + if not login: + raise InvalidAuth class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): @@ -72,8 +68,6 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): try: await validate_input(self.hass, data) - except ConnectionError: - errors["base"] = "cannot_connect" except InvalidAuth: errors["base"] = "invalid_auth" else: @@ -107,8 +101,6 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): try: await validate_input(self.hass, user_input) - except ConnectionError: - errors["base"] = "cannot_connect" except InvalidAuth: errors["base"] = "invalid_auth" diff --git a/homeassistant/components/aladdin_connect/cover.py b/homeassistant/components/aladdin_connect/cover.py index 9e18abe21f6..da3e6b81663 100644 --- a/homeassistant/components/aladdin_connect/cover.py +++ b/homeassistant/components/aladdin_connect/cover.py @@ -21,7 +21,7 @@ from homeassistant.const import ( STATE_OPENING, ) from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.exceptions import PlatformNotReady import homeassistant.helpers.config_validation as cv from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType @@ -63,13 +63,10 @@ async def async_setup_entry( ) -> None: """Set up the Aladdin Connect platform.""" acc = hass.data[DOMAIN][config_entry.entry_id] - try: - doors = await hass.async_add_executor_job(acc.get_doors) - - except (TypeError, KeyError, NameError, ValueError) as ex: - _LOGGER.error("%s", ex) - raise ConfigEntryNotReady from ex + doors = await hass.async_add_executor_job(acc.get_doors) + if doors is None: + raise PlatformNotReady("Error from Aladdin Connect getting doors") async_add_entities( (AladdinDevice(acc, door) for door in doors), update_before_add=True, diff --git a/tests/components/aladdin_connect/test_config_flow.py b/tests/components/aladdin_connect/test_config_flow.py index 37ec64ba6f0..899aa0a7e55 100644 --- a/tests/components/aladdin_connect/test_config_flow.py +++ b/tests/components/aladdin_connect/test_config_flow.py @@ -2,7 +2,6 @@ from unittest.mock import patch from homeassistant import config_entries -from homeassistant.components.aladdin_connect.config_flow import InvalidAuth from homeassistant.components.aladdin_connect.const import DOMAIN from homeassistant.const import CONF_PASSWORD, CONF_USERNAME from homeassistant.core import HomeAssistant @@ -48,15 +47,15 @@ async def test_form(hass: HomeAssistant) -> None: assert len(mock_setup_entry.mock_calls) == 1 -async def test_form_invalid_auth(hass: HomeAssistant) -> None: - """Test we handle invalid auth.""" +async def test_form_failed_auth(hass: HomeAssistant) -> None: + """Test we handle failed authentication error.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) with patch( "homeassistant.components.aladdin_connect.config_flow.AladdinConnectClient.login", - side_effect=InvalidAuth, + return_value=False, ): result2 = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -69,43 +68,6 @@ async def test_form_invalid_auth(hass: HomeAssistant) -> None: assert result2["type"] == RESULT_TYPE_FORM assert result2["errors"] == {"base": "invalid_auth"} - -async def test_form_cannot_connect(hass: HomeAssistant) -> None: - """Test we handle cannot connect error.""" - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} - ) - - with patch( - "homeassistant.components.aladdin_connect.config_flow.AladdinConnectClient.login", - side_effect=ConnectionError, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_USERNAME: "test-username", - CONF_PASSWORD: "test-password", - }, - ) - - assert result2["type"] == RESULT_TYPE_FORM - assert result2["errors"] == {"base": "cannot_connect"} - - with patch( - "homeassistant.components.aladdin_connect.config_flow.AladdinConnectClient.login", - side_effect=TypeError, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_USERNAME: "test-username", - CONF_PASSWORD: "test-password", - }, - ) - - assert result2["type"] == RESULT_TYPE_FORM - assert result2["errors"] == {"base": "cannot_connect"} - with patch( "homeassistant.components.aladdin_connect.config_flow.AladdinConnectClient.login", return_value=False, @@ -236,7 +198,7 @@ async def test_reauth_flow(hass: HomeAssistant) -> None: async def test_reauth_flow_auth_error(hass: HomeAssistant) -> None: - """Test a successful reauth flow.""" + """Test an authorization error reauth flow.""" mock_entry = MockConfigEntry( domain=DOMAIN, @@ -277,47 +239,3 @@ async def test_reauth_flow_auth_error(hass: HomeAssistant) -> None: assert result2["type"] == RESULT_TYPE_FORM assert result2["errors"] == {"base": "invalid_auth"} - - -async def test_reauth_flow_other_error(hass: HomeAssistant) -> None: - """Test an unsuccessful reauth flow.""" - - mock_entry = MockConfigEntry( - domain=DOMAIN, - data={"username": "test-username", "password": "test-password"}, - unique_id="test-username", - ) - mock_entry.add_to_hass(hass) - - result = await hass.config_entries.flow.async_init( - DOMAIN, - context={ - "source": config_entries.SOURCE_REAUTH, - "unique_id": mock_entry.unique_id, - "entry_id": mock_entry.entry_id, - }, - data={"username": "test-username", "password": "new-password"}, - ) - - assert result["step_id"] == "reauth_confirm" - assert result["type"] == RESULT_TYPE_FORM - assert result["errors"] == {} - - with patch( - "homeassistant.components.aladdin_connect.cover.async_setup_platform", - return_value=True, - ), patch( - "homeassistant.components.aladdin_connect.config_flow.AladdinConnectClient.login", - side_effect=ValueError, - ), patch( - "homeassistant.components.aladdin_connect.cover.async_setup_entry", - return_value=True, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - {CONF_PASSWORD: "new-password"}, - ) - await hass.async_block_till_done() - - assert result2["type"] == RESULT_TYPE_FORM - assert result2["errors"] == {"base": "cannot_connect"} diff --git a/tests/components/aladdin_connect/test_cover.py b/tests/components/aladdin_connect/test_cover.py index 4904d904ad4..c1571ed9fa2 100644 --- a/tests/components/aladdin_connect/test_cover.py +++ b/tests/components/aladdin_connect/test_cover.py @@ -4,7 +4,6 @@ from unittest.mock import patch import pytest from homeassistant.components.aladdin_connect.const import DOMAIN -import homeassistant.components.aladdin_connect.cover as cover from homeassistant.components.cover import DOMAIN as COVER_DOMAIN from homeassistant.config_entries import ConfigEntryState from homeassistant.const import ( @@ -77,18 +76,7 @@ DEVICE_CONFIG_BAD_NO_DOOR = { } -@pytest.mark.parametrize( - "side_effect", - [ - (TypeError), - (KeyError), - (NameError), - (ValueError), - ], -) -async def test_setup_get_doors_errors( - hass: HomeAssistant, side_effect: Exception -) -> None: +async def test_setup_get_doors_errors(hass: HomeAssistant) -> None: """Test component setup Get Doors Errors.""" config_entry = MockConfigEntry( domain=DOMAIN, @@ -101,23 +89,14 @@ async def test_setup_get_doors_errors( return_value=True, ), patch( "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.get_doors", - side_effect=side_effect, + return_value=None, ): assert await hass.config_entries.async_setup(config_entry.entry_id) is True await hass.async_block_till_done() assert len(hass.states.async_all()) == 0 -@pytest.mark.parametrize( - "side_effect", - [ - (TypeError), - (KeyError), - (NameError), - (ValueError), - ], -) -async def test_setup_login_error(hass: HomeAssistant, side_effect: Exception) -> None: +async def test_setup_login_error(hass: HomeAssistant) -> None: """Test component setup Login Errors.""" config_entry = MockConfigEntry( domain=DOMAIN, @@ -127,11 +106,9 @@ async def test_setup_login_error(hass: HomeAssistant, side_effect: Exception) -> config_entry.add_to_hass(hass) with patch( "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.login", - side_effect=side_effect, + return_value=False, ): assert await hass.config_entries.async_setup(config_entry.entry_id) is False - await hass.async_block_till_done() - assert len(hass.states.async_all()) == 0 async def test_setup_component_noerror(hass: HomeAssistant) -> None: @@ -183,36 +160,27 @@ async def test_cover_operation(hass: HomeAssistant) -> None: with patch( "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.open_door", return_value=True, - ): - await hass.services.async_call( - "cover", "open_cover", {"entity_id": "cover.home"}, blocking=True - ) - - with patch( - "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.close_door", - return_value=True, - ): - await hass.services.async_call( - "cover", "close_cover", {"entity_id": "cover.home"}, blocking=True - ) - with patch( - "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.get_doors", - return_value=[DEVICE_CONFIG_CLOSED], - ): - await hass.services.async_call( - "homeassistant", "update_entity", {"entity_id": "cover.home"}, blocking=True - ) - assert hass.states.get("cover.home").state == STATE_CLOSED - - with patch( + ), patch( "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.get_doors", return_value=[DEVICE_CONFIG_OPEN], ): await hass.services.async_call( - "homeassistant", "update_entity", {"entity_id": "cover.home"}, blocking=True + "cover", "open_cover", {"entity_id": "cover.home"}, blocking=True ) assert hass.states.get("cover.home").state == STATE_OPEN + with patch( + "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.close_door", + return_value=True, + ), patch( + "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.get_doors", + return_value=[DEVICE_CONFIG_CLOSED], + ): + await hass.services.async_call( + "cover", "close_cover", {"entity_id": "cover.home"}, blocking=True + ) + assert hass.states.get("cover.home").state == STATE_CLOSED + with patch( "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.get_doors", return_value=[DEVICE_CONFIG_OPENING], @@ -261,9 +229,19 @@ async def test_yaml_import(hass: HomeAssistant, caplog: pytest.LogCaptureFixture "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.get_doors", return_value=[DEVICE_CONFIG_CLOSED], ): - await cover.async_setup_platform(hass, YAML_CONFIG, None) + await async_setup_component( + hass, + COVER_DOMAIN, + { + COVER_DOMAIN: { + "platform": DOMAIN, + "username": "test-user", + "password": "test-password", + } + }, + ) await hass.async_block_till_done() - + assert hass.config_entries.async_entries(DOMAIN) assert "Configuring Aladdin Connect through yaml is deprecated" in caplog.text assert hass.config_entries.async_entries(DOMAIN) diff --git a/tests/components/aladdin_connect/test_init.py b/tests/components/aladdin_connect/test_init.py index c9814adb051..0ba9b317dfb 100644 --- a/tests/components/aladdin_connect/test_init.py +++ b/tests/components/aladdin_connect/test_init.py @@ -4,39 +4,14 @@ from unittest.mock import patch from homeassistant.components.aladdin_connect.const import DOMAIN from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant -from homeassistant.setup import async_setup_component from tests.common import MockConfigEntry YAML_CONFIG = {"username": "test-user", "password": "test-password"} -async def test_unload_entry(hass: HomeAssistant): - """Test successful unload of entry.""" - entry = MockConfigEntry( - domain=DOMAIN, - data={"username": "test-user", "password": "test-password"}, - ) - entry.add_to_hass(hass) - - with patch( - "homeassistant.components.aladdin_connect.cover.AladdinConnectClient.login", - return_value=True, - ): - - assert (await async_setup_component(hass, DOMAIN, entry)) is True - - assert len(hass.config_entries.async_entries(DOMAIN)) == 1 - assert entry.state is ConfigEntryState.LOADED - - assert await hass.config_entries.async_unload(entry.entry_id) - await hass.async_block_till_done() - - assert entry.state is ConfigEntryState.NOT_LOADED - - async def test_entry_password_fail(hass: HomeAssistant): - """Test successful unload of entry.""" + """Test password fail during entry.""" entry = MockConfigEntry( domain=DOMAIN, data={"username": "test-user", "password": "test-password"}, @@ -48,7 +23,8 @@ async def test_entry_password_fail(hass: HomeAssistant): return_value=False, ): - assert (await async_setup_component(hass, DOMAIN, entry)) is True + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() assert entry.state is ConfigEntryState.SETUP_ERROR