Trim comment essays and hoist function-local test imports

This commit is contained in:
J. Nick Koston
2026-08-22 11:34:32 -05:00
parent 2c229debe8
commit e99e0cc8b8
11 changed files with 32 additions and 107 deletions
+2 -5
View File
@@ -865,11 +865,8 @@ def compile_program(args: ArgsProtocol, config: ConfigType) -> int:
RuntimeError,
ValueError,
) as err:
# The firmware already built; idedata is a bonus artifact here.
# Broad on purpose: a vanished compiler (OSError), a failed
# include probe (RuntimeError), or a truncated compile DB
# (ValueError/LookupError) must not fail a successful build
# either.
# Broad on purpose: the firmware already built; an idedata
# failure must not fail a successful build.
_LOGGER.warning("Could not generate idedata: %s", err)
else:
from esphome.platformio import toolchain
+4 -9
View File
@@ -123,10 +123,8 @@ def _pick_entry(entries: list[dict]) -> dict:
raise ValueError("no C++ translation unit found in compile_commands.json")
# Compiler launchers that may prefix a compile command. A closed denylist is
# sturdier than trying to enumerate compiler names: launchers are few and
# stable, while compilers (cross prefixes, versioned names, icx, armcc, ...)
# are an open set.
# 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"})
@@ -300,17 +298,14 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d
representative = _pick_entry(entries)
cxx_path, defines, rep_includes, cxx_flags = parse_entry(representative, launcher)
if _is_launcher(cxx_path):
# Checked before the toolchain probe (which would fail opaquely on
# a launcher) so the unusable compile DB is named, and never
# cached or conflated with "nothing built yet"
# Reject before the toolchain probe, which would fail opaquely on
# a launcher; never cache the unusable compile DB
raise EsphomeError(
f"compile_commands.json names the launcher {cxx_path} as the "
"compiler; the compile database is unusable"
)
# Seed with the representative's includes so it is not parsed twice
# (per-file -c/-o arguments make every command distinct, so memoizing
# whole commands would never hit)
build_includes: dict[str, None] = dict.fromkeys(
rep_includes if _is_esphome_src(representative["file"]) else ()
)
+1 -5
View File
@@ -4,11 +4,7 @@ from __future__ import annotations
def format_bar(used: int, total: int) -> str:
"""Match PlatformIO's ``_format_availale_bytes`` (pioupload.py) exactly.
The upstream helper's name really is spelled that way; keep the citation
verbatim so it stays greppable in the PlatformIO source.
"""
"""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)
+2 -4
View File
@@ -92,10 +92,8 @@ def generate_cmakelists_txt(component: IDFComponent) -> str:
build_flags = ensure_list(
component.data.get("build", {}).get("flags", DEFAULT_BUILD_FLAGS)
)
# PlatformIO shell-lexes each build.flags entry, so one entry can carry a
# flag and its argument (e.g. "-include cp_custom_alloc.h"); bare
# -I/-L/-l/-D tokens re-glue to their argument ("-I foo" -> "-Ifoo") so
# prefix classifiers below still route them.
# PlatformIO shell-lexes each build.flags entry; bare -I/-L/-l/-D tokens
# re-glue to their argument so the prefix classifiers below route them.
# Joined per entry, as SCons's ParseFlags lexes each string
# independently: a dangling -I ending one entry must warn, not absorb
# the next entry's first token.
+3 -6
View File
@@ -141,9 +141,7 @@ class _FakeSConsEnv:
def Append(self, **kwargs) -> None: # noqa: N802 (SCons API name)
for key, value in kwargs.items():
if key not in _CAPTURED_KEYS:
# Diagnosable from the build log when a script configures
# something this shim does not translate; once per key so a
# loop of Appends cannot spam
# Warn once per key so a loop of Appends cannot spam
if key not in self._warned_keys:
self._warned_keys.add(key)
_LOGGER.warning(
@@ -239,9 +237,8 @@ def run_extra_script(
)
return ExtraScriptResult()
except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught
# Discard any partial capture: folding half a script's flags into the
# build could produce wrong-output firmware that links cleanly. The
# warning plus the resulting loud link error point back here.
# Discard any partial capture: half-applied flags could build wrong
# firmware that links cleanly.
_LOGGER.warning(
"PIO extra-script %s (in %s) raised %r; ignoring its output",
script_path,
+2 -5
View File
@@ -47,11 +47,8 @@ DEFAULT_BUILD_SRC_FILTER = (
DEFAULT_BUILD_SRC_DIRS = "src"
DEFAULT_BUILD_INCLUDE_DIR = "include"
DEFAULT_BUILD_FLAGS = []
# Source suffix -> compiler kind, PlatformIO's CSUFFIXES/CXXSUFFIXES/ASSUFFIXES
# split. Native build generators map the kind to their compile rules. "asm"
# deliberately merges SCons's AS (.s/.asm) and ASPP (.S/.spp/.sx) sets: the
# ninja rules compile all of them as assembler-with-cpp, whose asm-mode
# preprocessor passes non-directive text through unchanged.
# Suffix -> compiler kind (PlatformIO's CSUFFIXES/CXXSUFFIXES/ASSUFFIXES).
# "asm" merges SCons's AS and ASPP sets: all compile as assembler-with-cpp.
SOURCE_KIND_FOR_SUFFIX: dict[str, str] = {
".c": "c",
".cpp": "cxx",
@@ -429,8 +429,7 @@ def test_load_or_build_idedata_corrupted_cache_is_logged(
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 by name,
before the toolchain probe could fail opaquely, and never cached."""
"""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(
+2 -2
View File
@@ -6,6 +6,7 @@ from unittest.mock import MagicMock
import pytest
from esphome.components import esp32 as esp32_module
from esphome.const import (
KEY_CORE,
KEY_TARGET_FRAMEWORK,
@@ -15,6 +16,7 @@ from esphome.const import (
)
from esphome.core import CORE, Library
from esphome.espidf.component import (
_emit_idf_component,
generate_cmakelists_txt,
generate_idf_component_yml,
generate_idf_components,
@@ -1072,8 +1074,6 @@ def test_idf_component_download_passes_salt() -> None:
def test_emit_idf_component_wires_esp32_target(tmp_path, monkeypatch):
"""Emitting a component resolves the esp32 variant into the shared
extraScript helper."""
from esphome.components import esp32 as esp32_module
from esphome.espidf.component import _emit_idf_component
monkeypatch.setattr(esp32_module, "get_esp32_variant", lambda: "ESP32")
(tmp_path / "src").mkdir()
+2 -25
View File
@@ -101,6 +101,7 @@ from esphome.const import (
CONF_WEB_SERVER,
CONF_WIFI,
KEY_CORE,
KEY_TARGET_FRAMEWORK,
KEY_TARGET_PLATFORM,
PLATFORM_BK72XX,
PLATFORM_ESP32,
@@ -7139,7 +7140,7 @@ def test_warn_source_tree_mismatch_falls_back_when_stat_fails(
RuntimeError("Could not query builtin include dirs"),
ValueError("no C++ translation unit found"),
KeyError("command"),
None, # replaced with EsphomeError inside (import is function-local)
None, # replaced with EsphomeError inside
],
)
def test_compile_program_espidf_idedata_failure_does_not_fail_build(
@@ -7147,14 +7148,6 @@ def test_compile_program_espidf_idedata_failure_does_not_fail_build(
caplog: pytest.LogCaptureFixture,
) -> None:
"""A post-compile idedata error is a warning: the firmware already built."""
from esphome.const import (
KEY_CORE,
KEY_TARGET_FRAMEWORK,
KEY_TARGET_PLATFORM,
Toolchain,
)
from esphome.core import CORE, EsphomeError
if error is None:
error = EsphomeError("compile database is unusable")
CORE.toolchain = Toolchain.ESP_IDF
@@ -7178,14 +7171,6 @@ def test_compile_program_espidf_idedata_success_is_silent(
caplog: pytest.LogCaptureFixture,
) -> None:
"""The healthy path: idedata generated, nothing to warn about."""
from esphome.const import (
KEY_CORE,
KEY_TARGET_FRAMEWORK,
KEY_TARGET_PLATFORM,
Toolchain,
)
from esphome.core import CORE
CORE.toolchain = Toolchain.ESP_IDF
CORE.data[KEY_CORE] = {
KEY_TARGET_PLATFORM: "esp32",
@@ -7207,14 +7192,6 @@ def test_compile_program_espidf_idedata_none_warns(
caplog: pytest.LogCaptureFixture,
) -> None:
"""A silent None from the post-compile idedata refresh is made visible."""
from esphome.const import (
KEY_CORE,
KEY_TARGET_FRAMEWORK,
KEY_TARGET_PLATFORM,
Toolchain,
)
from esphome.core import CORE
CORE.toolchain = Toolchain.ESP_IDF
CORE.data[KEY_CORE] = {
KEY_TARGET_PLATFORM: "esp32",
@@ -2,19 +2,25 @@
from __future__ import annotations
import logging
import os
from pathlib import Path
from unittest.mock import patch
import pytest
from esphome.core import EsphomeError
from esphome.platformio.extra_script import (
ExtraScriptResult,
_FakeSConsEnv,
apply_extra_script,
captured_as_build_flags,
run_extra_script,
)
from esphome.platformio.library import ConvertedLibrary as IDFComponent, URLSource
def test_extra_script_captures_libpath_libs_and_defines(tmp_path):
from esphome.platformio.extra_script import (
captured_as_build_flags,
run_extra_script,
)
(tmp_path / "src" / "esp32").mkdir(parents=True)
script = tmp_path / "extra_script.py"
@@ -58,10 +64,6 @@ 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.platformio.extra_script import (
ExtraScriptResult,
captured_as_build_flags,
)
(tmp_path / "lib" / "esp32").mkdir(parents=True)
elsewhere = tmp_path.parent / "not_the_library_dir"
@@ -76,10 +78,6 @@ def test_extra_script_libpath_relative_resolves_against_library_dir(
def test_extra_script_libpath_absolute_outside_library_dir(tmp_path):
from esphome.platformio.extra_script import (
ExtraScriptResult,
captured_as_build_flags,
)
outside = tmp_path.parent / "system_lib"
outside.mkdir(exist_ok=True)
@@ -90,7 +88,6 @@ def test_extra_script_libpath_absolute_outside_library_dir(tmp_path):
def test_extra_script_failure_returns_empty_result(tmp_path, caplog):
from esphome.platformio.extra_script import run_extra_script
script = tmp_path / "broken.py"
script.write_text("raise RuntimeError('boom')\n")
@@ -106,7 +103,6 @@ def test_extra_script_failure_returns_empty_result(tmp_path, caplog):
def test_apply_extra_script_path_traversal_is_rejected(tmp_path):
from esphome.platformio.extra_script import apply_extra_script
library_dir = tmp_path / "lib"
library_dir.mkdir()
@@ -117,8 +113,6 @@ def test_apply_extra_script_path_traversal_is_rejected(tmp_path):
c.path = library_dir
c.data = {"build": {"extraScript": "../evil.py"}}
from esphome.core import EsphomeError
with pytest.raises(EsphomeError, match="escapes the library directory"):
apply_extra_script(c, board_mcu=lambda: "esp32", pio_platform="espressif32")
# Nothing was folded into flags: the traversal was rejected before
@@ -127,7 +121,6 @@ def test_apply_extra_script_path_traversal_is_rejected(tmp_path):
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"
@@ -146,8 +139,6 @@ def test_apply_extra_script_merges_into_existing_flags(tmp_path):
def test_apply_extra_script_malformed_flags_raises(tmp_path) -> None:
"""A null/dict build.flags fails naming the library instead of injecting
a non-string into the compiler command line."""
from esphome.core import EsphomeError
from esphome.platformio.extra_script import apply_extra_script
(tmp_path / "src").mkdir()
script = tmp_path / "extra.py"
@@ -164,7 +155,6 @@ def test_apply_extra_script_malformed_flags_raises(tmp_path) -> None:
def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None:
"""The shared helper resolves the board_mcu callable lazily and normalizes
a string ``build.flags`` value into a list before extending it."""
from esphome.platformio.extra_script import apply_extra_script
(tmp_path / "src").mkdir()
script = tmp_path / "extra.py"
@@ -180,7 +170,6 @@ def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None:
def test_apply_extra_script_no_script_and_no_flags(tmp_path) -> None:
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"))
@@ -203,9 +192,6 @@ def test_apply_extra_script_no_script_and_no_flags(tmp_path) -> None:
def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path, caplog) -> None:
"""Un-captured env vars and unsupported env methods are skipped but
diagnosable from the build log."""
import logging
from esphome.platformio.extra_script import apply_extra_script
caplog.set_level(logging.DEBUG)
script = tmp_path / "extra.py"
@@ -223,7 +209,6 @@ def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path, caplog) -> No
def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None:
"""A raising extra-script is best-effort: logged and skipped."""
from esphome.platformio.extra_script import apply_extra_script
script = tmp_path / "extra.py"
script.write_text("raise RuntimeError('boom')\n")
@@ -237,7 +222,6 @@ def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None:
def test_apply_extra_script_pio_platform(tmp_path) -> None:
"""The backend's platform token is exposed to the script as PIOPLATFORM."""
from esphome.platformio.extra_script import apply_extra_script
script = tmp_path / "extra.py"
script.write_text("env.Append(LIBS=[env.get('PIOPLATFORM')])\n")
@@ -251,8 +235,6 @@ def test_apply_extra_script_pio_platform(tmp_path) -> None:
def test_apply_extra_script_missing_script_raises(tmp_path) -> None:
"""A declared but absent extraScript is a broken package and fails by
name, as it would under PlatformIO."""
from esphome.core import EsphomeError
from esphome.platformio.extra_script import apply_extra_script
c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
c.path = tmp_path
@@ -264,7 +246,6 @@ def test_apply_extra_script_missing_script_raises(tmp_path) -> None:
def test_run_extra_script_failure_discards_partial_capture(tmp_path, caplog) -> None:
"""A crashed script yields an empty result: half-applied flags could
build wrong-output firmware that links cleanly."""
from esphome.platformio.extra_script import run_extra_script
script = tmp_path / "extra.py"
script.write_text("env.Append(LIBS=['algobsec'])\nraise RuntimeError('boom')\n")
@@ -278,7 +259,6 @@ def test_run_extra_script_failure_discards_partial_capture(tmp_path, caplog) ->
def test_run_extra_script_syntax_error_is_best_effort(tmp_path, caplog) -> None:
"""A vendored script that does not even compile warns and skips instead
of aborting the build."""
from esphome.platformio.extra_script import run_extra_script
script = tmp_path / "extra.py"
script.write_text("def broken(:\n")
@@ -291,7 +271,6 @@ def test_run_extra_script_syntax_error_is_best_effort(tmp_path, caplog) -> None:
def test_unsupported_env_method_warns_once(caplog) -> None:
"""Repeated calls to the same unsupported method warn only once."""
from esphome.platformio.extra_script import _FakeSConsEnv
env = _FakeSConsEnv(
board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266"
@@ -304,7 +283,6 @@ def test_unsupported_env_method_warns_once(caplog) -> None:
def test_run_extra_script_sys_exit_is_best_effort(tmp_path, caplog) -> None:
"""A nonzero sys.exit() in a vendored script must not kill the esphome
run, and its output is discarded."""
from esphome.platformio.extra_script import run_extra_script
script = tmp_path / "extra.py"
script.write_text("import sys\nenv.Append(LIBS=['x'])\nsys.exit(3)\n")
@@ -317,7 +295,6 @@ def test_run_extra_script_sys_exit_is_best_effort(tmp_path, caplog) -> None:
def test_run_extra_script_sys_exit_zero_is_success(tmp_path, caplog) -> None:
"""sys.exit(0) is a normal PlatformIO script ending: the capture is kept."""
from esphome.platformio.extra_script import run_extra_script
script = tmp_path / "extra.py"
script.write_text("import sys\nenv.Append(LIBS=['algobsec'])\nsys.exit(0)\n")
@@ -330,10 +307,6 @@ def test_run_extra_script_sys_exit_zero_is_success(tmp_path, caplog) -> None:
def test_run_extra_script_unreadable_raises(tmp_path) -> None:
"""An unreadable declared script is a broken package, like a missing one."""
from unittest.mock import patch
from esphome.core import EsphomeError
from esphome.platformio.extra_script import run_extra_script
script = tmp_path / "extra.py"
script.write_text("")
@@ -348,7 +321,6 @@ def test_run_extra_script_unreadable_raises(tmp_path) -> None:
def test_run_extra_script_bad_encoding_is_best_effort(tmp_path, caplog) -> None:
"""Undecodable content warns and skips, like a SyntaxError."""
from esphome.platformio.extra_script import run_extra_script
script = tmp_path / "extra.py"
script.write_bytes(b"\xff\xfe\x00bad")
@@ -361,7 +333,6 @@ def test_run_extra_script_bad_encoding_is_best_effort(tmp_path, caplog) -> None:
def test_uncaptured_append_key_warns_once(caplog) -> None:
"""A loop of Appends to the same uncaptured key warns once."""
from esphome.platformio.extra_script import _FakeSConsEnv
env = _FakeSConsEnv(
board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266"
+3 -5
View File
@@ -13,6 +13,7 @@ import pytest
from esphome.core import EsphomeError, Library
import esphome.platformio.library as lib
from esphome.platformio.library import (
SOURCE_KIND_FOR_SUFFIX,
ConvertedLibrary,
GitSource,
InvalidLibrary,
@@ -23,6 +24,8 @@ from esphome.platformio.library import (
_resolve_registry_version,
check_library_data,
convert_libraries,
join_flag_args,
split_flag_entry,
)
@@ -535,7 +538,6 @@ def test_convert_libraries_skips_incompatible_dependency(tmp_path, monkeypatch):
def test_split_flag_entry_unbalanced_quote_is_clean() -> None:
"""A malformed flags entry raises EsphomeError, not a raw ValueError."""
from esphome.platformio.library import split_flag_entry
assert split_flag_entry('-DX="a b"', "library x") == ["-DX=a b"]
with pytest.raises(EsphomeError, match=r"Malformed build flag.*library x"):
@@ -544,7 +546,6 @@ def test_split_flag_entry_unbalanced_quote_is_clean() -> None:
def test_join_flag_args_reglues_spaced_define() -> None:
"""A spaced -D re-glues to its argument, as ParseFlags does."""
from esphome.platformio.library import join_flag_args
assert join_flag_args(["-D", "FOO=1", "-Os"], "x") == ["-DFOO=1", "-Os"]
@@ -552,7 +553,6 @@ def test_join_flag_args_reglues_spaced_define() -> None:
def test_join_flag_args_trailing_bare_flag_warns(
caplog: pytest.LogCaptureFixture,
) -> None:
from esphome.platformio.library import join_flag_args
assert join_flag_args(["-Os", "-l"], "library x") == ["-Os"]
assert "Ignoring trailing '-l'" in caplog.text
@@ -561,7 +561,6 @@ def test_join_flag_args_trailing_bare_flag_warns(
def test_split_flag_entry_non_string_is_clean() -> None:
"""A dict or number from a third-party manifest fails naming the entry,
not with an opaque shlex traceback."""
from esphome.platformio.library import split_flag_entry
with pytest.raises(EsphomeError, match="Malformed build flag"):
split_flag_entry({"esp32": ["-DX"]}, "lib x")
@@ -572,7 +571,6 @@ def test_split_flag_entry_non_string_is_clean() -> None:
def test_source_kind_map_shape() -> None:
"""The kind values the native compile rules key on, and the deliberate
AS/ASPP merge (.s and .S both map to asm)."""
from esphome.platformio.library import SOURCE_KIND_FOR_SUFFIX
assert set(SOURCE_KIND_FOR_SUFFIX.values()) == {"c", "cxx", "asm"}
assert SOURCE_KIND_FOR_SUFFIX[".s"] == "asm"