Trim comment essays and hoist function-local test imports

esphome/build_helpers/ccache.py

@@ -1,13 +1,5 @@
-"""Shared ccache policy for build backends.
-
-``ccache_defaults_env`` serves the backends that export ``CCACHE_*`` into a
-build subprocess (native ESP-IDF and Arduino); ``resolve_ccache_path``
-carries the probe and enable rules (PlatformIO and the native Arduino
-build). The ESP-IDF backend keeps ``IDF_CCACHE_ENABLE`` as a
-higher-precedence override and falls back to the shared resolver (probe
-included) when it is unset; PlatformIO feeds its SCons wrapper script
-through env channels instead of ``CCACHE_*`` defaults.
-"""
+"""Shared ccache policy for build backends: env-knob parsing, binary
+resolution, and default ``CCACHE_*`` values."""
 
 from __future__ import annotations
 
@@ -50,12 +42,8 @@ def parse_enable_env(name: str) -> bool | None:
 def resolve_ccache_path() -> str | None:
     """The ccache binary to wrap compiles with, or None when disabled.
 
-    Shared policy for every backend: on by default when a runnable ccache is
-    on PATH, ``ESPHOME_CCACHE_ENABLE=0`` opts out, and an explicit ``=1``
-    warns when no binary is found and skips the runnability probe;
-    any other value warns and is treated as unset. The
-    Windows extended-length prefix is stripped before probing so the probe
-    validates the exact string the build will execute (#18399).
+    An explicit ``ESPHOME_CCACHE_ENABLE=1`` skips the runnability probe; the
+    Windows extended-length prefix is stripped before probing (#18399).
     """
     import shutil
 
@@ -85,9 +73,8 @@ def ccache_defaults_env(cache_dir: Path) -> dict[str, str]:
     """
     from esphome.core import CORE
 
-    # build_path is set during preload for every config-loading command; unset
-    # means the caller built the environment too early. Fail loudly rather
-    # than silently drop CCACHE_BASEDIR (losing cross-device cache hits).
+    # An unset build_path means the env was built before preload; fail loudly
+    # rather than silently drop CCACHE_BASEDIR.
     if CORE.build_path is None:
         raise ValueError(
             "CORE.build_path must be set before constructing the build environment"

esphome/build_helpers/ninja.py

@@ -25,11 +25,7 @@ def _ninja_runs(binary: str) -> bool:
 
 def find_ninja() -> Path:
     """Locate the ninja binary: a runnable PATH hit first, else the ninja
-    PyPI wheel.
-
-    The wheel is a requirements.txt dependency, so pip has already
-    integrity-checked it; no download logic is needed here.
-    """
+    PyPI wheel."""
     if binary := shutil.which("ninja"):
         binary = strip_win_long_path_prefix(binary)
         if _ninja_runs(binary):
@@ -58,13 +54,9 @@ def escape(value: Path | str) -> str:
 
 
 def quote_arg(tok: str) -> str:
-    """Wrap a token in double quotes with the Windows argv rule.
-
-    Same escaping rule as ``subprocess.list2cmdline``: a backslash run
-    doubles only immediately before a quote (or the closing quote), and the
-    quote itself is escaped. CreateProcess-only; POSIX sh collapses
-    backslash runs inside double quotes, so shell_token single-quotes
-    there instead. ``$`` must already be doubled for ninja.
+    """Quote with the CreateProcess argv rule (as ``subprocess.list2cmdline``):
+    backslash runs double only before a quote. Windows-only; ``$`` must
+    already be doubled for ninja.
     """
     quoted = re.sub(r'(\\*)"', lambda m: m.group(1) * 2 + '\\"', tok)
     quoted = re.sub(r"(\\+)\Z", lambda m: m.group(1) * 2, quoted)
@@ -78,16 +70,11 @@ _NEEDS_QUOTE = re.compile(r"[^\w@%+=:,./-]")
 
 
 def shell_token(tok: str, force: bool = False) -> str:
-    """Quote a lexed token only when needed; ``force`` always quotes.
+    """Re-quote a lexed token for the platform shell; ``force`` always quotes.
 
-    Lexing strips the quoting a user wrote (``-DX="a b"`` becomes the single
-    token ``-DX=a b``); re-quote on the way out so the compiler receives the
-    same argv element SCons would pass under PlatformIO. Ninja hands POSIX
-    commands to ``/bin/sh -c`` and Windows commands to CreateProcess, so the
-    quoting style is chosen per platform: single quotes on POSIX (sh expands
-    nothing inside them, matching SCons's no-shell spawn) and the argv rule
-    on Windows. ``$`` is doubled first in either case because ninja expands
-    ``$`` before the command reaches the shell.
+    Single quotes on POSIX (/bin/sh), the argv rule on Windows
+    (CreateProcess). ``$`` is doubled first because ninja expands it before
+    the command reaches the shell.
     """
     tok = tok.replace("$", "$$")  # ninja would expand a bare $ to nothing
     if not (force or not tok or _NEEDS_QUOTE.search(tok)):

esphome/espidf/framework.py

@@ -1155,11 +1155,8 @@ def _ccache_env() -> dict[str, str]:
     Only values the user has not already set in the environment are returned, so
     a custom ``CCACHE_DIR`` / ``CCACHE_MAXSIZE`` / etc. is respected.
     """
-    # Honor an explicit choice already in the environment (opt-out or opt-in).
-    # IDF_CCACHE_ENABLE (this backend's native knob) wins over the shared
-    # ESPHOME_CCACHE_ENABLE, which resolve_ccache_path parses; without it a
-    # user disabling ccache to debug a miscompile would silently keep it
-    # enabled here.
+    # IDF_CCACHE_ENABLE (the backend-native knob) wins over the shared
+    # ESPHOME_CCACHE_ENABLE.
     idf_knob = parse_enable_env("IDF_CCACHE_ENABLE")
     if idf_knob is False:
         return {}
@@ -1167,8 +1164,6 @@ def _ccache_env() -> dict[str, str]:
         # ESP-IDF silently skips ccache without the binary; don't enable it.
         return {}
 
-    # ccache is enabled past here; the shared helper carries the CCACHE_*
-    # policy (and the fail-loud build_path guard).
     env = ccache_defaults_env(get_idf_tools_path() / "ccache")
     if idf_knob is None:
         # An unparsable IDF_CCACHE_ENABLE must not leak to idf.py as truthy

esphome/framework_helpers.py

@@ -199,10 +199,8 @@ def run_command(
 def tool_version_runs(binary: str, warning: str) -> bool:
     """Probe ``binary --version``; on failure warn with ``warning`` % binary.
 
-    ``shutil.which`` proves existence, not runnability: on Windows it also
-    matches ``.bat``/``.cmd`` wrappers and stale package-manager shims whose
-    target is gone. Callers probe once and fall back instead of failing
-    every build step with an opaque OS error.
+    ``shutil.which`` proves existence, not runnability (Windows .bat/.cmd
+    shims, stale package-manager shims).
     """
     try:
         subprocess.run(

esphome/platformio/registry.py

@@ -1,9 +1,5 @@
-"""Install packages from the PlatformIO registry without PlatformIO.
-
-Native toolchains install the exact registry packages the PlatformIO backend
-uses, so the bits are identical, but resolve and verify them with esphome's
-own download machinery instead of importing the platformio package.
-"""
+"""Install packages from the PlatformIO registry without importing the
+platformio package (identical bits, esphome's own download machinery)."""
 
 from __future__ import annotations
 
@@ -33,11 +29,9 @@ _REGISTRY_URL = (
 def get_systype() -> str:
     """The registry system tag for the current host.
 
-    A transliteration of ``platformio.util.get_systype()``, honoring the same
-    ``PLATFORMIO_SYSTEM_TYPE`` override, so this module never imports the
-    platformio package. One deviation: windows-arm64 maps straight to
-    ``windows_amd64``: the registry ships no arm64 toolchains and those hosts
-    run x86 binaries via emulation, which upstream leaves to the override.
+    Transliterates ``platformio.util.get_systype()`` (same
+    ``PLATFORMIO_SYSTEM_TYPE`` override). Deviation: windows-arm64 maps to
+    ``windows_amd64`` (no arm64 toolchains; x86 emulation).
     """
     if systype := os.environ.get("PLATFORMIO_SYSTEM_TYPE"):
         return systype
@@ -100,10 +94,8 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None]
                     f"Unexpected package registry response for {package}: "
                     f"{str(ver)[:200]}"
                 )
-            # Only a MISSING key means "any system"; an explicitly empty
-            # list must not match (a wrong-architecture download would be
-            # cached as a good install). A bare string would make ``in`` a
-            # substring test.
+            # Only a missing key means "any system"; an empty list must not
+            # match, and a bare string would make ``in`` a substring test.
             systems = file.get("system")
             if systems is None:
                 systems = ["*"]
@@ -138,12 +130,8 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None]
 
 
 def _check_layout(name: str, dest: Path, expect: Collection[str]) -> None:
-    """Raise when an install tree is missing an expected directory.
-
-    Runs on fresh extracts and on marker hits: a marked tree that later
-    lost files (manual deletion, antivirus quarantine) must fail by name
-    instead of surfacing as an opaque toolchain error.
-    """
+    """Raise when an install tree is missing an expected directory (runs on
+    fresh extracts and on marker hits)."""
     for rel in expect:
         if not (dest / rel).is_dir():
             raise EsphomeError(
@@ -177,21 +165,16 @@ def install_package(
         return
     from filelock import FileLock
 
-    # The cache is machine-global; serialize concurrent cold builds so one
-    # process cannot wipe the directory another is extracting into (same
-    # filelock pattern as platformio/toolchain.py and git.py).
+    # Serialize concurrent cold builds (same filelock pattern as git.py).
     dest.parent.mkdir(parents=True, exist_ok=True)
-    # fallback_to_soft would silently degrade to an existence lock on a
-    # flock-less filesystem; a hard-killed run would then hang every later
-    # build forever (same hazard git.py documents).
+    # A soft-lock fallback would turn a hard-killed run into a permanent
+    # hang (see git.py).
     with FileLock(f"{dest}.lock", fallback_to_soft=False):
         if marker.is_file():
             # Another process finished the install while we waited
             return
         rmdir(dest, msg=f"Clean up incomplete {name} install")
-        # A persistent download location (not a temp dir) so an interrupted
-        # download resumes across esphome runs via download_with_resume's
-        # .part file, mirroring the espidf dist/ convention.
+        # Persistent location so an interrupted download resumes across runs.
         downloads_dir.mkdir(parents=True, exist_ok=True)
         archive = downloads_dir / f"{name}-{version}"
         _LOGGER.info("Downloading %s %s ...", name, version)

tests/unit_tests/build_helpers/test_ninja.py

@@ -4,6 +4,7 @@ from __future__ import annotations
 
 import os
 from pathlib import Path
+import subprocess
 import sys
 from unittest.mock import MagicMock, patch
 
@@ -84,7 +85,6 @@ def test_shell_token_quotes_shell_metacharacters() -> None:
 def test_shell_token_posix_roundtrips_through_sh() -> None:
     """Backslash runs, $, backticks, and quotes must reach the compiler
     exactly as lexed once ninja un-doubles $$ and /bin/sh strips quotes."""
-    import subprocess
 
     if sys.platform == "win32":
         pytest.skip("POSIX sh quoting")

tests/unit_tests/test_espidf_framework.py

@@ -1395,8 +1395,6 @@ def test_get_framework_env_without_python_env_uses_os_path(tmp_path: Path) -> No
 
 def _ccache_patches(tmp_path: Path, which: str | None, build_path: Path | None):
     return (
-        # The gate defers to the shared resolver (which carries the PATH
-        # lookup, ESPHOME_CCACHE_ENABLE parse, and runnability probe)
         patch("esphome.espidf.framework.resolve_ccache_path", return_value=which),
         patch(
             "esphome.espidf.framework.get_idf_tools_path",