Add check_config helper (#24557)

* check_config

* no ignore

* tests

* try tests again

homeassistant/config.py

@@ -745,13 +745,13 @@ async def async_check_ha_config_file(hass: HomeAssistant) -> Optional[str]:
 
     This method is a coroutine.
     """
-    from homeassistant.scripts.check_config import check_ha_config_file
+    import homeassistant.helpers.check_config as check_config
 
-    res = await check_ha_config_file(hass)  # type: ignore
+    res = await check_config.async_check_ha_config_file(hass)
 
     if not res.errors:
         return None
-    return '\n'.join([err.message for err in res.errors])
+    return res.error_str
 
 
 @callback

homeassistant/helpers/check_config.py

@@ -0,0 +1,181 @@
+"""Helper to check the configuration file."""
+from collections import OrderedDict, namedtuple
+# from typing import Dict, List, Sequence
+
+import attr
+import voluptuous as vol
+
+from homeassistant import loader, requirements
+from homeassistant.core import HomeAssistant
+from homeassistant.config import (
+    CONF_CORE, CORE_CONFIG_SCHEMA,
+    CONF_PACKAGES, merge_packages_config, _format_config_error,
+    find_config_file, load_yaml_config_file,
+    extract_domain_configs, config_per_platform)
+
+import homeassistant.util.yaml.loader as yaml_loader
+from homeassistant.exceptions import HomeAssistantError
+
+
+CheckConfigError = namedtuple(
+    'CheckConfigError', "message domain config")
+
+
+@attr.s
+class HomeAssistantConfig(OrderedDict):
+    """Configuration result with errors attribute."""
+
+    errors = attr.ib(default=attr.Factory(list))
+
+    def add_error(self, message, domain=None, config=None):
+        """Add a single error."""
+        self.errors.append(CheckConfigError(str(message), domain, config))
+        return self
+
+    @property
+    def error_str(self) -> str:
+        """Return errors as a string."""
+        return '\n'.join([err.message for err in self.errors])
+
+
+async def async_check_ha_config_file(hass: HomeAssistant) -> \
+        HomeAssistantConfig:
+    """Load and check if Home Assistant configuration file is valid.
+
+    This method is a coroutine.
+    """
+    config_dir = hass.config.config_dir
+    result = HomeAssistantConfig()
+
+    def _pack_error(package, component, config, message):
+        """Handle errors from packages: _log_pkg_error."""
+        message = "Package {} setup failed. Component {} {}".format(
+            package, component, message)
+        domain = 'homeassistant.packages.{}.{}'.format(package, component)
+        pack_config = core_config[CONF_PACKAGES].get(package, config)
+        result.add_error(message, domain, pack_config)
+
+    def _comp_error(ex, domain, config):
+        """Handle errors from components: async_log_exception."""
+        result.add_error(
+            _format_config_error(ex, domain, config), domain, config)
+
+    # Load configuration.yaml
+    try:
+        config_path = await hass.async_add_executor_job(
+            find_config_file, config_dir)
+        if not config_path:
+            return result.add_error("File configuration.yaml not found.")
+        config = await hass.async_add_executor_job(
+            load_yaml_config_file, config_path)
+    except FileNotFoundError:
+        return result.add_error("File not found: {}".format(config_path))
+    except HomeAssistantError as err:
+        return result.add_error(
+            "Error loading {}: {}".format(config_path, err))
+    finally:
+        yaml_loader.clear_secret_cache()
+
+    # Extract and validate core [homeassistant] config
+    try:
+        core_config = config.pop(CONF_CORE, {})
+        core_config = CORE_CONFIG_SCHEMA(core_config)
+        result[CONF_CORE] = core_config
+    except vol.Invalid as err:
+        result.add_error(err, CONF_CORE, core_config)
+        core_config = {}
+
+    # Merge packages
+    await merge_packages_config(
+        hass, config, core_config.get(CONF_PACKAGES, {}), _pack_error)
+    core_config.pop(CONF_PACKAGES, None)
+
+    # Filter out repeating config sections
+    components = set(key.split(' ')[0] for key in config.keys())
+
+    # Process and validate config
+    for domain in components:
+        try:
+            integration = await loader.async_get_integration(hass, domain)
+        except loader.IntegrationNotFound:
+            result.add_error("Integration not found: {}".format(domain))
+            continue
+
+        if (not hass.config.skip_pip and integration.requirements and
+                not await requirements.async_process_requirements(
+                    hass, integration.domain, integration.requirements)):
+            result.add_error("Unable to install all requirements: {}".format(
+                ', '.join(integration.requirements)))
+            continue
+
+        try:
+            component = integration.get_component()
+        except ImportError:
+            result.add_error("Component not found: {}".format(domain))
+            continue
+
+        if hasattr(component, 'CONFIG_SCHEMA'):
+            try:
+                config = component.CONFIG_SCHEMA(config)
+                result[domain] = config[domain]
+            except vol.Invalid as ex:
+                _comp_error(ex, domain, config)
+                continue
+
+        component_platform_schema = getattr(
+            component, 'PLATFORM_SCHEMA_BASE',
+            getattr(component, 'PLATFORM_SCHEMA', None))
+
+        if component_platform_schema is None:
+            continue
+
+        platforms = []
+        for p_name, p_config in config_per_platform(config, domain):
+            # Validate component specific platform schema
+            try:
+                p_validated = component_platform_schema(  # type: ignore
+                    p_config)
+            except vol.Invalid as ex:
+                _comp_error(ex, domain, config)
+                continue
+
+            # Not all platform components follow same pattern for platforms
+            # So if p_name is None we are not going to validate platform
+            # (the automation component is one of them)
+            if p_name is None:
+                platforms.append(p_validated)
+                continue
+
+            try:
+                p_integration = await loader.async_get_integration(hass,
+                                                                   p_name)
+            except loader.IntegrationNotFound:
+                result.add_error(
+                    "Integration {} not found when trying to verify its {} "
+                    "platform.".format(p_name, domain))
+                continue
+
+            try:
+                platform = p_integration.get_platform(domain)
+            except ImportError:
+                result.add_error(
+                    "Platform not found: {}.{}".format(domain, p_name))
+                continue
+
+            # Validate platform specific schema
+            if hasattr(platform, 'PLATFORM_SCHEMA'):
+                try:
+                    p_validated = platform.PLATFORM_SCHEMA(p_validated)
+                except vol.Invalid as ex:
+                    _comp_error(
+                        ex, '{}.{}'.format(domain, p_name), p_validated)
+                    continue
+
+            platforms.append(p_validated)
+
+        # Remove config for current component and add validated config back in.
+        for filter_comp in extract_domain_configs(config, domain):
+            del config[filter_comp]
+        result[domain] = platforms
+
+    return result

homeassistant/scripts/check_config.py

@@ -3,21 +3,14 @@
 import argparse
 import logging
 import os
-from collections import OrderedDict, namedtuple
+from collections import OrderedDict
 from glob import glob
 from typing import Dict, List, Sequence
 from unittest.mock import patch
 
-import attr
+from homeassistant import bootstrap, core
-import voluptuous as vol
+from homeassistant.config import get_default_config_dir
-
+from homeassistant.helpers.check_config import async_check_ha_config_file
-from homeassistant import bootstrap, core, loader, requirements
-from homeassistant.config import (
-    get_default_config_dir, CONF_CORE, CORE_CONFIG_SCHEMA,
-    CONF_PACKAGES, merge_packages_config, _format_config_error,
-    find_config_file, load_yaml_config_file,
-    extract_domain_configs, config_per_platform)
-
 import homeassistant.util.yaml.loader as yaml_loader
 from homeassistant.exceptions import HomeAssistantError
 
@@ -206,9 +199,8 @@ def check(config_dir, secrets=False):
         hass.config.config_dir = config_dir
 
         res['components'] = hass.loop.run_until_complete(
-            check_ha_config_file(hass))
+            async_check_ha_config_file(hass))
         res['secret_cache'] = OrderedDict(yaml_loader.__SECRET_CACHE)
-
         for err in res['components'].errors:
             domain = err.domain or ERROR_STR
             res['except'].setdefault(domain, []).append(err.message)
@@ -268,158 +260,3 @@ def dump_dict(layer, indent_count=3, listi=False, **kwargs):
                 dump_dict(i, indent_count + 2, True)
             else:
                 print(' ', indent_str, i)
-
-
-CheckConfigError = namedtuple(
-    'CheckConfigError', "message domain config")
-
-
-@attr.s
-class HomeAssistantConfig(OrderedDict):
-    """Configuration result with errors attribute."""
-
-    errors = attr.ib(default=attr.Factory(list))
-
-    def add_error(self, message, domain=None, config=None):
-        """Add a single error."""
-        self.errors.append(CheckConfigError(str(message), domain, config))
-        return self
-
-
-async def check_ha_config_file(hass):
-    """Check if Home Assistant configuration file is valid."""
-    config_dir = hass.config.config_dir
-    result = HomeAssistantConfig()
-
-    def _pack_error(package, component, config, message):
-        """Handle errors from packages: _log_pkg_error."""
-        message = "Package {} setup failed. Integration {} {}".format(
-            package, component, message)
-        domain = 'homeassistant.packages.{}.{}'.format(package, component)
-        pack_config = core_config[CONF_PACKAGES].get(package, config)
-        result.add_error(message, domain, pack_config)
-
-    def _comp_error(ex, domain, config):
-        """Handle errors from components: async_log_exception."""
-        result.add_error(
-            _format_config_error(ex, domain, config), domain, config)
-
-    # Load configuration.yaml
-    try:
-        config_path = await hass.async_add_executor_job(
-            find_config_file, config_dir)
-        if not config_path:
-            return result.add_error("File configuration.yaml not found.")
-        config = await hass.async_add_executor_job(
-            load_yaml_config_file, config_path)
-    except FileNotFoundError:
-        return result.add_error("File not found: {}".format(config_path))
-    except HomeAssistantError as err:
-        return result.add_error(
-            "Error loading {}: {}".format(config_path, err))
-    finally:
-        yaml_loader.clear_secret_cache()
-
-    # Extract and validate core [homeassistant] config
-    try:
-        core_config = config.pop(CONF_CORE, {})
-        core_config = CORE_CONFIG_SCHEMA(core_config)
-        result[CONF_CORE] = core_config
-    except vol.Invalid as err:
-        result.add_error(err, CONF_CORE, core_config)
-        core_config = {}
-
-    # Merge packages
-    await merge_packages_config(
-        hass, config, core_config.get(CONF_PACKAGES, {}), _pack_error)
-    core_config.pop(CONF_PACKAGES, None)
-
-    # Filter out repeating config sections
-    components = set(key.split(' ')[0] for key in config.keys())
-
-    # Process and validate config
-    for domain in components:
-        try:
-            integration = await loader.async_get_integration(hass, domain)
-        except loader.IntegrationNotFound:
-            result.add_error("Integration not found: {}".format(domain))
-            continue
-
-        if (not hass.config.skip_pip and integration.requirements and
-                not await requirements.async_process_requirements(
-                    hass, integration.domain, integration.requirements)):
-            result.add_error("Unable to install all requirements: {}".format(
-                ', '.join(integration.requirements)))
-            continue
-
-        try:
-            component = integration.get_component()
-        except ImportError:
-            result.add_error("Integration not found: {}".format(domain))
-            continue
-
-        if hasattr(component, 'CONFIG_SCHEMA'):
-            try:
-                config = component.CONFIG_SCHEMA(config)
-                result[domain] = config[domain]
-            except vol.Invalid as ex:
-                _comp_error(ex, domain, config)
-                continue
-
-        component_platform_schema = getattr(
-            component, 'PLATFORM_SCHEMA_BASE',
-            getattr(component, 'PLATFORM_SCHEMA', None))
-
-        if component_platform_schema is None:
-            continue
-
-        platforms = []
-        for p_name, p_config in config_per_platform(config, domain):
-            # Validate component specific platform schema
-            try:
-                p_validated = component_platform_schema(  # type: ignore
-                    p_config)
-            except vol.Invalid as ex:
-                _comp_error(ex, domain, config)
-                continue
-
-            # Not all platform components follow same pattern for platforms
-            # So if p_name is None we are not going to validate platform
-            # (the automation component is one of them)
-            if p_name is None:
-                platforms.append(p_validated)
-                continue
-
-            try:
-                p_integration = await loader.async_get_integration(hass,
-                                                                   p_name)
-            except loader.IntegrationNotFound:
-                result.add_error(
-                    "Integration {} not found when trying to verify its {} "
-                    "platform.".format(p_name, domain))
-                continue
-
-            try:
-                platform = p_integration.get_platform(domain)
-            except ImportError:
-                result.add_error(
-                    "Platform not found: {}.{}".format(domain, p_name))
-                continue
-
-            # Validate platform specific schema
-            if hasattr(platform, 'PLATFORM_SCHEMA'):
-                try:
-                    p_validated = platform.PLATFORM_SCHEMA(p_validated)
-                except vol.Invalid as ex:
-                    _comp_error(
-                        ex, '{}.{}'.format(domain, p_name), p_validated)
-                    continue
-
-            platforms.append(p_validated)
-
-        # Remove config for current component and add validated config back in.
-        for filter_comp in extract_domain_configs(config, domain):
-            del config[filter_comp]
-        result[domain] = platforms
-
-    return result

tests/common.py

@@ -16,7 +16,6 @@ from unittest.mock import MagicMock, Mock, patch
 
 import homeassistant.util.dt as date_util
 import homeassistant.util.yaml.loader as yaml_loader
-import homeassistant.util.yaml.dumper as yaml_dumper
 
 from homeassistant import auth, config_entries, core as ha, loader
 from homeassistant.auth import (
@@ -682,7 +681,6 @@ def patch_yaml_files(files_dict, endswith=True):
         raise FileNotFoundError("File not found: {}".format(fname))
 
     return patch.object(yaml_loader, 'open', mock_open_f, create=True)
-    return patch.object(yaml_dumper, 'open', mock_open_f, create=True)
 
 
 def mock_coro(return_value=None, exception=None):

tests/helpers/test_check_config.py

@@ -0,0 +1,149 @@
+"""Test check_config helper."""
+import logging
+import os  # noqa: F401 pylint: disable=unused-import
+from unittest.mock import patch
+
+from homeassistant.helpers.check_config import (
+    async_check_ha_config_file, CheckConfigError)
+from homeassistant.config import YAML_CONFIG_FILE
+from tests.common import patch_yaml_files
+
+_LOGGER = logging.getLogger(__name__)
+
+BASE_CONFIG = (
+    'homeassistant:\n'
+    '  name: Home\n'
+    '  latitude: -26.107361\n'
+    '  longitude: 28.054500\n'
+    '  elevation: 1600\n'
+    '  unit_system: metric\n'
+    '  time_zone: GMT\n'
+    '\n\n'
+)
+
+BAD_CORE_CONFIG = (
+    'homeassistant:\n'
+    '  unit_system: bad\n'
+    '\n\n'
+)
+
+
+def log_ha_config(conf):
+    """Log the returned config."""
+    cnt = 0
+    _LOGGER.debug("CONFIG - %s lines - %s errors", len(conf), len(conf.errors))
+    for key, val in conf.items():
+        _LOGGER.debug("#%s - %s: %s", cnt, key, val)
+        cnt += 1
+    for cnt, err in enumerate(conf.errors):
+        _LOGGER.debug("error[%s] = %s", cnt, err)
+
+
+async def test_bad_core_config(hass, loop):
+    """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):
+        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
+
+
+async def test_config_platform_valid(hass, loop):
+    """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):
+        res = await async_check_ha_config_file(hass)
+        log_ha_config(res)
+
+        assert res.keys() == {'homeassistant', 'light'}
+        assert res['light'] == [{'platform': 'demo'}]
+        assert not res.errors
+
+
+async def test_component_platform_not_found(hass, loop):
+    """Test errors if component or platform not found."""
+    # Make sure they don't exist
+    files = {
+        YAML_CONFIG_FILE: BASE_CONFIG + 'beer:',
+    }
+    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.keys() == {'homeassistant'}
+        assert res.errors[0] == CheckConfigError(
+            'Integration not found: beer', None, None)
+
+        # Only 1 error expected
+        res.errors.pop(0)
+        assert not res.errors
+
+
+async def test_component_platform_not_found_2(hass, loop):
+    """Test errors if component or platform not found."""
+    # Make sure they don't exist
+    files = {
+        YAML_CONFIG_FILE: BASE_CONFIG + 'light:\n  platform: beer',
+    }
+    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.keys() == {'homeassistant', 'light'}
+        assert res['light'] == []
+
+        assert res.errors[0] == CheckConfigError(
+            'Integration beer not found when trying to verify its '
+            'light platform.', None, None)
+
+        # Only 1 error expected
+        res.errors.pop(0)
+        assert not res.errors
+
+
+async def test_package_invalid(hass, loop):
+    """Test a valid platform setup."""
+    files = {
+        YAML_CONFIG_FILE: BASE_CONFIG + (
+            '  packages:\n'
+            '    p1:\n'
+            '      group: ["a"]'),
+    }
+    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 == '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'}
+
+
+async def test_bootstrap_error(hass, loop):
+    """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):
+        res = await async_check_ha_config_file(hass)
+        log_ha_config(res)
+
+        res.errors[0].domain is None
+
+        # Only 1 error expected
+        res.errors.pop(0)
+        assert not res.errors

tests/scripts/test_check_config.py

@@ -5,7 +5,7 @@ from unittest.mock import patch
 
 import homeassistant.scripts.check_config as check_config
 from homeassistant.config import YAML_CONFIG_FILE
-from tests.common import patch_yaml_files, get_test_config_dir
+from tests.common import get_test_config_dir, patch_yaml_files
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -34,7 +34,6 @@ def normalize_yaml_files(check_dict):
             for key in sorted(check_dict['yaml_files'].keys())]
 
 
-# pylint: disable=no-self-use,invalid-name
 @patch('os.path.isfile', return_value=True)
 def test_bad_core_config(isfile_patch, loop):
     """Test a bad core config setup."""

tests/test_config.py

@@ -30,7 +30,7 @@ from homeassistant.components.config.automation import (
     CONFIG_PATH as AUTOMATIONS_CONFIG_PATH)
 from homeassistant.components.config.script import (
     CONFIG_PATH as SCRIPTS_CONFIG_PATH)
-import homeassistant.scripts.check_config as check_config
+import homeassistant.helpers.check_config as check_config
 
 from tests.common import (
     get_test_config_dir, patch_yaml_files)
@@ -555,7 +555,7 @@ async def test_loading_configuration_from_packages(hass):
 
 
 @asynctest.mock.patch(
-    'homeassistant.scripts.check_config.check_ha_config_file')
+    'homeassistant.helpers.check_config.async_check_ha_config_file')
 async def test_check_ha_config_file_correct(mock_check, hass):
     """Check that restart propagates to stop."""
     mock_check.return_value = check_config.HomeAssistantConfig()
@@ -563,7 +563,7 @@ async def test_check_ha_config_file_correct(mock_check, hass):
 
 
 @asynctest.mock.patch(
-    'homeassistant.scripts.check_config.check_ha_config_file')
+    'homeassistant.helpers.check_config.async_check_ha_config_file')
 async def test_check_ha_config_file_wrong(mock_check, hass):
     """Check that restart with a bad config doesn't propagate to stop."""
     mock_check.return_value = check_config.HomeAssistantConfig()