From e1de36fda80aeb57e3246a1bd573d363672c63d0 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Thu, 26 Nov 2020 22:25:21 +0100 Subject: [PATCH] Fix check config (#43663) --- homeassistant/components/blueprint/models.py | 24 +++++++++-- homeassistant/helpers/check_config.py | 28 +++++++++---- tests/components/blueprint/test_models.py | 12 +++++- tests/helpers/test_check_config.py | 42 ++++++++++++++++---- 4 files changed, 85 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/blueprint/models.py b/homeassistant/components/blueprint/models.py index 85c87b5baa7..6e79b6da842 100644 --- a/homeassistant/components/blueprint/models.py +++ b/homeassistant/components/blueprint/models.py @@ -212,7 +212,13 @@ class DomainBlueprints: """Load a blueprint.""" try: blueprint_data = yaml.load_yaml(self.blueprint_folder / blueprint_path) - except (HomeAssistantError, FileNotFoundError) as err: + except FileNotFoundError as err: + raise FailedToLoad( + self.domain, + blueprint_path, + FileNotFoundError(f"Unable to find {blueprint_path}"), + ) from err + except HomeAssistantError as err: raise FailedToLoad(self.domain, blueprint_path, err) from err return Blueprint( @@ -251,13 +257,25 @@ class DomainBlueprints: async def async_get_blueprint(self, blueprint_path: str) -> Blueprint: """Get a blueprint.""" + + def load_from_cache(): + """Load blueprint from cache.""" + blueprint = self._blueprints[blueprint_path] + if blueprint is None: + raise FailedToLoad( + self.domain, + blueprint_path, + FileNotFoundError(f"Unable to find {blueprint_path}"), + ) + return blueprint + if blueprint_path in self._blueprints: - return self._blueprints[blueprint_path] + return load_from_cache() async with self._load_lock: # Check it again if blueprint_path in self._blueprints: - return self._blueprints[blueprint_path] + return load_from_cache() try: blueprint = await self.hass.async_add_executor_job( diff --git a/homeassistant/helpers/check_config.py b/homeassistant/helpers/check_config.py index 018bbe5cfe0..c98b563ac7e 100644 --- a/homeassistant/helpers/check_config.py +++ b/homeassistant/helpers/check_config.py @@ -1,9 +1,9 @@ """Helper to check the configuration file.""" from collections import OrderedDict +import logging import os from typing import List, NamedTuple, Optional -import attr import voluptuous as vol from homeassistant import loader @@ -36,11 +36,13 @@ class CheckConfigError(NamedTuple): config: Optional[ConfigType] -@attr.s class HomeAssistantConfig(OrderedDict): """Configuration result with errors attribute.""" - errors: List[CheckConfigError] = attr.ib(factory=list) + def __init__(self) -> None: + """Initialize HA config.""" + super().__init__() + self.errors: List[CheckConfigError] = [] def add_error( self, @@ -139,14 +141,24 @@ async def async_check_ha_config_file(hass: HomeAssistant) -> HomeAssistantConfig config_validator, "async_validate_config" ): try: - return await config_validator.async_validate_config( # type: ignore - hass, config - ) + result[domain] = ( + await config_validator.async_validate_config( # type: ignore + hass, config + ) + )[domain] + continue except (vol.Invalid, HomeAssistantError) as ex: _comp_error(ex, domain, config) continue - except Exception: # pylint: disable=broad-except - result.add_error("Unknown error calling %s config validator", domain) + except Exception as err: # pylint: disable=broad-except + logging.getLogger(__name__).exception( + "Unexpected error validating config" + ) + result.add_error( + f"Unexpected error calling config validator: {err}", + domain, + config.get(domain), + ) continue config_schema = getattr(component, "CONFIG_SCHEMA", None) diff --git a/tests/components/blueprint/test_models.py b/tests/components/blueprint/test_models.py index 5c2d5f965ff..f5d94a9301a 100644 --- a/tests/components/blueprint/test_models.py +++ b/tests/components/blueprint/test_models.py @@ -203,8 +203,8 @@ async def test_domain_blueprints_get_blueprint_errors(hass, domain_bps): with patch( "homeassistant.util.yaml.load_yaml", return_value={"blueprint": "invalid"} - ): - assert await domain_bps.async_get_blueprint("non-existing-path") is None + ), pytest.raises(errors.FailedToLoad): + await domain_bps.async_get_blueprint("non-existing-path") async def test_domain_blueprints_caching(domain_bps): @@ -258,3 +258,11 @@ async def test_domain_blueprints_add_blueprint(domain_bps, blueprint_1): with patch.object(domain_bps, "_load_blueprint") as mock_load: assert await domain_bps.async_get_blueprint("something.yaml") == blueprint_1 assert not mock_load.mock_calls + + +async def test_inputs_from_config_nonexisting_blueprint(domain_bps): + """Test referring non-existing blueprint.""" + with pytest.raises(errors.FailedToLoad): + await domain_bps.async_inputs_from_config( + {"use_blueprint": {"path": "non-existing.yaml"}} + ) diff --git a/tests/helpers/test_check_config.py b/tests/helpers/test_check_config.py index 1c25dbedcde..13ce52a840f 100644 --- a/tests/helpers/test_check_config.py +++ b/tests/helpers/test_check_config.py @@ -7,8 +7,8 @@ from homeassistant.helpers.check_config import ( async_check_ha_config_file, ) -from tests.async_mock import patch -from tests.common import patch_yaml_files +from tests.async_mock import Mock, patch +from tests.common import mock_platform, patch_yaml_files _LOGGER = logging.getLogger(__name__) @@ -37,7 +37,7 @@ def log_ha_config(conf): _LOGGER.debug("error[%s] = %s", cnt, err) -async def test_bad_core_config(hass, loop): +async def test_bad_core_config(hass): """Test a bad core config setup.""" files = {YAML_CONFIG_FILE: BAD_CORE_CONFIG} with patch("os.path.isfile", return_value=True), patch_yaml_files(files): @@ -53,7 +53,7 @@ async def test_bad_core_config(hass, loop): assert not res.errors -async def test_config_platform_valid(hass, loop): +async def test_config_platform_valid(hass): """Test a valid platform setup.""" files = {YAML_CONFIG_FILE: BASE_CONFIG + "light:\n platform: demo"} with patch("os.path.isfile", return_value=True), patch_yaml_files(files): @@ -65,7 +65,7 @@ async def test_config_platform_valid(hass, loop): assert not res.errors -async def test_component_platform_not_found(hass, loop): +async def test_component_platform_not_found(hass): """Test errors if component or platform not found.""" # Make sure they don't exist files = {YAML_CONFIG_FILE: BASE_CONFIG + "beer:"} @@ -83,7 +83,7 @@ async def test_component_platform_not_found(hass, loop): assert not res.errors -async def test_component_platform_not_found_2(hass, loop): +async def test_component_platform_not_found_2(hass): """Test errors if component or platform not found.""" # Make sure they don't exist files = {YAML_CONFIG_FILE: BASE_CONFIG + "light:\n platform: beer"} @@ -103,7 +103,7 @@ async def test_component_platform_not_found_2(hass, loop): assert not res.errors -async def test_package_invalid(hass, loop): +async def test_package_invalid(hass): """Test a valid platform setup.""" files = { YAML_CONFIG_FILE: BASE_CONFIG + (" packages:\n p1:\n" ' group: ["a"]') @@ -121,7 +121,7 @@ async def test_package_invalid(hass, loop): assert res.keys() == {"homeassistant"} -async def test_bootstrap_error(hass, loop): +async def test_bootstrap_error(hass): """Test a valid platform setup.""" files = {YAML_CONFIG_FILE: BASE_CONFIG + "automation: !include no.yaml"} with patch("os.path.isfile", return_value=True), patch_yaml_files(files): @@ -146,6 +146,7 @@ automation: input: trigger_event: blueprint_event service_to_call: test.automation +input_datetime: """, hass.config.path( "blueprints/automation/test_event_service.yaml" @@ -166,3 +167,28 @@ action: with patch("os.path.isfile", return_value=True), patch_yaml_files(files): res = await async_check_ha_config_file(hass) assert len(res.get("automation", [])) == 1 + assert len(res.errors) == 0 + assert "input_datetime" in res + + +async def test_config_platform_raise(hass): + """Test bad config validation platform.""" + mock_platform( + hass, + "bla.config", + Mock(async_validate_config=Mock(side_effect=Exception("Broken"))), + ) + files = { + YAML_CONFIG_FILE: BASE_CONFIG + + """ +bla: + value: 1 +""", + } + with patch("os.path.isfile", return_value=True), patch_yaml_files(files): + res = await async_check_ha_config_file(hass) + assert len(res.errors) == 1 + err = res.errors[0] + assert err.domain == "bla" + assert err.message == "Unexpected error calling config validator: Broken" + assert err.config == {"value": 1}