From 0c40c8465e368f1f070ce3e961b2ee709ae71883 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Thu, 5 Oct 2023 19:56:47 +0200 Subject: [PATCH] Correct checks for deprecated forecast in weather (#101392) Co-authored-by: Robert Resch --- homeassistant/components/weather/__init__.py | 110 ++++++---- homeassistant/components/weather/strings.json | 10 +- tests/components/weather/test_init.py | 194 ++++++++++++++++-- .../custom_components/test/weather.py | 36 ---- 4 files changed, 253 insertions(+), 97 deletions(-) diff --git a/homeassistant/components/weather/__init__.py b/homeassistant/components/weather/__init__.py index 4ec9ea91f89..648201f16d2 100644 --- a/homeassistant/components/weather/__init__.py +++ b/homeassistant/components/weather/__init__.py @@ -8,7 +8,6 @@ from contextlib import suppress from dataclasses import dataclass from datetime import timedelta from functools import partial -import inspect import logging from typing import ( Any, @@ -56,6 +55,7 @@ from homeassistant.helpers.update_coordinator import ( DataUpdateCoordinator, TimestampDataUpdateCoordinator, ) +from homeassistant.loader import async_get_issue_tracker, async_suggest_report_issue from homeassistant.util.dt import utcnow from homeassistant.util.json import JsonValueType from homeassistant.util.unit_system import US_CUSTOMARY_SYSTEM @@ -296,7 +296,8 @@ class WeatherEntity(Entity, PostInit): Literal["daily", "hourly", "twice_daily"], list[Callable[[list[JsonValueType] | None], None]], ] - __weather_legacy_forecast: bool = False + __weather_reported_legacy_forecast = False + __weather_legacy_forecast = False _weather_option_temperature_unit: str | None = None _weather_option_pressure_unit: str | None = None @@ -311,15 +312,12 @@ class WeatherEntity(Entity, PostInit): def __init_subclass__(cls, **kwargs: Any) -> None: """Post initialisation processing.""" super().__init_subclass__(**kwargs) - if any( - method in cls.__dict__ for method in ("_attr_forecast", "forecast") - ) and not any( - method in cls.__dict__ - for method in ( - "async_forecast_daily", - "async_forecast_hourly", - "async_forecast_twice_daily", - ) + if ( + "forecast" in cls.__dict__ + and cls.async_forecast_daily is WeatherEntity.async_forecast_daily + and cls.async_forecast_hourly is WeatherEntity.async_forecast_hourly + and cls.async_forecast_twice_daily + is WeatherEntity.async_forecast_twice_daily ): cls.__weather_legacy_forecast = True @@ -332,38 +330,55 @@ class WeatherEntity(Entity, PostInit): ) -> None: """Start adding an entity to a platform.""" super().add_to_platform_start(hass, platform, parallel_updates) - _reported_forecast = False - if self.__weather_legacy_forecast and not _reported_forecast: - module = inspect.getmodule(self) - if module and module.__file__ and "custom_components" in module.__file__: - # Do not report on core integrations as they are already fixed or PR is open. - report_issue = "report it to the custom integration author." - _LOGGER.warning( - ( - "%s::%s is using a forecast attribute on an instance of " - "WeatherEntity, this is deprecated and will be unsupported " - "from Home Assistant 2024.3. Please %s" - ), - self.__module__, - self.entity_id, - report_issue, - ) - ir.async_create_issue( - self.hass, - DOMAIN, - f"deprecated_weather_forecast_{self.platform.platform_name}", - breaks_in_ha_version="2024.3.0", - is_fixable=False, - is_persistent=False, - issue_domain=self.platform.platform_name, - severity=ir.IssueSeverity.WARNING, - translation_key="deprecated_weather_forecast", - translation_placeholders={ - "platform": self.platform.platform_name, - "report_issue": report_issue, - }, - ) - _reported_forecast = True + if self.__weather_legacy_forecast: + self._report_legacy_forecast(hass) + + def _report_legacy_forecast(self, hass: HomeAssistant) -> None: + """Log warning and create an issue if the entity imlpements legacy forecast.""" + if "custom_components" not in type(self).__module__: + # Do not report core integrations as they are already fixed or PR is open. + return + + report_issue = async_suggest_report_issue( + hass, + integration_domain=self.platform.platform_name, + module=type(self).__module__, + ) + _LOGGER.warning( + ( + "%s::%s implements the `forecast` property or sets " + "`self._attr_forecast` in a subclass of WeatherEntity, this is " + "deprecated and will be unsupported from Home Assistant 2024.3." + " Please %s" + ), + self.platform.platform_name, + self.__class__.__name__, + report_issue, + ) + + translation_placeholders = {"platform": self.platform.platform_name} + translation_key = "deprecated_weather_forecast_no_url" + issue_tracker = async_get_issue_tracker( + hass, + integration_domain=self.platform.platform_name, + module=type(self).__module__, + ) + if issue_tracker: + translation_placeholders["issue_tracker"] = issue_tracker + translation_key = "deprecated_weather_forecast_url" + ir.async_create_issue( + self.hass, + DOMAIN, + f"deprecated_weather_forecast_{self.platform.platform_name}", + breaks_in_ha_version="2024.3.0", + is_fixable=False, + is_persistent=False, + issue_domain=self.platform.platform_name, + severity=ir.IssueSeverity.WARNING, + translation_key=translation_key, + translation_placeholders=translation_placeholders, + ) + self.__weather_reported_legacy_forecast = True async def async_internal_added_to_hass(self) -> None: """Call when the weather entity is added to hass.""" @@ -554,6 +569,15 @@ class WeatherEntity(Entity, PostInit): Should not be overridden by integrations. Kept for backwards compatibility. """ + if ( + self._attr_forecast is not None + and type(self).async_forecast_daily is WeatherEntity.async_forecast_daily + and type(self).async_forecast_hourly is WeatherEntity.async_forecast_hourly + and type(self).async_forecast_twice_daily + is WeatherEntity.async_forecast_twice_daily + and not self.__weather_reported_legacy_forecast + ): + self._report_legacy_forecast(self.hass) return self._attr_forecast async def async_forecast_daily(self) -> list[Forecast] | None: diff --git a/homeassistant/components/weather/strings.json b/homeassistant/components/weather/strings.json index 26388c217eb..f76e93c66c3 100644 --- a/homeassistant/components/weather/strings.json +++ b/homeassistant/components/weather/strings.json @@ -100,9 +100,13 @@ } }, "issues": { - "deprecated_weather_forecast": { - "title": "The {platform} integration is using deprecated forecast", - "description": "The integration `{platform}` is using the deprecated forecast attribute.\n\nPlease {report_issue}." + "deprecated_weather_forecast_url": { + "title": "The {platform} custom integration is using deprecated weather forecast", + "description": "The custom integration `{platform}` implements the `forecast` property or sets `self._attr_forecast` in a subclass of WeatherEntity.\n\nPlease create a bug report at {issue_tracker}.\n\nOnce an updated version of `{platform}` is available, install it and restart Home Assistant to fix this issue." + }, + "deprecated_weather_forecast_no_url": { + "title": "[%key:component::weather::issues::deprecated_weather_forecast_url::title%]", + "description": "The custom integration `{platform}` implements the `forecast` property or sets `self._attr_forecast` in a subclass of WeatherEntity.\n\nPlease report it to the author of the {platform} integration.\n\nOnce an updated version of `{platform}` is available, install it and restart Home Assistant to fix this issue." } } } diff --git a/tests/components/weather/test_init.py b/tests/components/weather/test_init.py index db3a18db914..231f08c7cc1 100644 --- a/tests/components/weather/test_init.py +++ b/tests/components/weather/test_init.py @@ -1,4 +1,5 @@ """The test for weather entity.""" +from collections.abc import Generator from datetime import datetime from typing import Any @@ -8,13 +9,23 @@ from homeassistant.components.weather import ( ATTR_CONDITION_SUNNY, ATTR_FORECAST, ATTR_FORECAST_APPARENT_TEMP, + ATTR_FORECAST_CLOUD_COVERAGE, ATTR_FORECAST_DEW_POINT, ATTR_FORECAST_HUMIDITY, + ATTR_FORECAST_NATIVE_APPARENT_TEMP, + ATTR_FORECAST_NATIVE_DEW_POINT, + ATTR_FORECAST_NATIVE_PRECIPITATION, + ATTR_FORECAST_NATIVE_PRESSURE, + ATTR_FORECAST_NATIVE_TEMP, + ATTR_FORECAST_NATIVE_TEMP_LOW, + ATTR_FORECAST_NATIVE_WIND_GUST_SPEED, + ATTR_FORECAST_NATIVE_WIND_SPEED, ATTR_FORECAST_PRECIPITATION, ATTR_FORECAST_PRESSURE, ATTR_FORECAST_TEMP, ATTR_FORECAST_TEMP_LOW, ATTR_FORECAST_UV_INDEX, + ATTR_FORECAST_WIND_BEARING, ATTR_FORECAST_WIND_GUST_SPEED, ATTR_FORECAST_WIND_SPEED, ATTR_WEATHER_APPARENT_TEMPERATURE, @@ -44,6 +55,7 @@ from homeassistant.components.weather.const import ( ATTR_WEATHER_DEW_POINT, ATTR_WEATHER_HUMIDITY, ) +from homeassistant.config_entries import ConfigEntry, ConfigFlow from homeassistant.const import ( PRECISION_HALVES, PRECISION_TENTHS, @@ -56,6 +68,7 @@ from homeassistant.const import ( from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import entity_registry as er +from homeassistant.helpers.entity_platform import AddEntitiesCallback import homeassistant.helpers.issue_registry as ir from homeassistant.setup import async_setup_component from homeassistant.util import dt as dt_util @@ -69,6 +82,14 @@ from homeassistant.util.unit_system import METRIC_SYSTEM, US_CUSTOMARY_SYSTEM from . import create_entity +from tests.common import ( + MockConfigEntry, + MockModule, + MockPlatform, + mock_config_flow, + mock_integration, + mock_platform, +) from tests.testing_config.custom_components.test import weather as WeatherPlatform from tests.testing_config.custom_components.test_weather import ( weather as NewWeatherPlatform, @@ -950,7 +971,150 @@ async def test_get_forecast_unsupported( ) -async def test_issue_forecast_deprecated( +class MockFlow(ConfigFlow): + """Test flow.""" + + +@pytest.fixture +def config_flow_fixture(hass: HomeAssistant) -> Generator[None, None, None]: + """Mock config flow.""" + mock_platform(hass, "test.config_flow") + + with mock_config_flow("test", MockFlow): + yield + + +ISSUE_TRACKER = "https://blablabla.com" + + +@pytest.mark.parametrize( + ("manifest_extra", "translation_key", "translation_placeholders_extra", "report"), + [ + ( + {}, + "deprecated_weather_forecast_no_url", + {}, + "report it to the author of the 'test' custom integration", + ), + ( + {"issue_tracker": ISSUE_TRACKER}, + "deprecated_weather_forecast_url", + {"issue_tracker": ISSUE_TRACKER}, + "create a bug report at https://blablabla.com", + ), + ], +) +async def test_issue_forecast_property_deprecated( + hass: HomeAssistant, + caplog: pytest.LogCaptureFixture, + config_flow_fixture: None, + manifest_extra: dict[str, str], + translation_key: str, + translation_placeholders_extra: dict[str, str], + report: str, +) -> None: + """Test the issue is raised on deprecated forecast attributes.""" + + class MockWeatherMockLegacyForecastOnly(WeatherPlatform.MockWeather): + """Mock weather class with mocked legacy forecast.""" + + def __init__(self, **values: Any) -> None: + """Initialize.""" + super().__init__(**values) + self.forecast_list: list[Forecast] | None = [ + { + ATTR_FORECAST_NATIVE_TEMP: self.native_temperature, + ATTR_FORECAST_NATIVE_APPARENT_TEMP: self.native_apparent_temperature, + ATTR_FORECAST_NATIVE_TEMP_LOW: self.native_temperature, + ATTR_FORECAST_NATIVE_DEW_POINT: self.native_dew_point, + ATTR_FORECAST_CLOUD_COVERAGE: self.cloud_coverage, + ATTR_FORECAST_NATIVE_PRESSURE: self.native_pressure, + ATTR_FORECAST_NATIVE_WIND_GUST_SPEED: self.native_wind_gust_speed, + ATTR_FORECAST_NATIVE_WIND_SPEED: self.native_wind_speed, + ATTR_FORECAST_WIND_BEARING: self.wind_bearing, + ATTR_FORECAST_UV_INDEX: self.uv_index, + ATTR_FORECAST_NATIVE_PRECIPITATION: self._values.get( + "native_precipitation" + ), + ATTR_FORECAST_HUMIDITY: self.humidity, + } + ] + + @property + def forecast(self) -> list[Forecast] | None: + """Return the forecast.""" + return self.forecast_list + + # Fake that the class belongs to a custom integration + MockWeatherMockLegacyForecastOnly.__module__ = "custom_components.test.weather" + + kwargs = { + "native_temperature": 38, + "native_temperature_unit": UnitOfTemperature.CELSIUS, + } + weather_entity = MockWeatherMockLegacyForecastOnly( + name="Testing", + entity_id="weather.testing", + condition=ATTR_CONDITION_SUNNY, + **kwargs, + ) + + async def async_setup_entry_init( + hass: HomeAssistant, config_entry: ConfigEntry + ) -> bool: + """Set up test config entry.""" + await hass.config_entries.async_forward_entry_setups(config_entry, [DOMAIN]) + return True + + async def async_setup_entry_weather_platform( + hass: HomeAssistant, + config_entry: ConfigEntry, + async_add_entities: AddEntitiesCallback, + ) -> None: + """Set up test weather platform via config entry.""" + async_add_entities([weather_entity]) + + mock_integration( + hass, + MockModule( + "test", + async_setup_entry=async_setup_entry_init, + partial_manifest=manifest_extra, + ), + built_in=False, + ) + mock_platform( + hass, + "test.weather", + MockPlatform(async_setup_entry=async_setup_entry_weather_platform), + ) + + config_entry = MockConfigEntry(domain="test") + config_entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert weather_entity.state == ATTR_CONDITION_SUNNY + + issues = ir.async_get(hass) + issue = issues.async_get_issue("weather", "deprecated_weather_forecast_test") + assert issue + assert issue.issue_domain == "test" + assert issue.issue_id == "deprecated_weather_forecast_test" + assert issue.translation_key == translation_key + assert ( + issue.translation_placeholders + == {"platform": "test"} | translation_placeholders_extra + ) + + assert ( + "test::MockWeatherMockLegacyForecastOnly implements the `forecast` property or " + "sets `self._attr_forecast` in a subclass of WeatherEntity, this is deprecated " + f"and will be unsupported from Home Assistant 2024.3. Please {report}" + ) in caplog.text + + +async def test_issue_forecast_attr_deprecated( hass: HomeAssistant, enable_custom_integrations: None, caplog: pytest.LogCaptureFixture, @@ -964,14 +1128,14 @@ async def test_issue_forecast_deprecated( platform: WeatherPlatform = getattr(hass.components, "test.weather") caplog.clear() platform.init(empty=True) - platform.ENTITIES.append( - platform.MockWeatherMockLegacyForecastOnly( - name="Testing", - entity_id="weather.testing", - condition=ATTR_CONDITION_SUNNY, - **kwargs, - ) + weather = platform.MockWeather( + name="Testing", + entity_id="weather.testing", + condition=ATTR_CONDITION_SUNNY, + **kwargs, ) + weather._attr_forecast = [] + platform.ENTITIES.append(weather) entity0 = platform.ENTITIES[0] assert await async_setup_component( @@ -986,15 +1150,15 @@ async def test_issue_forecast_deprecated( assert issue assert issue.issue_domain == "test" assert issue.issue_id == "deprecated_weather_forecast_test" - assert issue.translation_placeholders == { - "platform": "test", - "report_issue": "report it to the custom integration author.", - } + assert issue.translation_key == "deprecated_weather_forecast_no_url" + assert issue.translation_placeholders == {"platform": "test"} assert ( - "custom_components.test.weather::weather.testing is using a forecast attribute on an instance of WeatherEntity" - in caplog.text - ) + "test::MockWeather implements the `forecast` property or " + "sets `self._attr_forecast` in a subclass of WeatherEntity, this is deprecated " + "and will be unsupported from Home Assistant 2024.3. Please report it to the " + "author of the 'test' custom integration" + ) in caplog.text async def test_issue_forecast_deprecated_no_logging( diff --git a/tests/testing_config/custom_components/test/weather.py b/tests/testing_config/custom_components/test/weather.py index 84864c1dbb2..633a5e4c389 100644 --- a/tests/testing_config/custom_components/test/weather.py +++ b/tests/testing_config/custom_components/test/weather.py @@ -125,11 +125,6 @@ class MockWeather(MockEntity, WeatherEntity): """Return the unit of measurement for visibility.""" return self._handle("native_visibility_unit") - @property - def forecast(self) -> list[Forecast] | None: - """Return the forecast.""" - return self._handle("forecast") - @property def native_precipitation_unit(self) -> str | None: """Return the native unit of measurement for accumulated precipitation.""" @@ -291,34 +286,3 @@ class MockWeatherMockForecast(MockWeather): ATTR_FORECAST_HUMIDITY: self.humidity, } ] - - -class MockWeatherMockLegacyForecastOnly(MockWeather): - """Mock weather class with mocked legacy forecast.""" - - def __init__(self, **values: Any) -> None: - """Initialize.""" - super().__init__(**values) - self.forecast_list: list[Forecast] | None = [ - { - ATTR_FORECAST_NATIVE_TEMP: self.native_temperature, - ATTR_FORECAST_NATIVE_APPARENT_TEMP: self.native_apparent_temperature, - ATTR_FORECAST_NATIVE_TEMP_LOW: self.native_temperature, - ATTR_FORECAST_NATIVE_DEW_POINT: self.native_dew_point, - ATTR_FORECAST_CLOUD_COVERAGE: self.cloud_coverage, - ATTR_FORECAST_NATIVE_PRESSURE: self.native_pressure, - ATTR_FORECAST_NATIVE_WIND_GUST_SPEED: self.native_wind_gust_speed, - ATTR_FORECAST_NATIVE_WIND_SPEED: self.native_wind_speed, - ATTR_FORECAST_WIND_BEARING: self.wind_bearing, - ATTR_FORECAST_UV_INDEX: self.uv_index, - ATTR_FORECAST_NATIVE_PRECIPITATION: self._values.get( - "native_precipitation" - ), - ATTR_FORECAST_HUMIDITY: self.humidity, - } - ] - - @property - def forecast(self) -> list[Forecast] | None: - """Return the forecast.""" - return self.forecast_list