github
/
ha-core
mirror of https://github.com/home-assistant/core
1
Fork 0
Code Releases Activity

Differentiate between warnings and errors in check_config helper (#102902) 

* Differentiate between warnings and errors in check_config helper

* Update tests

* Treat configuration errors in frontend and its dependencies as errors

* Improve test coverage

* Address review comments

* Improve test coverage

* Improve test coverage

* Address review comments

* Add comment
This commit is contained in:
Erik Montnemery 2023-11-05 03:08:04 +01:00 committed by GitHub
parent 936956a430
commit 3ba8a82243
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 290 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
homeassistant/components/config/core.py
View File

@ -7,9 +7,8 @@ import voluptuous as vol
from homeassistant.components import websocket_api
from homeassistant.components.http import HomeAssistantView, require_admin
from homeassistant.components.sensor import async_update_suggested_units
from homeassistant.config import async_check_ha_config_file
from homeassistant.core import HomeAssistant
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers import check_config, config_validation as cv
from homeassistant.helpers.aiohttp_client import async_get_clientsession
from homeassistant.util import location, unit_system
@ -31,11 +30,18 @@ class CheckConfigView(HomeAssistantView):
@require_admin
async def post(self, request):
"""Validate configuration and return results."""
errors = await async_check_ha_config_file(request.app["hass"])
state = "invalid" if errors else "valid"
res = await check_config.async_check_ha_config_file(request.app["hass"])
return self.json({"result": state, "errors": errors})
state = "invalid" if res.errors else "valid"
return self.json(
{
"result": state,
"errors": res.error_str or None,
"warnings": res.warning_str or None,
}
)
@websocket_api.require_admin

75
homeassistant/helpers/check_config.py
View File

@ -48,6 +48,7 @@ class HomeAssistantConfig(OrderedDict):
"""Initialize HA config."""
super().__init__()
self.errors: list[CheckConfigError] = []
self.warnings: list[CheckConfigError] = []
def add_error(
self,
@ -55,15 +56,30 @@ class HomeAssistantConfig(OrderedDict):
domain: str | None = None,
config: ConfigType | None = None,
) -> Self:
"""Add a single error."""
"""Add an error."""
self.errors.append(CheckConfigError(str(message), domain, config))
return self
@property
def error_str(self) -> str:
"""Return errors as a string."""
"""Concatenate all errors to a string."""
return "\n".join([err.message for err in self.errors])
def add_warning(
self,
message: str,
domain: str | None = None,
config: ConfigType | None = None,
) -> Self:
"""Add a warning."""
self.warnings.append(CheckConfigError(str(message), domain, config))
return self
@property
def warning_str(self) -> str:
"""Concatenate all warnings to a string."""
return "\n".join([err.message for err in self.warnings])
async def async_check_ha_config_file( # noqa: C901
hass: HomeAssistant,
@ -82,11 +98,36 @@ async def async_check_ha_config_file( # noqa: C901
message = f"Package {package} setup failed. Component {component} {message}"
domain = f"homeassistant.packages.{package}.{component}"
pack_config = core_config[CONF_PACKAGES].get(package, config)
result.add_error(message, domain, pack_config)
result.add_warning(message, domain, pack_config)
def _comp_error(ex: Exception, domain: str, config: ConfigType) -> None:
"""Handle errors from components: async_log_exception."""
result.add_error(_format_config_error(ex, domain, config)[0], domain, config)
if domain in frontend_dependencies:
result.add_error(
_format_config_error(ex, domain, config)[0], domain, config
)
else:
result.add_warning(
_format_config_error(ex, domain, config)[0], domain, config
)
async def _get_integration(
hass: HomeAssistant, domain: str
) -> loader.Integration | None:
"""Get an integration."""
integration: loader.Integration | None = None
try:
integration = await async_get_integration_with_requirements(hass, domain)
except loader.IntegrationNotFound as ex:
# We get this error if an integration is not found. In recovery mode and
# safe mode, this currently happens for all custom integrations. Don't
# show errors for a missing integration in recovery mode or safe mode to
# not confuse the user.
if not hass.config.recovery_mode and not hass.config.safe_mode:
result.add_warning(f"Integration error: {domain} - {ex}")
except RequirementsNotFound as ex:
result.add_warning(f"Integration error: {domain} - {ex}")
return integration
# Load configuration.yaml
config_path = hass.config.path(YAML_CONFIG_FILE)
@ -122,22 +163,22 @@ async def async_check_ha_config_file( # noqa: C901
# Filter out repeating config sections
components = {key.partition(" ")[0] for key in config}
frontend_dependencies: set[str] = set()
if "frontend" in components or "default_config" in components:
frontend = await _get_integration(hass, "frontend")
if frontend:
await frontend.resolve_dependencies()
frontend_dependencies = frontend.all_dependencies | {"frontend"}
# Process and validate config
for domain in components:
try:
integration = await async_get_integration_with_requirements(hass, domain)
except loader.IntegrationNotFound as ex:
if not hass.config.recovery_mode and not hass.config.safe_mode:
result.add_error(f"Integration error: {domain} - {ex}")
continue
except RequirementsNotFound as ex:
result.add_error(f"Integration error: {domain} - {ex}")
if not (integration := await _get_integration(hass, domain)):
continue
try:
component = integration.get_component()
except ImportError as ex:
result.add_error(f"Component error: {domain} - {ex}")
result.add_warning(f"Component error: {domain} - {ex}")
continue
# Check if the integration has a custom config validator
@ -216,14 +257,18 @@ async def async_check_ha_config_file( # noqa: C901
)
platform = p_integration.get_platform(domain)
except loader.IntegrationNotFound as ex:
# We get this error if an integration is not found. In recovery mode and
# safe mode, this currently happens for all custom integrations. Don't
# show errors for a missing integration in recovery mode or safe mode to
# not confuse the user.
if not hass.config.recovery_mode and not hass.config.safe_mode:
result.add_error(f"Platform error {domain}.{p_name} - {ex}")
result.add_warning(f"Platform error {domain}.{p_name} - {ex}")
continue
except (
RequirementsNotFound,
ImportError,
) as ex:
result.add_error(f"Platform error {domain}.{p_name} - {ex}")
result.add_warning(f"Platform error {domain}.{p_name} - {ex}")
continue
# Validate platform specific schema

22
homeassistant/scripts/check_config.py
View File

@ -40,6 +40,7 @@ PATCHES: dict[str, Any] = {}
C_HEAD = "bold"
ERROR_STR = "General Errors"
WARNING_STR = "General Warnings"
def color(the_color, *args, reset=None):
@ -116,6 +117,18 @@ def run(script_args: list) -> int:
dump_dict(config, reset="red")
print(color("reset"))
if res["warn"]:
print(color("bold_white", "Incorrect config"))
for domain, config in res["warn"].items():
domain_info.append(domain)
print(
" ",
color("bold_yellow", domain + ":"),
color("yellow", "", reset="yellow"),
)
dump_dict(config, reset="yellow")
print(color("reset"))
if domain_info:
if "all" in domain_info:
print(color("bold_white", "Successful config (all)"))
@ -160,7 +173,8 @@ def check(config_dir, secrets=False):
res: dict[str, Any] = {
"yaml_files": OrderedDict(), # yaml_files loaded
"secrets": OrderedDict(), # secret cache and secrets loaded
"except": OrderedDict(), # exceptions raised (with config)
"except": OrderedDict(), # critical exceptions raised (with config)
"warn": OrderedDict(), # non critical exceptions raised (with config)
#'components' is a HomeAssistantConfig # noqa: E265
"secret_cache": {},
}
@ -215,6 +229,12 @@ def check(config_dir, secrets=False):
if err.config:
res["except"].setdefault(domain, []).append(err.config)
for err in res["components"].warnings:
domain = err.domain or WARNING_STR
res["warn"].setdefault(domain, []).append(err.message)
if err.config:
res["warn"].setdefault(domain, []).append(err.config)
except Exception as err: # pylint: disable=broad-except
print(color("red", "Fatal error while loading config:"), str(err))
res["except"].setdefault(ERROR_STR, []).append(str(err))

39
tests/components/config/test_core.py
View File

@ -1,6 +1,6 @@
"""Test core config."""
from http import HTTPStatus
from unittest.mock import patch
from unittest.mock import Mock, patch
import pytest
@ -37,9 +37,14 @@ async def test_validate_config_ok(
client = await hass_client()
no_error = Mock()
no_error.errors = None
no_error.error_str = ""
no_error.warning_str = ""
with patch(
"homeassistant.components.config.core.async_check_ha_config_file",
return_value=None,
"homeassistant.components.config.core.check_config.async_check_ha_config_file",
return_value=no_error,
):
resp = await client.post("/api/config/core/check_config")
@ -47,10 +52,16 @@ async def test_validate_config_ok(
result = await resp.json()
assert result["result"] == "valid"
assert result["errors"] is None
assert result["warnings"] is None
error_warning = Mock()
error_warning.errors = ["beer"]
error_warning.error_str = "beer"
error_warning.warning_str = "milk"
with patch(
"homeassistant.components.config.core.async_check_ha_config_file",
return_value="beer",
"homeassistant.components.config.core.check_config.async_check_ha_config_file",
return_value=error_warning,
):
resp = await client.post("/api/config/core/check_config")
@ -58,6 +69,24 @@ async def test_validate_config_ok(
result = await resp.json()
assert result["result"] == "invalid"
assert result["errors"] == "beer"
assert result["warnings"] == "milk"
warning = Mock()
warning.errors = None
warning.error_str = ""
warning.warning_str = "milk"
with patch(
"homeassistant.components.config.core.check_config.async_check_ha_config_file",
return_value=warning,
):
resp = await client.post("/api/config/core/check_config")
assert resp.status == HTTPStatus.OK
result = await resp.json()
assert result["result"] == "valid"
assert result["errors"] is None
assert result["warnings"] == "milk"
async def test_validate_config_requires_admin(

199
tests/helpers/test_check_config.py
View File

@ -2,10 +2,13 @@
import logging
from unittest.mock import Mock, patch
import pytest
from homeassistant.config import YAML_CONFIG_FILE
from homeassistant.core import HomeAssistant
from homeassistant.helpers.check_config import (
CheckConfigError,
HomeAssistantConfig,
async_check_ha_config_file,
)
import homeassistant.helpers.config_validation as cv
@ -40,6 +43,28 @@ def log_ha_config(conf):
_LOGGER.debug("error[%s] = %s", cnt, err)
def _assert_warnings_errors(
res: HomeAssistantConfig,
expected_warnings: list[CheckConfigError],
expected_errors: list[CheckConfigError],
) -> None:
assert len(res.warnings) == len(expected_warnings)
assert len(res.errors) == len(expected_errors)
expected_warning_str = ""
expected_error_str = ""
for idx, expected_warning in enumerate(expected_warnings):
assert res.warnings[idx] == expected_warning
expected_warning_str += expected_warning.message
assert res.warning_str == expected_warning_str
for idx, expected_error in enumerate(expected_errors):
assert res.errors[idx] == expected_error
expected_error_str += expected_error.message
assert res.error_str == expected_error_str
async def test_bad_core_config(hass: HomeAssistant) -> None:
"""Test a bad core config setup."""
files = {YAML_CONFIG_FILE: BAD_CORE_CONFIG}
@ -47,13 +72,12 @@ async def test_bad_core_config(hass: HomeAssistant) -> None:
res = await async_check_ha_config_file(hass)
log_ha_config(res)
assert isinstance(res.errors[0].message, str)
assert res.errors[0].domain == "homeassistant"
assert res.errors[0].config == {"unit_system": "bad"}
# Only 1 error expected
res.errors.pop(0)
assert not res.errors
error = CheckConfigError(
"not a valid value for dictionary value @ data['unit_system']",
"homeassistant",
{"unit_system": "bad"},
)
_assert_warnings_errors(res, [], [error])
async def test_config_platform_valid(hass: HomeAssistant) -> None:
@ -65,7 +89,7 @@ async def test_config_platform_valid(hass: HomeAssistant) -> None:
assert res.keys() == {"homeassistant", "light"}
assert res["light"] == [{"platform": "demo"}]
assert not res.errors
_assert_warnings_errors(res, [], [])
async def test_component_platform_not_found(hass: HomeAssistant) -> None:
@ -77,13 +101,10 @@ async def test_component_platform_not_found(hass: HomeAssistant) -> None:
log_ha_config(res)
assert res.keys() == {"homeassistant"}
assert res.errors[0] == CheckConfigError(
warning = CheckConfigError(
"Integration error: beer - Integration 'beer' not found.", None, None
)
# Only 1 error expected
res.errors.pop(0)
assert not res.errors
_assert_warnings_errors(res, [warning], [])
async def test_component_requirement_not_found(hass: HomeAssistant) -> None:
@ -98,7 +119,7 @@ async def test_component_requirement_not_found(hass: HomeAssistant) -> None:
log_ha_config(res)
assert res.keys() == {"homeassistant"}
assert res.errors[0] == CheckConfigError(
warning = CheckConfigError(
(
"Integration error: test_custom_component - Requirements for"
" test_custom_component not found: ['any']."
@ -106,10 +127,7 @@ async def test_component_requirement_not_found(hass: HomeAssistant) -> None:
None,
None,
)
# Only 1 error expected
res.errors.pop(0)
assert not res.errors
_assert_warnings_errors(res, [warning], [])
async def test_component_not_found_recovery_mode(hass: HomeAssistant) -> None:
@ -122,7 +140,7 @@ async def test_component_not_found_recovery_mode(hass: HomeAssistant) -> None:
log_ha_config(res)
assert res.keys() == {"homeassistant"}
assert not res.errors
_assert_warnings_errors(res, [], [])
async def test_component_not_found_safe_mode(hass: HomeAssistant) -> None:
@ -135,7 +153,55 @@ async def test_component_not_found_safe_mode(hass: HomeAssistant) -> None:
log_ha_config(res)
assert res.keys() == {"homeassistant"}
assert not res.errors
_assert_warnings_errors(res, [], [])
async def test_component_import_error(hass: HomeAssistant) -> None:
"""Test errors if component with a requirement not found not found."""
# Make sure they don't exist
files = {YAML_CONFIG_FILE: BASE_CONFIG + "light:"}
with patch(
"homeassistant.loader.Integration.get_component",
side_effect=ImportError("blablabla"),
), patch("os.path.isfile", return_value=True), patch_yaml_files(files):
res = await async_check_ha_config_file(hass)
log_ha_config(res)
assert res.keys() == {"homeassistant"}
warning = CheckConfigError(
"Component error: light - blablabla",
None,
None,
)
_assert_warnings_errors(res, [warning], [])
@pytest.mark.parametrize(
("component", "errors", "warnings", "message"),
[
("frontend", 1, 0, "[blah] is an invalid option for [frontend]"),
("http", 1, 0, "[blah] is an invalid option for [http]"),
("logger", 0, 1, "[blah] is an invalid option for [logger]"),
],
)
async def test_component_schema_error(
hass: HomeAssistant, component: str, errors: int, warnings: int, message: str
) -> None:
"""Test schema error in component."""
# Make sure they don't exist
files = {YAML_CONFIG_FILE: BASE_CONFIG + f"frontend:\n{component}:\n blah:"}
hass.config.safe_mode = True
with patch("os.path.isfile", return_value=True), patch_yaml_files(files):
res = await async_check_ha_config_file(hass)
log_ha_config(res)
assert len(res.errors) == errors
assert len(res.warnings) == warnings
for err in res.errors:
assert message in err.message
for warn in res.warnings:
assert message in warn.message
async def test_component_platform_not_found_2(hass: HomeAssistant) -> None:
@ -149,13 +215,10 @@ async def test_component_platform_not_found_2(hass: HomeAssistant) -> None:
assert res.keys() == {"homeassistant", "light"}
assert res["light"] == []
assert res.errors[0] == CheckConfigError(
warning = CheckConfigError(
"Platform error light.beer - Integration 'beer' not found.", None, None
)
# Only 1 error expected
res.errors.pop(0)
assert not res.errors
_assert_warnings_errors(res, [warning], [])
async def test_platform_not_found_recovery_mode(hass: HomeAssistant) -> None:
@ -170,7 +233,7 @@ async def test_platform_not_found_recovery_mode(hass: HomeAssistant) -> None:
assert res.keys() == {"homeassistant", "light"}
assert res["light"] == []
assert not res.errors
_assert_warnings_errors(res, [], [])
async def test_platform_not_found_safe_mode(hass: HomeAssistant) -> None:
@ -185,7 +248,47 @@ async def test_platform_not_found_safe_mode(hass: HomeAssistant) -> None:
assert res.keys() == {"homeassistant", "light"}
assert res["light"] == []
assert not res.errors
_assert_warnings_errors(res, [], [])
async def test_component_config_platform_import_error(hass: HomeAssistant) -> None:
"""Test errors if config platform fails to import."""
# Make sure they don't exist
files = {YAML_CONFIG_FILE: BASE_CONFIG + "light:\n platform: beer"}
with patch(
"homeassistant.loader.Integration.get_platform",
side_effect=ImportError("blablabla"),
), patch("os.path.isfile", return_value=True), patch_yaml_files(files):
res = await async_check_ha_config_file(hass)
log_ha_config(res)
assert res.keys() == {"homeassistant"}
error = CheckConfigError(
"Error importing config platform light: blablabla",
None,
None,
)
_assert_warnings_errors(res, [], [error])
async def test_component_platform_import_error(hass: HomeAssistant) -> None:
"""Test errors if component or platform not found."""
# Make sure they don't exist
files = {YAML_CONFIG_FILE: BASE_CONFIG + "light:\n platform: demo"}
with patch(
"homeassistant.loader.Integration.get_platform",
side_effect=[None, ImportError("blablabla")],
), patch("os.path.isfile", return_value=True), patch_yaml_files(files):
res = await async_check_ha_config_file(hass)
log_ha_config(res)
assert res.keys() == {"homeassistant", "light"}
warning = CheckConfigError(
"Platform error light.demo - blablabla",
None,
None,
)
_assert_warnings_errors(res, [warning], [])
async def test_package_invalid(hass: HomeAssistant) -> None:
@ -195,27 +298,32 @@ async def test_package_invalid(hass: HomeAssistant) -> None:
res = await async_check_ha_config_file(hass)
log_ha_config(res)
assert res.errors[0].domain == "homeassistant.packages.p1.group"
assert res.errors[0].config == {"group": ["a"]}
# Only 1 error expected
res.errors.pop(0)
assert not res.errors
assert res.keys() == {"homeassistant"}
warning = CheckConfigError(
(
"Package p1 setup failed. Component group cannot be merged. Expected a "
"dict."
),
"homeassistant.packages.p1.group",
{"group": ["a"]},
)
_assert_warnings_errors(res, [warning], [])
async def test_bootstrap_error(hass: HomeAssistant) -> None:
"""Test a valid platform setup."""
async def test_missing_included_file(hass: HomeAssistant) -> None:
"""Test missing included file."""
files = {YAML_CONFIG_FILE: BASE_CONFIG + "automation: !include no.yaml"}
with patch("os.path.isfile", return_value=True), patch_yaml_files(files):
res = await async_check_ha_config_file(hass)
log_ha_config(res)
assert res.errors[0].domain is None
assert len(res.errors) == 1
assert len(res.warnings) == 0
# Only 1 error expected
res.errors.pop(0)
assert not res.errors
assert res.errors[0].message.startswith("Error loading")
assert res.errors[0].domain is None
assert res.errors[0].config is None
async def test_automation_config_platform(hass: HomeAssistant) -> None:
@ -251,6 +359,7 @@ action:
res = await async_check_ha_config_file(hass)
assert len(res.get("automation", [])) == 1
assert len(res.errors) == 0
assert len(res.warnings) == 0
assert "input_datetime" in res
@ -270,11 +379,12 @@ bla:
}
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}
error = CheckConfigError(
"Unexpected error calling config validator: Broken",
"bla",
{"value": 1},
)
_assert_warnings_errors(res, [], [error])
async def test_removed_yaml_support(hass: HomeAssistant) -> None:
@ -292,3 +402,4 @@ async def test_removed_yaml_support(hass: HomeAssistant) -> None:
log_ha_config(res)
assert res.keys() == {"homeassistant"}
_assert_warnings_errors(res, [], [])

14
tests/scripts/test_check_config.py
View File

@ -49,6 +49,7 @@ def test_bad_core_config(mock_is_file, event_loop, mock_hass_config_yaml: None)
res = check_config.check(get_test_config_dir())
assert res["except"].keys() == {"homeassistant"}
assert res["except"]["homeassistant"][1] == {"unit_system": "bad"}
assert res["warn"] == {}
@pytest.mark.parametrize("hass_config_yaml", [BASE_CONFIG + "light:\n platform: demo"])
@ -62,6 +63,7 @@ def test_config_platform_valid(
assert res["except"] == {}
assert res["secret_cache"] == {}
assert res["secrets"] == {}
assert res["warn"] == {}
assert len(res["yaml_files"]) == 1
@ -87,9 +89,10 @@ def test_component_platform_not_found(
# Make sure they don't exist
res = check_config.check(get_test_config_dir())
assert res["components"].keys() == platforms
assert res["except"] == {check_config.ERROR_STR: [error]}
assert res["except"] == {}
assert res["secret_cache"] == {}
assert res["secrets"] == {}
assert res["warn"] == {check_config.WARNING_STR: [error]}
assert len(res["yaml_files"]) == 1
@ -123,6 +126,7 @@ def test_secrets(mock_is_file, event_loop, mock_hass_config_yaml: None) -> None:
get_test_config_dir("secrets.yaml"): {"http_pw": "http://google.com"}
}
assert res["secrets"] == {"http_pw": "http://google.com"}
assert res["warn"] == {}
assert normalize_yaml_files(res) == [
".../configuration.yaml",
".../secrets.yaml",
@ -136,13 +140,12 @@ def test_package_invalid(mock_is_file, event_loop, mock_hass_config_yaml: None)
"""Test an invalid package."""
res = check_config.check(get_test_config_dir())
assert res["except"].keys() == {"homeassistant.packages.p1.group"}
assert res["except"]["homeassistant.packages.p1.group"][1] == {"group": ["a"]}
assert len(res["except"]) == 1
assert res["except"] == {}
assert res["components"].keys() == {"homeassistant"}
assert len(res["components"]) == 1
assert res["secret_cache"] == {}
assert res["secrets"] == {}
assert res["warn"].keys() == {"homeassistant.packages.p1.group"}
assert res["warn"]["homeassistant.packages.p1.group"][1] == {"group": ["a"]}
assert len(res["yaml_files"]) == 1
@ -158,4 +161,5 @@ def test_bootstrap_error(event_loop, mock_hass_config_yaml: None) -> None:
assert res["components"] == {} # No components, load failed
assert res["secret_cache"] == {}
assert res["secrets"] == {}
assert res["warn"] == {}
assert res["yaml_files"] == {}