Apply suggestions from #42697 to synology_dsm (#43197)

* apply suggestions from #42697

* fix tests

* use MockConfigEntry for test

* use hass.config_entries.async_setup()

* disable default fixture

* rename marker to no_bypass_setup

homeassistant/components/synology_dsm/__init__.py

@@ -185,7 +185,7 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry):
     try:
         await api.async_setup()
     except (SynologyDSMLoginFailedException, SynologyDSMRequestException) as err:
-        _LOGGER.debug("async_setup_entry - Unable to connect to DSM: %s", str(err))
+        _LOGGER.debug("async_setup_entry - Unable to connect to DSM: %s", err)
         raise ConfigEntryNotReady from err
 
     undo_listener = entry.add_update_listener(_async_update_listener)
@@ -244,9 +244,6 @@ async def _async_setup_services(hass: HomeAssistantType):
 
     async def service_handler(call: ServiceCall):
         """Handle service call."""
-        _LOGGER.debug(
-            "service_handler - called as '%s' with data: %s", call.service, call.data
-        )
         serial = call.data.get(CONF_SERIAL)
         dsm_devices = hass.data[DOMAIN]
 
@@ -268,7 +265,7 @@ async def _async_setup_services(hass: HomeAssistantType):
             )
             return
 
-        _LOGGER.info("%s DSM with serial %s", call.service, serial)
+        _LOGGER.debug("%s DSM with serial %s", call.service, serial)
         dsm_api = dsm_device[SYNO_API]
         if call.service == SERVICE_REBOOT:
             await dsm_api.async_reboot()
@@ -276,9 +273,6 @@ async def _async_setup_services(hass: HomeAssistantType):
             await dsm_api.system.shutdown()
 
     for service in SERVICES:
-        _LOGGER.debug(
-            "_async_setup_services - register service %s on domain %s", service, DOMAIN
-        )
         hass.services.async_register(DOMAIN, service, service_handler)
 
 
@@ -445,14 +439,14 @@ class SynoApi:
         if not self.system:
             _LOGGER.debug("async_reboot - System API not ready: %s", self)
             return
-        self._hass.async_add_executor_job(self.system.reboot)
+        await self._hass.async_add_executor_job(self.system.reboot)
 
     async def async_shutdown(self):
         """Shutdown NAS."""
         if not self.system:
             _LOGGER.debug("async_shutdown - System API not ready: %s", self)
             return
-        self._hass.async_add_executor_job(self.system.shutdown)
+        await self._hass.async_add_executor_job(self.system.shutdown)
 
     async def async_unload(self):
         """Stop interacting with the NAS and prepare for removal from hass."""
@@ -465,13 +459,14 @@ class SynoApi:
             await self._hass.async_add_executor_job(
                 self.dsm.update, self._with_information
             )
-            async_dispatcher_send(self._hass, self.signal_sensor_update)
         except (SynologyDSMLoginFailedException, SynologyDSMRequestException) as err:
             _LOGGER.warning(
                 "async_update - connection error during update, fallback by reloading the entry"
             )
-            _LOGGER.debug("async_update - exception: %s", str(err))
+            _LOGGER.debug("async_update - exception: %s", err)
             await self._hass.config_entries.async_reload(self._entry.entry_id)
+            return
+        async_dispatcher_send(self._hass, self.signal_sensor_update)
 
 
 class SynologyDSMEntity(Entity):

tests/components/synology_dsm/conftest.py

@@ -4,10 +4,20 @@ import pytest
 from tests.async_mock import patch
 
 
+def pytest_configure(config):
+    """Register custom marker for tests."""
+    config.addinivalue_line(
+        "markers", "no_bypass_setup: mark test to disable bypass_setup_fixture"
+    )
+
+
 @pytest.fixture(name="bypass_setup", autouse=True)
-def bypass_setup_fixture():
+def bypass_setup_fixture(request):
     """Mock component setup."""
-    with patch(
+    if "no_bypass_setup" in request.keywords:
-        "homeassistant.components.synology_dsm.async_setup_entry", return_value=True
-    ):
         yield
+    else:
+        with patch(
+            "homeassistant.components.synology_dsm.async_setup_entry", return_value=True
+        ):
+            yield

tests/components/synology_dsm/consts.py

@@ -0,0 +1,14 @@
+"""Constants for the Synology DSM component tests."""
+
+HOST = "nas.meontheinternet.com"
+SERIAL = "mySerial"
+HOST_2 = "nas.worldwide.me"
+SERIAL_2 = "mySerial2"
+PORT = 1234
+USE_SSL = True
+VERIFY_SSL = False
+USERNAME = "Home_Assistant"
+PASSWORD = "password"
+DEVICE_TOKEN = "Dév!cè_T0k€ñ"
+
+MACS = ["00-11-32-XX-XX-59", "00-11-32-XX-XX-5A"]

tests/components/synology_dsm/test_config_flow.py

@@ -10,7 +10,6 @@ from synology_dsm.exceptions import (
 
 from homeassistant import data_entry_flow, setup
 from homeassistant.components import ssdp
-from homeassistant.components.synology_dsm import _async_setup_services
 from homeassistant.components.synology_dsm.config_flow import CONF_OTP_CODE
 from homeassistant.components.synology_dsm.const import (
     CONF_VOLUMES,
@@ -21,7 +20,6 @@ from homeassistant.components.synology_dsm.const import (
     DEFAULT_USE_SSL,
     DEFAULT_VERIFY_SSL,
     DOMAIN,
-    SERVICES,
 )
 from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_SSDP, SOURCE_USER
 from homeassistant.const import (
@@ -38,22 +36,23 @@ from homeassistant.const import (
 )
 from homeassistant.helpers.typing import HomeAssistantType
 
+from .consts import (
+    DEVICE_TOKEN,
+    HOST,
+    HOST_2,
+    MACS,
+    PASSWORD,
+    PORT,
+    SERIAL,
+    SERIAL_2,
+    USE_SSL,
+    USERNAME,
+    VERIFY_SSL,
+)
+
 from tests.async_mock import MagicMock, Mock, patch
 from tests.common import MockConfigEntry
 
-HOST = "nas.meontheinternet.com"
-SERIAL = "mySerial"
-HOST_2 = "nas.worldwide.me"
-SERIAL_2 = "mySerial2"
-PORT = 1234
-USE_SSL = True
-VERIFY_SSL = False
-USERNAME = "Home_Assistant"
-PASSWORD = "password"
-DEVICE_TOKEN = "Dév!cè_T0k€ñ"
-
-MACS = ["00-11-32-XX-XX-59", "00-11-32-XX-XX-5A"]
-
 
 @pytest.fixture(name="service")
 def mock_controller_service():
@@ -498,23 +497,3 @@ async def test_options_flow(hass: HomeAssistantType, service: MagicMock):
     assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
     assert config_entry.options[CONF_SCAN_INTERVAL] == 2
     assert config_entry.options[CONF_TIMEOUT] == 30
-
-
-async def test_services_registered(hass: HomeAssistantType):
-    """Test if all services are registered."""
-    with patch(
-        "homeassistant.core.ServiceRegistry.async_register", return_value=Mock(True)
-    ) as async_register:
-        await hass.config_entries.flow.async_init(
-            DOMAIN,
-            context={"source": SOURCE_USER},
-            data={
-                CONF_HOST: HOST,
-                CONF_PORT: PORT,
-                CONF_SSL: USE_SSL,
-                CONF_USERNAME: USERNAME,
-                CONF_PASSWORD: PASSWORD,
-            },
-        )
-        await _async_setup_services(hass)
-        assert async_register.call_count == len(SERVICES)

tests/components/synology_dsm/test_init.py

@@ -0,0 +1,41 @@
+"""Tests for the Synology DSM component."""
+import pytest
+
+from homeassistant.components.synology_dsm.const import DOMAIN, SERVICES
+from homeassistant.const import (
+    CONF_HOST,
+    CONF_MAC,
+    CONF_PASSWORD,
+    CONF_PORT,
+    CONF_SSL,
+    CONF_USERNAME,
+)
+from homeassistant.helpers.typing import HomeAssistantType
+
+from .consts import HOST, MACS, PASSWORD, PORT, USE_SSL, USERNAME
+
+from tests.async_mock import patch
+from tests.common import MockConfigEntry
+
+
+@pytest.mark.no_bypass_setup
+async def test_services_registered(hass: HomeAssistantType):
+    """Test if all services are registered."""
+    with patch(
+        "homeassistant.components.synology_dsm.SynoApi.async_setup", return_value=True
+    ), patch("homeassistant.components.synology_dsm.PLATFORMS", return_value=[]):
+        entry = MockConfigEntry(
+            domain=DOMAIN,
+            data={
+                CONF_HOST: HOST,
+                CONF_PORT: PORT,
+                CONF_SSL: USE_SSL,
+                CONF_USERNAME: USERNAME,
+                CONF_PASSWORD: PASSWORD,
+                CONF_MAC: MACS[0],
+            },
+        )
+        entry.add_to_hass(hass)
+        assert await hass.config_entries.async_setup(entry.entry_id)
+        for service in SERVICES:
+            assert hass.services.has_service(DOMAIN, service)