Refactor Bond integration to remove duplication (#37740)

* Refactor Bond integration to remove duplication in Entity code and unit tests

* Refactor Bond integration to remove duplication in Entity code and unit tests (PR feedback)

homeassistant/components/bond/cover.py

@@ -1,15 +1,15 @@
 """Support for Bond covers."""
-from typing import Any, Callable, Dict, List, Optional
+from typing import Any, Callable, List, Optional
 
 from bond import BOND_DEVICE_TYPE_MOTORIZED_SHADES, Bond
 
 from homeassistant.components.cover import DEVICE_CLASS_SHADE, CoverEntity
 from homeassistant.config_entries import ConfigEntry
-from homeassistant.const import ATTR_NAME
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.entity import Entity
 
 from .const import DOMAIN
+from .entity import BondEntity
 from .utils import BondDevice, get_bond_devices
 
 
@@ -32,13 +32,13 @@ async def async_setup_entry(
     async_add_entities(covers, True)
 
 
-class BondCover(CoverEntity):
+class BondCover(BondEntity, CoverEntity):
     """Representation of a Bond cover."""
 
     def __init__(self, bond: Bond, device: BondDevice):
         """Create HA entity representing Bond cover."""
-        self._bond = bond
+        super().__init__(bond, device)
-        self._device = device
+
         self._closed: Optional[bool] = None
 
     @property
@@ -46,26 +46,6 @@ class BondCover(CoverEntity):
         """Get device class."""
         return DEVICE_CLASS_SHADE
 
-    @property
-    def unique_id(self) -> Optional[str]:
-        """Get unique ID for the entity."""
-        return self._device.device_id
-
-    @property
-    def name(self) -> Optional[str]:
-        """Get entity name."""
-        return self._device.name
-
-    @property
-    def device_info(self) -> Optional[Dict[str, Any]]:
-        """Get a an HA device representing this cover."""
-        return {ATTR_NAME: self.name, "identifiers": {(DOMAIN, self._device.device_id)}}
-
-    @property
-    def assumed_state(self) -> bool:
-        """Let HA know this entity relies on an assumed state tracked by Bond."""
-        return True
-
     def update(self):
         """Fetch assumed state of the cover from the hub using API."""
         state: dict = self._bond.getDeviceState(self._device.device_id)

homeassistant/components/bond/entity.py

@@ -0,0 +1,38 @@
+"""An abstract class common to all Bond entities."""
+from typing import Any, Dict, Optional
+
+from bond import Bond
+
+from homeassistant.components.bond.utils import BondDevice
+from homeassistant.const import ATTR_NAME
+
+from .const import DOMAIN
+
+
+class BondEntity:
+    """Generic Bond entity encapsulating common features of any Bond controlled device."""
+
+    def __init__(self, bond: Bond, device: BondDevice):
+        """Initialize entity with API and device info."""
+        self._bond = bond
+        self._device = device
+
+    @property
+    def unique_id(self) -> Optional[str]:
+        """Get unique ID for the entity."""
+        return self._device.device_id
+
+    @property
+    def name(self) -> Optional[str]:
+        """Get entity name."""
+        return self._device.name
+
+    @property
+    def device_info(self) -> Optional[Dict[str, Any]]:
+        """Get a an HA device representing this Bond controlled device."""
+        return {ATTR_NAME: self.name, "identifiers": {(DOMAIN, self._device.device_id)}}
+
+    @property
+    def assumed_state(self) -> bool:
+        """Let HA know this entity relies on an assumed state tracked by Bond."""
+        return True

homeassistant/components/bond/fan.py

@@ -1,5 +1,5 @@
 """Support for Bond fans."""
-from typing import Any, Callable, Dict, List, Optional
+from typing import Any, Callable, List, Optional
 
 from bond import BOND_DEVICE_TYPE_CEILING_FAN, Bond
 
@@ -15,8 +15,8 @@ from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.entity import Entity
 
-from ...const import ATTR_NAME
 from .const import DOMAIN
+from .entity import BondEntity
 from .utils import BondDevice, get_bond_devices
 
 
@@ -39,37 +39,16 @@ async def async_setup_entry(
     async_add_entities(fans, True)
 
 
-class BondFan(FanEntity):
+class BondFan(BondEntity, FanEntity):
     """Representation of a Bond fan."""
 
     def __init__(self, bond: Bond, device: BondDevice):
         """Create HA entity representing Bond fan."""
-        self._bond = bond
+        super().__init__(bond, device)
-        self._device = device
 
         self._power: Optional[bool] = None
         self._speed: Optional[int] = None
 
-    @property
-    def unique_id(self) -> Optional[str]:
-        """Get unique ID for the entity."""
-        return self._device.device_id
-
-    @property
-    def name(self) -> Optional[str]:
-        """Get entity name."""
-        return self._device.name
-
-    @property
-    def device_info(self) -> Optional[Dict[str, Any]]:
-        """Get a an HA device representing this fan."""
-        return {ATTR_NAME: self.name, "identifiers": {(DOMAIN, self._device.device_id)}}
-
-    @property
-    def assumed_state(self) -> bool:
-        """Let HA know this entity relies on an assumed state tracked by Bond."""
-        return True
-
     @property
     def supported_features(self) -> int:
         """Flag supported features."""

tests/components/bond/common.py

@@ -1,4 +1,7 @@
 """Common methods used across tests for Bond."""
+from typing import Any, Dict
+
+from homeassistant import core
 from homeassistant.components.bond.const import DOMAIN as BOND_DOMAIN
 from homeassistant.const import CONF_ACCESS_TOKEN, CONF_HOST
 from homeassistant.setup import async_setup_component
@@ -7,7 +10,9 @@ from tests.async_mock import patch
 from tests.common import MockConfigEntry
 
 
-async def setup_platform(hass, platform):
+async def setup_platform(
+    hass: core.HomeAssistant, platform: str, discovered_device: Dict[str, Any]
+):
     """Set up the specified Bond platform."""
     mock_entry = MockConfigEntry(
         domain=BOND_DOMAIN,
@@ -15,7 +20,14 @@ async def setup_platform(hass, platform):
     )
     mock_entry.add_to_hass(hass)
 
-    with patch("homeassistant.components.bond.PLATFORMS", [platform]):
+    with patch("homeassistant.components.bond.PLATFORMS", [platform]), patch(
+        "homeassistant.components.bond.Bond.getDeviceIds",
+        return_value=["bond-device-id"],
+    ), patch(
+        "homeassistant.components.bond.Bond.getDevice", return_value=discovered_device
+    ), patch(
+        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
+    ):
         assert await async_setup_component(hass, BOND_DOMAIN, {})
         await hass.async_block_till_done()
 

tests/components/bond/test_cover.py

@@ -22,21 +22,15 @@ from tests.common import async_fire_time_changed
 
 _LOGGER = logging.getLogger(__name__)
 
-TEST_DEVICE_IDS = ["device-1"]
+
-TEST_COVER_DEVICE = {"name": "name-1", "type": BOND_DEVICE_TYPE_MOTORIZED_SHADES}
+def shades(name: str):
+    """Create motorized shades with given name."""
+    return {"name": name, "type": BOND_DEVICE_TYPE_MOTORIZED_SHADES}
 
 
 async def test_entity_registry(hass: core.HomeAssistant):
     """Tests that the devices are registered in the entity registry."""
-
+    await setup_platform(hass, COVER_DOMAIN, shades("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, COVER_DOMAIN)
 
     registry: EntityRegistry = await hass.helpers.entity_registry.async_get_registry()
     assert [key for key in registry.entities.keys()] == ["cover.name_1"]
@@ -44,15 +38,7 @@ async def test_entity_registry(hass: core.HomeAssistant):
 
 async def test_open_cover(hass: core.HomeAssistant):
     """Tests that open cover command delegates to API."""
-
+    await setup_platform(hass, COVER_DOMAIN, shades("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, COVER_DOMAIN)
 
     with patch("homeassistant.components.bond.Bond.open") as mock_open:
         await hass.services.async_call(
@@ -67,15 +53,7 @@ async def test_open_cover(hass: core.HomeAssistant):
 
 async def test_close_cover(hass: core.HomeAssistant):
     """Tests that close cover command delegates to API."""
-
+    await setup_platform(hass, COVER_DOMAIN, shades("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, COVER_DOMAIN)
 
     with patch("homeassistant.components.bond.Bond.close") as mock_close:
         await hass.services.async_call(
@@ -90,15 +68,7 @@ async def test_close_cover(hass: core.HomeAssistant):
 
 async def test_stop_cover(hass: core.HomeAssistant):
     """Tests that stop cover command delegates to API."""
-
+    await setup_platform(hass, COVER_DOMAIN, shades("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, COVER_DOMAIN)
 
     with patch("homeassistant.components.bond.Bond.hold") as mock_hold:
         await hass.services.async_call(
@@ -113,15 +83,7 @@ async def test_stop_cover(hass: core.HomeAssistant):
 
 async def test_update_reports_open_cover(hass: core.HomeAssistant):
     """Tests that update command sets correct state when Bond API reports cover is open."""
-
+    await setup_platform(hass, COVER_DOMAIN, shades("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, COVER_DOMAIN)
 
     with patch(
         "homeassistant.components.bond.Bond.getDeviceState", return_value={"open": 1}
@@ -134,15 +96,7 @@ async def test_update_reports_open_cover(hass: core.HomeAssistant):
 
 async def test_update_reports_closed_cover(hass: core.HomeAssistant):
     """Tests that update command sets correct state when Bond API reports cover is closed."""
-
+    await setup_platform(hass, COVER_DOMAIN, shades("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_COVER_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, COVER_DOMAIN)
 
     with patch(
         "homeassistant.components.bond.Bond.getDeviceState", return_value={"open": 0}

tests/components/bond/test_fan.py

@@ -15,25 +15,19 @@ from .common import setup_platform
 
 from tests.async_mock import patch
 
-TEST_DEVICE_IDS = ["device-1"]
+
-TEST_FAN_DEVICE = {
+def ceiling_fan(name: str):
-    "name": "name-1",
+    """Create a ceiling fan with given name."""
+    return {
+        "name": name,
         "type": BOND_DEVICE_TYPE_CEILING_FAN,
         "actions": ["SetSpeed"],
-}
+    }
 
 
 async def test_entity_registry(hass: core.HomeAssistant):
     """Tests that the devices are registered in the entity registry."""
-
+    await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, FAN_DOMAIN)
 
     registry: EntityRegistry = await hass.helpers.entity_registry.async_get_registry()
     assert [key for key in registry.entities.keys()] == ["fan.name_1"]
@@ -41,15 +35,7 @@ async def test_entity_registry(hass: core.HomeAssistant):
 
 async def test_turn_on_fan(hass: core.HomeAssistant):
     """Tests that turn on command delegates to API."""
-
+    await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, FAN_DOMAIN)
 
     with patch("homeassistant.components.bond.Bond.turnOn") as mock_turn_on, patch(
         "homeassistant.components.bond.Bond.setSpeed"
@@ -68,15 +54,7 @@ async def test_turn_on_fan(hass: core.HomeAssistant):
 
 async def test_turn_off_fan(hass: core.HomeAssistant):
     """Tests that turn off command delegates to API."""
-
+    await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, FAN_DOMAIN)
 
     with patch("homeassistant.components.bond.Bond.turnOff") as mock_turn_off:
         await hass.services.async_call(
@@ -88,15 +66,7 @@ async def test_turn_off_fan(hass: core.HomeAssistant):
 
 async def test_update_reports_fan_on(hass: core.HomeAssistant):
     """Tests that update command sets correct state when Bond API reports fan power is on."""
-
+    await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, FAN_DOMAIN)
 
     with patch(
         "homeassistant.components.bond.Bond.getDeviceState",
@@ -110,15 +80,7 @@ async def test_update_reports_fan_on(hass: core.HomeAssistant):
 
 async def test_update_reports_fan_off(hass: core.HomeAssistant):
     """Tests that update command sets correct state when Bond API reports fan power is off."""
-
+    await setup_platform(hass, FAN_DOMAIN, ceiling_fan("name-1"))
-    with patch(
-        "homeassistant.components.bond.Bond.getDeviceIds", return_value=TEST_DEVICE_IDS
-    ), patch(
-        "homeassistant.components.bond.Bond.getDevice", return_value=TEST_FAN_DEVICE
-    ), patch(
-        "homeassistant.components.bond.Bond.getDeviceState", return_value={}
-    ):
-        await setup_platform(hass, FAN_DOMAIN)
 
     with patch(
         "homeassistant.components.bond.Bond.getDeviceState",