Trim comment essays and hoist function-local test imports

esphome/build_gen/arduino8266.py

@@ -55,11 +55,6 @@ if TYPE_CHECKING:
 
 _LOGGER = logging.getLogger(__name__)
 
-# Compile rule per source suffix, derived from the shared suffix -> kind map
-# so every source extension a library manifest can select has a rule.
-# Compile rule names are exactly the shared suffix -> kind values (c, cxx,
-# asm), so every source extension a library manifest can select has a rule.
-
 # Always excluded from the core build: ESPHome uses its own native OTA
 # backend, so the Arduino Updater (and its 228-byte global) never links.
 _CORE_EXCLUDE_ALWAYS = {"Updater.cpp"}
@@ -657,9 +652,7 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool:
     config = _resolve_build_config(flag_defines)
     esp8266_data = CORE.data[KEY_ESP8266]
     board = esp8266_data[KEY_BOARD]
-    # _validate_native_toolchain rejects unknown boards at config time and a
-    # test pins the two board tables equal; this backstop covers callers
-    # that bypassed validation
+    # Backstop; config validation already rejects unknown boards
     if board not in ESP8266_BOARD_BUILD:
         raise EsphomeError(f"Board '{board}' is not supported by the native toolchain")
     board_build = ESP8266_BOARD_BUILD[board]
@@ -730,10 +723,8 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool:
         + common
         + [_shell_token(f) for f in get_project_cxx_compile_flags()]
     )
-    # PlatformIO's ASPPCOM carries defines and includes but not CCFLAGS,
-    # so only -D/-I user flags reach assembly there; match it. The tokens
-    # are already shell-quoted (per-platform style), so test past a
-    # leading quote too.
+    # PlatformIO's ASPPCOM passes only -D/-I user flags to assembly; match
+    # it (tokens arrive shell-quoted, hence the lstrip)
     asflags = (
         _ASFLAGS
         + defines
@@ -781,10 +772,8 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool:
 
     build_tool = Path(__file__).parent / "build_tool.py"
 
-    # $in/$out stay unquoted in the rule commands: ninja shell-escapes its
-    # built-in path variables itself when expanding a command (POSIX and
-    # Windows), so adding quotes would wrap ninja's own quoting and break
-    # space-containing paths. Only literal paths need _q().
+    # $in/$out stay unquoted: ninja escapes its built-in path variables
+    # itself; only literal paths need _q().
     lines = [
         "# Auto-generated by ESPHome",
         "ninja_required_version = 1.5",
@@ -797,6 +786,7 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool:
         f"buildtool = {_q(build_tool)}",
         f"ccache = {_q(ccache) if ccache else ''}",
         "",
+        # Rule names match SOURCE_KIND_FOR_SUFFIX values (c, cxx, asm)
         "rule c",
         "  command = $ccache $cc -MMD -MF $out.d $cflags $flags -c $in -o $out",
         "  depfile = $out.d",
@@ -823,10 +813,8 @@ def write_project(paths: InstalledPaths, ccache: str | None) -> bool:
         "  rspfile_content = $in_newline",
         "  description = LINK $out",
         "rule elf2bin",
-        # --flash_freq 40: upstream derives this from the board JSON's
-        # f_flash, but all 45 supported boards ship 40 MHz (audited against
-        # platform-espressif8266); re-check if a future platform bump adds
-        # a board with a different f_flash
+        # --flash_freq 40: every supported board's f_flash is 40 MHz;
+        # re-check on a platform bump
         f"  command = $python {_q(framework / 'tools' / 'elf2bin.py')} --eboot {_q(framework / 'bootloaders' / 'eboot' / 'eboot.elf')} --app $in --flash_mode {esp8266_data[KEY_FLASH_MODE]} --flash_freq 40 --flash_size {_flash_size_str(BOARDS[board][KEY_FLASH_SIZE])} --path {_q(toolchain_bin)} --out $out",
         "  description = BIN $out",
         "rule copy",

tests/unit_tests/build_gen/test_arduino8266.py

@@ -1,14 +1,8 @@
 """Drift tests for the native ESP8266 Arduino build generator.
 
-These pin the ESPHome side of the transliteration (the knob-define
-precedence, the define/flag sets, and the linker-script generation) against
-literals audited from the PlatformIO builder
-(framework-arduinoespressif8266/tools/platformio-build.py and
-platform-espressif8266/builder/main.py): the knob-define precedence, the
-define/flag sets, the link line, and the core source exclusions. They catch
-an accidental edit on this side; an upstream change in a new framework
-release is caught by the A/B byte-identical build check on a version bump,
-not by these tests.
+Pin the transliterated flag/define/link sets against literals audited from
+the PlatformIO builder. Upstream drift is caught by the A/B build check on
+version bumps, not here.
 """
 
 from __future__ import annotations
@@ -17,10 +11,12 @@ from collections.abc import Generator
 import logging
 import os
 from pathlib import Path
+import shutil
 from unittest.mock import MagicMock, patch
 
 import pytest
 
+from esphome.arduino.library import ArduinoLibrary
 from esphome.arduino8266.framework import InstalledPaths, toolchain_tool
 from esphome.build_gen import arduino8266
 from esphome.build_gen.arduino8266 import (
@@ -30,7 +26,7 @@ from esphome.build_gen.arduino8266 import (
     _resolve_build_config,
     get_flash_ld_path,
 )
-from esphome.components.esp8266.boards import ESP8266_BOARD_BUILD
+from esphome.components.esp8266.boards import BOARDS, ESP8266_BOARD_BUILD
 from esphome.components.esp8266.build_surgery import RATETABLE_RULE
 from esphome.components.esp8266.const import (
     KEY_BOARD,
@@ -255,9 +251,7 @@ def test_write_project_link_line_and_exclusions(tmp_path: Path) -> None:
     assert "-T eagle.flash.4m.ld" in content
     # scanf float disabled: the forced-link flag must not appear
     assert "_scanf_float" not in content
-    # $in/$out must stay UNQUOTED: ninja shell-escapes its built-in path
-    # variables itself, so added quotes would wrap ninja's quoting and break
-    # space-containing paths.
+    # $in/$out must stay unquoted; ninja escapes its own path variables
     assert "-c $in -o $out" in content
     assert "--app $in --flash_mode" in content
     assert '"$in"' not in content
@@ -492,7 +486,6 @@ def test_generate_ld_scripts_testing_mode(tmp_path: Path) -> None:
 def test_write_project_libraries_and_variant(
     tmp_path: Path, caplog: pytest.LogCaptureFixture
 ) -> None:
-    from esphome.arduino.library import ArduinoLibrary
 
     paths = _make_framework(tmp_path)
     variant_src = paths.framework / "variants" / "nodemcu" / "variant.cpp"
@@ -572,7 +565,6 @@ def test_write_project_testing_mode(tmp_path: Path) -> None:
 
 def test_write_project_missing_framework_dir_raises(tmp_path: Path) -> None:
     """An incomplete framework install fails naming the missing path."""
-    import shutil
 
     paths = _make_framework(tmp_path)
     shutil.rmtree(paths.framework / "tools" / "sdk" / "lwip2")
@@ -891,7 +883,6 @@ def test_write_project_asm_keeps_quoted_defines(tmp_path: Path) -> None:
 
 def test_write_project_unarchived_library_links_objects(tmp_path: Path) -> None:
     """A libArchive:false library's objects reach the link directly."""
-    from esphome.arduino.library import ArduinoLibrary
 
     paths = _make_framework(tmp_path)
     lib_src = tmp_path / "gdb" / "src"
@@ -1219,11 +1210,7 @@ def test_generate_ld_scripts_unreadable_header_forces_regeneration(
 
 
 def test_board_tables_are_equal() -> None:
-    """write_project rejects a board missing from either table, so the two
-    must stay exactly in sync (the build-surgery test only checks the
-    subset direction, which is how d1_wroom_02 went missing)."""
-    from esphome.components.esp8266.boards import BOARDS
-
+    """BOARDS and ESP8266_BOARD_BUILD must stay exactly in sync."""
     assert set(BOARDS) == set(ESP8266_BOARD_BUILD)