From 9bd548ba92eb4a9bb2c418390f7a0f04423eaeac Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 13:35:07 -0500 Subject: [PATCH] Move the extraScript machinery into the platformio package and tidy the extraction --- esphome/build_helpers/idedata.py | 10 +- esphome/espidf/clang_tidy.py | 6 +- esphome/espidf/component.py | 8 +- .../extra_script.py | 28 ++--- esphome/platformio/library.py | 10 +- script/determine-jobs.py | 13 ++- tests/unit_tests/build_helpers/__init__.py | 0 .../unit_tests/build_helpers/test_idedata.py | 77 +++++++++---- .../build_helpers/test_size_summary.py | 10 ++ tests/unit_tests/test_espidf_clang_tidy.py | 14 +-- tests/unit_tests/test_espidf_component.py | 16 +++ tests/unit_tests/test_espidf_toolchain.py | 108 ------------------ ...ipt.py => test_platformio_extra_script.py} | 64 ++++++----- tests/unit_tests/test_size_summary.py | 7 -- 14 files changed, 159 insertions(+), 212 deletions(-) rename esphome/{build_helpers => platformio}/extra_script.py (93%) create mode 100644 tests/unit_tests/build_helpers/__init__.py create mode 100644 tests/unit_tests/build_helpers/test_size_summary.py rename tests/unit_tests/{build_helpers/test_extra_script.py => test_platformio_extra_script.py} (76%) diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index fd01b4fb28..5bf2a4b5ec 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -126,7 +126,7 @@ def _pick_entry(entries: list[dict]) -> dict: _COMPILER_STEM = re.compile(r"(?:gcc|g\+\+|cc|c\+\+|clang|clang\+\+)$") -def _parse_entry( +def parse_entry( entry: dict, launcher: str | None = None ) -> tuple[str, list[str], list[str], list[str]]: """Parse one compile_commands entry -> (cxx_path, defines, includes, cxx_flags).""" @@ -188,7 +188,7 @@ def _parse_entry( return cxx_path, defines, includes, cxx_flags -def _get_toolchain_includes(cxx_path: str) -> list[str]: +def get_toolchain_includes(cxx_path: str) -> list[str]: """Query the compiler for its builtin ``#include <...>`` search dirs.""" result = subprocess.run( [cxx_path, "-E", "-x", "c++", "-", "-v"], @@ -287,13 +287,13 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d provides). """ entries = json.loads(Path(compile_commands).read_text(encoding="utf-8")) - cxx_path, defines, _, cxx_flags = _parse_entry(_pick_entry(entries), launcher) + cxx_path, defines, _, cxx_flags = parse_entry(_pick_entry(entries), launcher) build_includes: dict[str, None] = {} for entry in entries: if not _is_esphome_src(entry["file"]): continue - for inc in _parse_entry(entry, launcher)[2]: + for inc in parse_entry(entry, launcher)[2]: build_includes.setdefault(inc, None) return { @@ -303,6 +303,6 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d "defines": defines, "includes": { "build": list(build_includes), - "toolchain": _get_toolchain_includes(cxx_path), + "toolchain": get_toolchain_includes(cxx_path), }, } diff --git a/esphome/espidf/clang_tidy.py b/esphome/espidf/clang_tidy.py index fc98d2c21f..61ce94fc9d 100644 --- a/esphome/espidf/clang_tidy.py +++ b/esphome/espidf/clang_tidy.py @@ -23,7 +23,7 @@ from dataclasses import dataclass import os from pathlib import Path -from esphome.build_helpers.idedata import _get_toolchain_includes, _parse_entry +from esphome.build_helpers.idedata import get_toolchain_includes, parse_entry TIDY_PROJECT_NAME = "esphome_tidy" @@ -421,7 +421,7 @@ def _idedata_from_tidy_project(compile_commands: Path) -> dict: entry = next((e for e in entries if e["file"].endswith("tidy.cpp")), None) if entry is None: raise RuntimeError(f"tidy.cpp not found in {compile_commands}") - cxx_path, defines, includes, cxx_flags = _parse_entry(entry) + cxx_path, defines, includes, cxx_flags = parse_entry(entry) return { "cxx_path": cxx_path, @@ -429,7 +429,7 @@ def _idedata_from_tidy_project(compile_commands: Path) -> dict: "defines": defines, "includes": { "build": includes, - "toolchain": _get_toolchain_includes(cxx_path), + "toolchain": get_toolchain_includes(cxx_path), }, } diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index 4847c834f0..604a61af00 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -43,10 +43,14 @@ def _idf_framework() -> str: def _apply_extra_script(component: IDFComponent) -> None: - from esphome.build_helpers.extra_script import apply_extra_script from esphome.components.esp32 import get_esp32_variant + from esphome.platformio.extra_script import apply_extra_script - apply_extra_script(component, lambda: variant_to_idf_target(get_esp32_variant())) + apply_extra_script( + component, + board_mcu=lambda: variant_to_idf_target(get_esp32_variant()), + pio_platform="espressif32", + ) def generate_cmakelists_txt(component: IDFComponent) -> str: diff --git a/esphome/build_helpers/extra_script.py b/esphome/platformio/extra_script.py similarity index 93% rename from esphome/build_helpers/extra_script.py rename to esphome/platformio/extra_script.py index 6ca06ad5ba..f1c2ad7a9a 100644 --- a/esphome/build_helpers/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -35,6 +35,8 @@ import os from pathlib import Path from typing import TYPE_CHECKING +from esphome.platformio.library import ensure_list + if TYPE_CHECKING: from esphome.platformio.library import ConvertedLibrary @@ -43,14 +45,14 @@ _LOGGER = logging.getLogger(__name__) def apply_extra_script( component: ConvertedLibrary, - idf_target: str | Callable[[], str], - pio_platform: str = "espressif32", + board_mcu: str | Callable[[], str], + pio_platform: str, ) -> None: """Run a library's PIO ``extraScript`` and fold its captured env vars into ``component.data["build"]["flags"]`` so the backend's -L/-l/-D extraction picks them up. Shared by the ESP-IDF and ESP8266 Arduino backends. - ``idf_target`` may be a callable so a backend whose target lookup needs + ``board_mcu`` may be a callable so a backend whose target lookup needs build state (the esp32 variant) resolves it only when a script will run. ``pio_platform`` is exposed to the script as PlatformIO's ``PIOPLATFORM``. """ @@ -78,20 +80,18 @@ def apply_extra_script( component.name, ) return - if callable(idf_target): - idf_target = idf_target() + if callable(board_mcu): + board_mcu = board_mcu() result = run_extra_script( script_path, library_dir=source_path, - idf_target=idf_target, + board_mcu=board_mcu, pio_platform=pio_platform, ) extra_flags = captured_as_build_flags(result, library_dir=source_path) if not extra_flags: return - flags = component.data.setdefault("build", {}).setdefault("flags", []) - if isinstance(flags, str): - flags = [flags] + flags = ensure_list(component.data.setdefault("build", {}).setdefault("flags", [])) flags.extend(extra_flags) component.data["build"]["flags"] = flags @@ -155,12 +155,12 @@ def run_extra_script( script_path: Path, *, library_dir: Path, - idf_target: str, - pio_platform: str = "espressif32", + board_mcu: str, + pio_platform: str, ) -> ExtraScriptResult: """Execute ``script_path`` with a fake SCons env and return captured vars. - ``idf_target`` is the active ESP-IDF target name (e.g. ``esp32``, + ``board_mcu`` is the active MCU name (e.g. ``esp32``, ``esp32s3``); it's exposed to the script as PlatformIO's ``BOARD_MCU`` so chip-conditional logic resolves the same way it would under PIO. The script runs with ``library_dir`` as the @@ -172,8 +172,8 @@ def run_extra_script( script shouldn't block the build. """ env = _FakeSConsEnv( - board_mcu=idf_target, - pio_env=f"esphome_{idf_target}", + board_mcu=board_mcu, + pio_env=f"esphome_{board_mcu}", pio_platform=pio_platform, ) code = compile(script_path.read_text(encoding="utf-8"), str(script_path), "exec") diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 753bbdd8a3..4ae256444a 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -23,6 +23,7 @@ import logging import os from pathlib import Path, PurePosixPath import re +import shlex import tempfile from typing import Any from urllib.parse import urlsplit, urlunsplit @@ -555,8 +556,6 @@ def _resolve_registry_version( def split_flag_entry(entry: str, owner: str) -> list[str]: """``shlex.split`` with a clean error naming the offending flags entry.""" - import shlex - try: return shlex.split(entry) except ValueError as err: @@ -769,9 +768,6 @@ def convert_libraries( else "" ) - def is_ignored(name: str | None) -> bool: - return is_lib_ignored(name, lib_ignore) - def add_spec(name: str | None, version: str | None, repository: str | None) -> str: key, kind, locator = _node_key(name, version, repository) node = nodes.get(key) or _LibNode(key=key, is_git=kind == "git") @@ -820,7 +816,7 @@ def convert_libraries( top_level = [ add_spec(library.name, library.version, library.repository) for library in libraries - if not is_ignored(library.name) + if not is_lib_ignored(library.name, lib_ignore) ] # Collect + resolve to a fixpoint: a node is (re)resolved whenever its @@ -919,7 +915,7 @@ def convert_libraries( dep_name = _owner_pkgname_to_name( dependency.get("owner"), dependency.get("name") ) - if is_ignored(dep_name): + if is_lib_ignored(dep_name, lib_ignore): _LOGGER.debug("Skip ignored dependency %s", dep_name) continue # The version field may actually be a URL (git/archive dependency). diff --git a/script/determine-jobs.py b/script/determine-jobs.py index 910db9525e..4e97275563 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -525,13 +525,20 @@ def _esp32_platformio_path_or_file_trigger(files: list[str]) -> bool: return False -# ESP-IDF infra: changes under esphome/espidf/ or to the IDF build generator -# affect every esp32 IDF build (now the default toolchain) but aren't +# Native-build infra: changes under esphome/espidf/, the shared +# esphome/build_helpers/ package, or the modules the native ESP-IDF build +# imports affect every esp32 IDF build (now the default toolchain) but aren't # components, so the component matrix wouldn't otherwise force any esp32 # compile. When they change we fold the `esp32` component into the matrix so # the default native-IDF build path is still compiled on an infra-only PR. ESP_IDF_INFRA_TRIGGER_PATH_PREFIXES = ("esphome/espidf/", "esphome/build_helpers/") -ESP_IDF_INFRA_TRIGGER_FILES = frozenset({"esphome/build_gen/espidf.py"}) +ESP_IDF_INFRA_TRIGGER_FILES = frozenset( + { + "esphome/build_gen/espidf.py", + "esphome/platformio/library.py", + "esphome/platformio/extra_script.py", + } +) def _esp_idf_infra_changed(files: list[str]) -> bool: diff --git a/tests/unit_tests/build_helpers/__init__.py b/tests/unit_tests/build_helpers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index c82feb4790..eca74ded35 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -21,7 +21,7 @@ def _entry(directory: str, file: str, command: str) -> dict: return {"directory": directory, "file": file, "command": command} -def test_parse_entry_extracts_fields() -> None: +def testparse_entry_extracts_fields() -> None: """cxx_path, defines, includes and remaining flags are split apart.""" entry = _entry( f"{ABS}build", @@ -30,7 +30,7 @@ def test_parse_entry_extracts_fields() -> None: f"-I{ABS}inc/a -isystem {ABS}sys/b -std=gnu++20 -c app.cpp -o app.cpp.o", ) - cxx_path, defines, includes, cxx_flags = idedata._parse_entry(entry) + cxx_path, defines, includes, cxx_flags = idedata.parse_entry(entry) assert cxx_path == "/tools/xtensa-esp32-elf-g++" assert "USE_ESP32" in defines @@ -45,7 +45,7 @@ def test_parse_entry_extracts_fields() -> None: assert "app.cpp.o" not in cxx_flags -def test_parse_entry_space_separated_args() -> None: +def testparse_entry_space_separated_args() -> None: """``-D X`` / ``-I path`` (separate arg) and ``-isystem`` (joined).""" entry = _entry( f"{ABS}build", @@ -53,14 +53,14 @@ def test_parse_entry_space_separated_args() -> None: f"g++ -D FOO=1 -I {ABS}inc/sep -isystem{ABS}sys/joined -c x.cpp", ) - _, defines, includes, _ = idedata._parse_entry(entry) + _, defines, includes, _ = idedata.parse_entry(entry) assert "FOO=1" in defines assert f"{ABS}inc/sep" in includes assert f"{ABS}sys/joined" in includes -def test_parse_entry_resolves_relative_includes() -> None: +def testparse_entry_resolves_relative_includes() -> None: """Relative includes are resolved against the entry's ``directory``.""" directory = f"{ABS}build/proj" entry = _entry( @@ -69,10 +69,10 @@ def test_parse_entry_resolves_relative_includes() -> None: "g++ -Iconfig -I../shared -isystem rel/sys -c x.cpp", ) - _, _, includes, _ = idedata._parse_entry(entry) + _, _, includes, _ = idedata.parse_entry(entry) def resolved(rel: str) -> str: - # _parse_entry emits forward slashes for consistency (normpath would + # parse_entry emits forward slashes for consistency (normpath would # yield backslashes on Windows). return os.path.normpath(Path(directory) / rel).replace("\\", "/") @@ -83,7 +83,7 @@ def test_parse_entry_resolves_relative_includes() -> None: assert all(Path(inc).is_absolute() for inc in includes) -def test_parse_entry_skips_dependency_flags() -> None: +def testparse_entry_skips_dependency_flags() -> None: """Dependency-generation flags (and their args) are dropped.""" entry = _entry( "/build", @@ -91,7 +91,7 @@ def test_parse_entry_skips_dependency_flags() -> None: "g++ -MD -MT x.cpp.o -MF x.cpp.o.d -c x.cpp -o x.cpp.o", ) - _, _, _, cxx_flags = idedata._parse_entry(entry) + _, _, _, cxx_flags = idedata.parse_entry(entry) for tok in ("-MD", "-MT", "x.cpp.o", "-MF", "x.cpp.o.d", "-c", "-o", "x.cpp"): assert tok not in cxx_flags @@ -198,17 +198,17 @@ def test_idedata_from_build(tmp_path: Path) -> None: assert data["includes"]["toolchain"] == ["/tc/inc/c++", "/tc/inc"] -def test_get_toolchain_includes_raises_on_probe_failure() -> None: +def testget_toolchain_includes_raises_on_probe_failure() -> None: """A failed compiler probe is a hard error, not a silent empty list.""" fake_proc = MagicMock(returncode=1, stderr="xtensa-esp32-elf-g++: not found") with ( patch.object(idedata.subprocess, "run", return_value=fake_proc), pytest.raises(RuntimeError, match="builtin include dirs"), ): - idedata._get_toolchain_includes("/bad/compiler") + idedata.get_toolchain_includes("/bad/compiler") -def test_get_toolchain_includes_raises_when_no_dirs_found() -> None: +def testget_toolchain_includes_raises_when_no_dirs_found() -> None: """Markers present but no dirs (anomalous output) also raises.""" fake_proc = MagicMock( returncode=0, @@ -218,7 +218,7 @@ def test_get_toolchain_includes_raises_when_no_dirs_found() -> None: patch.object(idedata.subprocess, "run", return_value=fake_proc), pytest.raises(RuntimeError, match="builtin include dirs"), ): - idedata._get_toolchain_includes("/some/compiler") + idedata.get_toolchain_includes("/some/compiler") # ESP-IDF's compile_commands.json on Windows mixes literal backslash path @@ -248,7 +248,7 @@ def test_split_command_empty_returns_empty() -> None: @pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization") -def test_parse_entry_normalizes_windows_cxx_path() -> None: +def testparse_entry_normalizes_windows_cxx_path() -> None: """A backslash compiler path is emitted forward-slashed; define unescaped.""" entry = _entry( r"C:\b", @@ -256,7 +256,7 @@ def test_parse_entry_normalizes_windows_cxx_path() -> None: r"C:\esp\bin\g++.exe -DVER=\"1.2.3\" -IC:/inc/a -c x.cpp", ) - cxx_path, defines, includes, _ = idedata._parse_entry(entry) + cxx_path, defines, includes, _ = idedata.parse_entry(entry) assert cxx_path == "C:/esp/bin/g++.exe" assert "\\" not in cxx_path @@ -264,7 +264,7 @@ def test_parse_entry_normalizes_windows_cxx_path() -> None: assert "C:/inc/a" in includes -def test_parse_entry_strips_launcher_prefix() -> None: +def testparse_entry_strips_launcher_prefix() -> None: """A launcher-wrapped compile names the compiler second; the exact configured launcher is stripped, not anything ccache-shaped.""" entry = _entry( @@ -273,18 +273,18 @@ def test_parse_entry_strips_launcher_prefix() -> None: "/opt/homebrew/bin/ccache /tools/xtensa-lx106-elf-g++ -DUSE_ESP8266 " "-c app.cpp -o app.cpp.o", ) - cxx_path, defines, _, _ = idedata._parse_entry( + cxx_path, defines, _, _ = idedata.parse_entry( entry, launcher="/opt/homebrew/bin/ccache" ) assert cxx_path == "/tools/xtensa-lx106-elf-g++" assert defines == ["USE_ESP8266"] # Without a configured launcher nothing is stripped, even a token that # happens to be named ccache -- but the surprise is warned about - cxx_path, _, _, _ = idedata._parse_entry(entry) + cxx_path, _, _, _ = idedata.parse_entry(entry) assert cxx_path == "/opt/homebrew/bin/ccache" -def test_parse_entry_warns_when_first_token_is_not_a_compiler( +def testparse_entry_warns_when_first_token_is_not_a_compiler( caplog: pytest.LogCaptureFixture, ) -> None: entry = _entry( @@ -292,10 +292,10 @@ def test_parse_entry_warns_when_first_token_is_not_a_compiler( f"{ABS}build/src/esphome/core/application.cpp", "/opt/homebrew/bin/ccache /tools/xtensa-lx106-elf-g++ -c a.cpp -o a.o", ) - idedata._parse_entry(entry) + idedata.parse_entry(entry) assert "does not start with a compiler" in caplog.text caplog.clear() - idedata._parse_entry(entry, launcher="/opt/homebrew/bin/ccache") + idedata.parse_entry(entry, launcher="/opt/homebrew/bin/ccache") assert "does not start with a compiler" not in caplog.text @@ -328,7 +328,7 @@ def test_load_or_build_idedata_builds_and_caches(tmp_path: Path) -> None: compile_commands = _write_compile_commands(tmp_path) cache = tmp_path / "cache" / "test.json" with patch.object( - idedata, "_get_toolchain_includes", return_value=["/toolchain/include"] + idedata, "get_toolchain_includes", return_value=["/toolchain/include"] ): data = idedata.load_or_build_idedata( compile_commands, tmp_path / "firmware.elf", cache @@ -355,8 +355,39 @@ def test_load_or_build_idedata_rebuilds_bad_cache(tmp_path: Path) -> None: for bad in ("not json", json.dumps({"no_cc_path": True})): cache.write_text(bad) os.utime(cache, (compile_commands.stat().st_mtime + 10,) * 2) - with patch.object(idedata, "_get_toolchain_includes", return_value=[]): + with patch.object(idedata, "get_toolchain_includes", return_value=[]): data = idedata.load_or_build_idedata( compile_commands, tmp_path / "f.elf", cache ) assert "cc_path" in data + + +def test_load_or_build_idedata_rebuilds_when_compile_db_newer(tmp_path: Path) -> None: + """A compile DB newer than the cache forces regeneration.""" + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "cache.json" + cache.write_text(json.dumps({"cc_path": "stale"})) + os.utime(compile_commands, (cache.stat().st_mtime + 10,) * 2) + with patch.object(idedata, "get_toolchain_includes", return_value=[]): + data = idedata.load_or_build_idedata( + compile_commands, tmp_path / "f.elf", cache + ) + assert data["cc_path"] != "stale" + + +def test_load_or_build_idedata_rebuilds_non_dict_cache(tmp_path: Path) -> None: + """Valid JSON that is not an object is regenerated, never handed out. + + A bare string would otherwise pass the cc_path check by substring. + """ + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "cache.json" + for bad in ('"cc_path is a string"', "[]", "42"): + cache.write_text(bad) + os.utime(cache, (compile_commands.stat().st_mtime + 10,) * 2) + with patch.object(idedata, "get_toolchain_includes", return_value=[]): + data = idedata.load_or_build_idedata( + compile_commands, tmp_path / "f.elf", cache + ) + assert isinstance(data, dict) + assert "cc_path" in data diff --git a/tests/unit_tests/build_helpers/test_size_summary.py b/tests/unit_tests/build_helpers/test_size_summary.py new file mode 100644 index 0000000000..2ccdb96613 --- /dev/null +++ b/tests/unit_tests/build_helpers/test_size_summary.py @@ -0,0 +1,10 @@ +"""Tests for the shared PlatformIO-format size bar.""" + +from __future__ import annotations + +from esphome.build_helpers.size_summary import format_bar + + +def test_format_bar_zero_total() -> None: + """A zero total must not divide by zero.""" + assert format_bar(0, 0) == "[ ] 0.0% (used 0 bytes from 0 bytes)" diff --git a/tests/unit_tests/test_espidf_clang_tidy.py b/tests/unit_tests/test_espidf_clang_tidy.py index 88195e4872..4cc445e29b 100644 --- a/tests/unit_tests/test_espidf_clang_tidy.py +++ b/tests/unit_tests/test_espidf_clang_tidy.py @@ -1,10 +1,13 @@ """Tests for esphome.espidf.clang_tidy tidy-project generation.""" +import json import os from pathlib import Path +from unittest.mock import patch import pytest +from esphome.espidf import clang_tidy from esphome.espidf.clang_tidy import _Settings, _setup_core, _write_tidy_project REPO_ROOT = Path(__file__).resolve().parents[2] @@ -68,11 +71,6 @@ def test_setup_core_sets_arduino_env( def test_idedata_from_tidy_project(tmp_path) -> None: """The tidy TU's compile entry is assembled into consumer-shaped idedata.""" - import json - from unittest.mock import patch - - from esphome.espidf import clang_tidy - compile_commands = tmp_path / "compile_commands.json" compile_commands.write_text( json.dumps( @@ -87,7 +85,7 @@ def test_idedata_from_tidy_project(tmp_path) -> None: ) ) with patch( - "esphome.espidf.clang_tidy._get_toolchain_includes", return_value=["/tc/inc"] + "esphome.espidf.clang_tidy.get_toolchain_includes", return_value=["/tc/inc"] ): data = clang_tidy._idedata_from_tidy_project(compile_commands) assert data["cxx_path"] == "/tc/xtensa-esp32-elf-g++" @@ -97,10 +95,6 @@ def test_idedata_from_tidy_project(tmp_path) -> None: def test_idedata_from_tidy_project_missing_tu_raises(tmp_path) -> None: - import json - - from esphome.espidf import clang_tidy - compile_commands = tmp_path / "compile_commands.json" compile_commands.write_text(json.dumps([])) with pytest.raises(RuntimeError, match="tidy.cpp not found"): diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index 0fd09d80dc..0ffc7fc61e 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -1067,3 +1067,19 @@ def test_idf_component_download_passes_salt() -> None: "owner/name", force=True, salt="abcd1234", namespace="idf" ) assert c.path == Path("/converted/owner/name") + + +def test_apply_extra_script_wrapper_wires_esp32_target(tmp_path, monkeypatch): + """The espidf wrapper resolves the esp32 variant into the shared helper.""" + from esphome.components import esp32 as esp32_module + from esphome.espidf.component import _apply_extra_script + + monkeypatch.setattr(esp32_module, "get_esp32_variant", lambda: "ESP32") + (tmp_path / "src").mkdir() + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=[env.get('BOARD_MCU')])\n") + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py"}} + _apply_extra_script(c) + assert c.data["build"]["flags"] == ["-lesp32"] diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index a65bd0a21b..6c11a74d48 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -151,114 +151,6 @@ def test_get_idedata_generates_and_caches(setup_core: Path) -> None: assert json.loads(cache.read_text()) == {"cxx_path": "g++", "prog_path": prog_path} -def test_get_idedata_uses_cache_when_valid(setup_core: Path) -> None: - """A cache at least as new as the compile DB is reused without regenerating.""" - compile_commands, cache = _setup_build(setup_core) - compile_commands.parent.mkdir(parents=True, exist_ok=True) - compile_commands.write_text("[]") - cache.parent.mkdir(parents=True, exist_ok=True) - cache.write_text('{"cc_path": "cached-gcc", "cxx_path": "cached"}') - cc_mtime = compile_commands.stat().st_mtime - os.utime(cache, (cc_mtime + 1, cc_mtime + 1)) - - with patch("esphome.build_helpers.idedata.idedata_from_build") as mock_transform: - result = toolchain.get_idedata() - - mock_transform.assert_not_called() - assert result == {"cc_path": "cached-gcc", "cxx_path": "cached"} - - -def test_get_idedata_regenerates_cache_without_cc_path(setup_core: Path) -> None: - """A cache predating cc_path is rebuilt even though it is newer. - - Such a cache stays newer than the compile DB forever, so consumers that - derive the binutils paths from cc_path would keep failing on it. - """ - compile_commands, cache = _setup_build(setup_core) - compile_commands.parent.mkdir(parents=True, exist_ok=True) - compile_commands.write_text("[]") - cache.parent.mkdir(parents=True, exist_ok=True) - cache.write_text('{"cxx_path": "cached"}') - cc_mtime = compile_commands.stat().st_mtime - os.utime(cache, (cc_mtime + 1, cc_mtime + 1)) - - with patch( - "esphome.build_helpers.idedata.idedata_from_build", - return_value={"cc_path": "gcc", "cxx_path": "g++"}, - ) as mock_transform: - result = toolchain.get_idedata() - - mock_transform.assert_called_once() - assert result["cc_path"] == "gcc" - - -def test_get_idedata_regenerates_when_compile_commands_newer(setup_core: Path) -> None: - """A compile DB newer than the cache forces regeneration.""" - compile_commands, cache = _setup_build(setup_core) - cache.parent.mkdir(parents=True, exist_ok=True) - cache.write_text('{"cxx_path": "stale"}') - compile_commands.parent.mkdir(parents=True, exist_ok=True) - compile_commands.write_text("[]") - cache_mtime = cache.stat().st_mtime - os.utime(compile_commands, (cache_mtime + 1, cache_mtime + 1)) - - with patch( - "esphome.build_helpers.idedata.idedata_from_build", - return_value={"cxx_path": "fresh"}, - ) as mock_transform: - result = toolchain.get_idedata() - - mock_transform.assert_called_once() - assert result == {"cxx_path": "fresh", "prog_path": str(toolchain.get_elf_path())} - - -@pytest.mark.parametrize("cached", ['"cc_path is a string"', "[]", "42"]) -def test_get_idedata_regenerates_on_non_dict_cache( - setup_core: Path, cached: str -) -> None: - """A newer cache holding valid JSON that is not an object is regenerated. - - A bare string would otherwise pass the cc_path check by substring and be - handed to consumers expecting a dict. - """ - compile_commands, cache = _setup_build(setup_core) - compile_commands.parent.mkdir(parents=True, exist_ok=True) - compile_commands.write_text("[]") - cache.parent.mkdir(parents=True, exist_ok=True) - cache.write_text(cached) - cc_mtime = compile_commands.stat().st_mtime - os.utime(cache, (cc_mtime + 1, cc_mtime + 1)) - - with patch( - "esphome.build_helpers.idedata.idedata_from_build", - return_value={"cc_path": "gcc", "cxx_path": "g++"}, - ) as mock_transform: - result = toolchain.get_idedata() - - mock_transform.assert_called_once() - assert isinstance(result, dict) - - -def test_get_idedata_regenerates_on_corrupted_cache(setup_core: Path) -> None: - """An unparseable (but newer) cache falls back to regeneration.""" - compile_commands, cache = _setup_build(setup_core) - compile_commands.parent.mkdir(parents=True, exist_ok=True) - compile_commands.write_text("[]") - cache.parent.mkdir(parents=True, exist_ok=True) - cache.write_text("{not json") - cc_mtime = compile_commands.stat().st_mtime - os.utime(cache, (cc_mtime + 1, cc_mtime + 1)) - - with patch( - "esphome.build_helpers.idedata.idedata_from_build", - return_value={"cxx_path": "regen"}, - ) as mock_transform: - result = toolchain.get_idedata() - - mock_transform.assert_called_once() - assert result == {"cxx_path": "regen", "prog_path": str(toolchain.get_elf_path())} - - def test_get_idedata_prog_path_points_at_firmware_elf(setup_core: Path) -> None: """The idedata exposes prog_path (the ELF) so consumers like build-action can locate firmware.factory.bin / firmware.ota.bin as its siblings.""" diff --git a/tests/unit_tests/build_helpers/test_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py similarity index 76% rename from tests/unit_tests/build_helpers/test_extra_script.py rename to tests/unit_tests/test_platformio_extra_script.py index e99921df51..16e9c9b58b 100644 --- a/tests/unit_tests/build_helpers/test_extra_script.py +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -1,4 +1,4 @@ -"""Tests for the shared extraScript machinery (build_helpers.extra_script).""" +"""Tests for the shared extraScript machinery (platformio.extra_script).""" from __future__ import annotations @@ -11,7 +11,7 @@ from esphome.platformio.library import ConvertedLibrary as IDFComponent, URLSour def test_extra_script_captures_libpath_libs_and_defines(tmp_path): - from esphome.build_helpers.extra_script import ( + from esphome.platformio.extra_script import ( captured_as_build_flags, run_extra_script, ) @@ -33,7 +33,9 @@ def test_extra_script_captures_libpath_libs_and_defines(tmp_path): # shim's exec namespace can resolve it. script.write_text("from os.path import join\n" + script.read_text()) - result = run_extra_script(script, library_dir=tmp_path, idf_target="esp32") + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) assert result.libpath == [str(Path("src") / "esp32")] assert result.libs == ["algobsec"] @@ -56,7 +58,7 @@ def test_extra_script_libpath_relative_resolves_against_library_dir( """Relative LIBPATH entries must resolve against ``library_dir``, not the caller's CWD (the shim restores CWD before ``captured_as_build_flags`` runs).""" - from esphome.build_helpers.extra_script import ( + from esphome.platformio.extra_script import ( ExtraScriptResult, captured_as_build_flags, ) @@ -74,7 +76,7 @@ def test_extra_script_libpath_relative_resolves_against_library_dir( def test_extra_script_libpath_absolute_outside_library_dir(tmp_path): - from esphome.build_helpers.extra_script import ( + from esphome.platformio.extra_script import ( ExtraScriptResult, captured_as_build_flags, ) @@ -88,13 +90,15 @@ def test_extra_script_libpath_absolute_outside_library_dir(tmp_path): def test_extra_script_failure_returns_empty_result(tmp_path, caplog): - from esphome.build_helpers.extra_script import run_extra_script + from esphome.platformio.extra_script import run_extra_script script = tmp_path / "broken.py" script.write_text("raise RuntimeError('boom')\n") with caplog.at_level("WARNING"): - result = run_extra_script(script, library_dir=tmp_path, idf_target="esp32") + result = run_extra_script( + script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" + ) assert result.libpath == [] assert result.libs == [] @@ -102,7 +106,7 @@ def test_extra_script_failure_returns_empty_result(tmp_path, caplog): def test_apply_extra_script_path_traversal_is_rejected(tmp_path): - from esphome.espidf.component import _apply_extra_script + from esphome.platformio.extra_script import apply_extra_script library_dir = tmp_path / "lib" library_dir.mkdir() @@ -113,19 +117,15 @@ def test_apply_extra_script_path_traversal_is_rejected(tmp_path): c.path = library_dir c.data = {"build": {"extraScript": "../evil.py"}} - _apply_extra_script(c) + apply_extra_script(c, board_mcu="esp32", pio_platform="espressif32") # Nothing was folded into flags: the traversal was rejected before # the script could run. assert "flags" not in c.data["build"] -def test_apply_extra_script_merges_into_existing_flags(tmp_path, monkeypatch): - from esphome.components import esp32 as esp32_module - - monkeypatch.setattr(esp32_module, "get_esp32_variant", lambda: "ESP32") - - from esphome.espidf.component import _apply_extra_script +def test_apply_extra_script_merges_into_existing_flags(tmp_path): + from esphome.platformio.extra_script import apply_extra_script (tmp_path / "src").mkdir() script = tmp_path / "extra.py" @@ -135,16 +135,16 @@ def test_apply_extra_script_merges_into_existing_flags(tmp_path, monkeypatch): c.path = tmp_path c.data = {"build": {"extraScript": "extra.py", "flags": ["-DEXISTING"]}} - _apply_extra_script(c) + apply_extra_script(c, board_mcu="esp32", pio_platform="espressif32") assert "-DEXISTING" in c.data["build"]["flags"] assert "-lalgobsec" in c.data["build"]["flags"] def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None: - """The shared helper resolves a callable idf_target lazily and normalizes + """The shared helper resolves a callable board_mcu lazily and normalizes a string ``build.flags`` value into a list before extending it.""" - from esphome.build_helpers.extra_script import apply_extra_script + from esphome.platformio.extra_script import apply_extra_script (tmp_path / "src").mkdir() script = tmp_path / "extra.py" @@ -154,31 +154,35 @@ def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None: c.path = tmp_path c.data = {"build": {"extraScript": "extra.py", "flags": "-DBASE=1"}} - apply_extra_script(c, lambda: "esp8266") + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") assert c.data["build"]["flags"] == ["-DBASE=1", "-lesp8266"] def test_apply_extra_script_no_script_and_no_flags(tmp_path) -> None: - from esphome.build_helpers.extra_script import apply_extra_script + from esphome.platformio.extra_script import apply_extra_script # No extraScript declared: nothing happens, the target is never resolved c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) c.path = tmp_path c.data = {"build": {}} - apply_extra_script(c, lambda: pytest.fail("target resolved without a script")) + apply_extra_script( + c, + board_mcu=lambda: pytest.fail("target resolved without a script"), + pio_platform="espressif8266", + ) # A script that captures nothing leaves the flags untouched script = tmp_path / "noop.py" script.write_text("pass\n") c.data = {"build": {"extraScript": "noop.py"}} - apply_extra_script(c, "esp8266") + apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266") assert "flags" not in c.data["build"] def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path) -> None: """Un-captured env vars and unsupported env methods are silent no-ops.""" - from esphome.build_helpers.extra_script import apply_extra_script + from esphome.platformio.extra_script import apply_extra_script script = tmp_path / "extra.py" script.write_text( @@ -187,44 +191,44 @@ def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path) -> None: c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) c.path = tmp_path c.data = {"build": {"extraScript": "extra.py"}} - apply_extra_script(c, "esp8266") + apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266") assert c.data["build"]["flags"] == ["-lsingle"] def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None: """A raising extra-script is best-effort: logged and skipped.""" - from esphome.build_helpers.extra_script import apply_extra_script + from esphome.platformio.extra_script import apply_extra_script script = tmp_path / "extra.py" script.write_text("raise RuntimeError('boom')\n") c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) c.path = tmp_path c.data = {"build": {"extraScript": "extra.py"}} - apply_extra_script(c, "esp8266") + apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266") assert "flags" not in c.data["build"] assert "skipping" in caplog.text def test_apply_extra_script_pio_platform(tmp_path) -> None: """The backend's platform token is exposed to the script as PIOPLATFORM.""" - from esphome.build_helpers.extra_script import apply_extra_script + from esphome.platformio.extra_script import apply_extra_script script = tmp_path / "extra.py" script.write_text("env.Append(LIBS=[env.get('PIOPLATFORM')])\n") c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) c.path = tmp_path c.data = {"build": {"extraScript": "extra.py"}} - apply_extra_script(c, "esp8266", pio_platform="espressif8266") + apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266") 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.""" - from esphome.build_helpers.extra_script import apply_extra_script + 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, "esp8266") + apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266") assert "not found" in caplog.text diff --git a/tests/unit_tests/test_size_summary.py b/tests/unit_tests/test_size_summary.py index 37ec17f8fa..816e65be18 100644 --- a/tests/unit_tests/test_size_summary.py +++ b/tests/unit_tests/test_size_summary.py @@ -128,13 +128,6 @@ def test_print_summary_handles_no_memory_types( assert capsys.readouterr().out == "" -def test_format_bar_zero_total() -> None: - """A zero total must not divide by zero.""" - from esphome.build_helpers.size_summary import format_bar - - assert format_bar(0, 0) == "[ ] 0.0% (used 0 bytes from 0 bytes)" - - def test_print_summary_flash_line( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: