From c8ebe461d13491ca3406d680c85b8cb31dc261dc Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:11:35 +1200 Subject: [PATCH 1/2] [ci] Skip CodSpeed benchmarks on beta and release pull requests (#18703) --- .github/workflows/ci.yml | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 35148de0c0..9da0937555 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -112,6 +112,12 @@ jobs: component-test-batches: ${{ steps.determine.outputs.component-test-batches }} validate-only-components: ${{ steps.determine.outputs.validate-only-components }} benchmarks: ${{ steps.determine.outputs.benchmarks }} + # "true" when this run is a pull request into one of the release + # branches. Those pull requests are batches of changes already tested on + # their original dev pull requests, so several jobs below trade coverage + # for turnaround time on them. Matched exactly, not by prefix, so an + # ordinary branch named e.g. "release-notes" is not caught by it. + release-pr: ${{ github.base_ref == 'beta' || github.base_ref == 'release' }} steps: - name: Check out code from GitHub uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -230,7 +236,7 @@ jobs: runs-on: ubuntu-latest needs: - determine-jobs - if: github.event_name == 'pull_request' && !startsWith(github.base_ref, 'beta') && !startsWith(github.base_ref, 'release') && needs.determine-jobs.outputs.core-ci == 'true' + if: github.event_name == 'pull_request' && needs.determine-jobs.outputs.release-pr == 'false' && needs.determine-jobs.outputs.core-ci == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -449,10 +455,21 @@ jobs: if: >- github.repository == 'esphome/esphome' && ( (github.event_name == 'push' && github.ref_name == 'dev') || - (github.event_name == 'pull_request' && needs.determine-jobs.outputs.benchmarks == 'true') + ( + github.event_name == 'pull_request' && + needs.determine-jobs.outputs.release-pr == 'false' && + needs.determine-jobs.outputs.benchmarks == 'true' + ) ) # CodSpeed benchmarks require a CodSpeed account linked to the repository to run # (https://codspeed.io) -- disabled on forks that aren't esphome/esphome itself. + # + # Pull requests into beta and release are skipped as well. CodSpeed compares a + # pull request against the newest commit of its base branch that has a benchmark + # run of its own, and only dev is benchmarked. A release pull request therefore + # falls back to dev's latest run, so every speed-up merged into dev since the + # release branched is reported as a regression in the release. The changes there + # have already been benchmarked on their original dev pull requests. steps: - name: Check out code from GitHub uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -929,7 +946,7 @@ jobs: ESPHOME_SDK_NRF_PREFIX: ~/.esphome-sdk-nrf strategy: fail-fast: false - max-parallel: ${{ (startsWith(github.base_ref, 'beta') || startsWith(github.base_ref, 'release')) && 32 || 16 }} + max-parallel: ${{ needs.determine-jobs.outputs.release-pr == 'true' && 32 || 16 }} matrix: batch: ${{ fromJson(needs.determine-jobs.outputs.component-test-batches) }} steps: @@ -1016,7 +1033,7 @@ jobs: # - This catches pin conflicts and other issues in directly changed code # - Grouped tests use --testing-mode to allow config merging (disables some checks) # - Dependencies are safe to group since they weren't modified in this PR - if [[ "${{ github.base_ref }}" == beta* ]] || [[ "${{ github.base_ref }}" == release* ]]; then + if [[ "${{ needs.determine-jobs.outputs.release-pr }}" == "true" ]]; then directly_changed_csv="" echo "Testing components: $components_csv" echo "Target branch: ${{ github.base_ref }} - grouping all components" From 9974ad97fa13a8b0c61b21837db8ef3ca14abecd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 23 Aug 2026 21:59:19 -0500 Subject: [PATCH 2/2] [core] Extract the shared idedata and size-summary helpers into build_helpers (#18661) --- esphome/__main__.py | 15 +- esphome/build_helpers/__init__.py | 1 + esphome/{espidf => build_helpers}/idedata.py | 201 +++++- esphome/build_helpers/size_summary.py | 24 + esphome/espidf/clang_tidy.py | 13 +- esphome/espidf/size_summary.py | 18 +- esphome/espidf/toolchain.py | 33 +- script/determine-jobs.py | 15 +- tests/script/test_determine_jobs.py | 18 +- tests/unit_tests/__init__.py | 0 .../analyze_memory/test_build_artifacts.py | 2 +- tests/unit_tests/build_gen/__init__.py | 0 tests/unit_tests/build_helpers/__init__.py | 0 .../unit_tests/build_helpers/test_idedata.py | 678 ++++++++++++++++++ .../build_helpers/test_size_summary.py | 22 + tests/unit_tests/test_core.py | 2 +- tests/unit_tests/test_espidf_clang_tidy.py | 35 + tests/unit_tests/test_espidf_idedata.py | 264 ------- tests/unit_tests/test_espidf_toolchain.py | 112 +-- tests/unit_tests/test_main.py | 104 ++- tests/unit_tests/test_size_summary.py | 17 + 21 files changed, 1091 insertions(+), 483 deletions(-) create mode 100644 esphome/build_helpers/__init__.py rename esphome/{espidf => build_helpers}/idedata.py (51%) create mode 100644 esphome/build_helpers/size_summary.py create mode 100644 tests/unit_tests/__init__.py create mode 100644 tests/unit_tests/build_gen/__init__.py create mode 100644 tests/unit_tests/build_helpers/__init__.py create mode 100644 tests/unit_tests/build_helpers/test_idedata.py create mode 100644 tests/unit_tests/build_helpers/test_size_summary.py delete mode 100644 tests/unit_tests/test_espidf_idedata.py diff --git a/esphome/__main__.py b/esphome/__main__.py index 769b66ecc8..0da86b3ec0 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -857,7 +857,20 @@ def compile_program(args: ArgsProtocol, config: ConfigType) -> int: toolchain.create_factory_bin() toolchain.create_ota_bin() toolchain.create_elf_copy() - toolchain.get_idedata() + from esphome.build_helpers.idedata import IDEDATA_BEST_EFFORT_ERRORS + + try: + if toolchain.get_idedata() is None: + _LOGGER.warning("No idedata was generated for this build") + except IDEDATA_BEST_EFFORT_ERRORS as err: + # The firmware already built; an idedata failure must not fail + # a successful build. + _LOGGER.warning( + "Could not generate idedata: %s (IDE, clang-tidy, and " + "memory-analysis data will be unavailable for this build)", + err, + ) + _LOGGER.debug("Idedata failure detail", exc_info=True) else: from esphome.platformio import toolchain diff --git a/esphome/build_helpers/__init__.py b/esphome/build_helpers/__init__.py new file mode 100644 index 0000000000..df956a2509 --- /dev/null +++ b/esphome/build_helpers/__init__.py @@ -0,0 +1 @@ +"""Build helpers shared by the native (non-PlatformIO) toolchains.""" diff --git a/esphome/espidf/idedata.py b/esphome/build_helpers/idedata.py similarity index 51% rename from esphome/espidf/idedata.py rename to esphome/build_helpers/idedata.py index 0047d568e2..038fe64970 100644 --- a/esphome/espidf/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -1,10 +1,10 @@ -"""Derive idedata from an ESP-IDF native-toolchain ``compile_commands.json``. +"""Derive idedata from a native (non-PlatformIO) build's ``compile_commands.json``. -PlatformIO exposes a curated ``pio run -t idedata`` JSON; the native ESP-IDF -toolchain has no such command, but its CMake build emits -``build/compile_commands.json`` (CMAKE_EXPORT_COMPILE_COMMANDS). This module -turns that file into the same fields consumers (IDE integration, clang-tidy) -expect: +PlatformIO exposes a curated ``pio run -t idedata`` JSON; the native +toolchains have no such command, but each build produces a +``compile_commands.json`` (CMAKE_EXPORT_COMPILE_COMMANDS for ESP-IDF, ninja's +compdb tool otherwise). This module turns that file into the same fields +consumers (IDE integration, clang-tidy) expect: {cc_path, cxx_path, cxx_flags, defines, includes: {build, toolchain}} """ @@ -18,6 +18,19 @@ from pathlib import Path import shlex import subprocess +from esphome.core import EsphomeError +from esphome.helpers import write_file + +# Everything idedata generation may raise after a successful link; idedata +# is a bonus artifact, so consumers warn instead of failing the build +IDEDATA_BEST_EFFORT_ERRORS = ( + EsphomeError, + LookupError, + OSError, + RuntimeError, + ValueError, +) + _LOGGER = logging.getLogger(__name__) # C++ translation-unit suffixes used to identify ESPHome source files. @@ -30,12 +43,8 @@ _ESPHOME_SRC_MARKER = "/src/esphome/" def _is_esphome_src(file: str) -> bool: - """Whether ``file`` is an ESPHome C++ translation unit. - - ``compile_commands.json`` ``file`` paths use the OS-native separator, so on - Windows they contain backslashes; normalize to ``/`` before testing the - marker, otherwise no source matches and the build-include union is empty. - """ + """Whether ``file`` is an ESPHome C++ translation unit; normalized to + ``/`` first since Windows compile DBs use backslashes.""" return _ESPHOME_SRC_MARKER in file.replace("\\", "/") and file.endswith( _CXX_SUFFIXES ) @@ -106,11 +115,8 @@ def _expand_response_files(tokens: list[str], directory: Path) -> list[str]: def _pick_entry(entries: list[dict]) -> dict: - """Pick a representative ESPHome C++ translation unit. - - All ESPHome sources share the same component flags/defines, so any one of - them yields the cxx_path / cxx_flags / defines we need. - """ + """Pick a representative ESPHome C++ TU; all share the same component + flags/defines.""" for entry in entries: if _is_esphome_src(entry["file"]): return entry @@ -120,25 +126,46 @@ def _pick_entry(entries: list[dict]) -> dict: raise ValueError("no C++ translation unit found in compile_commands.json") -def _parse_entry(entry: dict) -> tuple[str, list[str], list[str], list[str]]: +# Compiler launchers that may prefix a compile command; a closed launcher +# denylist beats enumerating compiler names, an open set. +_LAUNCHER_STEMS = frozenset({"ccache", "sccache", "distcc", "icecc", "buildcache"}) + + +def _is_launcher(token: str) -> bool: + return Path(token).stem.lower() in _LAUNCHER_STEMS + + +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).""" directory = Path(entry["directory"]) tokens = _expand_response_files(_split_command(entry["command"]), directory) def _include(raw: str) -> str: - # Include paths in compile_commands are interpreted relative to the - # entry's ``directory`` (e.g. build-local ``-Iconfig``); resolve them - # so the cached idedata is usable regardless of the consumer's cwd. - # Emit forward slashes (``normpath`` yields ``\`` on Windows) so the - # paths match the absolute, already-forward-slash entries in the JSON. + # Resolve against the entry's ``directory`` so cached idedata works + # from any cwd; emit forward slashes to match the JSON's own entries raw = raw.strip() if raw and not Path(raw).is_absolute(): raw = os.path.normpath(directory / raw) return raw.replace("\\", "/") + # A launcher-wrapped command ("ccache g++ ...") names the compiler second + if launcher is not None and tokens[:1] == [launcher]: + tokens = tokens[1:] + if not tokens: + # An empty command, or one that was only the launcher; fail by name + raise ValueError(f"empty compile command for {entry.get('file')}") + if _is_launcher(tokens[0]) and len(tokens) > 1 and not tokens[1].startswith("-"): + # Stale DB built with a launcher this run no longer configures; the + # real compiler is the next token + _LOGGER.warning("Stripping unconfigured launcher %s", tokens[0]) + tokens = tokens[1:] # token0 is the compiler path; the rest of the command already uses forward # slashes on Windows, so normalize it too for a consistent idedata file. cxx_path = tokens[0].replace("\\", "/") + # Enforced here so no caller can record ccache as the compiler + reject_launcher_compiler(cxx_path) defines: list[str] = [] includes: list[str] = [] cxx_flags: list[str] = [] @@ -168,7 +195,7 @@ def _parse_entry(entry: dict) -> tuple[str, list[str], list[str], list[str]]: 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"], @@ -219,26 +246,128 @@ def _cc_path_from_cxx(cxx_path: str) -> str: return f"{stem}{suffix}" -def idedata_from_build(compile_commands: Path) -> dict: +def _cache_usable(cached: object) -> bool: + """Check a cached idedata dict against the guarantees of the write path. + + Caches written by older versions predate the launcher rejection and the + include-union shape; serving one would bypass both. The dict check also + keeps "in" from substring-matching a bare JSON string. + """ + if not isinstance(cached, dict) or "cc_path" not in cached: + return False + cxx_path = cached.get("cxx_path") + if not isinstance(cxx_path, str) or _is_launcher(cxx_path): + return False + includes = cached.get("includes") + return isinstance(includes, dict) and isinstance(includes.get("build"), list) + + +def load_or_build_idedata( + compile_commands: Path, + elf_path: Path, + cache: Path, + launcher: str | None = None, +) -> dict | None: + """Return idedata for a compile_commands.json build, cached on mtime. + + Shared by the native ESP-IDF and ESP8266 Arduino toolchains. Returns None + when the compile DB doesn't exist yet (nothing was built). ``launcher`` + is the compiler-launcher path (ccache) the build was generated with, if + any; commands in the compile DB are prefixed with it. + """ + if not compile_commands.is_file(): + _LOGGER.debug("No %s yet; skipping idedata generation", compile_commands) + return None + + if cache.is_file() and cache.stat().st_mtime >= compile_commands.stat().st_mtime: + try: + cached = json.loads(cache.read_text(encoding="utf-8")) + except (ValueError, OSError) as err: + # A recurring cause (interrupted write, disk full) would otherwise + # look like unexplained slow builds + _LOGGER.warning("Discarding unreadable idedata cache %s: %s", cache, err) + else: + if _cache_usable(cached): + # Re-stamp so a relocated build dir cannot serve a stale ELF path + cached["prog_path"] = str(elf_path) + return cached + _LOGGER.debug("Regenerating idedata: cache %s fails validation", cache) + + data = idedata_from_build(compile_commands, launcher) + data["prog_path"] = str(elf_path) + cache.parent.mkdir(parents=True, exist_ok=True) + # Atomic so a crash mid-write cannot leave a truncated cache + write_file(cache, json.dumps(data, indent=2) + "\n") + return data + + +def reject_launcher_compiler(cxx_path: str) -> None: + """Reject a compile DB naming a launcher (ccache) as the compiler; it + must never be probed, cached, or consumed.""" + if _is_launcher(cxx_path): + raise EsphomeError( + f"compile_commands.json names the launcher {cxx_path} as the " + "compiler; the compile database is unusable" + ) + + +def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> dict: """Parse compile_commands.json into the idedata fields consumers expect. - A single ESP-IDF compile entry only carries its own component's REQUIRES - include set, but consumers (clang-tidy) analyze ESPHome headers that - transitively pull in other components. So take cxx_path / cxx_flags / - defines from a representative ESPHome TU, but union the include dirs across - all ESPHome TUs to get a project-wide superset (as PlatformIO's idedata - provides). + A single compile entry only carries the include set its own translation + unit was built with (per-component under ESP-IDF), but consumers + (clang-tidy) analyze ESPHome headers that transitively pull in other + components. So take cxx_path / cxx_flags / defines from a representative + ESPHome TU, but union the include dirs across all ESPHome TUs to get a + project-wide superset (as PlatformIO's idedata provides). """ entries = json.loads(Path(compile_commands).read_text(encoding="utf-8")) - cxx_path, defines, _, cxx_flags = _parse_entry(_pick_entry(entries)) + if not isinstance(entries, list) or not all(isinstance(e, dict) for e in entries): + # A TypeError here would escape IDEDATA_BEST_EFFORT_ERRORS + raise EsphomeError(f"{compile_commands} is not a compile-command list") - build_includes: dict[str, None] = {} + representative = _pick_entry(entries) + cxx_path, defines, rep_includes, cxx_flags = parse_entry(representative, launcher) + + # Seed with the representative's includes so it is not parsed twice + has_esphome_tu = _is_esphome_src(representative["file"]) + build_includes: dict[str, None] = dict.fromkeys( + rep_includes if has_esphome_tu else () + ) + + def _shape(entry: dict) -> str: + # directory + command minus TU-specific paths: same shape means the + # same include set, so tokenize once per shape. Response-file + # commands never dedupe (the .rsp contents differ per object) + command = entry["command"] + directory = entry.get("directory", "") + if "@" in command: + return f"unique:{directory}|{entry.get('output') or command}" + stripped = command.replace(entry.get("file", ""), "").replace( + entry.get("output", ""), "" + ) + return f"{directory}|{stripped}" + + seen_shapes = {_shape(representative)} for entry in entries: - if not _is_esphome_src(entry["file"]): + if entry is representative or not _is_esphome_src(entry["file"]): continue - for inc in _parse_entry(entry)[2]: + has_esphome_tu = True + if (shape := _shape(entry)) in seen_shapes: + _LOGGER.debug("Include union: %s shares a command shape", entry["file"]) + continue + seen_shapes.add(shape) + for inc in parse_entry(entry, launcher)[2]: build_includes.setdefault(inc, None) + if not has_esphome_tu: + # An arbitrary fallback TU breaks clang-tidy/IDE consumers, and a + # warning would be cached into permanence; call sites downgrade this + raise EsphomeError( + f"No ESPHome translation unit found in {compile_commands}; " + "refusing to cache unusable idedata" + ) + return { "cc_path": _cc_path_from_cxx(cxx_path), "cxx_path": cxx_path, @@ -246,6 +375,6 @@ def idedata_from_build(compile_commands: Path) -> dict: "defines": defines, "includes": { "build": list(build_includes), - "toolchain": _get_toolchain_includes(cxx_path), + "toolchain": get_toolchain_includes(cxx_path), }, } diff --git a/esphome/build_helpers/size_summary.py b/esphome/build_helpers/size_summary.py new file mode 100644 index 0000000000..b888111044 --- /dev/null +++ b/esphome/build_helpers/size_summary.py @@ -0,0 +1,24 @@ +"""The PlatformIO-format size bar shared by the native toolchains.""" + +from __future__ import annotations + + +def format_bar(used: int, total: int) -> str: + """Match PlatformIO's ``_format_availale_bytes`` (sic, pioupload.py) exactly.""" + pct_raw = used / total if total else 0 + blocks = 10 + filled = min(int(round(blocks * pct_raw)), blocks) + progress = "=" * filled + return ( + f"[{progress:<{blocks}}] {pct_raw: 6.1%} " + f"(used {used:d} bytes from {total:d} bytes)" + ) + + +def print_size_line(label: str, used: int, total: int) -> None: + """One PlatformIO-format summary line (``RAM``/``Flash``). + + The label padding is part of the format: ``script/ci_memory_impact_extract.py`` + matches these lines verbatim. + """ + print(f"{label + ':':<7}{format_bar(used, total)}") diff --git a/esphome/espidf/clang_tidy.py b/esphome/espidf/clang_tidy.py index c91db775a3..9a70f1a99a 100644 --- a/esphome/espidf/clang_tidy.py +++ b/esphome/espidf/clang_tidy.py @@ -23,6 +23,12 @@ from dataclasses import dataclass import os from pathlib import Path +from esphome.build_helpers.idedata import ( + get_toolchain_includes, + parse_entry, + reject_launcher_compiler, +) + TIDY_PROJECT_NAME = "esphome_tidy" # A do-nothing C++ app: just enough for IDF to configure a valid project. It's @@ -415,13 +421,12 @@ def _idedata_from_tidy_project(compile_commands: Path) -> dict: """ import json - from esphome.espidf.idedata import _get_toolchain_includes, _parse_entry - entries = json.loads(Path(compile_commands).read_text(encoding="utf-8")) 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) + reject_launcher_compiler(cxx_path) return { "cxx_path": cxx_path, @@ -429,7 +434,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/size_summary.py b/esphome/espidf/size_summary.py index 7a5305ff0c..2be3634c69 100644 --- a/esphome/espidf/size_summary.py +++ b/esphome/espidf/size_summary.py @@ -28,6 +28,8 @@ import json import logging from pathlib import Path +from esphome.build_helpers.size_summary import print_size_line + _LOGGER = logging.getLogger(__name__) _SIZE_SUFFIXES = {"K": 1024, "M": 1024 * 1024} @@ -67,18 +69,6 @@ def _find_app_partition_size(partitions_csv: Path) -> int: raise ValueError(f"No app+factory or app+ota_0 partition in {partitions_csv}") -def _format_bar(used: int, total: int) -> str: - """Match PlatformIO's ``_format_availale_bytes`` (pioupload.py) exactly.""" - pct_raw = used / total if total else 0 - blocks = 10 - filled = min(int(round(blocks * pct_raw)), blocks) - progress = "=" * filled - return ( - f"[{progress:<{blocks}}] {pct_raw: 6.1%} " - f"(used {used:d} bytes from {total:d} bytes)" - ) - - def print_summary(size_json: Path, partitions_csv: Path | None) -> None: """Print PlatformIO-shaped RAM and Flash one-liners. @@ -99,7 +89,7 @@ def print_summary(size_json: Path, partitions_csv: Path | None) -> None: ram_used = ram_region.get("used") ram_total = ram_region.get("size") if ram_total and ram_used is not None: - print(f"RAM: {_format_bar(ram_used, ram_total)}") + print_size_line("RAM", ram_used, ram_total) image_size = data.get("image_size") if image_size is None or partitions_csv is None: @@ -109,4 +99,4 @@ def print_summary(size_json: Path, partitions_csv: Path | None) -> None: except ValueError as e: _LOGGER.debug("Skipping Flash summary: %s", e) return - print(f"Flash: {_format_bar(image_size, app_size)}") + print_size_line("Flash", image_size, app_size) diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 07ba03e2cf..3c5c4803c2 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -526,32 +526,15 @@ def get_idedata() -> dict | None: idedata fields IDE integrations and clang-tidy expect, cached alongside the PlatformIO idedata path. Returns None if the compile DB doesn't exist yet. """ - from esphome.espidf.idedata import idedata_from_build + from esphome.build_helpers.idedata import load_or_build_idedata - compile_commands = CORE.relative_build_path("build", "compile_commands.json") - if not compile_commands.is_file(): - _LOGGER.debug("No %s yet; skipping idedata generation", compile_commands) - return None - - cache = CORE.relative_internal_path("idedata", f"{CORE.name}.json") - if cache.is_file() and cache.stat().st_mtime >= compile_commands.stat().st_mtime: - try: - cached = json.loads(cache.read_text(encoding="utf-8")) - except ValueError: - pass - else: - # Caches written before cc_path was emitted stay newer than - # compile_commands.json forever, so rebuild them on the field rather - # than on the timestamp. Check the type too: a corrupted cache can - # still be valid JSON, and "in" would match a substring of a string. - if isinstance(cached, dict) and "cc_path" in cached: - return cached - - data = idedata_from_build(compile_commands) - data["prog_path"] = str(get_elf_path()) - cache.parent.mkdir(parents=True, exist_ok=True) - cache.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") - return data + # No launcher: CMake excludes CMAKE__COMPILER_LAUNCHER (ccache) + # from the exported compile database, unlike ninja's compdb dump. + return load_or_build_idedata( + CORE.relative_build_path("build", "compile_commands.json"), + get_elf_path(), + CORE.relative_internal_path("idedata", f"{CORE.name}.json"), + ) def create_factory_bin() -> bool: diff --git a/script/determine-jobs.py b/script/determine-jobs.py index e4d002975c..722e2370fe 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/",) -ESP_IDF_INFRA_TRIGGER_FILES = frozenset({"esphome/build_gen/espidf.py"}) +ESP_IDF_INFRA_TRIGGER_PATH_PREFIXES = ("esphome/espidf/", "esphome/build_helpers/") +ESP_IDF_INFRA_TRIGGER_FILES = frozenset( + { + "esphome/build_gen/espidf.py", + "esphome/framework_helpers.py", + "esphome/platformio/library.py", + } +) def _esp_idf_infra_changed(files: list[str]) -> bool: diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 80f572d9fe..1a568ca6c6 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -1120,7 +1120,13 @@ def test_should_run_esp32_platformio_with_branch() -> None: (["esphome/espidf/runner.py"], True), (["esphome/espidf/framework.py"], True), (["esphome/build_gen/espidf.py"], True), - # PlatformIO build gen and esp32 component are NOT IDF-infra triggers + # Shared native-build modules the IDF build imports -> trigger + (["esphome/build_helpers/idedata.py"], True), + (["esphome/platformio/library.py"], True), + (["esphome/framework_helpers.py"], True), + # PlatformIO build gen, its toolchain, and the esp32 component are + # NOT IDF-infra triggers + (["esphome/platformio/toolchain.py"], False), (["esphome/build_gen/platformio.py"], False), (["esphome/components/esp32/__init__.py"], False), (["README.md"], False), @@ -1132,6 +1138,16 @@ def test_esp_idf_infra_changed(changed_files: list[str], expected: bool) -> None assert determine_jobs._esp_idf_infra_changed(changed_files) is expected +def test_esp_idf_infra_trigger_paths_exist() -> None: + """A renamed or moved trigger module must fail here, not silently stop + forcing the esp32 IDF compile.""" + repo_root = Path(__file__).resolve().parents[2] + for file in determine_jobs.ESP_IDF_INFRA_TRIGGER_FILES: + assert (repo_root / file).is_file(), f"trigger file {file} moved or renamed" + for prefix in determine_jobs.ESP_IDF_INFRA_TRIGGER_PATH_PREFIXES: + assert (repo_root / prefix).is_dir(), f"trigger dir {prefix} moved or renamed" + + @pytest.mark.parametrize( ("changed_files", "expected_result"), [ diff --git a/tests/unit_tests/__init__.py b/tests/unit_tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit_tests/analyze_memory/test_build_artifacts.py b/tests/unit_tests/analyze_memory/test_build_artifacts.py index d97ee94e1c..ed37858b86 100644 --- a/tests/unit_tests/analyze_memory/test_build_artifacts.py +++ b/tests/unit_tests/analyze_memory/test_build_artifacts.py @@ -9,7 +9,7 @@ from esphome.analyze_memory.toolchain import ( find_idedata_path, idedata_candidates, ) -from esphome.espidf.idedata import _cc_path_from_cxx +from esphome.build_helpers.idedata import _cc_path_from_cxx from esphome.platformio.toolchain import IDEData diff --git a/tests/unit_tests/build_gen/__init__.py b/tests/unit_tests/build_gen/__init__.py new file mode 100644 index 0000000000..e69de29bb2 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 new file mode 100644 index 0000000000..fcf9c67086 --- /dev/null +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -0,0 +1,678 @@ +"""Tests for esphome.build_helpers.idedata (compile_commands.json -> idedata).""" + +# pylint: disable=protected-access + +import json +import logging +import os +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from esphome.build_helpers import idedata +from esphome.core import EsphomeError + +# An absolute, forward-slash (shlex-safe) path prefix valid on the host OS, so +# tests exercise the same is-absolute / normalize behavior as a real compile DB +# (a drive-qualified path on Windows, a leading slash elsewhere). +ABS = "C:/" if os.name == "nt" else "/" + + +def _entry(directory: str, file: str, command: str) -> dict: + return {"directory": directory, "file": file, "command": command} + + +def test_parse_entry_extracts_fields() -> None: + """cxx_path, defines, includes and remaining flags are split apart.""" + entry = _entry( + f"{ABS}build", + f"{ABS}build/src/esphome/core/application.cpp", + f"/tools/xtensa-esp32-elf-g++ -DUSE_ESP32 -DESPHOME_LOG_LEVEL=5 " + 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) + + assert cxx_path == "/tools/xtensa-esp32-elf-g++" + assert "USE_ESP32" in defines + assert "ESPHOME_LOG_LEVEL=5" in defines + assert f"{ABS}inc/a" in includes + assert f"{ABS}sys/b" in includes + assert "-std=gnu++20" in cxx_flags + # input/output files and their flags are not treated as flags + assert "-c" not in cxx_flags + assert "-o" not in cxx_flags + assert "app.cpp" not in cxx_flags + assert "app.cpp.o" not in cxx_flags + + +def test_parse_entry_space_separated_args() -> None: + """``-D X`` / ``-I path`` (separate arg) and ``-isystem`` (joined).""" + entry = _entry( + f"{ABS}build", + f"{ABS}build/src/esphome/x.cpp", + f"g++ -D FOO=1 -I {ABS}inc/sep -isystem{ABS}sys/joined -c x.cpp", + ) + + _, 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: + """Relative includes are resolved against the entry's ``directory``.""" + directory = f"{ABS}build/proj" + entry = _entry( + directory, + f"{directory}/src/esphome/x.cpp", + "g++ -Iconfig -I../shared -isystem rel/sys -c x.cpp", + ) + + _, _, includes, _ = idedata.parse_entry(entry) + + def resolved(rel: str) -> str: + # parse_entry emits forward slashes for consistency (normpath would + # yield backslashes on Windows). + return os.path.normpath(Path(directory) / rel).replace("\\", "/") + + assert resolved("config") in includes + assert resolved("../shared") in includes # ../ normalized away + assert resolved("rel/sys") in includes + # nothing is left relative + assert all(Path(inc).is_absolute() for inc in includes) + + +def test_parse_entry_skips_dependency_flags() -> None: + """Dependency-generation flags (and their args) are dropped.""" + entry = _entry( + "/build", + "/build/src/esphome/x.cpp", + "g++ -MD -MT x.cpp.o -MF x.cpp.o.d -c x.cpp -o x.cpp.o", + ) + + _, _, _, 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 + + +def test_expand_response_files(tmp_path: Path) -> None: + """``@file`` arguments are inlined relative to the directory.""" + rsp = tmp_path / "flags.rsp" + rsp.write_text("-DFROM_RSP -I/rsp/inc") + + tokens = idedata._expand_response_files( + ["g++", f"@{rsp.name}", "-c", "x.cpp"], tmp_path + ) + + assert "-DFROM_RSP" in tokens + assert "-I/rsp/inc" in tokens + assert not any(t.startswith("@") for t in tokens) + + +def test_expand_response_files_keeps_literal_when_missing(tmp_path: Path) -> None: + """An unreadable ``@file`` token is kept verbatim rather than dropped.""" + tokens = idedata._expand_response_files(["g++", "@nope.rsp"], tmp_path) + assert "@nope.rsp" in tokens + + +def test_pick_entry_prefers_esphome_tu() -> None: + """A ``/src/esphome/`` C++ TU is picked over other compile entries.""" + entries = [ + _entry("/b", "/b/managed_components/foo/foo.c", "gcc -c foo.c"), + _entry("/b", "/b/src/esphome/core/app.cpp", "g++ -c app.cpp"), + ] + assert idedata._pick_entry(entries)["file"].endswith("app.cpp") + + +def test_pick_entry_falls_back_to_any_cxx_tu() -> None: + """With no ``/src/esphome/`` TU present, the first C++ entry is the fallback.""" + entries = [ + _entry("/b", "/b/managed_components/foo/foo.c", "gcc -c foo.c"), + _entry("/b", "/b/components/x/x.cpp", "g++ -c x.cpp"), + ] + assert idedata._pick_entry(entries)["file"].endswith("x.cpp") + + +def test_is_esphome_src_handles_backslash_paths() -> None: + r"""The src marker must match Windows ``\src\esphome\`` paths too. + + compile_commands ``file`` entries use the OS-native separator; if the + marker only matched forward slashes no source would match on Windows and + the build-include union would be silently empty. + """ + assert idedata._is_esphome_src(r"C:\b\src\esphome\core\app.cpp") + assert idedata._is_esphome_src("/b/src/esphome/core/app.cpp") + # non-esphome and non-C++ still rejected regardless of separator + assert not idedata._is_esphome_src(r"C:\b\managed_components\x\x.cpp") + assert not idedata._is_esphome_src(r"C:\b\src\esphome\core\app.h") + + +@pytest.mark.parametrize( + ("command", "launcher"), + [ + ("", None), + # A command that is only the launcher strips to nothing + ("/usr/bin/ccache", "/usr/bin/ccache"), + ], +) +def test_parse_entry_empty_command_raises(command: str, launcher: str | None) -> None: + """A blank (or launcher-only) command fails with a named ValueError, + not an IndexError.""" + entry = {"directory": "/b", "file": "/b/src/x.cpp", "command": command} + with pytest.raises(ValueError, match="empty compile command"): + idedata.parse_entry(entry, launcher) + + +def test_idedata_from_build_empty_includes_raises(tmp_path: Path) -> None: + """A compile DB with no ESPHome TU is never usable idedata and must + not be cached (call sites downgrade the raise to a build warning).""" + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text( + json.dumps( + [ + _entry( + f"{ABS}build", + f"{ABS}build/other/lib.cpp", + "/tools/g++ -c other/lib.cpp -o lib.o", + ) + ] + ) + ) + with ( + patch.object(idedata, "get_toolchain_includes", return_value=[]), + pytest.raises(EsphomeError, match="No ESPHome translation unit found"), + ): + idedata.idedata_from_build(compile_commands) + + +def test_idedata_from_build_rsp_commands_never_dedupe(tmp_path: Path) -> None: + """Per-object response files strip to one shape while holding different + include sets; @-commands must tokenize per TU.""" + entries = [] + for name in ("a", "b"): + rsp = tmp_path / f"{name}.cpp.o.rsp" + rsp.write_text(f"-I{ABS}inc/{name}") + file = f"{ABS}build/src/esphome/core/{name}.cpp" + entries.append( + { + "directory": str(tmp_path), + "file": file, + "command": f"/tools/g++ @{rsp.name} -c {file} -o {name}.o", + "output": f"{name}.o", + } + ) + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text(json.dumps(entries)) + with patch.object(idedata, "get_toolchain_includes", return_value=[]): + data = idedata.idedata_from_build(compile_commands) + joined = " ".join(data["includes"]["build"]) + assert "inc/a" in joined and "inc/b" in joined + + +def test_idedata_from_build_dedupes_identical_command_shapes( + tmp_path: Path, +) -> None: + """Translation units sharing one ninja rule (same command modulo + file/output) carry + identical includes, so only one per shape is tokenized; a differing + shape still contributes its includes.""" + + def _tu(name: str, inc: str) -> dict: + # ninja's compdb embeds the file and output strings verbatim + file = f"{ABS}build/src/esphome/core/{name}.cpp" + return _entry( + f"{ABS}build", file, f"/tools/g++ -I{ABS}inc/{inc} -c {file} -o {name}.o" + ) | {"output": f"{name}.o"} + + entries = [_tu(name, "shared") for name in ("application", "component", "helpers")] + entries.append(_tu("extra", "extra")) + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text(json.dumps(entries)) + with ( + patch.object(idedata, "get_toolchain_includes", return_value=[]), + patch.object(idedata, "parse_entry", wraps=idedata.parse_entry) as spy, + ): + data = idedata.idedata_from_build(compile_commands) + includes = set(data["includes"]["build"]) + assert f"{ABS}inc/shared".replace("\\", "/") in { + i.replace("\\", "/") for i in includes + } + assert any("inc/extra" in i for i in includes) + # Representative + one distinct shape; the two same-shape duplicates + # are never tokenized + assert spy.call_count == 2 + + +def test_idedata_from_build(tmp_path: Path) -> None: + """Full transform: representative entry + include union + toolchain dirs.""" + compile_commands = tmp_path / "compile_commands.json" + entries = [ + _entry( + f"{ABS}b", + f"{ABS}b/src/esphome/core/app.cpp", + f"g++ -DUSE_ESP32 -I{ABS}inc/core -std=gnu++20 -c app.cpp -o app.cpp.o", + ), + _entry( + f"{ABS}b", + f"{ABS}b/src/esphome/sensor/s.cpp", + f"g++ -DUSE_ESP32 -I{ABS}inc/sensor -c s.cpp -o s.cpp.o", + ), + # non-esphome TU: its includes must not leak into the union + _entry( + f"{ABS}b", + f"{ABS}b/managed_components/x/x.c", + f"gcc -I{ABS}inc/managed -c x.c", + ), + ] + compile_commands.write_text(json.dumps(entries)) + + fake_proc = MagicMock( + returncode=0, + stderr=( + "ignored\n" + "#include <...> search starts here:\n" + " /tc/inc/c++\n" + " /tc/inc\n" + "End of search list.\n" + "more ignored\n" + ), + ) + with patch.object(idedata.subprocess, "run", return_value=fake_proc): + data = idedata.idedata_from_build(compile_commands) + + assert data["cxx_path"] == "g++" + assert "USE_ESP32" in data["defines"] + assert "-std=gnu++20" in data["cxx_flags"] + # include dirs unioned across all esphome TUs + assert f"{ABS}inc/core" in data["includes"]["build"] + assert f"{ABS}inc/sensor" in data["includes"]["build"] + # the non-esphome TU is excluded from the union + assert f"{ABS}inc/managed" not in data["includes"]["build"] + # toolchain search dirs parsed from the compiler's -v output + assert data["includes"]["toolchain"] == ["/tc/inc/c++", "/tc/inc"] + + +def test_get_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") + + +def test_get_toolchain_includes_raises_when_no_dirs_found() -> None: + """Markers present but no dirs (anomalous output) also raises.""" + fake_proc = MagicMock( + returncode=0, + stderr="#include <...> search starts here:\nEnd of search list.\n", + ) + with ( + patch.object(idedata.subprocess, "run", return_value=fake_proc), + pytest.raises(RuntimeError, match="builtin include dirs"), + ): + idedata.get_toolchain_includes("/some/compiler") + + +# ESP-IDF's compile_commands.json on Windows mixes literal backslash path +# separators in the compiler path with shell ``\"`` quote-escaping in defines, +# which only the real Windows argv parser handles. These exercise that path. +@pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization") +def test_split_command_preserves_paths_and_unescapes_quotes() -> None: + r"""Backslash paths survive while ``\"`` define-quoting is unescaped.""" + command = r"C:\esp\bin\riscv32-esp-elf-g++.exe -DVER=\"1.2.3\" -IC:/inc/a -c x.cpp" + + tokens = idedata._split_command(command) + + assert tokens[0] == r"C:\esp\bin\riscv32-esp-elf-g++.exe" + assert '-DVER="1.2.3"' in tokens + assert "-IC:/inc/a" in tokens + + +@pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization") +def test_split_command_empty_returns_empty() -> None: + """An empty or blank command tokenizes to ``[]`` (e.g. an empty response file). + + Guards against ``CommandLineToArgvW("")`` returning the current process name + instead of an empty list. + """ + assert idedata._split_command("") == [] + assert idedata._split_command(" ") == [] + + +@pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization") +def test_parse_entry_normalizes_windows_cxx_path() -> None: + """A backslash compiler path is emitted forward-slashed; define unescaped.""" + entry = _entry( + r"C:\b", + r"C:\b\src\esphome\x.cpp", + r"C:\esp\bin\g++.exe -DVER=\"1.2.3\" -IC:/inc/a -c x.cpp", + ) + + cxx_path, defines, includes, _ = idedata.parse_entry(entry) + + assert cxx_path == "C:/esp/bin/g++.exe" + assert "\\" not in cxx_path + assert 'VER="1.2.3"' in defines + assert "C:/inc/a" in includes + + +def test_parse_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( + f"{ABS}build", + f"{ABS}build/src/esphome/core/application.cpp", + "/opt/homebrew/bin/ccache /tools/xtensa-lx106-elf-g++ -DUSE_ESP8266 " + "-c app.cpp -o app.cpp.o", + ) + cxx_path, defines, _, _ = idedata.parse_entry( + entry, launcher="/opt/homebrew/bin/ccache" + ) + assert cxx_path == "/tools/xtensa-lx106-elf-g++" + assert defines == ["USE_ESP8266"] + + +def test_parse_entry_recovers_from_unconfigured_launcher( + caplog: pytest.LogCaptureFixture, +) -> None: + """A stale compile DB built with a launcher this run no longer configures + still yields the real compiler (the next token), not the launcher.""" + entry = _entry( + f"{ABS}build", + f"{ABS}build/src/esphome/core/application.cpp", + "/opt/homebrew/bin/ccache /tools/xtensa-lx106-elf-g++ -c a.cpp -o a.o", + ) + caplog.set_level(logging.DEBUG) + cxx_path, _, _, _ = idedata.parse_entry(entry) + assert cxx_path == "/tools/xtensa-lx106-elf-g++" + assert "Stripping unconfigured launcher" in caplog.text + + +def test_parse_entry_rejects_launcher_without_program() -> None: + """A launcher followed only by flags is rejected in the parser itself, + so no caller can record ccache as the compiler.""" + entry = _entry( + f"{ABS}build", + f"{ABS}build/src/esphome/core/application.cpp", + "/opt/homebrew/bin/ccache -c a.cpp -o a.o", + ) + with pytest.raises(EsphomeError, match="compile database is unusable"): + idedata.parse_entry(entry) + + +def _write_compile_commands(tmp_path: Path) -> Path: + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text( + json.dumps( + [ + _entry( + f"{ABS}build", + f"{ABS}build/src/esphome/core/application.cpp", + "/tools/g++ -DUSE_ESP8266 -c app.cpp -o app.cpp.o", + ) + ] + ) + ) + return compile_commands + + +def test_load_or_build_idedata_missing_compile_db(tmp_path: Path) -> None: + assert ( + idedata.load_or_build_idedata( + tmp_path / "compile_commands.json", tmp_path / "f.elf", tmp_path / "c.json" + ) + is None + ) + + +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"] + ): + data = idedata.load_or_build_idedata( + compile_commands, tmp_path / "firmware.elf", cache + ) + assert data["cc_path"] == "/tools/gcc" + assert data["prog_path"] == str(tmp_path / "firmware.elf") + assert json.loads(cache.read_text()) == data + + # A fresh cache is served without re-parsing the compile DB + os.utime(cache, (compile_commands.stat().st_mtime + 10,) * 2) + with patch.object(idedata, "idedata_from_build") as mock_build: + assert ( + idedata.load_or_build_idedata( + compile_commands, tmp_path / "firmware.elf", cache + ) + == data + ) + mock_build.assert_not_called() + + +def test_load_or_build_idedata_rebuilds_bad_cache(tmp_path: Path) -> None: + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "cache.json" + 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=[]): + 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 + + +def test_is_launcher_matches_only_known_launchers() -> None: + """Compilers of any shape pass; only the closed launcher set matches.""" + for token in ("/t/g++-13", "gcc-8.4.0", "clang++-17", "armcc", "icx", "cc"): + assert not idedata._is_launcher(token) + for token in ("/opt/homebrew/bin/ccache", "CCACHE.EXE", "distcc", "sccache"): + assert idedata._is_launcher(token) + + +def test_load_or_build_idedata_corrupted_cache_is_logged( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A truncated cache is diagnosable, not a silent slow-build cause.""" + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "c.json" + cache.write_text('{"cc_path": trunc') + os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 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["cxx_path"] == "/tools/g++" + assert "Discarding unreadable idedata cache" in caplog.text + + +def test_load_or_build_idedata_discards_unreadable_cache_file( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """An OSError on the cache read (permissions, I/O) regenerates like a + parse failure instead of aborting the consumer.""" + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "c.json" + cache.write_text("{}") + os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 2) + real_read_text = Path.read_text + + def fail_cache_read(self: Path, *args: object, **kwargs: object) -> str: + # chmod(0) cannot revoke read access on Windows, so fault the read + # itself for a platform-independent OSError + if self == cache: + raise OSError("permission denied") + return real_read_text(self, *args, **kwargs) + + with ( + patch.object(idedata, "get_toolchain_includes", return_value=[]), + patch.object(Path, "read_text", fail_cache_read), + ): + data = idedata.load_or_build_idedata( + compile_commands, tmp_path / "f.elf", cache + ) + assert data["cxx_path"] == "/tools/g++" + assert "Discarding unreadable idedata cache" in caplog.text + + +def test_load_or_build_idedata_never_caches_a_launcher(tmp_path: Path) -> None: + """A compile DB naming a launcher as the compiler is rejected, never cached.""" + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text( + json.dumps( + [ + _entry( + f"{ABS}build", + f"{ABS}build/src/esphome/core/application.cpp", + "/opt/homebrew/bin/ccache -c app.cpp -o app.cpp.o", + ) + ] + ) + ) + cache = tmp_path / "c.json" + # No probe patch needed: the launcher is rejected before the probe runs + with pytest.raises(EsphomeError, match="compile database is unusable"): + idedata.load_or_build_idedata(compile_commands, tmp_path / "f.elf", cache) + assert not cache.exists() + + +@pytest.mark.parametrize( + "cached", + ( + {"cc_path": "/x/gcc", "cxx_path": "/opt/homebrew/bin/ccache"}, + {"cc_path": "/x/gcc", "cxx_path": "/tools/g++"}, + {"cc_path": "/x/gcc", "cxx_path": "/tools/g++", "includes": {}}, + ), + ids=("launcher-cxx", "no-includes", "no-build-list"), +) +def test_load_or_build_idedata_regenerates_invalid_cache( + tmp_path: Path, cached: dict +) -> None: + """A cache written by an older version fails validation and regenerates.""" + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "c.json" + cache.write_text(json.dumps(cached)) + os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 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["cxx_path"] == "/tools/g++" + assert "includes" in data + + +def test_load_or_build_idedata_cache_hit_restamps_prog_path(tmp_path: Path) -> None: + """A served cache carries the current ELF path, not the one it was written with.""" + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "c.json" + cache.write_text( + json.dumps( + { + "cc_path": "/tools/gcc", + "cxx_path": "/tools/g++", + "includes": {"build": [], "toolchain": []}, + "prog_path": "/old/location/firmware.elf", + } + ) + ) + os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 2) + data = idedata.load_or_build_idedata( + compile_commands, tmp_path / "firmware.elf", cache + ) + assert data["prog_path"] == str(tmp_path / "firmware.elf") + + +def test_idedata_from_build_non_list_compile_db_raises(tmp_path: Path) -> None: + """Valid JSON that is not a list raises by name, inside the best-effort tuple.""" + compile_commands = tmp_path / "compile_commands.json" + for bad in ("{}", "null", '"text"', '["a", "b"]', "[1, 2]"): + compile_commands.write_text(bad) + with pytest.raises(EsphomeError, match="not a compile-command list"): + idedata.idedata_from_build(compile_commands) + + +def test_idedata_from_build_same_file_rsp_commands_never_dedupe( + tmp_path: Path, +) -> None: + """Two objects built from one source with different .rsp files keep both + include sets; the rsp sentinel keys on the output, not the source.""" + file = f"{ABS}build/src/esphome/core/shared.cpp" + entries = [] + for name in ("a", "b"): + rsp = tmp_path / f"{name}.o.rsp" + rsp.write_text(f"-I{ABS}inc/{name}") + entries.append( + { + "directory": str(tmp_path), + "file": file, + "command": f"/tools/g++ @{rsp.name} -c {file} -o {name}.o", + "output": f"{name}.o", + } + ) + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text(json.dumps(entries)) + with patch.object(idedata, "get_toolchain_includes", return_value=[]): + data = idedata.idedata_from_build(compile_commands) + joined = " ".join(data["includes"]["build"]) + assert "inc/a" in joined and "inc/b" in joined + + +def test_load_or_build_idedata_cache_hit_skips_rebuild(tmp_path: Path) -> None: + """A valid cache newer than the compile DB is served without re-parsing.""" + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "c.json" + cache.write_text( + json.dumps( + { + "cc_path": "/tools/gcc", + "cxx_path": "/tools/g++", + "includes": {"build": ["/inc"], "toolchain": []}, + "cached": True, + } + ) + ) + os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 2) + with patch.object(idedata, "idedata_from_build") as mock_build: + data = idedata.load_or_build_idedata( + compile_commands, tmp_path / "f.elf", cache + ) + mock_build.assert_not_called() + assert data["cached"] is True 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..231ae271b2 --- /dev/null +++ b/tests/unit_tests/build_helpers/test_size_summary.py @@ -0,0 +1,22 @@ +"""Tests for the shared PlatformIO-format size bar.""" + +from __future__ import annotations + +import pytest + +from esphome.build_helpers.size_summary import format_bar, print_size_line + + +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)" + + +def test_print_size_line_label_padding(capsys: pytest.CaptureFixture[str]) -> None: + """The label column is exactly what ci_memory_impact_extract.py greps.""" + print_size_line("RAM", 47932, 180736) + print_size_line("Flash", 888511, 1835008) + out = capsys.readouterr().out.splitlines() + assert out[0].startswith("RAM: [") + assert out[1].startswith("Flash: [") + assert "26.5% (used 47932 bytes from 180736 bytes)" in out[0] diff --git a/tests/unit_tests/test_core.py b/tests/unit_tests/test_core.py index c373116106..7adf955217 100644 --- a/tests/unit_tests/test_core.py +++ b/tests/unit_tests/test_core.py @@ -6,9 +6,9 @@ from unittest.mock import patch from hypothesis import given import pytest -from strategies import mac_addr_strings from esphome import const, core +from tests.unit_tests.strategies import mac_addr_strings class TestHexInt: diff --git a/tests/unit_tests/test_espidf_clang_tidy.py b/tests/unit_tests/test_espidf_clang_tidy.py index cb25535d8d..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] @@ -64,3 +67,35 @@ def test_setup_core_sets_arduino_env( _setup_core(tmp_path / "proj", _settings(target_framework=target_framework)) assert os.environ["ESPHOME_ARDUINO_COMPONENT"] == expected + + +def test_idedata_from_tidy_project(tmp_path) -> None: + """The tidy TU's compile entry is assembled into consumer-shaped idedata.""" + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text( + json.dumps( + [ + { + "directory": str(tmp_path), + "file": str(tmp_path / "main" / "tidy.cpp"), + "command": "/tc/xtensa-esp32-elf-g++ -DUSE_ESP32 " + f"-I{tmp_path}/inc -c main/tidy.cpp -o tidy.o", + } + ] + ) + ) + with patch( + "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++" + assert data["defines"] == ["USE_ESP32"] + assert data["includes"]["toolchain"] == ["/tc/inc"] + assert any(inc.endswith("/inc") for inc in data["includes"]["build"]) + + +def test_idedata_from_tidy_project_missing_tu_raises(tmp_path) -> None: + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text(json.dumps([])) + with pytest.raises(RuntimeError, match="tidy.cpp not found"): + clang_tidy._idedata_from_tidy_project(compile_commands) diff --git a/tests/unit_tests/test_espidf_idedata.py b/tests/unit_tests/test_espidf_idedata.py deleted file mode 100644 index 1088517ed1..0000000000 --- a/tests/unit_tests/test_espidf_idedata.py +++ /dev/null @@ -1,264 +0,0 @@ -"""Tests for esphome.espidf.idedata (compile_commands.json -> idedata).""" - -# pylint: disable=protected-access - -import json -import os -from pathlib import Path -from unittest.mock import MagicMock, patch - -import pytest - -from esphome.espidf import idedata - -# An absolute, forward-slash (shlex-safe) path prefix valid on the host OS, so -# tests exercise the same is-absolute / normalize behavior as a real compile DB -# (a drive-qualified path on Windows, a leading slash elsewhere). -ABS = "C:/" if os.name == "nt" else "/" - - -def _entry(directory: str, file: str, command: str) -> dict: - return {"directory": directory, "file": file, "command": command} - - -def test_parse_entry_extracts_fields() -> None: - """cxx_path, defines, includes and remaining flags are split apart.""" - entry = _entry( - f"{ABS}build", - f"{ABS}build/src/esphome/core/application.cpp", - f"/tools/xtensa-esp32-elf-g++ -DUSE_ESP32 -DESPHOME_LOG_LEVEL=5 " - 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) - - assert cxx_path == "/tools/xtensa-esp32-elf-g++" - assert "USE_ESP32" in defines - assert "ESPHOME_LOG_LEVEL=5" in defines - assert f"{ABS}inc/a" in includes - assert f"{ABS}sys/b" in includes - assert "-std=gnu++20" in cxx_flags - # input/output files and their flags are not treated as flags - assert "-c" not in cxx_flags - assert "-o" not in cxx_flags - assert "app.cpp" not in cxx_flags - assert "app.cpp.o" not in cxx_flags - - -def test_parse_entry_space_separated_args() -> None: - """``-D X`` / ``-I path`` (separate arg) and ``-isystem`` (joined).""" - entry = _entry( - f"{ABS}build", - f"{ABS}build/src/esphome/x.cpp", - f"g++ -D FOO=1 -I {ABS}inc/sep -isystem{ABS}sys/joined -c x.cpp", - ) - - _, 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: - """Relative includes are resolved against the entry's ``directory``.""" - directory = f"{ABS}build/proj" - entry = _entry( - directory, - f"{directory}/src/esphome/x.cpp", - "g++ -Iconfig -I../shared -isystem rel/sys -c x.cpp", - ) - - _, _, includes, _ = idedata._parse_entry(entry) - - def resolved(rel: str) -> str: - # _parse_entry emits forward slashes for consistency (normpath would - # yield backslashes on Windows). - return os.path.normpath(Path(directory) / rel).replace("\\", "/") - - assert resolved("config") in includes - assert resolved("../shared") in includes # ../ normalized away - assert resolved("rel/sys") in includes - # nothing is left relative - assert all(Path(inc).is_absolute() for inc in includes) - - -def test_parse_entry_skips_dependency_flags() -> None: - """Dependency-generation flags (and their args) are dropped.""" - entry = _entry( - "/build", - "/build/src/esphome/x.cpp", - "g++ -MD -MT x.cpp.o -MF x.cpp.o.d -c x.cpp -o x.cpp.o", - ) - - _, _, _, 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 - - -def test_expand_response_files(tmp_path: Path) -> None: - """``@file`` arguments are inlined relative to the directory.""" - rsp = tmp_path / "flags.rsp" - rsp.write_text("-DFROM_RSP -I/rsp/inc") - - tokens = idedata._expand_response_files( - ["g++", f"@{rsp.name}", "-c", "x.cpp"], tmp_path - ) - - assert "-DFROM_RSP" in tokens - assert "-I/rsp/inc" in tokens - assert not any(t.startswith("@") for t in tokens) - - -def test_expand_response_files_keeps_literal_when_missing(tmp_path: Path) -> None: - """An unreadable ``@file`` token is kept verbatim rather than dropped.""" - tokens = idedata._expand_response_files(["g++", "@nope.rsp"], tmp_path) - assert "@nope.rsp" in tokens - - -def test_pick_entry_prefers_esphome_tu() -> None: - """A ``/src/esphome/`` C++ TU is picked over other compile entries.""" - entries = [ - _entry("/b", "/b/managed_components/foo/foo.c", "gcc -c foo.c"), - _entry("/b", "/b/src/esphome/core/app.cpp", "g++ -c app.cpp"), - ] - assert idedata._pick_entry(entries)["file"].endswith("app.cpp") - - -def test_pick_entry_falls_back_to_any_cxx_tu() -> None: - """With no ``/src/esphome/`` TU present, the first C++ entry is the fallback.""" - entries = [ - _entry("/b", "/b/managed_components/foo/foo.c", "gcc -c foo.c"), - _entry("/b", "/b/components/x/x.cpp", "g++ -c x.cpp"), - ] - assert idedata._pick_entry(entries)["file"].endswith("x.cpp") - - -def test_is_esphome_src_handles_backslash_paths() -> None: - r"""The src marker must match Windows ``\src\esphome\`` paths too. - - compile_commands ``file`` entries use the OS-native separator; if the - marker only matched forward slashes no source would match on Windows and - the build-include union would be silently empty. - """ - assert idedata._is_esphome_src(r"C:\b\src\esphome\core\app.cpp") - assert idedata._is_esphome_src("/b/src/esphome/core/app.cpp") - # non-esphome and non-C++ still rejected regardless of separator - assert not idedata._is_esphome_src(r"C:\b\managed_components\x\x.cpp") - assert not idedata._is_esphome_src(r"C:\b\src\esphome\core\app.h") - - -def test_idedata_from_build(tmp_path: Path) -> None: - """Full transform: representative entry + include union + toolchain dirs.""" - compile_commands = tmp_path / "compile_commands.json" - entries = [ - _entry( - f"{ABS}b", - f"{ABS}b/src/esphome/core/app.cpp", - f"g++ -DUSE_ESP32 -I{ABS}inc/core -std=gnu++20 -c app.cpp -o app.cpp.o", - ), - _entry( - f"{ABS}b", - f"{ABS}b/src/esphome/sensor/s.cpp", - f"g++ -DUSE_ESP32 -I{ABS}inc/sensor -c s.cpp -o s.cpp.o", - ), - # non-esphome TU: its includes must not leak into the union - _entry( - f"{ABS}b", - f"{ABS}b/managed_components/x/x.c", - f"gcc -I{ABS}inc/managed -c x.c", - ), - ] - compile_commands.write_text(json.dumps(entries)) - - fake_proc = MagicMock( - returncode=0, - stderr=( - "ignored\n" - "#include <...> search starts here:\n" - " /tc/inc/c++\n" - " /tc/inc\n" - "End of search list.\n" - "more ignored\n" - ), - ) - with patch.object(idedata.subprocess, "run", return_value=fake_proc): - data = idedata.idedata_from_build(compile_commands) - - assert data["cxx_path"] == "g++" - assert "USE_ESP32" in data["defines"] - assert "-std=gnu++20" in data["cxx_flags"] - # include dirs unioned across all esphome TUs - assert f"{ABS}inc/core" in data["includes"]["build"] - assert f"{ABS}inc/sensor" in data["includes"]["build"] - # the non-esphome TU is excluded from the union - assert f"{ABS}inc/managed" not in data["includes"]["build"] - # toolchain search dirs parsed from the compiler's -v output - assert data["includes"]["toolchain"] == ["/tc/inc/c++", "/tc/inc"] - - -def test_get_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") - - -def test_get_toolchain_includes_raises_when_no_dirs_found() -> None: - """Markers present but no dirs (anomalous output) also raises.""" - fake_proc = MagicMock( - returncode=0, - stderr="#include <...> search starts here:\nEnd of search list.\n", - ) - with ( - patch.object(idedata.subprocess, "run", return_value=fake_proc), - pytest.raises(RuntimeError, match="builtin include dirs"), - ): - idedata._get_toolchain_includes("/some/compiler") - - -# ESP-IDF's compile_commands.json on Windows mixes literal backslash path -# separators in the compiler path with shell ``\"`` quote-escaping in defines, -# which only the real Windows argv parser handles. These exercise that path. -@pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization") -def test_split_command_preserves_paths_and_unescapes_quotes() -> None: - r"""Backslash paths survive while ``\"`` define-quoting is unescaped.""" - command = r"C:\esp\bin\riscv32-esp-elf-g++.exe -DVER=\"1.2.3\" -IC:/inc/a -c x.cpp" - - tokens = idedata._split_command(command) - - assert tokens[0] == r"C:\esp\bin\riscv32-esp-elf-g++.exe" - assert '-DVER="1.2.3"' in tokens - assert "-IC:/inc/a" in tokens - - -@pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization") -def test_split_command_empty_returns_empty() -> None: - """An empty or blank command tokenizes to ``[]`` (e.g. an empty response file). - - Guards against ``CommandLineToArgvW("")`` returning the current process name - instead of an empty list. - """ - assert idedata._split_command("") == [] - assert idedata._split_command(" ") == [] - - -@pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization") -def test_parse_entry_normalizes_windows_cxx_path() -> None: - """A backslash compiler path is emitted forward-slashed; define unescaped.""" - entry = _entry( - r"C:\b", - r"C:\b\src\esphome\x.cpp", - r"C:\esp\bin\g++.exe -DVER=\"1.2.3\" -IC:/inc/a -c x.cpp", - ) - - cxx_path, defines, includes, _ = idedata._parse_entry(entry) - - assert cxx_path == "C:/esp/bin/g++.exe" - assert "\\" not in cxx_path - assert 'VER="1.2.3"' in defines - assert "C:/inc/a" in includes diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index 2556397aef..6c11a74d48 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -140,7 +140,7 @@ def test_get_idedata_generates_and_caches(setup_core: Path) -> None: compile_commands.write_text("[]") with patch( - "esphome.espidf.idedata.idedata_from_build", + "esphome.build_helpers.idedata.idedata_from_build", return_value={"cxx_path": "g++"}, ) as mock_transform: result = toolchain.get_idedata() @@ -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.espidf.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.espidf.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.espidf.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.espidf.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.espidf.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.""" @@ -267,7 +159,7 @@ def test_get_idedata_prog_path_points_at_firmware_elf(setup_core: Path) -> None: compile_commands.write_text("[]") with patch( - "esphome.espidf.idedata.idedata_from_build", + "esphome.build_helpers.idedata.idedata_from_build", return_value={"cxx_path": "g++"}, ): result = toolchain.get_idedata() diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 1cb710ca58..c7a5c85638 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -17,6 +17,7 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest from pytest import CaptureFixture +import serial from zeroconf import ServiceStateChange from esphome import __main__ as main, yaml_util @@ -26,6 +27,7 @@ from esphome.__main__ import ( _make_crystal_freq_callback, _redact_with_legacy_fallback, _resolve_network_devices, + _should_subscribe_states, _split_network_devices, _unresolved_default_error, _validate_bootloader_binary, @@ -69,19 +71,21 @@ from esphome.__main__ import ( ) from esphome.address_cache import AddressCache from esphome.bundle import BUNDLE_EXTENSION, BundleFile, BundleResult -from esphome.components import esp32, esp8266 +from esphome.components import esp32, esp8266, mqtt from esphome.components.esp32 import ( KEY_ESP32, KEY_VARIANT, VARIANT_ESP32, get_esp32_variant, ) +from esphome.config import Config from esphome.const import ( CONF_API, CONF_AUTH, CONF_BAUD_RATE, CONF_BROKER, CONF_DISABLED, + CONF_DISCOVER_IP, CONF_ESPHOME, CONF_LEVEL, CONF_LOG, @@ -103,6 +107,7 @@ from esphome.const import ( CONF_WEB_SERVER, CONF_WIFI, KEY_CORE, + KEY_TARGET_FRAMEWORK, KEY_TARGET_PLATFORM, PLATFORM_BK72XX, PLATFORM_ESP32, @@ -567,8 +572,6 @@ def test_command_config__no_defaults_dumps_user_snapshot( ) -> None: """``--no-defaults`` dumps ``config.user_config`` instead of the validated config, so schema defaults don't leak into the output.""" - from esphome.config import Config - setup_core(tmp_path=tmp_path, config={"esphome": {"name": "test"}}) args = MockArgs() args.show_secrets = True @@ -621,8 +624,6 @@ def test_command_config__no_defaults_skips_strip_default_ids( ) -> None: """When ``--no-defaults`` is set, ``strip_default_ids`` isn't run -- the user snapshot is already free of schema-injected IDs.""" - from esphome.config import Config - setup_core(tmp_path=tmp_path, config={"esphome": {"name": "test"}}) args = MockArgs() args.show_secrets = True @@ -3440,9 +3441,6 @@ def test_get_port_type() -> None: def test_mqtt_reexports_discover_ip() -> None: """The old import path must keep working for external code.""" - from esphome.components import mqtt - from esphome.const import CONF_DISCOVER_IP - assert mqtt.CONF_DISCOVER_IP is CONF_DISCOVER_IP @@ -5909,8 +5907,6 @@ class MockSerial: chunk = self.chunks[self.chunk_index] if chunk is MOCK_SERIAL_END: # Sentinel means we're done - simulate port closed - import serial - raise serial.SerialException("Port closed") # Respect the requested size and keep any remaining bytes if size <= 0: @@ -5924,8 +5920,6 @@ class MockSerial: # Entire chunk consumed; advance to the next one self.chunk_index += 1 return data # type: ignore[return-value] - import serial - raise serial.SerialException("Port closed") @@ -6784,8 +6778,6 @@ def test_parse_args_argcomplete_only_runs_when_completing() -> None: def test_should_subscribe_states_default() -> None: """Test that states are shown by default when nothing is set.""" - from esphome.__main__ import _should_subscribe_states - args = parse_args(["esphome", "logs", "device.yaml"]) with patch.dict(os.environ, {}, clear=False): os.environ.pop("ESPHOME_LOG_STATES", None) @@ -6794,8 +6786,6 @@ def test_should_subscribe_states_default() -> None: def test_should_subscribe_states_env_suppresses() -> None: """Test that ESPHOME_LOG_STATES=false suppresses states by default.""" - from esphome.__main__ import _should_subscribe_states - args = parse_args(["esphome", "logs", "device.yaml"]) with patch.dict(os.environ, {"ESPHOME_LOG_STATES": "false"}): assert _should_subscribe_states(args) is False @@ -6803,8 +6793,6 @@ def test_should_subscribe_states_env_suppresses() -> None: def test_should_subscribe_states_env_enables() -> None: """Test that ESPHOME_LOG_STATES=true enables states by default.""" - from esphome.__main__ import _should_subscribe_states - args = parse_args(["esphome", "logs", "device.yaml"]) with patch.dict(os.environ, {"ESPHOME_LOG_STATES": "true"}): assert _should_subscribe_states(args) is True @@ -6812,8 +6800,6 @@ def test_should_subscribe_states_env_enables() -> None: def test_should_subscribe_states_flag_overrides_env() -> None: """Test that --states overrides ESPHOME_LOG_STATES=false.""" - from esphome.__main__ import _should_subscribe_states - args = parse_args(["esphome", "logs", "--states", "device.yaml"]) with patch.dict(os.environ, {"ESPHOME_LOG_STATES": "false"}): assert _should_subscribe_states(args) is True @@ -6821,8 +6807,6 @@ def test_should_subscribe_states_flag_overrides_env() -> None: def test_should_subscribe_states_no_flag_overrides_env() -> None: """Test that --no-states overrides ESPHOME_LOG_STATES=true.""" - from esphome.__main__ import _should_subscribe_states - args = parse_args(["esphome", "logs", "--no-states", "device.yaml"]) with patch.dict(os.environ, {"ESPHOME_LOG_STATES": "true"}): assert _should_subscribe_states(args) is False @@ -7135,6 +7119,82 @@ 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"), + KeyError("command"), + None, # replaced with EsphomeError inside + ], +) +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.""" + if error is None: + error = EsphomeError("compile database is unusable") + CORE.toolchain = Toolchain.ESP_IDF + CORE.data[KEY_CORE] = { + KEY_TARGET_PLATFORM: "esp32", + KEY_TARGET_FRAMEWORK: "esp-idf", + } + with ( + patch("esphome.espidf.toolchain.run_compile", return_value=0), + 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=error), + patch("esphome.__main__._check_and_emit_build_info"), + ): + assert compile_program(MagicMock(), {}) == 0 + assert "Could not generate idedata" in caplog.text + + +def test_compile_program_espidf_idedata_success_is_silent( + caplog: pytest.LogCaptureFixture, +) -> None: + """The healthy path: idedata generated, nothing to warn about.""" + CORE.toolchain = Toolchain.ESP_IDF + CORE.data[KEY_CORE] = { + KEY_TARGET_PLATFORM: "esp32", + KEY_TARGET_FRAMEWORK: "esp-idf", + } + with ( + patch("esphome.espidf.toolchain.run_compile", return_value=0), + 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", return_value={"cc_path": "x"}), + patch("esphome.__main__._check_and_emit_build_info"), + ): + assert compile_program(MagicMock(), {}) == 0 + assert "idedata" not in caplog.text + + +def test_compile_program_espidf_idedata_none_warns( + caplog: pytest.LogCaptureFixture, +) -> None: + """A silent None from the post-compile idedata refresh is made visible.""" + CORE.toolchain = Toolchain.ESP_IDF + CORE.data[KEY_CORE] = { + KEY_TARGET_PLATFORM: "esp32", + KEY_TARGET_FRAMEWORK: "esp-idf", + } + with ( + patch("esphome.espidf.toolchain.run_compile", return_value=0), + 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", return_value=None), + patch("esphome.__main__._check_and_emit_build_info"), + ): + assert compile_program(MagicMock(), {}) == 0 + assert "No idedata was generated" in caplog.text + + @pytest.mark.asyncio async def test_wrap_to_code_comment_is_insertion_order_independent() -> None: """The config comment dumps with sorted keys: voluptuous fills schema diff --git a/tests/unit_tests/test_size_summary.py b/tests/unit_tests/test_size_summary.py index 933be88476..0c0852a191 100644 --- a/tests/unit_tests/test_size_summary.py +++ b/tests/unit_tests/test_size_summary.py @@ -126,3 +126,20 @@ def test_print_summary_handles_no_memory_types( size_json = _write_size_json(tmp_path, {"image_size": 0}) print_summary(size_json, partitions_csv=None) assert capsys.readouterr().out == "" + + +def test_print_summary_flash_line( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A partition table with an app row yields the Flash line in the exact + padded shape script/ci_memory_impact_extract.py greps.""" + size_json = _write_size_json(tmp_path, _esp32_size_data()) + partitions = tmp_path / "partitions.csv" + partitions.write_text( + "# name, type, subtype, offset, size, flags\n" + "app0, app, ota_0, 0x10000, 0x1C0000,\n" + ) + print_summary(size_json, partitions) + out = capsys.readouterr().out + assert "Flash: " in out + assert "(used 827455 bytes from 1835008 bytes)" in out