diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index 5fb95fa0d5c..65309d9cdc1 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -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) diff --git a/tests/component_tests/packages/test_packages.py b/tests/component_tests/packages/test_packages.py index 0107314947a..fd7e9f12786 100644 --- a/tests/component_tests/packages/test_packages.py +++ b/tests/component_tests/packages/test_packages.py @@ -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") diff --git a/tests/unit_tests/test_bundle.py b/tests/unit_tests/test_bundle.py index 6a086767840..fe4bf197157 100644 --- a/tests/unit_tests/test_bundle.py +++ b/tests/unit_tests/test_bundle.py @@ -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: diff --git a/tests/unit_tests/test_yaml_util.py b/tests/unit_tests/test_yaml_util.py index 914b1948070..f0bfebcb8ac 100644 --- a/tests/unit_tests/test_yaml_util.py +++ b/tests/unit_tests/test_yaml_util.py @@ -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(