mirror of
https://github.com/esphome/esphome.git
synced 2026-10-15 10:42:52 +00:00
[core] Log discovery load failures once and test package fetch order
This commit is contained in:
+23
-11
@@ -391,6 +391,7 @@ def _load_include_candidates(
|
||||
include: IncludeFile,
|
||||
*,
|
||||
warn_on_unresolved: bool,
|
||||
warn_on_load_error: bool,
|
||||
seen: set[int],
|
||||
expanded_paths: set[Path],
|
||||
keepalive: list[Any],
|
||||
@@ -422,12 +423,12 @@ def _load_include_candidates(
|
||||
try:
|
||||
loaded = include.with_file(candidate.as_posix()).load()
|
||||
except (EsphomeError, Invalid) as err:
|
||||
# Unlike an unresolved pattern (expected during the discovery
|
||||
# re-parse), a matched on-disk candidate that fails to load is a
|
||||
# genuine user error; warn in every mode. The file itself is
|
||||
# still tracked (the load listener fires before parsing), only
|
||||
# its nested includes go undiscovered.
|
||||
_LOGGER.warning(
|
||||
# A matched on-disk candidate that fails to load is a genuine user
|
||||
# error; callers that report ``result.errors`` themselves pass
|
||||
# warn_on_load_error=False so it is not logged twice. The file
|
||||
# itself is still tracked (the load listener fires before
|
||||
# parsing), only its nested includes go undiscovered.
|
||||
(_LOGGER.warning if warn_on_load_error else _LOGGER.debug)(
|
||||
"Failed to load candidate %s for !include %s: %s",
|
||||
candidate,
|
||||
include.file,
|
||||
@@ -441,6 +442,7 @@ def _load_include_candidates(
|
||||
_force_load_include_files(
|
||||
loaded,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
warn_on_load_error=warn_on_load_error,
|
||||
_seen=seen,
|
||||
_expanded_paths=expanded_paths,
|
||||
_keepalive=keepalive,
|
||||
@@ -475,7 +477,7 @@ class ForceLoadResult(NamedTuple):
|
||||
|
||||
|
||||
def force_load_include_files(
|
||||
obj: Any, *, warn_on_unresolved: bool = True
|
||||
obj: Any, *, warn_on_unresolved: bool = True, warn_on_load_error: bool = True
|
||||
) -> ForceLoadResult:
|
||||
"""Recursively resolve any deferred ``IncludeFile`` instances in a YAML tree.
|
||||
|
||||
@@ -491,11 +493,16 @@ def force_load_include_files(
|
||||
expression could select. By default a warning is logged when no candidate
|
||||
exists; pass ``warn_on_unresolved=False`` (used by discovery paths that
|
||||
run on a fresh re-parse where substitutions haven't been applied yet) to
|
||||
demote it to a debug log.
|
||||
demote it to a debug log. Includes that fail to load are logged as
|
||||
warnings too; callers that report ``errors`` themselves pass
|
||||
``warn_on_load_error=False`` so each failure is shown once.
|
||||
"""
|
||||
result = ForceLoadResult([], [])
|
||||
_force_load_include_files(
|
||||
obj, warn_on_unresolved=warn_on_unresolved, _result=result
|
||||
obj,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
warn_on_load_error=warn_on_load_error,
|
||||
_result=result,
|
||||
)
|
||||
return result
|
||||
|
||||
@@ -504,6 +511,7 @@ def _force_load_include_files(
|
||||
obj: Any,
|
||||
*,
|
||||
warn_on_unresolved: bool = True,
|
||||
warn_on_load_error: bool = True,
|
||||
_seen: set[int] | None = None,
|
||||
_expanded_paths: set[Path] | None = None,
|
||||
_keepalive: list[Any] | None = None,
|
||||
@@ -532,6 +540,7 @@ def _force_load_include_files(
|
||||
_load_include_candidates(
|
||||
obj,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
warn_on_load_error=warn_on_load_error,
|
||||
seen=_seen,
|
||||
expanded_paths=_expanded_paths,
|
||||
keepalive=_keepalive,
|
||||
@@ -541,7 +550,7 @@ def _force_load_include_files(
|
||||
try:
|
||||
loaded = obj.load()
|
||||
except (EsphomeError, Invalid) as err:
|
||||
_LOGGER.warning(
|
||||
(_LOGGER.warning if warn_on_load_error else _LOGGER.debug)(
|
||||
"Failed to load !include %s (referenced from %s): %s",
|
||||
obj.file,
|
||||
obj.parent_file,
|
||||
@@ -552,6 +561,7 @@ def _force_load_include_files(
|
||||
_force_load_include_files(
|
||||
loaded,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
warn_on_load_error=warn_on_load_error,
|
||||
_seen=_seen,
|
||||
_expanded_paths=_expanded_paths,
|
||||
_keepalive=_keepalive,
|
||||
@@ -565,6 +575,7 @@ def _force_load_include_files(
|
||||
_force_load_include_files(
|
||||
value,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
warn_on_load_error=warn_on_load_error,
|
||||
_seen=_seen,
|
||||
_expanded_paths=_expanded_paths,
|
||||
_keepalive=_keepalive,
|
||||
@@ -578,6 +589,7 @@ def _force_load_include_files(
|
||||
_force_load_include_files(
|
||||
item,
|
||||
warn_on_unresolved=warn_on_unresolved,
|
||||
warn_on_load_error=warn_on_load_error,
|
||||
_seen=_seen,
|
||||
_expanded_paths=_expanded_paths,
|
||||
_keepalive=_keepalive,
|
||||
@@ -637,7 +649,7 @@ def discover_user_yaml_files(config_path: Path) -> DiscoveredYamlFiles:
|
||||
list(loaded), secrets, load_errors=[f"{config_path}: {err}"]
|
||||
)
|
||||
unresolved, load_errors = force_load_include_files(
|
||||
data, warn_on_unresolved=False
|
||||
data, warn_on_unresolved=False, warn_on_load_error=False
|
||||
)
|
||||
finally:
|
||||
_load_listeners.remove(_capture_secret)
|
||||
|
||||
@@ -652,11 +652,11 @@ def test_get_remote_package_sources_records_each_fetch_once(
|
||||
}
|
||||
packages_pass(config)
|
||||
|
||||
sources = get_remote_package_sources()
|
||||
assert sorted(sources, key=lambda source: source.url) == [
|
||||
RemotePackageSource("https://github.com/esphome/repo-a", "main"),
|
||||
# packages are processed last to first, so repo-b is fetched before repo-a
|
||||
assert get_remote_package_sources() == (
|
||||
RemotePackageSource("https://github.com/esphome/repo-b", None),
|
||||
]
|
||||
RemotePackageSource("https://github.com/esphome/repo-a", "main"),
|
||||
)
|
||||
|
||||
|
||||
@patch("esphome.yaml_util.load_yaml")
|
||||
|
||||
@@ -1363,7 +1363,8 @@ def test_discover_files_candidate_outside_config_dir_skipped(
|
||||
def test_discover_files_nested_include_load_failure(
|
||||
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A nested !include pointing at a missing file is logged and skipped."""
|
||||
"""A nested !include pointing at a missing file is reported in the bundle
|
||||
summary, not a second time by yaml_util, and skipped."""
|
||||
config_dir = _setup_config_dir(tmp_path)
|
||||
(config_dir / "test.yaml").write_text(
|
||||
"esphome:\n name: test\nwifi: !include missing.yaml\n"
|
||||
@@ -1374,10 +1375,12 @@ def test_discover_files_nested_include_load_failure(
|
||||
|
||||
paths = [f.path for f in files]
|
||||
assert "test.yaml" in paths
|
||||
warnings = [r.message for r in caplog.records if r.levelname == "WARNING"]
|
||||
assert any(
|
||||
"failed to load !include" in r.message.lower() and "missing.yaml" in r.message
|
||||
for r in caplog.records
|
||||
m.startswith("Bundle may be incomplete") and "missing.yaml" in m
|
||||
for m in warnings
|
||||
)
|
||||
assert not any("failed to load !include" in m.lower() for m in warnings)
|
||||
|
||||
|
||||
def test_force_load_skips_duplicate_include_file() -> None:
|
||||
|
||||
@@ -1309,6 +1309,20 @@ def test_force_load_include_files_warns_on_load_failure(
|
||||
assert result.unresolved == []
|
||||
|
||||
|
||||
def test_force_load_include_files_load_failure_quiet_when_caller_reports(
|
||||
patch_include_file: None,
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""With warn_on_load_error=False the failure is still returned but only
|
||||
logged at debug, so a caller that reports it does not duplicate it."""
|
||||
stub = _StubInclude("missing.yaml", raise_on_load=EsphomeError("boom"))
|
||||
with caplog.at_level("DEBUG", logger="esphome.yaml_util"):
|
||||
result = force_load_include_files({"k": stub}, warn_on_load_error=False)
|
||||
assert result.errors == [f"{stub.file}: boom"]
|
||||
failures = [r for r in caplog.records if "Failed to load !include" in r.message]
|
||||
assert [r.levelname for r in failures] == ["DEBUG"]
|
||||
|
||||
|
||||
def test_discovered_yaml_files_holds_files_and_secrets() -> None:
|
||||
"""`DiscoveredYamlFiles` is a small data carrier."""
|
||||
files = [Path("/tmp/a.yaml")]
|
||||
@@ -1617,9 +1631,9 @@ def test_discover_user_yaml_files_many_candidates_keep_nested_includes(
|
||||
def test_discover_user_yaml_files_bad_candidate_still_tracked(
|
||||
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""A matched candidate that fails to parse warns even during discovery,
|
||||
stays tracked (the load listener fires before parsing), and doesn't block
|
||||
other candidates."""
|
||||
"""A matched candidate that fails to parse is reported in ``load_errors``
|
||||
(logged only at debug, the caller reports it), stays tracked (the load
|
||||
listener fires before parsing), and doesn't block other candidates."""
|
||||
_write(tmp_path, "keys/good.yaml", "api:\n")
|
||||
_write(tmp_path, "keys/bad.yaml", "esphome: [unterminated\n")
|
||||
with caplog.at_level("DEBUG", logger="esphome.yaml_util"):
|
||||
@@ -1632,7 +1646,8 @@ def test_discover_user_yaml_files_bad_candidate_still_tracked(
|
||||
matching = [
|
||||
r.levelname for r in caplog.records if "Failed to load candidate" in r.message
|
||||
]
|
||||
assert matching == ["WARNING"]
|
||||
assert matching == ["DEBUG"]
|
||||
assert any("keys/bad.yaml" in e for e in discovered.load_errors)
|
||||
|
||||
|
||||
def test_discover_user_yaml_files_tolerates_templated_top_level_include(
|
||||
|
||||
Reference in New Issue
Block a user