diff --git a/.github/actions/cache-arduino8266/action.yml b/.github/actions/cache-arduino8266/action.yml index b23db96769..affe62b5aa 100644 --- a/.github/actions/cache-arduino8266/action.yml +++ b/.github/actions/cache-arduino8266/action.yml @@ -17,9 +17,9 @@ runs: id: version shell: bash run: | - # One owner for the install prefix: exporting it here (instead of a - # per-job env stanza) makes it impossible for a caller to install - # into a path other than the one cached below. + # One owner for the install prefix: exported here and referenced by + # the cache steps below via env, so the caller's install and the + # cached path cannot diverge. echo "ESPHOME_ARDUINO8266_PREFIX=$HOME/.esphome-arduino8266" >> "$GITHUB_ENV" . venv/bin/activate key=$(python -c 'from esphome.components.esp8266 import RECOMMENDED_ARDUINO_FRAMEWORK_VERSION as f; from esphome.arduino8266.framework import TOOLCHAIN_VERSION as t; print(f"{f}-{t}")') @@ -29,11 +29,11 @@ runs: if: github.ref == 'refs/heads/dev' uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: - path: ~/.esphome-arduino8266 + path: ${{ env.ESPHOME_ARDUINO8266_PREFIX }} key: ${{ runner.os }}-esp8266-native-${{ steps.version.outputs.key }} - name: Restore the native toolchain (off dev) if: github.ref != 'refs/heads/dev' uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: - path: ~/.esphome-arduino8266 + path: ${{ env.ESPHOME_ARDUINO8266_PREFIX }} key: ${{ runner.os }}-esp8266-native-${{ steps.version.outputs.key }} diff --git a/esphome/arduino8266/toolchain.py b/esphome/arduino8266/toolchain.py index dec3e7caf7..0d7fe51622 100644 --- a/esphome/arduino8266/toolchain.py +++ b/esphome/arduino8266/toolchain.py @@ -116,6 +116,11 @@ def run_compile(config: ConfigType, verbose: bool) -> int: cmd.append("-v") if jobs := config[CONF_ESPHOME].get(CONF_COMPILE_PROCESS_LIMIT): cmd += ["-j", str(jobs)] + # Explicit targets, not the default statement: a generator defect that + # drops them fails loudly with "unknown target" instead of a green + # no-op run that leaves stale artifacts in place + targets = ["firmware.factory.bin", "firmware.ota.bin"] + cmd += targets # A dry-run probe keeps a no-op rebuild quiet: ninja would only print # "no work to do". A freshly rewritten manifest all but guarantees work, @@ -124,7 +129,7 @@ def run_compile(config: ConfigType, verbose: bool) -> int: skip_build = False if not ninja_changed: probe = subprocess.run( - [str(paths.ninja), "-n"], + [str(paths.ninja), "-n", *targets], cwd=build_dir, env=env, capture_output=True, @@ -137,6 +142,10 @@ def run_compile(config: ConfigType, verbose: bool) -> int: # flags a generator bug; the skip branch would otherwise # swallow it forever _LOGGER.warning("ninja: %s", probe.stderr.strip()) + if probe.returncode != 0: + # An unknown target here is the defective-manifest case; fall + # through to the real build so the error prints attributably + _LOGGER.debug("ninja probe failed; running the full build") skip_build = probe.returncode == 0 and "no work to do" in probe.stdout if skip_build: _LOGGER.debug("ninja: nothing to rebuild") @@ -148,29 +157,19 @@ def run_compile(config: ConfigType, verbose: bool) -> int: if rc != 0: 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); - # the factory/ota copies are what upload and OTA actually consume + # ninja already refused a manifest missing the explicit targets above; + # existence covers the remaining hole (a rule that ran but wrote + # elsewhere). The factory/ota copies are what upload and OTA consume. build_dir_artifacts = ( get_elf_path(), build_dir / "firmware.bin", get_factory_firmware_path(), build_dir / "firmware.ota.bin", ) - ninja_mtime = ninja_file.stat().st_mtime for artifact in build_dir_artifacts: if not artifact.is_file(): _LOGGER.error("Build produced no %s", artifact) return 1 - if artifact.stat().st_mtime < ninja_mtime: - # A leftover from an older manifest must not pass as this - # build's output (a defective manifest with no default targets - # reports "no work to do" while building nothing) - _LOGGER.error( - "%s is older than build.ninja; run 'esphome clean-all' and retry", - artifact, - ) - return 1 if not _print_size_summary(build_dir, paths): # The cause was already warned; name the consequence so a build diff --git a/esphome/components/esp8266/__init__.py b/esphome/components/esp8266/__init__.py index 97b9ad8acd..7422bcd9b2 100644 --- a/esphome/components/esp8266/__init__.py +++ b/esphome/components/esp8266/__init__.py @@ -682,7 +682,9 @@ def _decode_pc(config: ConfigType, addr: str) -> None: return if "?? ??:0" in translation: - # Nothing useful + # A stale or mismatched ELF: mark it, or the frame silently reads + # as merely unmappable + _LOGGER.warning("Not decoded %s (address not in %s)", addr, elf) return translation = translation.replace(" at ??:?", "").replace(":?", "") _LOGGER.warning("Decoded %s", translation) diff --git a/esphome/writer.py b/esphome/writer.py index 37114f84e5..435c4804f1 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -612,12 +612,15 @@ def clean_build(clear_pio_cache: bool = True, *, full: bool = False): # The idedata caches are derived from the build but live under the data # dir, not the build path, so they must be removed separately in both - # modes (the .arduino.json variant is the native esp8266 toolchain's). - for cache_name in (f"{CORE.name}.json", f"{CORE.name}.arduino.json"): - idedata_cache = CORE.relative_internal_path("idedata", cache_name) - if idedata_cache.is_file(): - _LOGGER.info("Deleting %s", idedata_cache) - idedata_cache.unlink() + # modes. Globbed (name.json plus name..json) so a future + # backend suffix cannot silently drift out of clean-all. + idedata_dir = CORE.relative_internal_path("idedata") + for idedata_cache in ( + *idedata_dir.glob(f"{CORE.name}.json"), + *idedata_dir.glob(f"{CORE.name}.*.json"), + ): + _LOGGER.info("Deleting %s", idedata_cache) + idedata_cache.unlink() if not clear_pio_cache: return diff --git a/script/test_build_components.py b/script/test_build_components.py index 28026d5775..d50e9f5847 100755 --- a/script/test_build_components.py +++ b/script/test_build_components.py @@ -1062,17 +1062,19 @@ def test_components( # toolchain build. include_validate = esphome_command != "compile" - # Find all component tests + # Find all component tests; remember which patterns (wildcards + # included) matched anything, for the deferred no-tests accounting all_tests = {} + pattern_hits: dict[str, bool] = {} for pattern in component_patterns: # Skip empty patterns (happens when components list is empty string) if not pattern: continue - all_tests.update( - find_component_tests( - tests_dir, pattern, base_only, include_validate=include_validate - ) + found = find_component_tests( + tests_dir, pattern, base_only, include_validate=include_validate ) + pattern_hits[pattern] = bool(found) + all_tests.update(found) # The flag's contract is "no test matched fails": a fully blank pattern # list would otherwise slide into the reference-baseline fallback and @@ -1081,39 +1083,6 @@ def test_components( print("No components requested (blank component list)") return 1 - # Renamed or removed fixtures must shrink coverage loudly, and the - # reference-baseline fallback below must not mask an empty match. The - # check is per platform: a component whose fixture exists only for other - # platforms contributes nothing to this leg. - def _has_platform_test(component: str) -> bool: - # "all" fixtures build on every platform (mirrors the run loop) - return any( - not platform_filter - or (suffix := test.stem.split(".")[-1]) == "all" - or suffix.startswith(platform_filter) - for test in all_tests.get(component, []) - ) - - if fail_on_no_tests and ( - unmatched := [ - p - for p in component_patterns - if p and "*" not in p and not _has_platform_test(p) - ] - ): - target = f"{platform_filter} " if platform_filter else "" - print( - f"No {target}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 fail_on_no_tests and not all_tests: - # The synthetic baseline would report success having built nothing - # the caller asked for (e.g. a wildcard that matched no component) - print(f"No components found matching: {component_patterns}") - return 1 if not all_tests: print(f"No components found matching: {component_patterns}") print( @@ -1219,21 +1188,21 @@ def test_components( silent: list[str] = [] if fail_on_no_tests: - # A green run that built nothing for a requested component (renamed - # fixture, missing base file, version-suffix mismatch) must not pass - # CI. Per component: an all-or-nothing check would let one silent - # component hide behind the others. Opt-in: some legs (the esp32-ard - # smoke subset) legitimately match nothing. Failing is deferred past - # the summary so a real failure's reproduce commands still print. + # A green run that built nothing for a requested pattern (renamed + # fixture, missing base file, version-suffix mismatch, a wildcard + # matching no component) must not pass CI. Per pattern: an + # all-or-nothing check would let one silent pattern hide behind the + # others. Opt-in: some legs (the esp32-ard smoke subset) + # legitimately match nothing. Failing is deferred past the summary + # so a real failure's reproduce commands still print. built = {c for r in test_results for c in r.components} silent = [ - p for p in component_patterns if p and "*" not in p and p not in built + p + for p in component_patterns + if p and (not pattern_hits.get(p) or ("*" not in p and p not in built)) ] if silent: - print(f"No tests ran for requested component(s): {', '.join(silent)}") - elif not test_results: - print("No tests matched the requested components/platform") - return 1 + print(f"No tests ran for requested pattern(s): {', '.join(silent)}") # Separate results into passed and failed passed_results = [r for r in test_results if r.success] diff --git a/tests/script/test_test_build_components.py b/tests/script/test_test_build_components.py index 3f19935e0b..214c1e1273 100644 --- a/tests/script/test_test_build_components.py +++ b/tests/script/test_test_build_components.py @@ -245,10 +245,15 @@ def test_components_empty_match_fails_with_flag( 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 + ["logger"], + "zz-none", + "compile", + False, + enable_grouping=False, + fail_on_no_tests=True, ) assert rc == 1 - assert "No zz-none tests found" in capsys.readouterr().out + assert "No tests ran for requested pattern(s): logger" in (capsys.readouterr().out) def test_components_component_with_no_base_file_fails_with_flag( @@ -268,9 +273,7 @@ def test_components_component_with_no_base_file_fails_with_flag( fail_on_no_tests=True, ) assert rc == 1 - assert "No tests ran for requested component(s): logger" in ( - capsys.readouterr().out - ) + assert "No tests ran for requested pattern(s): logger" in (capsys.readouterr().out) def test_components_blank_list_fails_with_flag( @@ -291,10 +294,17 @@ def test_components_wildcard_no_match_fails_with_flag( """A wildcard matching nothing must not degrade to the synthetic baseline build and exit green under the flag.""" rc = tbc.test_components( - ["zz_no_such*"], "esp8266-ard", "compile", False, fail_on_no_tests=True + ["zz_no_such*"], + "esp8266-ard", + "compile", + False, + enable_grouping=False, + fail_on_no_tests=True, ) assert rc == 1 - assert "No components found matching" in capsys.readouterr().out + assert "No tests ran for requested pattern(s): zz_no_such*" in ( + capsys.readouterr().out + ) def test_components_empty_match_tolerated_without_flag() -> None: @@ -318,10 +328,10 @@ def test_components_unknown_component_fails_with_flag( "esp8266-ard", "compile", False, + enable_grouping=False, fail_on_no_tests=True, ) assert rc == 1 - assert ( - "No esp8266-ard tests found for requested component(s)" - in capsys.readouterr().out + assert "No tests ran for requested pattern(s): no_such_component_xyz" in ( + capsys.readouterr().out ) diff --git a/tests/unit_tests/test_arduino8266_toolchain.py b/tests/unit_tests/test_arduino8266_toolchain.py index a5c4df6c14..4ceeba670b 100644 --- a/tests/unit_tests/test_arduino8266_toolchain.py +++ b/tests/unit_tests/test_arduino8266_toolchain.py @@ -53,18 +53,6 @@ def _setup_core(tmp_path: Path) -> None: (build_dir / artifact).write_bytes(b"") -def _touch_artifacts(build_dir: Path) -> None: - """Re-date the fixture artifacts after a test rewrites build.ninja, so - the freshness guard sees them as this manifest's outputs.""" - for artifact in ( - "firmware.elf", - "firmware.bin", - "firmware.factory.bin", - "firmware.ota.bin", - ): - (build_dir / artifact).touch() - - def _paths(tmp_path: Path) -> framework.InstalledPaths: return framework.InstalledPaths( framework=tmp_path / "framework", @@ -122,9 +110,11 @@ def test_run_compile_success(tmp_path: Path) -> None: assert rc == 0 # The -n probe runs first, then the real build (cwd, no -C banner) ninja_calls = [c for c in mock_run.call_args_list if "ninja" in str(c[0][0][0])] - assert ninja_calls[0][0][0][-1] == "-n" + assert "-n" in ninja_calls[0][0][0] + # Explicit targets: a manifest missing them fails as "unknown target" + assert ninja_calls[0][0][0][-1] == "firmware.ota.bin" cmd = ninja_calls[1][0][0] - assert cmd[-2:] == ["-j", "4"] + assert cmd[-4:] == ["-j", "4", "firmware.factory.bin", "firmware.ota.bin"] assert "-C" not in cmd assert ninja_calls[1][1]["cwd"] is not None mock_compdb.assert_called_once() @@ -155,7 +145,9 @@ def test_run_compile_noop_skips_the_build_spawn(tmp_path: Path) -> None: assert rc == 0 ninja_calls = [c for c in mock_run.call_args_list if "ninja" in str(c[0][0][0])] assert len(ninja_calls) == 1 - assert ninja_calls[0][0][0][-1] == "-n" + assert "-n" in ninja_calls[0][0][0] + # Explicit targets: a manifest missing them fails as "unknown target" + assert ninja_calls[0][0][0][-1] == "firmware.ota.bin" def test_run_compile_regenerates_stale_compdb(tmp_path: Path) -> None: @@ -164,7 +156,6 @@ def test_run_compile_regenerates_stale_compdb(tmp_path: Path) -> None: build_dir = toolchain.get_build_dir() build_dir.mkdir(parents=True, exist_ok=True) (build_dir / "build.ninja").write_text("") - _touch_artifacts(build_dir) compdb = build_dir / "compile_commands.json" compdb.write_text("[]") os.utime(compdb, ((build_dir / "build.ninja").stat().st_mtime - 5,) * 2) @@ -437,7 +428,6 @@ def test_run_compile_skips_compdb_when_ninja_unchanged(tmp_path: Path) -> None: build_dir.mkdir(parents=True, exist_ok=True) # write_project (stubbed below) always leaves a build.ninja behind (build_dir / "build.ninja").write_text("# manifest") - _touch_artifacts(build_dir) def run(regenerate_expected: bool) -> None: with ( @@ -587,31 +577,6 @@ def test_run_compile_skipped_size_summary_names_consequence( assert "Firmware size summary unavailable for this build" in caplog.text -def test_run_compile_stale_artifact_fails( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: - """An artifact older than build.ninja is a leftover, not this build's - output; a defective manifest with no default targets must not pass on - it.""" - build_dir = toolchain.get_build_dir() - os.utime(build_dir / "firmware.bin", (0, 0)) - 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="", stderr=""), - ), - patch.object(toolchain, "_write_compile_commands"), - patch.object(toolchain, "_print_size_summary", return_value=True), - patch.object(toolchain, "get_idedata", return_value={}), - ): - assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == 1 - assert "older than build.ninja" in caplog.text - - def test_parse_app_size_non_utf8_ld_warns( tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: