diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad39b3f346..06c6c0fec1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -252,6 +252,8 @@ jobs: python-linters: ${{ steps.determine.outputs.python-linters }} import-time: ${{ steps.determine.outputs.import-time }} device-builder: ${{ steps.determine.outputs.device-builder }} + native-idf: ${{ steps.determine.outputs.native-idf }} + native-idf-components: ${{ steps.determine.outputs.native-idf-components }} changed-components: ${{ steps.determine.outputs.changed-components }} changed-components-with-tests: ${{ steps.determine.outputs.changed-components-with-tests }} directly-changed-components-with-tests: ${{ steps.determine.outputs.directly-changed-components-with-tests }} @@ -297,6 +299,8 @@ jobs: echo "python-linters=$(echo "$output" | jq -r '.python_linters')" >> $GITHUB_OUTPUT echo "import-time=$(echo "$output" | jq -r '.import_time')" >> $GITHUB_OUTPUT echo "device-builder=$(echo "$output" | jq -r '.device_builder')" >> $GITHUB_OUTPUT + echo "native-idf=$(echo "$output" | jq -r '.native_idf')" >> $GITHUB_OUTPUT + echo "native-idf-components=$(echo "$output" | jq -r '.native_idf_components')" >> $GITHUB_OUTPUT echo "changed-components=$(echo "$output" | jq -c '.changed_components')" >> $GITHUB_OUTPUT echo "changed-components-with-tests=$(echo "$output" | jq -c '.changed_components_with_tests')" >> $GITHUB_OUTPUT echo "directly-changed-components-with-tests=$(echo "$output" | jq -c '.directly_changed_components_with_tests')" >> $GITHUB_OUTPUT @@ -823,10 +827,14 @@ jobs: needs: - common - determine-jobs - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && needs.determine-jobs.outputs.native-idf == 'true' env: ESPHOME_ESP_IDF_PREFIX: ~/.esphome-idf - TEST_COMPONENTS: esp32,api,heatpumpir,bme280_i2c,bh1750,aht10,esp32_ble,esp32_ble_beacon,esp32_ble_client,esp32_ble_server,esp32_ble_tracker,ble_client,ble_presence,ble_rssi,ble_scanner + # Comma-joined subset of the native-IDF representative component list, + # computed by script/determine-jobs.py (native_idf_components_to_test). + # Single source of truth -- the full list lives in + # script/determine-jobs.py::NATIVE_IDF_TEST_COMPONENTS. + TEST_COMPONENTS: ${{ needs.determine-jobs.outputs.native-idf-components }} steps: - name: Check out code from GitHub uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 diff --git a/script/determine-jobs.py b/script/determine-jobs.py index 57b3c6eb88..0a55b2a848 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -495,6 +495,125 @@ def should_run_device_builder(branch: str | None = None) -> bool: return False +# Components tested by the native ESP-IDF compile-test job. This is the +# single source of truth: the workflow reads the comma-joined list from the +# `native-idf-components` output of `determine-jobs` and uses it as the +# `TEST_COMPONENTS` env on the `test-native-idf` job. +NATIVE_IDF_TEST_COMPONENTS = frozenset( + { + "esp32", + "api", + "heatpumpir", + "bme280_i2c", + "bh1750", + "aht10", + "esp32_ble", + "esp32_ble_beacon", + "esp32_ble_client", + "esp32_ble_server", + "esp32_ble_tracker", + "ble_client", + "ble_presence", + "ble_rssi", + "ble_scanner", + } +) + +# Path prefixes whose changes always trigger the native ESP-IDF compile +# test: anything under esphome/espidf/ (the native IDF runner / API / +# framework / component generator). +NATIVE_IDF_TRIGGER_PATH_PREFIXES = ("esphome/espidf/",) + +# Standalone files that, when changed, also trigger the native ESP-IDF +# compile test: +# - esphome/build_gen/espidf.py -- the native IDF build generator +# (other files under build_gen/ target PlatformIO and don't affect +# the native IDF path) +# - script/test_build_components.py -- the harness the job invokes +# - .github/workflows/ci.yml -- the job's own definition +NATIVE_IDF_TRIGGER_FILES = frozenset( + { + "esphome/build_gen/espidf.py", + "script/test_build_components.py", + ".github/workflows/ci.yml", + } +) + + +def _native_idf_path_or_file_trigger(files: list[str]) -> bool: + """Whether any changed file is a native IDF infrastructure / harness trigger.""" + for file in files: + if file in NATIVE_IDF_TRIGGER_FILES: + return True + if any(file.startswith(prefix) for prefix in NATIVE_IDF_TRIGGER_PATH_PREFIXES): + return True + return False + + +def native_idf_components_to_test(branch: str | None = None) -> list[str]: + """Subset of ``NATIVE_IDF_TEST_COMPONENTS`` the job needs to compile. + + The job builds components with the native ESP-IDF toolchain (no + PlatformIO). When only a specific component (or something it depends + on) changed, there's no value in re-building every other unrelated + component in the test list -- the regular ``component-test`` matrix + already covers them via PlatformIO. So we narrow to the intersection + of ``NATIVE_IDF_TEST_COMPONENTS`` and the changed-component dependency + closure. + + Returns the full list (sorted) when we can't safely narrow: + + 1. Core C++/Python files changed (``esphome/core/*``). + 2. Native IDF infrastructure changed (``esphome/espidf/*`` or + ``esphome/build_gen/espidf.py``). + 3. The test harness or workflow itself changed + (``script/test_build_components.py``, ``.github/workflows/ci.yml``). + + Otherwise returns the intersection (sorted), which may be empty -- an + empty list signals the job should be skipped. + + The dependency closure is derived from ``files`` via + ``get_components_with_dependencies()`` (the same primitive ``main()`` + uses) so the result honors ``branch``. ``get_changed_components()`` + is deliberately not used here: it re-invokes ``changed_files()`` with + its own default branch, which would silently ignore our ``branch`` + argument. + + Args: + branch: Branch to compare against. If None, uses default. + + Returns: + Sorted list of component names to compile. + """ + files = changed_files(branch) + + if core_changed(files) or _native_idf_path_or_file_trigger(files): + return sorted(NATIVE_IDF_TEST_COMPONENTS) + + component_files = [f for f in files if filter_component_and_test_files(f)] + changed = get_components_with_dependencies(component_files, True) + + return sorted(NATIVE_IDF_TEST_COMPONENTS & set(changed)) + + +def should_run_native_idf(branch: str | None = None) -> bool: + """Determine if the `test-native-idf` compile-test job should run. + + Runs whenever ``native_idf_components_to_test()`` returns a non-empty + list. Skipping the job on unrelated Python-only PRs avoids ~5 min of + CI per PR (worse on cold caches). The regular ``component-test`` + matrix still exercises the same components through PlatformIO when + those components change. + + Args: + branch: Branch to compare against. If None, uses default. + + Returns: + True if the native ESP-IDF compile test should run, False otherwise. + """ + return bool(native_idf_components_to_test(branch)) + + def determine_cpp_unit_tests( branch: str | None = None, ) -> tuple[bool, list[str]]: @@ -957,6 +1076,8 @@ def main() -> None: run_python_linters = should_run_python_linters(args.branch) run_import_time = should_run_import_time(args.branch) run_device_builder = should_run_device_builder(args.branch) + native_idf_components = native_idf_components_to_test(args.branch) + run_native_idf = bool(native_idf_components) changed_cpp_file_count = count_changed_cpp_files(args.branch) # Get changed components @@ -1102,6 +1223,8 @@ def main() -> None: "python_linters": run_python_linters, "import_time": run_import_time, "device_builder": run_device_builder, + "native_idf": run_native_idf, + "native_idf_components": ",".join(native_idf_components), "changed_components": changed_components, "changed_components_with_tests": changed_components_with_tests, "directly_changed_components_with_tests": list(directly_changed_with_tests), diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 5e2dd670dc..9139c6e095 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -70,6 +70,17 @@ def mock_should_run_device_builder() -> Generator[Mock, None, None]: yield mock +@pytest.fixture +def mock_native_idf_components_to_test() -> Generator[Mock, None, None]: + """Mock native_idf_components_to_test from determine_jobs. + + main() drives both the ``native_idf`` boolean output and the + ``native_idf_components`` CSV from this one function. + """ + with patch.object(determine_jobs, "native_idf_components_to_test") as mock: + yield mock + + @pytest.fixture def mock_determine_cpp_unit_tests() -> Generator[Mock, None, None]: """Mock determine_cpp_unit_tests from helpers.""" @@ -107,6 +118,7 @@ def test_main_all_tests_should_run( mock_should_run_python_linters: Mock, mock_should_run_import_time: Mock, mock_should_run_device_builder: Mock, + mock_native_idf_components_to_test: Mock, mock_changed_files: Mock, mock_determine_cpp_unit_tests: Mock, capsys: pytest.CaptureFixture[str], @@ -122,6 +134,7 @@ def test_main_all_tests_should_run( mock_should_run_python_linters.return_value = True mock_should_run_import_time.return_value = True mock_should_run_device_builder.return_value = True + mock_native_idf_components_to_test.return_value = ["api", "esp32"] mock_determine_cpp_unit_tests.return_value = (False, ["wifi", "api", "sensor"]) # Mock changed_files to return non-component files (to avoid memory impact) @@ -203,6 +216,8 @@ def test_main_all_tests_should_run( assert output["python_linters"] is True assert output["import_time"] is True assert output["device_builder"] is True + assert output["native_idf"] is True + assert output["native_idf_components"] == "api,esp32" assert output["changed_components"] == ["wifi", "api", "sensor"] # changed_components_with_tests will only include components that actually have test files assert "changed_components_with_tests" in output @@ -236,6 +251,7 @@ def test_main_no_tests_should_run( mock_should_run_python_linters: Mock, mock_should_run_import_time: Mock, mock_should_run_device_builder: Mock, + mock_native_idf_components_to_test: Mock, mock_changed_files: Mock, mock_determine_cpp_unit_tests: Mock, capsys: pytest.CaptureFixture[str], @@ -251,6 +267,7 @@ def test_main_no_tests_should_run( mock_should_run_python_linters.return_value = False mock_should_run_import_time.return_value = False mock_should_run_device_builder.return_value = False + mock_native_idf_components_to_test.return_value = [] mock_determine_cpp_unit_tests.return_value = (False, []) # Mock changed_files to return no component files @@ -291,6 +308,8 @@ def test_main_no_tests_should_run( assert output["python_linters"] is False assert output["import_time"] is False assert output["device_builder"] is False + assert output["native_idf"] is False + assert output["native_idf_components"] == "" assert output["changed_components"] == [] assert output["changed_components_with_tests"] == [] assert output["component_test_count"] == 0 @@ -313,6 +332,7 @@ def test_main_with_branch_argument( mock_should_run_python_linters: Mock, mock_should_run_import_time: Mock, mock_should_run_device_builder: Mock, + mock_native_idf_components_to_test: Mock, mock_changed_files: Mock, mock_determine_cpp_unit_tests: Mock, capsys: pytest.CaptureFixture[str], @@ -328,6 +348,7 @@ def test_main_with_branch_argument( mock_should_run_python_linters.return_value = True mock_should_run_import_time.return_value = True mock_should_run_device_builder.return_value = True + mock_native_idf_components_to_test.return_value = ["esp32"] mock_determine_cpp_unit_tests.return_value = (False, ["mqtt"]) # Mock changed_files to return non-component files (to avoid memory impact) @@ -366,6 +387,7 @@ def test_main_with_branch_argument( mock_should_run_python_linters.assert_called_once_with("main") mock_should_run_import_time.assert_called_once_with("main") mock_should_run_device_builder.assert_called_once_with("main") + mock_native_idf_components_to_test.assert_called_once_with("main") # Check output captured = capsys.readouterr() @@ -379,6 +401,8 @@ def test_main_with_branch_argument( assert output["python_linters"] is True assert output["import_time"] is True assert output["device_builder"] is True + assert output["native_idf"] is True + assert output["native_idf_components"] == "esp32" assert output["changed_components"] == ["mqtt"] # changed_components_with_tests will only include components that actually have test files assert "changed_components_with_tests" in output @@ -827,6 +851,142 @@ def test_should_run_device_builder_skips_beta_release(target_branch: str) -> Non mock_changed.assert_not_called() +_NATIVE_IDF_FULL_LIST_FILES = [ + # Core C++/Python changes -- caught by core_changed() + ["esphome/core/component.cpp"], + ["esphome/core/config.py"], + # Native IDF infrastructure paths + ["esphome/espidf/framework.py"], + ["esphome/espidf/component.py"], + ["esphome/espidf/api.py"], + ["esphome/build_gen/espidf.py"], + # Workflow / harness files + ["script/test_build_components.py"], + [".github/workflows/ci.yml"], +] + + +@pytest.mark.parametrize("changed_files", _NATIVE_IDF_FULL_LIST_FILES) +def test_native_idf_components_to_test_returns_full_list_on_infrastructure( + changed_files: list[str], +) -> None: + """Infrastructure / core / harness changes fall back to the full component list.""" + with ( + patch.object(determine_jobs, "changed_files", return_value=changed_files), + # The dep-closure path shouldn't be consulted at all -- if it is, + # the obviously-wrong "wifi" sneaks in and the assertion catches it. + patch.object( + determine_jobs, "get_components_with_dependencies", return_value=["wifi"] + ), + ): + result = determine_jobs.native_idf_components_to_test() + assert result == sorted(determine_jobs.NATIVE_IDF_TEST_COMPONENTS) + + +@pytest.mark.parametrize( + ("changed_files", "dependency_closure", "expected"), + [ + # Single tested component changed -- narrow to just that component. + ( + ["esphome/components/esp32/__init__.py"], + ["esp32"], + ["esp32"], + ), + # Dependency closure: multiple BLE components in the changed set + # are all intersected with the test list and returned sorted. + ( + ["esphome/components/esp32_ble/ble.cpp"], + ["esp32_ble", "esp32_ble_tracker", "ble_scanner"], + ["ble_scanner", "esp32_ble", "esp32_ble_tracker"], + ), + # api in the test set -- narrow to [api] even though the closure + # has other (unrelated to native-IDF coverage) entries. + ( + ["esphome/components/api/api_connection.cpp"], + ["api", "logger"], + ["api"], + ), + # Components outside the test set return an empty list (job skipped). + ( + ["esphome/components/wifi/wifi_component.cpp"], + ["wifi", "network"], + [], + ), + # Pure Python-only change outside trigger paths -> empty. + (["esphome/yaml_util.py"], [], []), + # Non-IDF files in esphome/build_gen/ do NOT trigger the full + # list -- only esphome/build_gen/espidf.py is a trigger. + (["esphome/build_gen/platformio.py"], [], []), + # Docs / unrelated files -> empty. + (["README.md"], [], []), + ([], [], []), + ], +) +def test_native_idf_components_to_test_narrowing( + changed_files: list[str], + dependency_closure: list[str], + expected: list[str], +) -> None: + """Component changes narrow the test list to the intersection.""" + with ( + patch.object(determine_jobs, "changed_files", return_value=changed_files), + patch.object( + determine_jobs, + "get_components_with_dependencies", + return_value=dependency_closure, + ), + ): + result = determine_jobs.native_idf_components_to_test() + assert result == expected + + +def test_native_idf_components_to_test_with_branch() -> None: + """native_idf_components_to_test passes branch argument through. + + Regression test: an earlier version called ``get_changed_components()``, + which silently ignored the branch argument because that helper re-runs + ``changed_files()`` with its own default. The current implementation + derives the closure from ``files = changed_files(branch)`` directly, + so a branch arg has to flow through ``changed_files``. + """ + with ( + patch.object(determine_jobs, "changed_files") as mock_changed, + patch.object( + determine_jobs, "get_components_with_dependencies", return_value=[] + ), + ): + mock_changed.return_value = [] + determine_jobs.native_idf_components_to_test("release") + mock_changed.assert_called_once_with("release") + + +@pytest.mark.parametrize( + ("components_to_test", "expected"), + [ + ([], False), + (["esp32"], True), + (["esp32", "api"], True), + ], +) +def test_should_run_native_idf(components_to_test: list[str], expected: bool) -> None: + """should_run_native_idf is a thin wrapper around the component list.""" + with patch.object( + determine_jobs, + "native_idf_components_to_test", + return_value=components_to_test, + ): + assert determine_jobs.should_run_native_idf() is expected + + +def test_should_run_native_idf_with_branch() -> None: + """Test should_run_native_idf passes branch argument through.""" + with patch.object( + determine_jobs, "native_idf_components_to_test", return_value=[] + ) as mock_inner: + determine_jobs.should_run_native_idf("release") + mock_inner.assert_called_once_with("release") + + @pytest.mark.parametrize( ("changed_files", "expected_result"), [