From 8b8f750c41b24b9388517fef4d3203ffff8e81e1 Mon Sep 17 00:00:00 2001 From: Franck Nijhof Date: Thu, 20 Jan 2022 13:48:24 +0100 Subject: [PATCH] Address after merge review comments on IntelliFire (#64553) * Remove unneeded coordinator update method parameter * Remove unused error * Remove unused translation strings * Remove leftover debug print from tests * Improve tests * Cleanup unused entry ID * Typing completions --- .../components/intellifire/binary_sensor.py | 5 +- .../components/intellifire/config_flow.py | 5 - .../components/intellifire/coordinator.py | 7 +- .../components/intellifire/strings.json | 3 +- .../intellifire/translations/en.json | 3 +- tests/components/intellifire/conftest.py | 29 +++++ .../intellifire/test_config_flow.py | 106 ++++-------------- 7 files changed, 59 insertions(+), 99 deletions(-) create mode 100644 tests/components/intellifire/conftest.py diff --git a/homeassistant/components/intellifire/binary_sensor.py b/homeassistant/components/intellifire/binary_sensor.py index 139087a6ec7..0c636ec597b 100644 --- a/homeassistant/components/intellifire/binary_sensor.py +++ b/homeassistant/components/intellifire/binary_sensor.py @@ -70,9 +70,7 @@ async def async_setup_entry( coordinator: IntellifireDataUpdateCoordinator = hass.data[DOMAIN][entry.entry_id] async_add_entities( - IntellifireBinarySensor( - coordinator=coordinator, entry_id=entry.entry_id, description=description - ) + IntellifireBinarySensor(coordinator=coordinator, description=description) for description in INTELLIFIRE_BINARY_SENSORS ) @@ -87,7 +85,6 @@ class IntellifireBinarySensor(CoordinatorEntity, BinarySensorEntity): def __init__( self, coordinator: IntellifireDataUpdateCoordinator, - entry_id: str, description: IntellifireBinarySensorEntityDescription, ) -> None: """Class initializer.""" diff --git a/homeassistant/components/intellifire/config_flow.py b/homeassistant/components/intellifire/config_flow.py index f47b434ae94..c541c733e08 100644 --- a/homeassistant/components/intellifire/config_flow.py +++ b/homeassistant/components/intellifire/config_flow.py @@ -11,7 +11,6 @@ from homeassistant import config_entries from homeassistant.const import CONF_HOST from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResult -from homeassistant.exceptions import HomeAssistantError from .const import DOMAIN @@ -62,7 +61,3 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): return self.async_show_form( step_id="user", data_schema=STEP_USER_DATA_SCHEMA, errors=errors ) - - -class CannotConnect(HomeAssistantError): - """Error to indicate we cannot connect.""" diff --git a/homeassistant/components/intellifire/coordinator.py b/homeassistant/components/intellifire/coordinator.py index be5e3110cac..4d169e07a17 100644 --- a/homeassistant/components/intellifire/coordinator.py +++ b/homeassistant/components/intellifire/coordinator.py @@ -24,11 +24,10 @@ class IntellifireDataUpdateCoordinator(DataUpdateCoordinator[IntellifirePollData LOGGER, name=DOMAIN, update_interval=timedelta(seconds=15), - update_method=self._async_update_data, ) self._api = api - async def _async_update_data(self): + async def _async_update_data(self) -> IntellifirePollData: LOGGER.debug("Calling update loop on IntelliFire") async with timeout(100): try: @@ -38,12 +37,12 @@ class IntellifireDataUpdateCoordinator(DataUpdateCoordinator[IntellifirePollData return self._api.data @property - def api(self): + def api(self) -> IntellifireAsync: """Return the API pointer.""" return self._api @property - def device_info(self): + def device_info(self) -> DeviceInfo: """Return the device info.""" return DeviceInfo( manufacturer="Hearth and Home", diff --git a/homeassistant/components/intellifire/strings.json b/homeassistant/components/intellifire/strings.json index 06315a7fc50..52d57eda809 100644 --- a/homeassistant/components/intellifire/strings.json +++ b/homeassistant/components/intellifire/strings.json @@ -8,8 +8,7 @@ } }, "error": { - "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", - "unknown": "[%key:common::config_flow::error::unknown%]" + "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]" }, "abort": { "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" diff --git a/homeassistant/components/intellifire/translations/en.json b/homeassistant/components/intellifire/translations/en.json index 0a4ba36e285..10441d21536 100644 --- a/homeassistant/components/intellifire/translations/en.json +++ b/homeassistant/components/intellifire/translations/en.json @@ -4,8 +4,7 @@ "already_configured": "Device is already configured" }, "error": { - "cannot_connect": "Failed to connect", - "unknown": "Unexpected error" + "cannot_connect": "Failed to connect" }, "step": { "user": { diff --git a/tests/components/intellifire/conftest.py b/tests/components/intellifire/conftest.py new file mode 100644 index 00000000000..2bbb3318090 --- /dev/null +++ b/tests/components/intellifire/conftest.py @@ -0,0 +1,29 @@ +"""Fixtures for IntelliFire integration tests.""" +from collections.abc import Generator +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import pytest + + +@pytest.fixture +def mock_setup_entry() -> Generator[AsyncMock, None, None]: + """Mock setting up a config entry.""" + with patch( + "homeassistant.components.intellifire.async_setup_entry", return_value=True + ) as mock_setup: + yield mock_setup + + +@pytest.fixture +def mock_intellifire_config_flow() -> Generator[None, MagicMock, None]: + """Return a mocked IntelliFire client.""" + data_mock = Mock() + data_mock.serial = "12345" + + with patch( + "homeassistant.components.intellifire.config_flow.IntellifireAsync", + autospec=True, + ) as intellifire_mock: + intellifire = intellifire_mock.return_value + intellifire.data = data_mock + yield intellifire diff --git a/tests/components/intellifire/test_config_flow.py b/tests/components/intellifire/test_config_flow.py index 44f31ab32c5..c9d08318d3c 100644 --- a/tests/components/intellifire/test_config_flow.py +++ b/tests/components/intellifire/test_config_flow.py @@ -1,14 +1,17 @@ """Test the IntelliFire config flow.""" -from unittest.mock import Mock, patch +from unittest.mock import AsyncMock, MagicMock from homeassistant import config_entries -from homeassistant.components.intellifire.config_flow import validate_input from homeassistant.components.intellifire.const import DOMAIN from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import RESULT_TYPE_CREATE_ENTRY, RESULT_TYPE_FORM -async def test_form(hass: HomeAssistant) -> None: +async def test_form( + hass: HomeAssistant, + mock_setup_entry: AsyncMock, + mock_intellifire_config_flow: MagicMock, +) -> None: """Test we get the form.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} @@ -16,98 +19,37 @@ async def test_form(hass: HomeAssistant) -> None: assert result["type"] == RESULT_TYPE_FORM assert result["errors"] is None - with patch( - "homeassistant.components.intellifire.config_flow.validate_input", - return_value={ - "title": "Living Room Fireplace", - "type": "Fireplace", - "serial": "abcd1234", + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { "host": "1.1.1.1", }, - ), patch( - "homeassistant.components.intellifire.async_setup_entry", return_value=True - ) as mock_setup_entry: - print("mock_setup_entry", mock_setup_entry) - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - "host": "1.1.1.1", - }, - ) - await hass.async_block_till_done() + ) + await hass.async_block_till_done() assert result2["type"] == RESULT_TYPE_CREATE_ENTRY assert result2["title"] == "Fireplace" assert result2["data"] == {"host": "1.1.1.1"} + assert len(mock_setup_entry.mock_calls) == 1 -async def test_form_cannot_connect(hass: HomeAssistant) -> None: + +async def test_form_cannot_connect( + hass: HomeAssistant, mock_intellifire_config_flow: MagicMock +) -> None: """Test we handle cannot connect error.""" + mock_intellifire_config_flow.poll.side_effect = ConnectionError + result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) - with patch( - "intellifire4py.IntellifireAsync.poll", - side_effect=ConnectionError, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - "host": "1.1.1.1", - }, - ) + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + "host": "1.1.1.1", + }, + ) assert result2["type"] == RESULT_TYPE_FORM assert result2["errors"] == {"base": "cannot_connect"} - - -async def test_form_good(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( - "intellifire4py.IntellifireAsync.poll", - side_effect=ConnectionError, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - "host": "1.1.1.1", - }, - ) - - assert result2["type"] == RESULT_TYPE_FORM - assert result2["errors"] == {"base": "cannot_connect"} - - -async def test_validate_input(hass: HomeAssistant) -> None: - """Test for the ideal case.""" - # Define a mock object - data_mock = Mock() - data_mock.serial = "12345" - - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} - ) - with patch( - "homeassistant.components.intellifire.config_flow.validate_input", - return_value="abcd1234", - ), patch("intellifire4py.IntellifireAsync.poll", return_value=3), patch( - "intellifire4py.IntellifireAsync.data", return_value="something" - ), patch( - "intellifire4py.IntellifireAsync.data.serial", return_value="1234" - ), patch( - "intellifire4py.intellifire_async.IntellifireAsync", return_value="1111" - ), patch( - "intellifire4py.IntellifireAsync", return_value=True - ), patch( - "intellifire4py.model.IntellifirePollData", new=data_mock - ) as mobj: - assert mobj.serial == "12345" - - result = await validate_input(hass, {"host": "127.0.0.1"}) - - assert result() == "1234"