From a58b4edb6ac12204274d021652f6bbffd82a37a7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 22 May 2026 18:39:06 -0500 Subject: [PATCH] [ci] Gate unconditional CI jobs on a single determine-jobs output instead of a path filter (#16580) --- .github/workflows/ci.yml | 17 +++--- script/determine-jobs.py | 80 ++++++++++++++++++++++++++++ tests/script/test_determine_jobs.py | 82 +++++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 43b03aec85..53516db913 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,14 +6,6 @@ on: branches: [dev, beta, release] pull_request: - paths: - - "**" - - "!.github/workflows/*.yml" - - "!.github/actions/build-image/*" - - ".github/workflows/ci.yml" - - "!.yamllint" - - "!.github/dependabot.yml" - - "!docker/**" merge_group: permissions: @@ -101,6 +93,8 @@ jobs: runs-on: ubuntu-24.04 needs: - common + - determine-jobs + if: needs.determine-jobs.outputs.core-ci == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -223,6 +217,8 @@ jobs: runs-on: ${{ matrix.os }} needs: - common + - determine-jobs + if: needs.determine-jobs.outputs.core-ci == 'true' steps: - name: Check out code from GitHub uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -261,6 +257,7 @@ jobs: needs: - common outputs: + core-ci: ${{ steps.determine.outputs.core-ci }} integration-tests: ${{ steps.determine.outputs.integration-tests }} integration-test-buckets: ${{ steps.determine.outputs.integration-test-buckets }} clang-tidy: ${{ steps.determine.outputs.clang-tidy }} @@ -314,6 +311,7 @@ jobs: echo "$output" | jq # Extract individual fields + echo "core-ci=$(echo "$output" | jq -r '.core_ci')" >> $GITHUB_OUTPUT echo "integration-tests=$(echo "$output" | jq -r '.integration_tests')" >> $GITHUB_OUTPUT echo "integration-test-buckets=$(echo "$output" | jq -c '.integration_test_buckets')" >> $GITHUB_OUTPUT echo "clang-tidy=$(echo "$output" | jq -r '.clang_tidy')" >> $GITHUB_OUTPUT @@ -969,7 +967,8 @@ jobs: runs-on: ubuntu-latest needs: - common - if: github.event_name == 'pull_request' && !startsWith(github.base_ref, 'beta') && !startsWith(github.base_ref, 'release') + - determine-jobs + if: github.event_name == 'pull_request' && !startsWith(github.base_ref, 'beta') && !startsWith(github.base_ref, 'release') && needs.determine-jobs.outputs.core-ci == 'true' 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 3259fb5836..ef2175eb79 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -5,6 +5,7 @@ This script is a centralized way to determine which CI jobs need to run based on what files have changed. It outputs JSON with the following structure: { + "core_ci": true/false, "integration_tests": true/false, "integration_test_buckets": [{"name": "1/3", "tests": ["tests/integration/test_foo.py", ...]}, ...], "clang_tidy": true/false, @@ -22,6 +23,11 @@ what files have changed. It outputs JSON with the following structure: } The CI workflow uses this information to: +- Gate the unconditional jobs (ci-custom, pytest, pre-commit-ci-lite) via core_ci; + false when a pull_request only touches CI-irrelevant meta paths (other workflow + files, .github/actions/build-image/*, .yamllint, .github/dependabot.yml, docker/**) + so workflow-only PRs satisfy the required CI Status check without running the + unconditional jobs. Always true on non-pull_request events and under --force-all. - Skip or run integration tests - Skip or run clang-tidy (and whether to do a full scan) - Skip or run clang-format @@ -712,6 +718,69 @@ def should_run_benchmarks(branch: str | None = None) -> bool: return any(get_component_from_path(f) in benchmarked_components for f in files) +# Files / path patterns whose changes alone don't warrant running the +# unconditional CI jobs (`ci-custom`, `pytest`, `pre-commit-ci-lite`). +# Single source of truth for what we treat as "CI-irrelevant" on +# pull_request events; ci.yml used to encode this in its own +# `pull_request.paths` filter, but that hid the required `CI Status` +# check on PRs that only touched these files (dependabot Action bumps, +# dependabot.yml edits, docker/ changes, etc.) and forced admin +# force-merges. +# +# ci.yml itself is deliberately *not* ignored — editing the CI workflow +# must still run CI. Workflows that have their own dedicated triggers +# (codeql.yml, ci-docker.yml, ...) are matched via the +# `.github/workflows/*.yml` prefix below and exclude ci.yml explicitly. +CI_IRRELEVANT_EXACT_FILES = frozenset( + { + ".yamllint", + ".github/dependabot.yml", + } +) + + +def _is_ci_irrelevant_path(path: str) -> bool: + """Whether a single changed path is irrelevant to the unconditional CI jobs.""" + if path in CI_IRRELEVANT_EXACT_FILES: + return True + # docker/** — all descendants + if path.startswith("docker/"): + return True + # .github/workflows/*.yml — top-level workflow files other than ci.yml + # (ci.yml itself must still trigger full CI when edited). + if path.startswith(".github/workflows/") and path.endswith(".yml"): + if path == ".github/workflows/ci.yml": + return False + if "/" not in path[len(".github/workflows/") :]: + return True + # .github/actions/build-image/* — direct children only, matches the + # single-star glob the workflow used to encode. + if path.startswith(".github/actions/build-image/"): + rest = path[len(".github/actions/build-image/") :] + if rest and "/" not in rest: + return True + return False + + +def should_run_core_ci(branch: str | None = None) -> bool: + """Determine if the unconditional CI jobs (ci-custom/pytest/pre-commit-ci-lite) should run. + + Returns False only when every changed file is in the CI-irrelevant set + above (see ``_is_ci_irrelevant_path``). Empty diffs return True so we + never accidentally skip CI when the diff probe fails. + + Args: + branch: Branch to compare against. If None, uses default. + + Returns: + True if the unconditional CI jobs should run, False otherwise. + """ + files = changed_files(branch) + if not files: + return True + return any(not _is_ci_irrelevant_path(f) for f in files) + + def _any_changed_file_endswith(branch: str | None, extensions: tuple[str, ...]) -> bool: """Check if a changed file ends with any of the specified extensions.""" return any(file.endswith(extensions) for file in changed_files(branch)) @@ -1075,6 +1144,16 @@ def main() -> None: args = parser.parse_args() # Determine what should run + # core_ci gates the unconditional jobs in ci.yml (ci-custom, pytest, + # pre-commit-ci-lite). Non-pull_request events (push to dev/beta/release + # and merge_group) always run them so behavior like venv-cache saves on + # push to dev is preserved. + event_name = os.environ.get("GITHUB_EVENT_NAME", "") + run_core_ci = ( + True + if args.force_all or event_name != "pull_request" + else should_run_core_ci(args.branch) + ) if args.force_all: integration_run_all, integration_test_files = True, [] run_clang_tidy = True @@ -1255,6 +1334,7 @@ def main() -> None: component_test_batches = [] output: dict[str, Any] = { + "core_ci": run_core_ci, "integration_tests": run_integration, "integration_test_buckets": integration_test_buckets, "clang_tidy": run_clang_tidy, diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 202ae9030f..7bb9fe2543 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -775,6 +775,88 @@ def test_should_run_import_time_with_branch() -> None: mock_changed.assert_called_once_with("release") +@pytest.mark.parametrize( + ("path", "expected_result"), + [ + # Exact-file matches in the CI-irrelevant set. + (".yamllint", True), + (".github/dependabot.yml", True), + # Other top-level workflow files are irrelevant; ci.yml itself is not. + (".github/workflows/codeql.yml", True), + (".github/workflows/release.yml", True), + (".github/workflows/ci.yml", False), + # Nested files under workflows/ are not matched by the single-star glob. + (".github/workflows/matchers/gcc.json", False), + # build-image action: direct children only (single-star glob). + (".github/actions/build-image/action.yml", True), + (".github/actions/build-image/nested/file.yml", False), + # Other actions are CI-relevant. + (".github/actions/restore-python/action.yml", False), + # docker/** covers everything under docker/. + ("docker/Dockerfile", True), + ("docker/scripts/run.sh", True), + # Regular source files are CI-relevant. + ("esphome/__main__.py", False), + ("esphome/components/wifi/wifi_component.cpp", False), + ("README.md", False), + ("tests/script/test_determine_jobs.py", False), + ], +) +def test_is_ci_irrelevant_path(path: str, expected_result: bool) -> None: + """Test _is_ci_irrelevant_path mirrors the historic ci.yml path filter.""" + assert determine_jobs._is_ci_irrelevant_path(path) == expected_result + + +@pytest.mark.parametrize( + ("changed_files", "expected_result"), + [ + # Empty diffs default to True — don't accidentally skip CI on a + # broken probe. + ([], True), + # Any CI-relevant file flips the result to True. + (["esphome/__main__.py"], True), + (["esphome/components/wifi/wifi_component.cpp"], True), + (["README.md"], True), + # All-irrelevant diffs return False. + ([".github/workflows/codeql.yml"], False), + ( + [".github/workflows/codeql.yml", ".github/workflows/release.yml"], + False, + ), + ([".yamllint"], False), + ([".github/dependabot.yml"], False), + (["docker/Dockerfile"], False), + ( + [ + ".github/workflows/codeql.yml", + ".github/dependabot.yml", + "docker/Dockerfile", + ], + False, + ), + # Mixed diffs always trigger CI. + ( + [".github/workflows/codeql.yml", "esphome/__main__.py"], + True, + ), + # ci.yml itself is treated as CI-relevant. + ([".github/workflows/ci.yml"], True), + ], +) +def test_should_run_core_ci(changed_files: list[str], expected_result: bool) -> None: + """Test should_run_core_ci function.""" + with patch.object(determine_jobs, "changed_files", return_value=changed_files): + assert determine_jobs.should_run_core_ci() == expected_result + + +def test_should_run_core_ci_with_branch() -> None: + """Test should_run_core_ci passes the branch through to changed_files.""" + with patch.object(determine_jobs, "changed_files") as mock_changed: + mock_changed.return_value = [] + determine_jobs.should_run_core_ci("release") + mock_changed.assert_called_once_with("release") + + @pytest.mark.parametrize( ("changed_files", "expected_result"), [