From 24ea73a626731f64c40d21385017d2cfd48a0dcc Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 21:47:03 -0500 Subject: [PATCH] Gate the zero-test guard behind a flag, keep the upload path import-free, verify all artifacts, regenerate a stale compile DB --- .github/workflows/ci.yml | 4 +- esphome/__main__.py | 6 +++ esphome/arduino8266/toolchain.py | 24 ++++++++++-- script/test_build_components.py | 23 ++++++++++- tests/script/test_test_build_components.py | 36 ++++++++++++++--- .../unit_tests/test_arduino8266_toolchain.py | 39 ++++++++++++++++++- 6 files changed, 117 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8f9e5dabab..d3dfe9b209 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1149,7 +1149,7 @@ jobs: # compile validates config first, so a separate config pass is # redundant for this smoke test. ESP-IDF framework via PlatformIO: - python3 script/test_build_components.py -e compile -t esp32-idf -c "$TEST_COMPONENTS" -f --toolchain platformio + python3 script/test_build_components.py -e compile -t esp32-idf -c "$TEST_COMPONENTS" -f --toolchain platformio --fail-on-no-tests echo "" echo "ESP-IDF-via-PlatformIO build passed! Starting Arduino smoke test..." @@ -1191,7 +1191,7 @@ jobs: # ESP8266 Arduino built directly (no PlatformIO); compile validates # config first, so a separate config pass is redundant. - python3 script/test_build_components.py -e compile -t esp8266-ard -c "$TEST_COMPONENTS" -f --toolchain arduino + python3 script/test_build_components.py -e compile -t esp8266-ard -c "$TEST_COMPONENTS" -f --toolchain arduino --fail-on-no-tests device-builder: name: Test downstream esphome/device-builder diff --git a/esphome/__main__.py b/esphome/__main__.py index 536abbb86a..43b205dd53 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -1939,6 +1939,12 @@ def command_update_all(args: ArgsProtocol) -> int | None: def _native_toolchain_module(): """The native build backend module for the resolved toolchain, via the target platform's ``native_toolchain_module`` hook.""" + if not CORE.using_native_toolchain: + # Both hooks return None for PlatformIO anyway; returning early + # keeps the serial upload/logs fast path from importing the + # platform component package (see the esp32 variant comment in + # upload_using_esptool) + return None module = importlib.import_module("esphome.components." + CORE.target_platform) get_native = getattr(module, "native_toolchain_module", None) native = get_native() if get_native is not None else None diff --git a/esphome/arduino8266/toolchain.py b/esphome/arduino8266/toolchain.py index fdc5fc239a..d7d228d74f 100644 --- a/esphome/arduino8266/toolchain.py +++ b/esphome/arduino8266/toolchain.py @@ -93,8 +93,18 @@ def run_compile(config: ConfigType, verbose: bool) -> int: env = framework.get_build_env(paths.toolchain, ccache) # Regenerate the compile DB before the build (a pure function of - # build.ninja); skip when unchanged. - if ninja_changed or not (build_dir / "compile_commands.json").is_file(): + # build.ninja); skip only when it is at least as fresh as build.ninja + # (an interrupted previous run may have rewritten the manifest without + # regenerating the DB). + compdb = build_dir / "compile_commands.json" + ninja_file = build_dir / "build.ninja" + if ( + ninja_changed + or not compdb.is_file() + or ( + ninja_file.is_file() and compdb.stat().st_mtime < ninja_file.stat().st_mtime + ) + ): _write_compile_commands(paths.ninja, build_dir, env) cmd = [str(paths.ninja)] @@ -135,8 +145,14 @@ def run_compile(config: ConfigType, verbose: bool) -> int: return rc # A generator defect emitting no default targets must not turn into a - # green build with no firmware (size summary and idedata only warn) - build_dir_artifacts = (get_elf_path(), get_build_dir() / "firmware.bin") + # green build with no firmware (size summary and idedata only warn); + # the factory/ota copies are what upload and OTA actually consume + build_dir_artifacts = ( + get_elf_path(), + get_build_dir() / "firmware.bin", + get_factory_firmware_path(), + get_build_dir() / "firmware.ota.bin", + ) for artifact in build_dir_artifacts: if not artifact.is_file(): _LOGGER.error("Build produced no %s", artifact) diff --git a/script/test_build_components.py b/script/test_build_components.py index 0e3281fd8c..e4911ab1d6 100755 --- a/script/test_build_components.py +++ b/script/test_build_components.py @@ -1027,6 +1027,7 @@ def test_components( isolated_components: set[str] | None = None, base_only: bool = False, toolchain: str | None = None, + fail_on_no_tests: bool = False, ) -> int: """Test components with optional intelligent grouping. @@ -1073,6 +1074,16 @@ def test_components( ) ) + # Renamed or removed fixtures must shrink coverage loudly, and the + # reference-baseline fallback below must not mask an empty match + if fail_on_no_tests and ( + unmatched := [ + p for p in component_patterns if p and "*" not in p and p not in all_tests + ] + ): + print(f"No tests found for requested component(s): {', '.join(unmatched)}") + return 1 + # If no components found, build a reference configuration for baseline comparison # Create a synthetic "empty" component test that will build just the base config if not all_tests: @@ -1178,9 +1189,10 @@ def test_components( toolchain=toolchain, ) - if not test_results: + if fail_on_no_tests and not test_results: # A green run that compiled nothing (renamed/removed test fixture, - # bad platform filter) must not pass CI + # bad platform filter) must not pass CI. Opt-in: some legs (the + # esp32-ard smoke subset) legitimately match nothing. print("No tests matched the requested components/platform") return 1 @@ -1270,6 +1282,12 @@ def main() -> int: "--toolchain", help="Select toolchain for compiling.", ) + parser.add_argument( + "--fail-on-no-tests", + action="store_true", + help="Exit non-zero when no test matched (for CI legs whose " + "components must all have fixtures)", + ) args = parser.parse_args() @@ -1288,6 +1306,7 @@ def main() -> int: continue_on_fail=args.continue_on_fail, enable_grouping=not args.no_grouping, isolated_components=isolated_components, + fail_on_no_tests=args.fail_on_no_tests, base_only=args.base_only, toolchain=args.toolchain, ) diff --git a/tests/script/test_test_build_components.py b/tests/script/test_test_build_components.py index 502d888c12..efd4a9e41d 100644 --- a/tests/script/test_test_build_components.py +++ b/tests/script/test_test_build_components.py @@ -238,10 +238,36 @@ def test_run_grouped_test_closes_group_when_subprocess_raises( assert "::endgroup::" in capsys.readouterr().out -def test_components_empty_match_fails(capsys: pytest.CaptureFixture[str]) -> None: - """A real component filtered to a platform with no matching test file - must not pass CI as a green zero-component compile. (An unknown - component name instead builds the reference baseline.)""" - rc = tbc.test_components(["logger"], "zz-none", "compile", False) +def test_components_empty_match_fails_with_flag( + capsys: pytest.CaptureFixture[str], +) -> None: + """Under --fail-on-no-tests, a real component filtered to a platform + with no matching test file must not pass CI as a green zero-component + compile.""" + rc = tbc.test_components( + ["logger"], "zz-none", "compile", False, fail_on_no_tests=True + ) assert rc == 1 assert "No tests matched" in capsys.readouterr().out + + +def test_components_empty_match_tolerated_without_flag() -> None: + """The esp32-ard smoke leg deliberately builds only the subset with a + matching fixture; without the flag an empty match stays green.""" + assert tbc.test_components(["logger"], "zz-none", "compile", False) == 0 + + +def test_components_unknown_component_fails_with_flag( + capsys: pytest.CaptureFixture[str], +) -> None: + """A renamed smoke-test component must shrink coverage loudly, not fall + into the reference-baseline build.""" + rc = tbc.test_components( + ["no_such_component_xyz"], + "esp8266-ard", + "compile", + False, + fail_on_no_tests=True, + ) + assert rc == 1 + assert "No tests found for requested component(s)" in capsys.readouterr().out diff --git a/tests/unit_tests/test_arduino8266_toolchain.py b/tests/unit_tests/test_arduino8266_toolchain.py index dd456b8ec7..0cf5962559 100644 --- a/tests/unit_tests/test_arduino8266_toolchain.py +++ b/tests/unit_tests/test_arduino8266_toolchain.py @@ -42,8 +42,13 @@ def _setup_core(tmp_path: Path) -> None: # that "produced" them (tests for the guard delete them again) build_dir = CORE.relative_pioenvs_path("test8266") build_dir.mkdir(parents=True, exist_ok=True) - (build_dir / "firmware.elf").write_bytes(b"") - (build_dir / "firmware.bin").write_bytes(b"") + for artifact in ( + "firmware.elf", + "firmware.bin", + "firmware.factory.bin", + "firmware.ota.bin", + ): + (build_dir / artifact).write_bytes(b"") def _paths(tmp_path: Path) -> framework.InstalledPaths: @@ -139,6 +144,36 @@ def test_run_compile_noop_skips_the_build_spawn(tmp_path: Path) -> None: assert ninja_calls[0][0][0][-1] == "-n" +def test_run_compile_regenerates_stale_compdb(tmp_path: Path) -> None: + """An interrupted run can leave build.ninja newer than the compile DB; + mere existence must not skip regeneration.""" + import os + + build_dir = toolchain.get_build_dir() + build_dir.mkdir(parents=True, exist_ok=True) + (build_dir / "build.ninja").write_text("") + compdb = build_dir / "compile_commands.json" + compdb.write_text("[]") + os.utime(compdb, ((build_dir / "build.ninja").stat().st_mtime - 5,) * 2) + with ( + patch.object(framework, "check_and_install", return_value=_paths(tmp_path)), + patch.object(framework, "get_build_env", return_value={}), + patch("esphome.build_gen.arduino8266.write_project", return_value=False), + patch.object( + toolchain.subprocess, + "run", + return_value=MagicMock( + returncode=0, stdout="ninja: no work to do.\n", stderr="" + ), + ), + patch.object(toolchain, "_write_compile_commands") as mock_compdb, + patch.object(toolchain, "_print_size_summary"), + patch.object(toolchain, "get_idedata"), + ): + assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == 0 + mock_compdb.assert_called_once() + + def test_run_compile_surfaces_probe_diagnostics( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: