From 5aebd7cb4149807b3cfa3f5a5ea691e11a6418eb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 23 Aug 2026 15:05:43 -0500 Subject: [PATCH] Match SCons' remaining CPPDEFINES spellings, drop dead test scaffolding A bare 2-tuple is one name=value pair (not -DFOO -D1), a dict maps names to values, and a None value is a bare define, per processDefines. The unused caplog_at_info helper and the progress kwarg the download stubs declared ahead of their PR are gone. --- esphome/platformio/extra_script.py | 29 +++++++++++++++---- .../test_platformio_extra_script.py | 18 ++++++++++++ tests/unit_tests/test_platformio_library.py | 21 +------------- 3 files changed, 43 insertions(+), 25 deletions(-) diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py index bdef357671..9fa73bafac 100644 --- a/esphome/platformio/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -14,7 +14,7 @@ import logging import os from pathlib import Path import shlex -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from esphome.core import EsphomeError @@ -98,6 +98,17 @@ class ExtraScriptResult: cppflags: list[str] = field(default_factory=list) +def _cppdefines_items(value: Any) -> list: + """Normalize SCons ``processDefines`` spellings: a bare 2-tuple is one + ``name=value`` pair, a dict maps names to values, a list is + element-wise.""" + if isinstance(value, tuple) and len(value) == 2: + return [value] + if isinstance(value, dict): + return list(value.items()) + return list(value) if isinstance(value, (list, tuple)) else [value] + + class _FakeSConsEnv: """Minimal SCons ``Environment`` stand-in: ``get`` and ``Append`` work; every other method is a swallowed no-op so scripts don't abort.""" @@ -147,7 +158,10 @@ class _FakeSConsEnv: key, ) continue - items = list(value) if isinstance(value, (list, tuple)) else [value] + if key == "CPPDEFINES": + items = _cppdefines_items(value) + else: + items = list(value) if isinstance(value, (list, tuple)) else [value] bucket = getattr(self.result, key.lower()) bucket.extend(items) @@ -285,14 +299,19 @@ def captured_as_build_flags( ) flags.extend(f"-l{shlex.quote(lib)}" for lib in _strs(result.libs, "LIBS")) for define in result.cppdefines: - # SCons also accepts dict/list CPPDEFINES; formatting those blind + # SCons also accepts nested containers; formatting those blind # would hand the compiler garbage like -D{'FOO': '1'} if ( isinstance(define, (tuple, list)) and len(define) == 2 - and all(isinstance(part, (str, int)) for part in define) + and isinstance(define[0], (str, int)) + and isinstance(define[1], (str, int, type(None))) ): - flags.append(shlex.quote(f"-D{define[0]}={define[1]}")) + if define[1] is None: + # {"FOO": None} / ("FOO", None) is a bare -DFOO in SCons + flags.append(shlex.quote(f"-D{define[0]}")) + else: + flags.append(shlex.quote(f"-D{define[0]}={define[1]}")) elif isinstance(define, str): flags.append(shlex.quote(f"-D{define}")) else: diff --git a/tests/unit_tests/test_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py index 4b839e30e8..ebe61d0d1f 100644 --- a/tests/unit_tests/test_platformio_extra_script.py +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -497,6 +497,24 @@ def test_env_unmodelled_subscript_degrades_one_branch(caplog) -> None: assert env.result.libs == ["still_captured"] +def test_cppdefines_scons_spellings(tmp_path) -> None: + """A bare 2-tuple is one name=value pair, a dict maps names to values, + and a None value is a bare define (SCons processDefines).""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + env.Append(CPPDEFINES=("FOO", "1")) + env.Append(CPPDEFINES={"BAR": "2", "BAZ": None}) + env.Append(CPPDEFINES=["PLAIN"]) + flags = captured_as_build_flags(env.result, library_dir=tmp_path) + assert lex_build_flags(flags, "test") == [ + "-DFOO=1", + "-DBAR=2", + "-DBAZ", + "-DPLAIN", + ] + + def test_uncaptured_append_key_warns_once(caplog) -> None: """A loop of Appends to the same uncaptured key warns once.""" diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index da4dba842f..0f90ad3e84 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -4,7 +4,6 @@ Covers the shared download/parse/resolve/dependency-walk paths in ``esphome.platformio.library`` directly (the ESP-IDF and Zephyr backends are exercised in their own test modules).""" -from contextlib import contextmanager import json import logging from pathlib import Path @@ -154,23 +153,6 @@ def test_localsource_download_returns_empty_build_dir(setup_core: Path) -> None: assert plain != out -@contextmanager -def caplog_at_info(): - records: list[logging.LogRecord] = [] - handler = logging.Handler() - handler.emit = records.append - logger = logging.getLogger("esphome.platformio.library") - logger.addHandler(handler) - # The level must actually admit INFO or the no-INFO assertions are vacuous - old_level = logger.level - logger.setLevel(logging.INFO) - try: - yield records - finally: - logger.setLevel(old_level) - logger.removeHandler(handler) - - def test_urlsource_download_extracts_then_reuses_marker(setup_core, monkeypatch): monkeypatch.setattr(lib, "rmdir", lambda path, msg="": None) dl_calls: list[list[str]] = [] @@ -236,7 +218,7 @@ def _patch_registry_resolve(monkeypatch: pytest.MonkeyPatch) -> None: def _patch_download_with_manifests(monkeypatch, tmp_path, manifests, *, properties=()): """Fake ConvertedLibrary.download to materialize canned manifests on disk.""" - def fake_download(self, force=False, salt="", namespace="", progress=None): + def fake_download(self, force=False, salt="", namespace=""): self.path = tmp_path / self.get_require_name() self.path.mkdir(parents=True, exist_ok=True) if self.name in properties: @@ -319,7 +301,6 @@ def _patch_download_without_manifest( force: bool = False, salt: str = "", namespace: str = "", - progress=None, ) -> None: calls.append(force) self.path = tmp_path / self.get_require_name()