From 8a7d2d0ca036f9a78a066136a6babc9674d75229 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:01:15 -0400 Subject: [PATCH] [ci] Enforce list form for platform domains in test fixtures (#17869) --- .github/workflows/ci.yml | 1 + script/ci_check_test_fixture_list_form.py | 103 ++++++++++++++++++ tests/components/esp32/test.esp32-p4-idf.yaml | 2 +- tests/components/espnow/common.yaml | 1 + tests/components/packet_transport/common.yaml | 36 +++--- .../packet_transport/test.host.yaml | 36 +++--- tests/components/syslog/test.host.yaml | 2 +- 7 files changed, 147 insertions(+), 34 deletions(-) create mode 100755 script/ci_check_test_fixture_list_form.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72ece5b4fd..b7e5f31cd8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -117,6 +117,7 @@ jobs: script/generate-esp32-boards.py --check script/generate-rp2-boards.py --check script/ci_check_duplicate_test_ids.py + script/ci_check_test_fixture_list_form.py import-time: name: Check import esphome.__main__ time diff --git a/script/ci_check_test_fixture_list_form.py b/script/ci_check_test_fixture_list_form.py new file mode 100755 index 0000000000..6da1f8337d --- /dev/null +++ b/script/ci_check_test_fixture_list_form.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +"""Fail when a test fixture writes a platform-list domain as a single dict. + +Component tests are merged and built in groups in CI (see +``script/merge_component_configs.py``). ESPHome's ``merge_config`` concatenates +two lists, but when one side is a dict it replaces the other side wholesale +(``esphome/config_helpers.py``). A domain such as ``one_wire:`` or ``ota:`` +written in single-dict form therefore deletes every entry other components +contributed to that domain before it in the merge, and is itself deleted by any +list that merges after it. The resulting failure only appears when the affected +components land in the same group -- usually a full component matrix run on an +unrelated PR long after the fixture was written (this is what broke the +dallas_temp tests when ds2484 was added, see #17868). + +This guard scans every fixture under ``tests/components/`` and rejects any +top-level domain written as a dict with a ``platform`` key. Such a domain is by +definition a platform list (single-dict form is only user-config sugar), so the +fix is always to write it as a one-element list: + + one_wire: + - platform: gpio + pin: 4 +""" + +from __future__ import annotations + +from pathlib import Path +import sys + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from esphome.core import EsphomeError # noqa: E402 +from script.analyze_component_buses import ISOLATED_COMPONENTS # noqa: E402 +from script.merge_component_configs import load_yaml_file # noqa: E402 + +# Resolved relative to this file (not the CWD) so the scan cannot silently cover +# nothing when run from a different directory. +ROOT_DIR = Path(__file__).resolve().parent.parent +TESTS_DIR = ROOT_DIR / "tests" / "components" + + +def main() -> int: + offenders: list[str] = [] + parse_errors: list[str] = [] + fixtures_scanned = 0 + + for fixture in sorted(TESTS_DIR.glob("*/*.yaml")): + # Isolated components are never merged with others, so dict form + # cannot clobber anyone there. + if fixture.parent.name in ISOLATED_COMPONENTS: + continue + try: + data = load_yaml_file(fixture) + except EsphomeError as err: + parse_errors.append(f"{fixture.relative_to(ROOT_DIR)}: {err}") + continue + fixtures_scanned += 1 + if not isinstance(data, dict): + continue + for key, value in data.items(): + if isinstance(value, dict) and "platform" in value: + offenders.append(f"{fixture.relative_to(ROOT_DIR)}: '{key}:'") + + if offenders: + print("Test fixtures with platform domains in single-dict form:\n") + for line in offenders: + print(f" - {line}") + print( + "\nWrite the domain as a one-element list ('- platform: ...') so " + "grouped CI builds can merge it with other components' entries; " + "in dict form it replaces or is replaced by their lists wholesale." + ) + + if parse_errors: + # A fixture we could not parse was never scanned, so the run is not a + # clean pass even if no offenders were found among the rest. + print( + f"\n{len(parse_errors)} test fixture(s) could not be parsed and " + "were not checked:" + ) + for line in parse_errors: + print(f" - {line}") + + if fixtures_scanned == 0: + # A scan that covered nothing is a false green -- the whole point of the + # guard is defeated. Fail loudly (wrong working directory or layout change). + print( + f"\nERROR: scanned 0 test fixtures under {TESTS_DIR}; " + "the guard covered nothing.", + file=sys.stderr, + ) + + if offenders or parse_errors or fixtures_scanned == 0: + return 1 + + print( + f"No single-dict platform domains found ({fixtures_scanned} fixtures scanned)." + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/components/esp32/test.esp32-p4-idf.yaml b/tests/components/esp32/test.esp32-p4-idf.yaml index fd42fac5a3..c16869d06b 100644 --- a/tests/components/esp32/test.esp32-p4-idf.yaml +++ b/tests/components/esp32/test.esp32-p4-idf.yaml @@ -21,7 +21,7 @@ esp32: disable_fatfs: true ota: - platform: esphome + - platform: esphome wifi: ssid: MySSID diff --git a/tests/components/espnow/common.yaml b/tests/components/espnow/common.yaml index ae43baa41a..2f82e794c4 100644 --- a/tests/components/espnow/common.yaml +++ b/tests/components/espnow/common.yaml @@ -74,6 +74,7 @@ sensor: id: espnow_temp_sensor - platform: packet_transport + transport_id: transport1 provider: test-provider remote_id: espnow_temp_sensor id: remote_temp diff --git a/tests/components/packet_transport/common.yaml b/tests/components/packet_transport/common.yaml index 9151cf27dc..5c6c8dd636 100644 --- a/tests/components/packet_transport/common.yaml +++ b/tests/components/packet_transport/common.yaml @@ -7,36 +7,40 @@ udp: addresses: ["239.0.60.53"] packet_transport: - platform: udp - update_interval: 5s - encryption: "our key goes here" - rolling_code_enable: true - ping_pong_enable: true - binary_sensors: - - binary_sensor_id1 - - id: binary_sensor_id1 - broadcast_id: other_id - sensors: - - sensor_id1 - - id: sensor_id1 - broadcast_id: other_id - providers: - - name: some-device-name - encryption: "their key goes here" + - platform: udp + id: transport_udp + update_interval: 5s + encryption: "our key goes here" + rolling_code_enable: true + ping_pong_enable: true + binary_sensors: + - binary_sensor_id1 + - id: binary_sensor_id1 + broadcast_id: other_id + sensors: + - sensor_id1 + - id: sensor_id1 + broadcast_id: other_id + providers: + - name: some-device-name + encryption: "their key goes here" sensor: - platform: template id: sensor_id1 - platform: packet_transport + transport_id: transport_udp provider: some-device-name id: our_id remote_id: some_sensor_id binary_sensor: - platform: packet_transport + transport_id: transport_udp provider: unencrypted-device id: other_binary_sensor_id - platform: packet_transport + transport_id: transport_udp provider: some-device-name type: status name: Some-Device Status diff --git a/tests/components/packet_transport/test.host.yaml b/tests/components/packet_transport/test.host.yaml index 49fdbbc9b2..f67b561226 100644 --- a/tests/components/packet_transport/test.host.yaml +++ b/tests/components/packet_transport/test.host.yaml @@ -3,36 +3,40 @@ udp: addresses: ["239.0.60.53"] packet_transport: - platform: udp - update_interval: 5s - encryption: "our key goes here" - rolling_code_enable: true - ping_pong_enable: true - binary_sensors: - - binary_sensor_id1 - - id: binary_sensor_id1 - broadcast_id: other_id - sensors: - - sensor_id1 - - id: sensor_id1 - broadcast_id: other_id - providers: - - name: some-device-name - encryption: "their key goes here" + - platform: udp + id: transport_udp + update_interval: 5s + encryption: "our key goes here" + rolling_code_enable: true + ping_pong_enable: true + binary_sensors: + - binary_sensor_id1 + - id: binary_sensor_id1 + broadcast_id: other_id + sensors: + - sensor_id1 + - id: sensor_id1 + broadcast_id: other_id + providers: + - name: some-device-name + encryption: "their key goes here" sensor: - platform: template id: sensor_id1 - platform: packet_transport + transport_id: transport_udp provider: some-device-name id: our_id remote_id: some_sensor_id binary_sensor: - platform: packet_transport + transport_id: transport_udp provider: unencrypted-device id: other_binary_sensor_id - platform: packet_transport + transport_id: transport_udp provider: some-device-name type: status name: Some-Device Status diff --git a/tests/components/syslog/test.host.yaml b/tests/components/syslog/test.host.yaml index 31122437d5..d9aa8e529b 100644 --- a/tests/components/syslog/test.host.yaml +++ b/tests/components/syslog/test.host.yaml @@ -2,7 +2,7 @@ udp: addresses: ["239.0.60.53"] time: - platform: host + - platform: host syslog: port: 514