From 5bd328e6c41230f4c888869f3778795c7e35f561 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 19:25:33 -0500 Subject: [PATCH] Fail on a declared-but-absent extraScript, warn once per unsupported env method, split and widen tests --- esphome/platformio/extra_script.py | 20 +++++++++++-------- tests/unit_tests/test_main.py | 17 ++++++++++++---- .../test_platformio_extra_script.py | 13 ++++++------ tests/unit_tests/test_platformio_library.py | 9 ++++++--- 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py index 1e0a233309..af6061a059 100644 --- a/esphome/platformio/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -72,14 +72,11 @@ def apply_extra_script( ) return if not script_path.is_file(): - # The script's captured -L/-l/-D flags are lost; surface that here - # instead of as undefined references at link time - _LOGGER.warning( - "extraScript %s of library %s not found; skipping", - extra_script, - component.name, + # A declared-but-absent script is a broken or half-downloaded + # package, not an unsupported script; PlatformIO fails on it too + raise EsphomeError( + f"extraScript {extra_script} of library {component.name} not found" ) - return result = run_extra_script( script_path, library_dir=source_path, @@ -134,6 +131,7 @@ class _FakeSConsEnv: "PIOENV": pio_env, } self.result = ExtraScriptResult() + self._warned_methods: set[str] = set() # ----- SCons env API the common scripts use ----- @@ -158,7 +156,13 @@ class _FakeSConsEnv: def __getattr__(self, name: str): def _noop(*args, **kwargs): - _LOGGER.debug("PIO extra-script env.%s(...) is a no-op here", name) + # Once per method: a script whose whole effect is env.Replace() + # must be diagnosable from a normal build log + if name not in self._warned_methods: + self._warned_methods.add(name) + _LOGGER.warning( + "PIO extra-script env.%s(...) is not supported; ignoring", name + ) return _noop diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index f81da9052b..b6a6e300c4 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -7132,7 +7132,17 @@ def test_warn_source_tree_mismatch_falls_back_when_stat_fails( assert not caplog.text +@pytest.mark.parametrize( + "error", + [ + FileNotFoundError("no such compiler"), + RuntimeError("Could not query builtin include dirs"), + ValueError("no C++ translation unit found"), + None, # replaced with EsphomeError inside (import is function-local) + ], +) def test_compile_program_espidf_idedata_failure_does_not_fail_build( + error: Exception, caplog: pytest.LogCaptureFixture, ) -> None: """A post-compile idedata error is a warning: the firmware already built.""" @@ -7144,6 +7154,8 @@ def test_compile_program_espidf_idedata_failure_does_not_fail_build( ) from esphome.core import CORE, EsphomeError + if error is None: + error = EsphomeError("compile database is unusable") CORE.toolchain = Toolchain.ESP_IDF CORE.data[KEY_CORE] = { KEY_TARGET_PLATFORM: "esp32", @@ -7154,10 +7166,7 @@ def test_compile_program_espidf_idedata_failure_does_not_fail_build( patch("esphome.espidf.toolchain.create_factory_bin"), patch("esphome.espidf.toolchain.create_ota_bin"), patch("esphome.espidf.toolchain.create_elf_copy"), - patch( - "esphome.espidf.toolchain.get_idedata", - side_effect=EsphomeError("compile database is unusable"), - ), + patch("esphome.espidf.toolchain.get_idedata", side_effect=error), patch("esphome.__main__._check_and_emit_build_info"), ): assert compile_program(MagicMock(), {}) == 0 diff --git a/tests/unit_tests/test_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py index 3de9ced3d8..f15f94f0e7 100644 --- a/tests/unit_tests/test_platformio_extra_script.py +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -216,7 +216,7 @@ def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path, caplog) -> No apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") assert c.data["build"]["flags"] == ["-lsingle"] assert "env.Append(UNCAPTURED=...) is not captured" in caplog.text - assert "env.Replace(...) is a no-op here" in caplog.text + assert "env.Replace(...) is not supported" in caplog.text def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None: @@ -246,16 +246,17 @@ def test_apply_extra_script_pio_platform(tmp_path) -> None: assert c.data["build"]["flags"] == ["-lespressif8266"] -def test_apply_extra_script_missing_script_logged(tmp_path, caplog) -> None: - """A declared but absent extraScript is skipped with a visible warning: - its captured link flags are lost.""" +def test_apply_extra_script_missing_script_raises(tmp_path) -> None: + """A declared but absent extraScript is a broken package and fails by + name, as it would under PlatformIO.""" + from esphome.core import EsphomeError from esphome.platformio.extra_script import apply_extra_script c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) c.path = tmp_path c.data = {"build": {"extraScript": "nope.py"}} - apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") - assert "not found" in caplog.text + with pytest.raises(EsphomeError, match="nope.py of library owner/name not found"): + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") def test_run_extra_script_failure_discards_partial_capture(tmp_path, caplog) -> None: diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 0acb0d69cb..9567ab985f 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -538,12 +538,15 @@ def test_split_flag_entry_unbalanced_quote_is_clean() -> None: from esphome.platformio.library import split_flag_entry assert split_flag_entry('-DX="a b"', "library x") == ["-DX=a b"] - # join_flag_args re-glues a spaced -D like ParseFlags does + with pytest.raises(EsphomeError, match=r"Malformed build flag.*library x"): + split_flag_entry('-DX="unclosed', "library x") + + +def test_join_flag_args_reglues_spaced_define() -> None: + """A spaced -D re-glues to its argument, as ParseFlags does.""" from esphome.platformio.library import join_flag_args assert join_flag_args(["-D", "FOO=1", "-Os"], "x") == ["-DFOO=1", "-Os"] - with pytest.raises(EsphomeError, match=r"Malformed build flag.*library x"): - split_flag_entry('-DX="unclosed', "library x") def test_join_flag_args_trailing_bare_flag_warns(