From eeaa8c78d84082fc7c3cf7f68e2a327759c81bef Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Sun, 11 Oct 2026 13:33:18 +1100 Subject: [PATCH] [cli] Add line format error message option (#20470) Co-authored-by: Claude Opus 5.5 --- esphome/__main__.py | 36 ++- esphome/components/substitutions/__init__.py | 17 +- esphome/config.py | 103 +++++- esphome/config_validation.py | 18 +- esphome/const.py | 9 + esphome/core/__init__.py | 4 + esphome/voluptuous_schema.py | 10 +- esphome/vscode.py | 13 +- tests/unit_tests/test_config_error_format.py | 312 +++++++++++++++++++ tests/unit_tests/test_main.py | 57 ++++ tests/unit_tests/test_vscode.py | 72 +++++ 11 files changed, 618 insertions(+), 33 deletions(-) create mode 100644 tests/unit_tests/test_config_error_format.py diff --git a/esphome/__main__.py b/esphome/__main__.py index 905339d2bcc..77c1c9f1ade 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -51,6 +51,7 @@ from esphome.const import ( KEY_ESP32, KEY_VARIANT, SECRETS_FILES, + ErrorFormat, Toolchain, ) from esphome.core import CORE, EsphomeError, coroutine @@ -2205,6 +2206,16 @@ def _add_states_args(parser: argparse.ArgumentParser) -> None: ) +def _error_format(value: str) -> ErrorFormat: + try: + return ErrorFormat(value.lower()) + except ValueError: + choices = ", ".join(ErrorFormat) + raise argparse.ArgumentTypeError( + f"invalid choice: '{value}' (choose from {choices})" + ) from None + + def parse_args(argv): options_parser = argparse.ArgumentParser(add_help=False) options_parser.add_argument( @@ -2267,6 +2278,17 @@ def parse_args(argv): ), ) + options_parser.add_argument( + "--error-format", + type=_error_format, + metavar="{" + ",".join(f.value for f in ErrorFormat) + "}", + help=( + "How configuration errors are shown. 'yaml' shows them inside the " + "failing YAML; 'line' prints one 'file:line:column: error: message' " + "line per error. Default: $ESPHOME_ERROR_FORMAT, else 'yaml'." + ), + ) + parser = argparse.ArgumentParser( description=f"ESPHome {const.__version__}", parents=[options_parser] ) @@ -2570,9 +2592,18 @@ def parse_args(argv): args, unknown_args = parser.parse_known_args(arguments) if unknown_args: _LOGGER.warning("Ignored unrecognized arguments: %s", unknown_args) - return args + else: + args = parser.parse_args(arguments) - return parser.parse_args(arguments) + # Read here rather than as the option default, so a bad value is reported + # against the environment variable instead of --error-format + if args.error_format is None: + env_value = os.getenv("ESPHOME_ERROR_FORMAT", ErrorFormat.YAML.value) + try: + args.error_format = _error_format(env_value) + except argparse.ArgumentTypeError as err: + parser.error(f"ESPHOME_ERROR_FORMAT: {err}") + return args def _warn_if_source_tree_mismatch() -> None: @@ -2645,6 +2676,7 @@ def run_esphome(argv): args = parse_args(argv) CORE.dashboard = args.dashboard CORE.testing_mode = args.testing_mode + CORE.error_format = args.error_format # Create address cache from command-line arguments CORE.address_cache = AddressCache.from_cli_args( diff --git a/esphome/components/substitutions/__init__.py b/esphome/components/substitutions/__init__.py index 93cf588e02a..711dd9e8157 100644 --- a/esphome/components/substitutions/__init__.py +++ b/esphome/components/substitutions/__init__.py @@ -68,8 +68,9 @@ def raise_first_undefined( for e, p_path, _ in errors[1:] ) _LOGGER.debug("Additional undefined variables in %s: %s", context_label, extras) - raise cv.Invalid( - f"Undefined variable in {context_label}: {err.message}\n{format_path(err_path, err_value)}" + raise cv.SourceTraceInvalid( + f"Undefined variable in {context_label}: {err.message}", + format_path(err_path, err_value), ) @@ -223,13 +224,13 @@ def _expand_substitutions( errors=errors, ) except JinjaError as err: - raise cv.Invalid( + raise cv.SourceTraceInvalid( f"{err.error_name()} Error evaluating jinja expression" f" '{value}': {str(err.parent())}." f"\nEvaluation stack: (most recent evaluation last)" f"\n{err.stack_trace_str()}" - f"\nRelevant context:\n{err.context_trace_str()}" - f"\n{format_path(path, orig_value)}", + f"\nRelevant context:\n{err.context_trace_str()}", + format_path(path, orig_value), path, ) from err else: @@ -389,9 +390,9 @@ def resolve_include( return include.load() if include.should_load() else {} except (esphome.core.EsphomeError, cv.Invalid) as err: resolved = f" (expanded from '{original}')" if substituted else "" - raise cv.Invalid( - f"Error including file '{filename}'{resolved}: {err}" - f"\n{format_path(path, original)}", + raise cv.SourceTraceInvalid( + f"Error including file '{filename}'{resolved}: {err}", + format_path(path, original), path + [f"<{filename}>"], ) from err diff --git a/esphome/config.py b/esphome/config.py index 28a688f15ab..682557ecfc9 100644 --- a/esphome/config.py +++ b/esphome/config.py @@ -12,6 +12,7 @@ import re from typing import TYPE_CHECKING, Any import voluptuous as vol +import yaml from esphome import core, loader, pins, yaml_util from esphome.components.substitutions import do_substitution_pass @@ -26,8 +27,9 @@ from esphome.const import ( CONF_PACKAGES, CONF_PLATFORM, CONF_SUBSTITUTIONS, + ErrorFormat, ) -from esphome.core import CORE, DocumentRange, EsphomeError +from esphome.core import CORE, DocumentLocation, DocumentRange, EsphomeError # `esphome.core.config` is imported lazily at its two use sites below. # It pulls in `esphome.automation` and `esphome.config_validation`, which @@ -38,7 +40,7 @@ from esphome.loader import ComponentManifest, get_component, get_platform from esphome.log import AnsiFore, color from esphome.types import ConfigFragmentType, ConfigType from esphome.util import OrderedDict, safe_print -from esphome.voluptuous_schema import ExtraKeysInvalid +from esphome.voluptuous_schema import ExtraKeysInvalid, KeyInvalid from esphome.yaml_util import ESPHomeDataBase, ESPLiteralValue, is_secret if TYPE_CHECKING: @@ -285,12 +287,8 @@ class Config(OrderedDict, fv.FinalValidateConfig): last_root = max( i for i, v in enumerate(error.path) if v is cv.ROOT_CONFIG_PATH ) - # can't change the path so re-create the error - error = vol.Invalid( - message=error.error_message, - path=error.path[last_root + 1 :], - error_type=error.error_type, - ) + # Trim in place, as Invalid.prepend() does, to keep the subclass + error._path = error.path[last_root + 1 :] # pylint: disable=protected-access self.errors.append(error) def add_validation_step(self, step: ConfigValidationStep): @@ -1513,6 +1511,7 @@ class InvalidYAMLError(EsphomeError): message = f"Invalid YAML syntax:\n\n{base}" super().__init__(message) self.base_exc = base_exc + self.base_message = base def _load_config( @@ -1555,6 +1554,86 @@ def load_config( raise EsphomeError(f"Error while parsing config: {err}") from err +def get_invalid_range(res: Config, invalid: vol.Invalid) -> DocumentRange | None: + """Return the source range of the YAML that caused *invalid*.""" + return res.get_deepest_document_range_for_path( + invalid.path, isinstance(invalid, KeyInvalid) + ) + + +def _format_location(location: DocumentLocation | None) -> str: + """Return a 1-based `file:line:column`, or the main config file if unknown.""" + if location is None: + return str(CORE.config_path) + return f"{location.document}:{location.line + 1}:{location.column + 1}" + + +def _print_line_message(location: str, kind: str, message: object) -> None: + """Print *message* as `location: kind: text`, one line per message line. + + Lines after the first become `note:` lines, so tools that read one + message per line still see every line with its location. + """ + lines = [line.strip() for line in str(message).splitlines() if line.strip()] + first, *rest = lines or [""] + safe_print(f"{location}: {kind}: {first}") + for line in rest: + safe_print(f"{location}: note: {line}") + + +def _find_marked_yaml_error(err: BaseException) -> yaml.MarkedYAMLError | None: + cause: BaseException | None = err + while cause is not None and not isinstance(cause, yaml.MarkedYAMLError): + cause = cause.__cause__ + return cause + + +def _print_marked_yaml_error(err: yaml.MarkedYAMLError) -> None: + # Errors raised as MarkedYAMLError(message, mark) put the message in context + mark = err.problem_mark or err.context_mark + location = DocumentLocation.from_mark(mark) if mark is not None else None + if not (message := err.problem or err.context): + # str() includes the note, so it is not printed separately + _print_line_message(_format_location(location), "error", str(err)) + return + _print_line_message(_format_location(location), "error", message) + if err.problem and err.context and err.context_mark is not None: + context_location = DocumentLocation.from_mark(err.context_mark) + _print_line_message(_format_location(context_location), "note", err.context) + if err.note: + _print_line_message(_format_location(location), "note", err.note) + + +def _print_line_errors(res: Config) -> None: + for err in res.errors: + doc_range = get_invalid_range(res, err) + location = _format_location( + doc_range.start_mark if doc_range is not None else None + ) + # A YAML error in an included file is wrapped in a validation error + if (yaml_err := _find_marked_yaml_error(err)) is not None: + _print_marked_yaml_error(yaml_err) + _print_line_message(location, "note", "included from here") + elif isinstance(err, cv.SourceTraceInvalid): + # The trace only repeats the location this line already starts with + _print_line_message(location, "error", humanize_error(res, err.detail)) + else: + _print_line_message(location, "error", _format_vol_invalid(err, res)) + if doc_range is None and err.path: + # No source position, so give the config path to help find it + path = "->".join(str(item) for item in err.path) + _print_line_message(location, "note", f"In: {path}") + + +def _print_line_load_error(err: EsphomeError) -> None: + if (yaml_err := _find_marked_yaml_error(err)) is not None: + _print_marked_yaml_error(yaml_err) + else: + # Drop the "Invalid YAML syntax" heading; it is wrong for unreadable files + message = err.base_message if isinstance(err, InvalidYAMLError) else err + _print_line_message(_format_location(None), "error", message) + + def line_info(config, path, highlight=True): """Display line config source.""" if not highlight: @@ -1703,7 +1782,13 @@ def read_config( snapshot_user_config=snapshot_user_config, ) except EsphomeError as err: - _LOGGER.error("Error while reading config: %s", err) + if CORE.error_format == ErrorFormat.LINE: + _print_line_load_error(err) + else: + _LOGGER.error("Error while reading config: %s", err) + return None + if res.errors and CORE.error_format == ErrorFormat.LINE: + _print_line_errors(res) return None if res.errors: if not CORE.verbose: diff --git a/esphome/config_validation.py b/esphome/config_validation.py index 79d7068838a..64d22f9a143 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -113,7 +113,7 @@ from esphome.schema_extractors import ( # Deprecated re-export for external components; remove before 2027.2.0 # pylint: disable-next=unused-import from esphome.util import parse_esphome_version # noqa: F401 -from esphome.voluptuous_schema import _Schema +from esphome.voluptuous_schema import KeyInvalid, _Schema from esphome.yaml_util import SensitiveStr, make_data_base if typing.TYPE_CHECKING: @@ -452,6 +452,20 @@ def with_visibility(schema: Schema, visibility: Visibility, *keys: str) -> Schem return schema.extend(overrides) +class SourceTraceInvalid(Invalid): + """An error whose message is followed by where the bad value came from. + + ``detail`` is the message without that trace, for output that already + gives the location. + """ + + def __init__( + self, detail: str, source_trace: str, path: list[typing.Hashable] | None = None + ) -> None: + super().__init__(f"{detail}\n{source_trace}", path) + self.detail = detail + + class FinalExternalInvalid(Invalid): """Represents an invalid value in the final validation phase where the path should not be prepended.""" @@ -2293,7 +2307,7 @@ def validate_registry_entry(name, registry): if key is None: raise Invalid(f"Key missing from {name}! Got {value}") if key not in registry: - raise Invalid(f"Unable to find {name} with the name '{key}'", [key]) + raise KeyInvalid(f"Unable to find {name} with the name '{key}'", [key]) key2 = next((x for x in value if x != key and x not in ignore_keys), None) if key2 is not None: raise Invalid( diff --git a/esphome/const.py b/esphome/const.py index 7774722795d..115ccad7470 100644 --- a/esphome/const.py +++ b/esphome/const.py @@ -27,6 +27,15 @@ class Toolchain(StrEnum): HOST = "host" +class ErrorFormat(StrEnum): + """How configuration errors are printed.""" + + # Errors embedded in a dump of the failing YAML + YAML = "yaml" + # One `file:line:col: error: message` line per error + LINE = "line" + + # Toolchains that drive their build natively and never read platformio.ini. # SDK_NRF is absent on purpose: the zephyr backend keeps consuming # platformio_options. diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index 1668263e337..61092ec4272 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -30,6 +30,7 @@ from esphome.const import ( PLATFORM_NRF52, PLATFORM_RP2, PLATFORM_RTL87XX, + ErrorFormat, Toolchain, ) @@ -681,6 +682,8 @@ class EsphomeCore: self.verbose = False # Whether ESPHome was started in quiet mode self.quiet = False + # How configuration errors are printed + self.error_format: ErrorFormat = ErrorFormat.YAML # A list of all known ID classes self.id_classes = {} # The current component being processed during validation @@ -734,6 +737,7 @@ class EsphomeCore: self._config_hash = None self.skip_external_update = False self.toolchain = None + self.error_format = ErrorFormat.YAML PIN_SCHEMA_REGISTRY.reset() @contextmanager diff --git a/esphome/voluptuous_schema.py b/esphome/voluptuous_schema.py index 904963ba4e8..a31f3252ce3 100644 --- a/esphome/voluptuous_schema.py +++ b/esphome/voluptuous_schema.py @@ -6,10 +6,14 @@ import voluptuous as vol from esphome.schema_extractors import schema_extractor_extended -class ExtraKeysInvalid(vol.Invalid): +class KeyInvalid(vol.Invalid): + """An error caused by a mapping key itself rather than by its value.""" + + +class ExtraKeysInvalid(KeyInvalid): def __init__(self, *arg, **kwargs): self.candidates = kwargs.pop("candidates") - vol.Invalid.__init__(self, *arg, **kwargs) + super().__init__(*arg, **kwargs) def ensure_multiple_invalid(err): @@ -192,7 +196,7 @@ class _Schema(vol.Schema): ) else: errors.append( - vol.Invalid("extra keys not allowed", key_path) + KeyInvalid("extra keys not allowed", key_path) ) # for any required keys left that weren't found and don't have defaults: diff --git a/esphome/vscode.py b/esphome/vscode.py index ba7b4e727b5..e4ff0bb6ea6 100644 --- a/esphome/vscode.py +++ b/esphome/vscode.py @@ -7,19 +7,13 @@ import sys import traceback from typing import Any -from esphome.config import Config, _format_vol_invalid, validate_config +from esphome.config import _format_vol_invalid, get_invalid_range, validate_config import esphome.config_validation as cv from esphome.const import __version__ as ESPHOME_VERSION from esphome.core import CORE, DocumentRange, EsphomeError from esphome.yaml_util import parse_yaml -def _get_invalid_range(res: Config, invalid: cv.Invalid) -> DocumentRange | None: - return res.get_deepest_document_range_for_path( - invalid.path, invalid.error_message == "extra keys not allowed" - ) - - def _dump_range(range: DocumentRange | None) -> dict | None: if range is None: return None @@ -155,8 +149,9 @@ def read_config(args): else: for err in res.errors: try: - range_ = _get_invalid_range(res, err) + range_ = get_invalid_range(res, err) vs.add_validation_error(range_, _format_vol_invalid(err, res)) except Exception: # noqa: BLE001 # pylint: disable=broad-except - continue + # Report the error without a range rather than drop it + vs.add_validation_error(None, str(err)) print(vs.dump()) diff --git a/tests/unit_tests/test_config_error_format.py b/tests/unit_tests/test_config_error_format.py new file mode 100644 index 00000000000..395253dd708 --- /dev/null +++ b/tests/unit_tests/test_config_error_format.py @@ -0,0 +1,312 @@ +"""Tests for the `--error-format line` output of read_config.""" + +from pathlib import Path + +import pytest +import voluptuous as vol +import yaml + +from esphome import config_validation as cv +from esphome.config import ( + Config, + InvalidYAMLError, + _print_line_errors, + _print_line_load_error, + _print_line_message, + _print_marked_yaml_error, + read_config, +) +from esphome.const import ErrorFormat +from esphome.core import CORE +from esphome.voluptuous_schema import ExtraKeysInvalid, KeyInvalid + + +def _read( + tmp_path: Path, files: dict[str, str], capsys: pytest.CaptureFixture[str] +) -> list[str]: + for name, content in files.items(): + (tmp_path / name).write_text(content) + CORE.config_path = tmp_path / "test.yaml" + CORE.error_format = ErrorFormat.LINE + assert read_config({}) is None + return capsys.readouterr().out.splitlines() + + +def test_validation_error(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + lines = _read( + tmp_path, + {"test.yaml": "esphome:\n name: test\nhost:\nlogger:\n levl: DEBUG\n"}, + capsys, + ) + assert lines == [ + ( + f"{tmp_path / 'test.yaml'}:5:3: error: [levl] is an invalid option for " + "[logger]. Did you mean [level]?" + ) + ] + + +def test_validation_error_in_package( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + lines = _read( + tmp_path, + { + "test.yaml": ( + "esphome:\n name: test\nhost:\npackages:\n pkg: !include pkg.yaml\n" + ), + "pkg.yaml": "logger:\n levl: DEBUG\n", + }, + capsys, + ) + assert lines == [ + ( + f"{tmp_path / 'pkg.yaml'}:2:3: error: [levl] is an invalid option for " + "[logger]. Did you mean [level]?" + ) + ] + + +def test_unknown_action_anchors_on_the_key( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # validate_registry_entry raises KeyInvalid for an unknown action, so the + # location is the key, not the action's value. + lines = _read( + tmp_path, + { + "test.yaml": ( + "esphome:\n name: test\nhost:\nlogger:\nbutton:\n" + ' - platform: template\n on_press:\n - loggr.log: "msg"\n' + ) + }, + capsys, + ) + assert lines == [ + ( + f"{tmp_path / 'test.yaml'}:8:9: error: Unable to find action " + "with the name 'loggr.log'." + ) + ] + + +def test_yaml_syntax_error(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + lines = _read(tmp_path, {"test.yaml": "esphome:\n name: [1, 2\nhost:\n"}, capsys) + assert lines == [ + f"{tmp_path / 'test.yaml'}:3:5: error: expected ',' or ']', but got ':'", + f"{tmp_path / 'test.yaml'}:2:9: note: while parsing a flow sequence", + ] + + +def test_yaml_syntax_error_in_package( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + lines = _read( + tmp_path, + { + "test.yaml": ( + "esphome:\n name: test\nhost:\npackages:\n pkg: !include pkg.yaml\n" + ), + "pkg.yaml": "logger:\n level: [1, 2\nwifi:\n", + }, + capsys, + ) + assert lines == [ + f"{tmp_path / 'pkg.yaml'}:3:5: error: expected ',' or ']', but got ':'", + f"{tmp_path / 'pkg.yaml'}:2:10: note: while parsing a flow sequence", + f"{tmp_path / 'test.yaml'}:5:3: note: included from here", + ] + + +def test_secret_syntax_error_in_package( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + (tmp_path / "sub").mkdir() + lines = _read( + tmp_path, + { + "test.yaml": ( + "esphome:\n name: test\nhost:\npackages:\n p: !include sub/pkg.yaml\n" + ), + "sub/pkg.yaml": "logger:\n level: !secret lvl\n", + "secrets.yaml": "lvl: [1, 2\nx: 1\n", + }, + capsys, + ) + assert lines == [ + f"{tmp_path / 'secrets.yaml'}:2:2: error: expected ',' or ']', but got ':'", + f"{tmp_path / 'secrets.yaml'}:1:6: note: while parsing a flow sequence", + f"{tmp_path / 'test.yaml'}:5:3: note: included from here", + ] + + +def test_undefined_secret(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + lines = _read( + tmp_path, + { + "test.yaml": "esphome:\n name: !secret nope\nhost:\n", + "secrets.yaml": "other: 1\n", + }, + capsys, + ) + assert lines == [f"{tmp_path / 'test.yaml'}:2:9: error: Secret 'nope' not defined"] + + +def test_missing_file(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + CORE.config_path = tmp_path / "missing.yaml" + CORE.error_format = ErrorFormat.LINE + assert read_config({}) is None + lines = capsys.readouterr().out.splitlines() + assert len(lines) == 1 + assert lines[0].startswith( + f"{tmp_path / 'missing.yaml'}: error: Error reading file " + ) + + +def test_source_trace_omitted( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + lines = _read( + tmp_path, + { + "test.yaml": ( + "esphome:\n name: test\nhost:\n" + "packages:\n p: !include ${ undefined_var }.yaml\n" + ), + }, + capsys, + ) + # The "In: packages->p in test.yaml 5:3" trace would repeat the location + assert lines == [ + ( + f"{tmp_path / 'test.yaml'}:5:3: error: Error including file " + "'${ undefined_var }.yaml': Cannot load include with unresolved " + "substitutions: ${ undefined_var }.yaml." + ), + ] + + +def test_print_line_message_skips_blank_lines( + capsys: pytest.CaptureFixture[str], +) -> None: + _print_line_message("a.yaml:1:1", "error", "first\n\n second \n") + assert capsys.readouterr().out.splitlines() == [ + "a.yaml:1:1: error: first", + "a.yaml:1:1: note: second", + ] + + +def test_reset_restores_default_format() -> None: + CORE.error_format = ErrorFormat.LINE + CORE.reset() + assert CORE.error_format is ErrorFormat.YAML + + +@pytest.mark.parametrize( + "error", + [ + KeyInvalid("Unable to find action", ["a", cv.ROOT_CONFIG_PATH, "b"]), + ExtraKeysInvalid( + "extra keys not allowed", + ["a", cv.ROOT_CONFIG_PATH, "b"], + candidates=["c"], + ), + ], +) +def test_add_error_keeps_error_class(error: KeyInvalid) -> None: + res = Config() + res.add_error(error) + (added,) = res.errors + assert type(added) is type(error) + assert added.path == ["b"] + assert getattr(added, "candidates", None) == getattr(error, "candidates", None) + + +def test_default_format_logs_load_error( + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + caplog: pytest.LogCaptureFixture, +) -> None: + CORE.config_path = tmp_path / "missing.yaml" + assert read_config({}) is None + assert capsys.readouterr().out == "" + assert "Error while reading config: Invalid YAML syntax" in caplog.text + + +def test_exception_as_error_message( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # "ms" with no number makes the time period validator raise Invalid(ValueError) + lines = _read( + tmp_path, + { + "test.yaml": ( + "esphome:\n name: test\nhost:\nsensor:\n - platform: template\n" + " name: foo\n update_interval: ms\n" + ) + }, + capsys, + ) + assert lines == [ + ( + f"{tmp_path / 'test.yaml'}:7:22: error: " + "could not convert string to float: ''." + ) + ] + + +def _mark(line: int, column: int) -> yaml.Mark: + return yaml.Mark("a.yaml", 0, line, column, None, None) + + +@pytest.mark.parametrize( + ("error", "expected"), + [ + ( + yaml.MarkedYAMLError( + problem="bad value", problem_mark=_mark(2, 1), note="a hint" + ), + ["a.yaml:3:2: error: bad value", "a.yaml:3:2: note: a hint"], + ), + # No problem or context: the whole error text is used, note included + ( + yaml.MarkedYAMLError(problem_mark=_mark(2, 1), note="a hint"), + [ + 'a.yaml:3:2: error: in "a.yaml", line 3, column 2', + "a.yaml:3:2: note: a hint", + ], + ), + ], +) +def test_marked_yaml_error_note( + capsys: pytest.CaptureFixture[str], + error: yaml.MarkedYAMLError, + expected: list[str], +) -> None: + _print_marked_yaml_error(error) + assert capsys.readouterr().out.splitlines() == expected + + +def test_error_without_range_gives_config_path( + capsys: pytest.CaptureFixture[str], +) -> None: + CORE.config_path = Path("main.yaml") + res = Config() + res.add_error(vol.Invalid("bad value", ["sensor", 0, "name"])) + _print_line_errors(res) + assert capsys.readouterr().out.splitlines() == [ + "main.yaml: error: bad value.", + "main.yaml: note: In: sensor->0->name", + ] + + +def test_load_error_with_unprintable_cause( + capsys: pytest.CaptureFixture[str], +) -> None: + class Unprintable(Exception): + def __str__(self) -> str: + raise UnicodeDecodeError("utf-8", b"\xff", 0, 1, "invalid start byte") + + CORE.config_path = Path("main.yaml") + _print_line_load_error(InvalidYAMLError(Unprintable())) + assert capsys.readouterr().out.splitlines() == ["main.yaml: error: Unprintable()"] diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 37d8e7486f4..3b8330f1591 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -115,6 +115,7 @@ from esphome.const import ( PLATFORM_HOST, PLATFORM_NRF52, PLATFORM_RP2, + ErrorFormat, Toolchain, ) from esphome.core import CORE, EsphomeError @@ -6462,6 +6463,62 @@ def test_parse_args_logs_states() -> None: assert args.states is True +@pytest.mark.parametrize( + ("argv", "env_value", "expected"), + [ + ([], None, ErrorFormat.YAML), + (["--error-format", "line"], None, ErrorFormat.LINE), + ([], "line", ErrorFormat.LINE), + (["--error-format", "yaml"], "line", ErrorFormat.YAML), + (["--error-format", "LINE"], None, ErrorFormat.LINE), + ([], "Line", ErrorFormat.LINE), + # The command line wins, so a bad environment value is not an error + (["--error-format", "line"], "bogus", ErrorFormat.LINE), + ], +) +def test_parse_args_error_format( + monkeypatch: pytest.MonkeyPatch, + argv: list[str], + env_value: str | None, + expected: ErrorFormat, +) -> None: + """--error-format overrides ESPHOME_ERROR_FORMAT, which overrides the default.""" + if env_value is None: + monkeypatch.delenv("ESPHOME_ERROR_FORMAT", raising=False) + else: + monkeypatch.setenv("ESPHOME_ERROR_FORMAT", env_value) + + args = parse_args(["esphome", *argv, "config", "device.yaml"]) + assert args.error_format is expected + + +@pytest.mark.parametrize( + ("argv", "env_value", "expected_name"), + [ + ([], "bogus", "ESPHOME_ERROR_FORMAT"), + (["--error-format", "bogus"], None, "--error-format"), + ], +) +def test_parse_args_error_format_invalid( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + argv: list[str], + env_value: str | None, + expected_name: str, +) -> None: + """A bad value is reported against the setting that supplied it.""" + if env_value is None: + monkeypatch.delenv("ESPHOME_ERROR_FORMAT", raising=False) + else: + monkeypatch.setenv("ESPHOME_ERROR_FORMAT", env_value) + + with pytest.raises(SystemExit): + parse_args(["esphome", *argv, "config", "device.yaml"]) + err = capsys.readouterr().err + assert expected_name in err + assert "invalid choice: 'bogus' (choose from yaml, line)" in err + + @pytest.mark.parametrize("command", ["logs", "run"]) @pytest.mark.parametrize( ("env_value", "expected"), diff --git a/tests/unit_tests/test_vscode.py b/tests/unit_tests/test_vscode.py index 7d401eaa653..178fd840203 100644 --- a/tests/unit_tests/test_vscode.py +++ b/tests/unit_tests/test_vscode.py @@ -192,3 +192,75 @@ def test_invalid_stays_plain() -> None: def test_format_unexpected_error_without_traceback() -> None: message = vscode._format_unexpected_error(ValueError("boom")) assert message == "Unexpected error while validating: ValueError: boom" + + +def test_reports_error_with_exception_message(): + source_path = str(Path("dir_path", "x.yaml")) + output_lines = _run_repl_test( + [ + _validate(source_path), + # read_file x.yaml + _file_response("""esphome: + name: test1 +host: +sensor: + - platform: template + name: foo + update_interval: ms # no number, so the validator raises Invalid(ValueError) +"""), + ] + ) + + error = json.loads(output_lines[-1]) + validation_error = error["validation_errors"][0] + assert validation_error["message"] == "could not convert string to float: ''." + assert validation_error["range"]["start_line"] == 6 + + +def test_unknown_action_range_is_the_key(): + source_path = str(Path("dir_path", "x.yaml")) + output_lines = _run_repl_test( + [ + _validate(source_path), + # read_file x.yaml + _file_response("""esphome: + name: test1 +host: +button: + - platform: template + on_press: + - loggr.log: "msg" +"""), + ] + ) + + error = json.loads(output_lines[-1]) + validation_error = error["validation_errors"][0] + assert ( + validation_error["message"] + == "Unable to find action with the name 'loggr.log'." + ) + assert validation_error["range"]["start_line"] == 6 + assert validation_error["range"]["start_col"] == 8 + + +def test_error_kept_when_range_lookup_fails(): + source_path = str(Path("dir_path", "x.yaml")) + with patch.object(vscode, "get_invalid_range", side_effect=RuntimeError("boom")): + output_lines = _run_repl_test( + [ + _validate(source_path), + # read_file x.yaml + _file_response("""esphome: + name: test1 +host: +logger: + levl: DEBUG +"""), + ] + ) + + error = json.loads(output_lines[-1]) + (validation_error,) = error["validation_errors"] + assert validation_error["range"] is None + assert "extra keys not allowed" in validation_error["message"]