From 93d74cafdc1f6c34b164b6674e03cd98eda427f7 Mon Sep 17 00:00:00 2001
From: G Johansson <goran.johansson@shiftit.se>
Date: Wed, 2 Nov 2022 17:52:36 +0100
Subject: [PATCH] Fix late review comments for Scrape (#81259)

* Review comments from #74325

* Remove moved test

* Fix init

* Handle no data

* Remove print

* Fix tests

* PlatformNotReady if no data

* Recover failed platform setup

* Fix broken test

* patch context

* reset test init

* Move to platform

* asyncio gather

* Remove duplicate code
---
 homeassistant/components/scrape/__init__.py | 36 ++++++-------
 homeassistant/components/scrape/sensor.py   | 55 ++++++++++----------
 tests/components/scrape/test_init.py        | 57 ++++++++++++++++++---
 tests/components/scrape/test_sensor.py      | 38 ++++++--------
 4 files changed, 113 insertions(+), 73 deletions(-)

diff --git a/homeassistant/components/scrape/__init__.py b/homeassistant/components/scrape/__init__.py
index 9b6fb9210f3..25a734dbd24 100644
--- a/homeassistant/components/scrape/__init__.py
+++ b/homeassistant/components/scrape/__init__.py
@@ -1,8 +1,10 @@
 """The scrape component."""
 from __future__ import annotations
 
+import asyncio
+from collections.abc import Coroutine
 from datetime import timedelta
-import logging
+from typing import Any
 
 import voluptuous as vol
 
@@ -15,7 +17,6 @@ from homeassistant.const import (
     Platform,
 )
 from homeassistant.core import HomeAssistant
-from homeassistant.exceptions import PlatformNotReady
 from homeassistant.helpers import discovery
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.template_entity import TEMPLATE_SENSOR_BASE_SCHEMA
@@ -24,9 +25,6 @@ from homeassistant.helpers.typing import ConfigType
 from .const import CONF_INDEX, CONF_SELECT, DEFAULT_SCAN_INTERVAL, DOMAIN
 from .coordinator import ScrapeCoordinator
 
-_LOGGER = logging.getLogger(__name__)
-
-
 SENSOR_SCHEMA = vol.Schema(
     {
         **TEMPLATE_SENSOR_BASE_SCHEMA.schema,
@@ -55,13 +53,12 @@ CONFIG_SCHEMA = vol.Schema(
 
 async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
     """Set up Scrape from yaml config."""
+    scrape_config: list[ConfigType] | None
     if not (scrape_config := config.get(DOMAIN)):
         return True
 
+    load_coroutines: list[Coroutine[Any, Any, None]] = []
     for resource_config in scrape_config:
-        if not (sensors := resource_config.get(SENSOR_DOMAIN)):
-            raise PlatformNotReady("No sensors configured")
-
         rest = create_rest_data_from_config(hass, resource_config)
         coordinator = ScrapeCoordinator(
             hass,
@@ -70,17 +67,20 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
                 seconds=resource_config.get(CONF_SCAN_INTERVAL, DEFAULT_SCAN_INTERVAL)
             ),
         )
-        await coordinator.async_refresh()
-        if coordinator.data is None:
-            raise PlatformNotReady
 
-        for sensor_config in sensors:
-            discovery.load_platform(
-                hass,
-                Platform.SENSOR,
-                DOMAIN,
-                {"coordinator": coordinator, "config": sensor_config},
-                config,
+        sensors: list[ConfigType] = resource_config.get(SENSOR_DOMAIN, [])
+        if sensors:
+            load_coroutines.append(
+                discovery.async_load_platform(
+                    hass,
+                    Platform.SENSOR,
+                    DOMAIN,
+                    {"coordinator": coordinator, "configs": sensors},
+                    config,
+                )
             )
 
+    if load_coroutines:
+        await asyncio.gather(*load_coroutines)
+
     return True
diff --git a/homeassistant/components/scrape/sensor.py b/homeassistant/components/scrape/sensor.py
index 176b556e189..e911ade5d72 100644
--- a/homeassistant/components/scrape/sensor.py
+++ b/homeassistant/components/scrape/sensor.py
@@ -83,6 +83,7 @@ async def async_setup_platform(
     discovery_info: DiscoveryInfoType | None = None,
 ) -> None:
     """Set up the Web scrape sensor."""
+    entities: list[ScrapeSensor] = []
     if discovery_info is None:
         async_create_issue(
             hass,
@@ -97,45 +98,47 @@ async def async_setup_platform(
         rest = create_rest_data_from_config(hass, resource_config)
 
         coordinator = ScrapeCoordinator(hass, rest, SCAN_INTERVAL)
-        await coordinator.async_refresh()
-        if coordinator.data is None:
-            raise PlatformNotReady
 
-        sensor_config = config
-        template_config = vol.Schema(
-            TEMPLATE_SENSOR_BASE_SCHEMA.schema, extra=vol.REMOVE_EXTRA
-        )(sensor_config)
+        sensors_config: list[tuple[ConfigType, ConfigType]] = [
+            (
+                config,
+                vol.Schema(TEMPLATE_SENSOR_BASE_SCHEMA.schema, extra=vol.REMOVE_EXTRA)(
+                    config
+                ),
+            )
+        ]
 
     else:
         coordinator = discovery_info["coordinator"]
-        sensor_config = discovery_info["config"]
-        template_config = sensor_config
+        sensors_config = [
+            (sensor_config, sensor_config)
+            for sensor_config in discovery_info["configs"]
+        ]
 
-    name: str = template_config[CONF_NAME]
-    unique_id: str | None = template_config.get(CONF_UNIQUE_ID)
-    select: str | None = sensor_config.get(CONF_SELECT)
-    attr: str | None = sensor_config.get(CONF_ATTRIBUTE)
-    index: int = sensor_config[CONF_INDEX]
-    value_template: Template | None = sensor_config.get(CONF_VALUE_TEMPLATE)
+    await coordinator.async_refresh()
+    if coordinator.data is None:
+        raise PlatformNotReady
 
-    if value_template is not None:
-        value_template.hass = hass
+    for sensor_config, template_config in sensors_config:
+        value_template: Template | None = sensor_config.get(CONF_VALUE_TEMPLATE)
+        if value_template is not None:
+            value_template.hass = hass
 
-    async_add_entities(
-        [
+        entities.append(
             ScrapeSensor(
                 hass,
                 coordinator,
                 template_config,
-                name,
-                unique_id,
-                select,
-                attr,
-                index,
+                template_config[CONF_NAME],
+                template_config.get(CONF_UNIQUE_ID),
+                sensor_config.get(CONF_SELECT),
+                sensor_config.get(CONF_ATTRIBUTE),
+                sensor_config[CONF_INDEX],
                 value_template,
             )
-        ],
-    )
+        )
+
+    async_add_entities(entities)
 
 
 class ScrapeSensor(CoordinatorEntity[ScrapeCoordinator], TemplateSensor):
diff --git a/tests/components/scrape/test_init.py b/tests/components/scrape/test_init.py
index 22c07a2acaf..e66b9a65fd4 100644
--- a/tests/components/scrape/test_init.py
+++ b/tests/components/scrape/test_init.py
@@ -1,15 +1,21 @@
 """Test Scrape component setup process."""
 from __future__ import annotations
 
+from datetime import datetime
 from unittest.mock import patch
 
+import pytest
+
 from homeassistant.components.scrape.const import DOMAIN
+from homeassistant.components.scrape.sensor import SCAN_INTERVAL
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers import entity_registry as er
 from homeassistant.setup import async_setup_component
 
 from . import MockRestData, return_integration_config
 
+from tests.common import async_fire_time_changed
+
 
 async def test_setup_config(hass: HomeAssistant) -> None:
     """Test setup from yaml."""
@@ -35,8 +41,10 @@ async def test_setup_config(hass: HomeAssistant) -> None:
     assert len(mock_setup.mock_calls) == 1
 
 
-async def test_setup_no_data_fails(hass: HomeAssistant) -> None:
-    """Test setup entry no data fails."""
+async def test_setup_no_data_fails_with_recovery(
+    hass: HomeAssistant, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test setup entry no data fails and recovers."""
     config = {
         DOMAIN: [
             return_integration_config(
@@ -45,15 +53,25 @@ async def test_setup_no_data_fails(hass: HomeAssistant) -> None:
         ]
     }
 
+    mocker = MockRestData("test_scrape_sensor_no_data")
     with patch(
-        "homeassistant.components.scrape.coordinator.RestData",
-        return_value=MockRestData("test_scrape_sensor_no_data"),
+        "homeassistant.components.rest.RestData",
+        return_value=mocker,
     ):
-        assert not await async_setup_component(hass, DOMAIN, config)
+        assert await async_setup_component(hass, DOMAIN, config)
+        await hass.async_block_till_done()
+
+        state = hass.states.get("sensor.ha_version")
+        assert state is None
+
+        assert "Platform scrape not ready yet" in caplog.text
+
+        mocker.payload = "test_scrape_sensor"
+        async_fire_time_changed(hass, datetime.utcnow() + SCAN_INTERVAL)
         await hass.async_block_till_done()
 
     state = hass.states.get("sensor.ha_version")
-    assert state is None
+    assert state.state == "Current Version: 2021.12.10"
 
 
 async def test_setup_config_no_configuration(hass: HomeAssistant) -> None:
@@ -65,3 +83,30 @@ async def test_setup_config_no_configuration(hass: HomeAssistant) -> None:
 
     entities = er.async_get(hass)
     assert entities.entities == {}
+
+
+async def test_setup_config_no_sensors(
+    hass: HomeAssistant, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test setup from yaml with no configured sensors finalize properly."""
+    config = {
+        DOMAIN: [
+            {
+                "resource": "https://www.address.com",
+                "verify_ssl": True,
+            },
+            {
+                "resource": "https://www.address2.com",
+                "verify_ssl": True,
+                "sensor": None,
+            },
+        ]
+    }
+
+    mocker = MockRestData("test_scrape_sensor")
+    with patch(
+        "homeassistant.components.rest.RestData",
+        return_value=mocker,
+    ):
+        assert await async_setup_component(hass, DOMAIN, config)
+        await hass.async_block_till_done()
diff --git a/tests/components/scrape/test_sensor.py b/tests/components/scrape/test_sensor.py
index 24e583f5a87..2d0c7b4c61d 100644
--- a/tests/components/scrape/test_sensor.py
+++ b/tests/components/scrape/test_sensor.py
@@ -4,6 +4,8 @@ from __future__ import annotations
 from datetime import datetime
 from unittest.mock import patch
 
+import pytest
+
 from homeassistant.components.scrape.sensor import SCAN_INTERVAL
 from homeassistant.components.sensor import (
     CONF_STATE_CLASS,
@@ -67,6 +69,7 @@ async def test_scrape_sensor_platform_yaml(hass: HomeAssistant) -> None:
                 name="Auth page2",
                 username="user@secret.com",
                 password="12345678",
+                template="{{value}}",
             ),
         ]
     }
@@ -248,7 +251,9 @@ async def test_scrape_sensor_authentication(hass: HomeAssistant) -> None:
     assert state2.state == "secret text"
 
 
-async def test_scrape_sensor_no_data(hass: HomeAssistant) -> None:
+async def test_scrape_sensor_no_data(
+    hass: HomeAssistant, caplog: pytest.LogCaptureFixture
+) -> None:
     """Test Scrape sensor fails on no data."""
     config = {
         DOMAIN: return_integration_config(
@@ -261,12 +266,14 @@ async def test_scrape_sensor_no_data(hass: HomeAssistant) -> None:
         "homeassistant.components.rest.RestData",
         return_value=mocker,
     ):
-        assert not await async_setup_component(hass, DOMAIN, config)
+        assert await async_setup_component(hass, DOMAIN, config)
         await hass.async_block_till_done()
 
     state = hass.states.get("sensor.ha_version")
     assert state is None
 
+    assert "Platform scrape not ready yet" in caplog.text
+
 
 async def test_scrape_sensor_no_data_refresh(hass: HomeAssistant) -> None:
     """Test Scrape sensor no data on refresh."""
@@ -286,13 +293,13 @@ async def test_scrape_sensor_no_data_refresh(hass: HomeAssistant) -> None:
         assert await async_setup_component(hass, DOMAIN, config)
         await hass.async_block_till_done()
 
-    state = hass.states.get("sensor.ha_version")
-    assert state
-    assert state.state == "Current Version: 2021.12.10"
+        state = hass.states.get("sensor.ha_version")
+        assert state
+        assert state.state == "Current Version: 2021.12.10"
 
-    mocker.payload = "test_scrape_sensor_no_data"
-    async_fire_time_changed(hass, datetime.utcnow() + SCAN_INTERVAL)
-    await hass.async_block_till_done()
+        mocker.payload = "test_scrape_sensor_no_data"
+        async_fire_time_changed(hass, datetime.utcnow() + SCAN_INTERVAL)
+        await hass.async_block_till_done()
 
     state = hass.states.get("sensor.ha_version")
     assert state is not None
@@ -398,18 +405,3 @@ async def test_scrape_sensor_unique_id(hass: HomeAssistant) -> None:
     entity = entity_reg.async_get("sensor.ha_version")
 
     assert entity.unique_id == "ha_version_unique_id"
-
-
-async def test_scrape_sensor_not_configured_sensor(hass: HomeAssistant, caplog) -> None:
-    """Test Scrape sensor with missing configured sensors."""
-    config = {DOMAIN: [return_integration_config(sensors=None)]}
-
-    mocker = MockRestData("test_scrape_sensor")
-    with patch(
-        "homeassistant.components.rest.RestData",
-        return_value=mocker,
-    ):
-        assert not await async_setup_component(hass, DOMAIN, config)
-        await hass.async_block_till_done()
-
-    assert "No sensors configured" in caplog.text
-- 
GitLab