This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch backport-ci-sync-v3-4-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit e5a48b904a1134bed3da3223c5f07b708924eec5 Author: Jarek Potiuk <[email protected]> AuthorDate: Sun Oct 25 19:40:15 2026 +0100 Run fork pushes with pull-request selective checks (#74820) * Run fork pushes with pull-request selective checks A push in a contributor fork would otherwise be treated as a post-merge push and run the full matrix, about 220 minutes on the contributor's 20-runner cap, for every push. Computing the change set against apache/airflow gives the same selection a pull request would get, independent of how stale the fork's main is, and fork runs must never select self-hosted runners that forks do not have. Generated-by: Claude Opus 5 * Read pull request labels and the merge base for fork pushes A fork push carries no labels, so full tests needed, allow provider dependency bump and the other labels did nothing in fork CI and a PR that needed one stayed red. Read them from the open apache/airflow pull request of the pushed branch. The pyproject.toml and provider dependency bump checks compared the pushed commit with its parent, which after a merge of upstream main is the contributor's own previous tip, so a release manager's bump from main counted as theirs. Read the previous version at the compare's merge base instead. Generated-by: Claude Opus 5 * Match the fork pull request by base when reading its labels One head branch can have open pull requests into several bases, so the label lookup now filters on the branch line the changes are compared against. The fork push selective-check tests move to test_ci_commands.py, next to the other selective_check command tests. Generated-by: Claude Opus 5 (cherry picked from commit 164a812559d09a812c19c782f5327764cd15c474) --- dev/breeze/doc/ci/04_selective_checks.md | 32 ++++- .../doc/ci/08_fork_ci_for_external_contributors.md | 19 ++- .../src/airflow_breeze/commands/ci_commands.py | 23 +++- .../src/airflow_breeze/utils/fork_compare.py | 151 +++++++++++++++++++++ .../src/airflow_breeze/utils/selective_checks.py | 87 ++++++++---- dev/breeze/tests/test_ci_commands.py | 52 +++++++ dev/breeze/tests/test_fork_compare.py | 134 ++++++++++++++++++ dev/breeze/tests/test_selective_checks.py | 120 ++++++++++++++++ 8 files changed, 586 insertions(+), 32 deletions(-) diff --git a/dev/breeze/doc/ci/04_selective_checks.md b/dev/breeze/doc/ci/04_selective_checks.md index 45937cf3b71..d15ff81dd5e 100644 --- a/dev/breeze/doc/ci/04_selective_checks.md +++ b/dev/breeze/doc/ci/04_selective_checks.md @@ -141,7 +141,11 @@ high-blast-radius signals: ```mermaid flowchart TD - A([_should_run_all_tests_and_versions]) --> B{event is<br/>PUSH / SCHEDULE / DISPATCH?} + A([_should_run_all_tests_and_versions]) --> T{fork push with<br/>truncated change set?} + T -->|yes| YES0[True<br/>run everything] + T -->|no| FP{fork push?} + FP -->|yes - treated as a pull request| E + FP -->|no| B{event is<br/>PUSH / SCHEDULE / DISPATCH?} B -->|yes| C{PUSH and only<br/>.txt/.md files changed?} C -->|yes| NO1[False<br/>skip full tests] C -->|no| D{PUSH with<br/>--selective-tests-on-push?} @@ -332,6 +336,23 @@ flowchart TD 6. **Scheduled nightly canary** (SCHEDULE on `apache/airflow`). Full matrix on all versions, every canary-only job. The most thorough run; it is what gives us confidence that the PR-time shortcuts never let a regression through. +7. **Push in a contributor fork** (AIP-120, see + [Fork CI for external contributors](08_fork_ci_for_external_contributors.md)). The changed files + come from the cross-repository compare API against `apache/airflow`'s branch line of the pushed + code (breeze's default branch at that commit: `main`, or the release branch it was cut from), + not from `git diff-tree`, with a renamed file counted under both paths, and the run is selected + exactly like a pull request with those files. `is_committer_build` is always `False` because + forks have no self-hosted runners. When the file list reaches the compare API's 300-file cap, + or the compare fails, everything runs, including the UI E2E tests, which otherwise only follow + matching files or the `full tests needed` label. Checks that compare a file with its previous version + (`pyproject.toml` and the provider dependency bump check) read the previous version at the + compare's merge base in `apache/airflow`, not at the pushed commit's parent, which after a + merge of upstream `main` is the contributor's own previous tip. A file missing at the merge + base counts as new; any other failure to read it fails the selective checks instead of + skipping the check. The labels come from the open + `apache/airflow` pull request of the pushed branch, so `full tests needed`, + `allow provider dependency bump` and the other labels work as on a pull request; a label + added later takes effect on the next fork run. ## Troubleshooting: my CI run does too much @@ -452,7 +473,11 @@ together using `pytest-xdist` (pytest-xdist distributes the tests among parallel checks are performed. See also [1] note below. Two exceptions narrow this: a PUSH that changed **only** `.txt`/`.md` files skips full tests, and a PUSH with `--selective-tests-on-push` (which `ci-amd.yml` passes for pushes to release branches) is treated like a pull request — selective tests on default - versions, not a `canary` run. The high-level flow and worked examples for these cases are in + versions, not a `canary` run. A PUSH in a contributor fork is not a PUSH for this rule either: it is + selected like a pull request against `apache/airflow`, and runs everything only when its change set + reaches the compare API's 300-file cap (a list of 300 may be missing files) or the compare fails. The + incremental base of a release-branch push (the last successful run of the branch) is only looked up in + `apache/airflow`. The high-level flow and worked examples for these cases are in [The decision pipeline](#the-decision-pipeline) and [Worked examples](#worked-examples-pull-requests-pushes-and-canary-runs) above. * Python, Kubernetes, Backend, Kind, Helm versions are limited to "defaults" only unless `Full tests` mode @@ -739,6 +764,9 @@ anything that matters has to be enforced outside the repository, by: time. RunsOn has no cap on instance sizes or counts and its budget only alerts, so the AWS account's EC2 vCPU quotas are the cost ceiling. +Pushes in a contributor fork are never committer builds, whoever pushes, because forks have no +self-hosted runners. + ## Changing behaviours of the CI runs by setting labels PRs from `apache/airflow` branches run on self-hosted RunsOn runners and PRs from forks run on public diff --git a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md index 164c86b7112..1c652c525ae 100644 --- a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md +++ b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md @@ -358,9 +358,22 @@ upstream yields no changed files, so such a run performs basic checks only. - **Changed files** come from the cross-repository compare API, `GET /repos/apache/airflow/compare/<base>...<fork-owner>:<sha>`, three-dot semantics, so the set equals what a pull request against upstream would show. The fork's `main` is never - used as a base, so a stale fork does not inflate the change set. `<base>` is `main` unless - the pushed branch name starts with a canonical release branch prefix. The API caps the file - list at 300; at the cap the run falls back to "run everything". + used as a base, so a stale fork does not inflate the change set. `<base>` is the branch line + of the pushed code, breeze's default branch at that commit (`main`, or the release branch it + was cut from), because a branch name such as `backport-<sha>-v3-1-test` cannot be trusted to + tell. A renamed file counts under both paths. The API caps the file list at 300; at the cap, + and when the compare fails, the run falls back to "run everything", UI E2E tests included, + rather than failing. +- **Previous versions** of changed files, for the `pyproject.toml` and provider dependency + bump checks, are read at the compare's merge base in `apache/airflow` + (`GET /repos/apache/airflow/contents/<path>?ref=<merge-base>`). The pushed commit's parent + would be the contributor's own previous tip after they merge upstream `main`, so a release + manager's bump from `main` would count as theirs. A file missing at the merge base is new; any + other read failure fails the run rather than letting the check pass unseen. +- **Labels** come from the open `apache/airflow` pull request whose head is + `<fork-owner>:<branch>`, since a push event carries none. `full tests needed`, + `allow provider dependency bump`, `skip common compat check` and the other labels work as on + a pull request; a label added later takes effect on the next fork run. - `_should_run_all_tests_and_versions` treats a fork push like a pull request. Today any `push` runs the full matrix, which would cost a contributor ~220 minutes per push. - `is_committer_build` is always false. Forks have no self-hosted runners; a committer pushing diff --git a/dev/breeze/src/airflow_breeze/commands/ci_commands.py b/dev/breeze/src/airflow_breeze/commands/ci_commands.py index 7c822539141..2db6a4e8cd9 100644 --- a/dev/breeze/src/airflow_breeze/commands/ci_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/ci_commands.py @@ -60,6 +60,7 @@ from airflow_breeze.utils.docker_command_utils import ( fix_ownership_using_docker, perform_environment_checks, ) +from airflow_breeze.utils.fork_compare import get_fork_changes, get_fork_pull_request_labels, is_fork_push from airflow_breeze.utils.github import format_github_token_scope_guidance, retrieve_github_token from airflow_breeze.utils.path_utils import AIRFLOW_HOME_PATH, AIRFLOW_ROOT_PATH from airflow_breeze.utils.run_utils import run_command @@ -323,7 +324,22 @@ def selective_check( github_context_dict = json.loads(github_context) if github_context else {} github_event = GithubEvents(github_event_name) changed_files: tuple[str, ...] = () - if commit_ref is not None and selective_tests_on_push and github_event == GithubEvents.PUSH: + fork_push = is_fork_push(github_event, github_repository) + fork_changes_truncated = False + fork_merge_base = None + labels = tuple(ast.literal_eval(pr_labels)) if pr_labels else () + fork_owner = github_repository.split("/")[0] + if fork_push and github_context_dict.get("ref_name"): + labels = get_fork_pull_request_labels( + fork_owner=fork_owner, branch=github_context_dict["ref_name"], base=default_branch + ) + if fork_push and commit_ref is not None: + fork_changes = get_fork_changes(fork_owner=fork_owner, sha=commit_ref, base=default_branch) + changed_files = fork_changes.files + fork_changes_truncated = fork_changes.truncated + fork_merge_base = fork_changes.merge_base + console_print(f"\n[info]Changed files against apache/airflow:[/]\n{changed_files}\n") + elif commit_ref is not None and selective_tests_on_push and github_event == GithubEvents.PUSH: # The base is the last successful run of the branch, so this run also covers the commits # whose push runs failed or were cancelled by a newer push. Without one, nothing after the # branch point is known to pass, so everything runs. @@ -344,13 +360,16 @@ def selective_check( files=changed_files, default_branch=default_branch, default_constraints_branch=default_constraints_branch, - pr_labels=tuple(ast.literal_eval(pr_labels)) if pr_labels else (), + pr_labels=labels, github_event=github_event, github_repository=github_repository, github_actor=github_actor, github_context_dict=github_context_dict, platform=ci_platform, selective_tests_on_push=selective_tests_on_push, + fork_push=fork_push, + fork_changes_truncated=fork_changes_truncated, + fork_merge_base=fork_merge_base, ) print(str(sc), file=sys.stderr) except Exception: diff --git a/dev/breeze/src/airflow_breeze/utils/fork_compare.py b/dev/breeze/src/airflow_breeze/utils/fork_compare.py new file mode 100644 index 00000000000..5ced24ec5e1 --- /dev/null +++ b/dev/breeze/src/airflow_breeze/utils/fork_compare.py @@ -0,0 +1,151 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Changed files of a push in a contributor fork, computed against apache/airflow (AIP-120).""" + +from __future__ import annotations + +import json +from dataclasses import dataclass + +from airflow_breeze.global_constants import APACHE_AIRFLOW_GITHUB_REPOSITORY, GithubEvents +from airflow_breeze.utils.console import console_print +from airflow_breeze.utils.github import run_gh_command + +# The compare API lists at most this many files: a list that reaches it may be missing some, so +# the change set is treated as truncated. +COMPARE_FILE_CAP = 300 + + +@dataclass(frozen=True) +class ForkChanges: + files: tuple[str, ...] + truncated: bool + merge_base: str | None = None + + +def is_fork_push(github_event: GithubEvents, github_repository: str) -> bool: + return github_event == GithubEvents.PUSH and github_repository != APACHE_AIRFLOW_GITHUB_REPOSITORY + + +def get_fork_changes(*, fork_owner: str, sha: str, base: str) -> ForkChanges: + """ + Three-dot compare of the pushed commit against the upstream base branch. + + Three-dot semantics give the same file list a pull request against upstream would show, + so a fork whose ``main`` is behind upstream does not inflate the change set. ``base`` is the + branch line the pushed code belongs to (breeze's default branch at that commit), which a + branch name cannot be trusted to tell. A renamed file counts under both paths, as in + ``git diff``. When the compare fails the change set is reported as truncated, so the run + covers everything instead of failing. + + ``merge_base`` is the upstream commit the changes are measured from. Checks that compare a + file with its previous version use it instead of the pushed commit's parent, which after a + merge of upstream ``main`` into the branch is the contributor's own previous tip. + """ + result = run_gh_command( + [ + "gh", + "api", + f"repos/{APACHE_AIRFLOW_GITHUB_REPOSITORY}/compare/{base}...{fork_owner}:{sha}", + "--jq", + "{files: [.files[] | [.filename, (.previous_filename // empty)]], " + "merge_base: .merge_base_commit.sha}", + ], + capture_output=True, + text=True, + ) + if result.returncode != 0: + console_print( + f"[warning]Could not compare {fork_owner}:{sha} with {APACHE_AIRFLOW_GITHUB_REPOSITORY}:{base}, " + f"so everything runs: {result.stderr.strip()}[/]" + ) + return ForkChanges(files=(), truncated=True) + compare = json.loads(result.stdout or "{}") + entries = compare.get("files", []) + files = tuple(path for entry in entries for path in entry) + return ForkChanges( + files=files, truncated=len(entries) >= COMPARE_FILE_CAP, merge_base=compare.get("merge_base") + ) + + +def get_fork_pull_request_labels(*, fork_owner: str, branch: str, base: str) -> tuple[str, ...]: + """ + Labels of the open apache/airflow pull request from ``fork_owner:branch`` into ``base``. + + A fork push carries no pull request, so labels such as ``full tests needed`` or + ``allow provider dependency bump`` are read here. A label added later takes effect on the + next fork run. One head branch can have open pull requests into several bases, so ``base`` is + the same branch line the changes are compared against. No pull request, or a failed call, + means no labels. + """ + result = run_gh_command( + [ + "gh", + "api", + "--method", + "GET", + f"repos/{APACHE_AIRFLOW_GITHUB_REPOSITORY}/pulls", + "-f", + f"head={fork_owner}:{branch}", + "-f", + f"base={base}", + "-f", + "state=open", + "--jq", + "[.[0].labels[]?.name]", + ], + capture_output=True, + text=True, + ) + if result.returncode != 0: + console_print( + f"[warning]Could not read the labels of the pull request from {fork_owner}:{branch}: " + f"{result.stderr.strip()}[/]" + ) + return () + return tuple(json.loads(result.stdout or "[]")) + + +class UpstreamFileError(RuntimeError): + """A file of apache/airflow could not be read for a reason other than its absence.""" + + +def get_upstream_file(*, path: str, ref: str) -> str | None: + """ + Content of ``path`` at ``ref`` in apache/airflow, or None when the file does not exist there. + + Any other failure raises: the checks that compare a file with its previous version would + otherwise pass silently for a change they never saw. + """ + result = run_gh_command( + [ + "gh", + "api", + "-H", + "Accept: application/vnd.github.raw", + f"repos/{APACHE_AIRFLOW_GITHUB_REPOSITORY}/contents/{path}?ref={ref}", + ], + capture_output=True, + text=True, + ) + if result.returncode == 0: + return result.stdout + if "404" in result.stderr or "Not Found" in result.stderr: + return None + raise UpstreamFileError( + f"Could not read {path} at {ref} in {APACHE_AIRFLOW_GITHUB_REPOSITORY}: {result.stderr.strip()}" + ) diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py index d272587c11e..2d420e8ef90 100644 --- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py +++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py @@ -73,6 +73,7 @@ from airflow_breeze.global_constants import ( ) from airflow_breeze.utils.console import console_print, get_console from airflow_breeze.utils.exclude_from_matrix import excluded_combos +from airflow_breeze.utils.fork_compare import get_upstream_file from airflow_breeze.utils.functools_cache import clearable_cache from airflow_breeze.utils.kubernetes_utils import get_kubernetes_python_combos from airflow_breeze.utils.packages import get_available_distributions, get_suspended_provider_ids @@ -896,7 +897,16 @@ def _get_test_list_as_json(list_of_list_of_types: list[list[str]]) -> list[dict[ class SelectiveChecks: - __HASHABLE_FIELDS = {"_files", "_default_branch", "_commit_ref", "_pr_labels", "_github_event"} + __HASHABLE_FIELDS = { + "_files", + "_default_branch", + "_commit_ref", + "_pr_labels", + "_github_event", + "_fork_push", + "_fork_changes_truncated", + "_fork_merge_base", + } def __init__( self, @@ -911,7 +921,17 @@ class SelectiveChecks: github_context_dict: dict[str, Any] | None = None, platform: str = CI_AMD_PLATFORM, selective_tests_on_push: bool = False, + fork_push: bool = False, + fork_changes_truncated: bool = False, + fork_merge_base: str | None = None, ): + """ + :param fork_push: the push happened in a contributor fork (AIP-120); ``files`` are the + changes against apache/airflow and the run is selected like a pull request. + :param fork_changes_truncated: the compare API capped the changed files of a fork push. + :param fork_merge_base: the apache/airflow commit a fork push is compared with; previous + versions of changed files are read there instead of from the pushed commit's parent. + """ self._files = files # A changed test helper selects the tests that import it, as if those test files had changed; # only helpers that cannot be narrowed to their importers still force the full set of tests. @@ -938,6 +958,9 @@ class SelectiveChecks: self._github_context_dict = github_context_dict or {} self._platform = platform self._selective_tests_on_push = selective_tests_on_push + self._fork_push = fork_push + self._fork_changes_truncated = fork_changes_truncated + self._fork_merge_base = fork_merge_base self._new_toml: dict[str, Any] = {} self._old_toml: dict[str, Any] = {} @@ -998,8 +1021,26 @@ class SelectiveChecks: return [GithubEvents.SCHEDULE, GithubEvents.WORKFLOW_DISPATCH] return [GithubEvents.PUSH, GithubEvents.SCHEDULE, GithubEvents.WORKFLOW_DISPATCH] + def _get_previous_file_content(self, path: str) -> str | None: + if self._fork_merge_base: + return get_upstream_file(path=path, ref=self._fork_merge_base) + result = run_command( + ["git", "show", f"{self._commit_ref}^:{path}"], + capture_output=True, + text=True, + cwd=AIRFLOW_ROOT_PATH, + check=False, + ) + return result.stdout if result.returncode == 0 else None + + def _is_pull_request_like(self) -> bool: + return self._github_event == GithubEvents.PULL_REQUEST or self._fork_push + def _should_run_all_tests_and_versions(self) -> bool: - if self._github_event in self._get_full_run_events(): + if self._fork_changes_truncated: + console_print("[warning]Running everything because the fork change set is truncated[/]") + return True + if not self._fork_push and self._github_event in self._get_full_run_events(): if self.only_text_non_doc_files_changed and self._github_event == GithubEvents.PUSH: console_print( f"[warning]Only text non doc files changed in {self._github_event}, skip full tests[/]" @@ -1269,6 +1310,13 @@ class SelectiveChecks: f"'{FULL_TESTS_NEEDED_LABEL}' label is set[/]" ) return True + # The UI files of a truncated fork change set may be among the files the compare API + # did not list, so the file match below cannot rule them out. + if self._fork_changes_truncated: + console_print( + f"[warning]{FileGroupForCi.UI_FILES} e2e enabled because the fork change set is truncated[/]" + ) + return True matched_files = self._matching_files(FileGroupForCi.UI_FILES, CI_FILE_GROUP_MATCHES) if matched_files: console_print( @@ -1851,21 +1899,14 @@ class SelectiveChecks: f"Could not get pyproject.toml from {self._commit_ref}[/]" ) return False - old_result = run_command( - ["git", "show", f"{self._commit_ref}^:pyproject.toml"], - capture_output=True, - text=True, - cwd=AIRFLOW_ROOT_PATH, - check=False, - ) - if old_result.returncode != 0: + old_content = self._get_previous_file_content("pyproject.toml") + if old_content is None: console_print( - f"[warning]Cannot determine pyproject.toml changes. " - f"Could not get pyproject.toml from {self._commit_ref}^[/]" + "[warning]Cannot determine pyproject.toml changes. Could not get its previous version[/]" ) return False self._new_toml = tomllib.loads(new_result.stdout) - self._old_toml = tomllib.loads(old_result.stdout) + self._old_toml = tomllib.loads(old_content) return True @cached_property @@ -2341,6 +2382,9 @@ class SelectiveChecks: def is_committer_build(self): if NON_COMMITTER_BUILD_LABEL in self._pr_labels: return False + # Forks have no self-hosted runners; a committer's fork push must stay on public runners. + if self._fork_push: + return False return self._github_actor in COMMITTERS def _find_all_providers_affected(self, include_docs: bool) -> list[str] | AllProvidersSentinel | None: @@ -2431,7 +2475,7 @@ class SelectiveChecks: FileGroupForCi.ALL_PYPROJECT_TOML_FILES, CI_FILE_GROUP_MATCHES, ) - if not pyproject_files or not self._github_event == GithubEvents.PULL_REQUEST: + if not pyproject_files or not self._is_pull_request_like(): return False violations = [] @@ -2447,20 +2491,13 @@ class SelectiveChecks: if new_result.returncode != 0: continue - # Get the old version of the file - old_result = run_command( - ["git", "show", f"{self._commit_ref}^:{pyproject_file}"], - capture_output=True, - text=True, - cwd=AIRFLOW_ROOT_PATH, - check=False, - ) - if old_result.returncode != 0: + old_content = self._get_previous_file_content(pyproject_file) + if old_content is None: continue try: new_toml = tomllib.loads(new_result.stdout) - old_toml = tomllib.loads(old_result.stdout) + old_toml = tomllib.loads(old_content) except Exception: continue @@ -2652,7 +2689,7 @@ class SelectiveChecks: Check if common.compat provider changed and other providers changed don't have '# use next version' comment for their common-compat dependency. """ - if self._github_event != GithubEvents.PULL_REQUEST: + if not self._is_pull_request_like(): return False # Only enforce on PRs targeting main. On release branches (e.g. v3-X-test) # cherry-picked common.compat changes don't follow the '# use next version' diff --git a/dev/breeze/tests/test_ci_commands.py b/dev/breeze/tests/test_ci_commands.py index 9b70a35530f..ca64a30b0b1 100644 --- a/dev/breeze/tests/test_ci_commands.py +++ b/dev/breeze/tests/test_ci_commands.py @@ -16,6 +16,7 @@ # under the License. from __future__ import annotations +import json from subprocess import CompletedProcess from unittest.mock import patch @@ -23,6 +24,7 @@ import pytest from click.testing import CliRunner from airflow_breeze.commands.ci_commands import get_changed_files_since, selective_check +from airflow_breeze.utils.fork_compare import ForkChanges PUSHED_COMMIT = "b" * 40 BASE_COMMIT = "a" * 40 @@ -112,3 +114,53 @@ def test_push_without_selective_tests_on_push_uses_the_pushed_commit( mock_get_changed_files_since.assert_not_called() mock_get_changed_files.assert_called_once_with(commit_ref=PUSHED_COMMIT) assert mock_selective_checks.call_args.kwargs["files"] == ("a.py",) + + +def _invoke_fork_selective_check(repository: str): + return _invoke_selective_check( + "--github-repository", + repository, + "--github-context", + json.dumps({"ref_name": "backport-abc-v3-1-test"}), + "--default-branch", + "v3-1-test", + ) + + +@patch("airflow_breeze.commands.ci_commands.get_fork_pull_request_labels", autospec=True) +@patch("airflow_breeze.commands.ci_commands.get_changed_files", autospec=True) +@patch("airflow_breeze.commands.ci_commands.get_fork_changes", autospec=True) +@patch("airflow_breeze.utils.selective_checks.SelectiveChecks", autospec=True) +def test_fork_push_takes_changed_files_from_compare( + mock_selective_checks, mock_fork_changes, mock_changed_files, mock_labels +): + mock_fork_changes.return_value = ForkChanges(files=("README.md",), truncated=True, merge_base="base-sha") + mock_labels.return_value = ("full tests needed",) + + _invoke_fork_selective_check("someone/airflow") + + mock_fork_changes.assert_called_once_with(fork_owner="someone", sha=PUSHED_COMMIT, base="v3-1-test") + mock_changed_files.assert_not_called() + mock_labels.assert_called_once_with( + fork_owner="someone", branch="backport-abc-v3-1-test", base="v3-1-test" + ) + kwargs = mock_selective_checks.call_args.kwargs + assert kwargs["files"] == ("README.md",) + assert kwargs["fork_push"] is True + assert kwargs["fork_changes_truncated"] is True + assert kwargs["fork_merge_base"] == "base-sha" + assert kwargs["pr_labels"] == ("full tests needed",) + + +@patch("airflow_breeze.commands.ci_commands.get_fork_pull_request_labels", autospec=True) +@patch("airflow_breeze.commands.ci_commands.get_changed_files", autospec=True, return_value=("README.md",)) +@patch("airflow_breeze.commands.ci_commands.get_fork_changes", autospec=True) +@patch("airflow_breeze.utils.selective_checks.SelectiveChecks", autospec=True) +def test_upstream_push_uses_git_diff( + mock_selective_checks, mock_fork_changes, mock_changed_files, mock_labels +): + _invoke_fork_selective_check("apache/airflow") + + mock_fork_changes.assert_not_called() + mock_labels.assert_not_called() + assert mock_selective_checks.call_args.kwargs["fork_push"] is False diff --git a/dev/breeze/tests/test_fork_compare.py b/dev/breeze/tests/test_fork_compare.py new file mode 100644 index 00000000000..4d745b84a95 --- /dev/null +++ b/dev/breeze/tests/test_fork_compare.py @@ -0,0 +1,134 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import json +import subprocess +from unittest import mock + +import pytest + +from airflow_breeze.global_constants import GithubEvents +from airflow_breeze.utils.fork_compare import ( + COMPARE_FILE_CAP, + ForkChanges, + UpstreamFileError, + get_fork_changes, + get_fork_pull_request_labels, + get_upstream_file, + is_fork_push, +) + + [email protected]( + ("event", "repository", "expected"), + [ + pytest.param(GithubEvents.PUSH, "someone/airflow", True, id="fork-push"), + pytest.param(GithubEvents.PUSH, "apache/airflow", False, id="upstream-push"), + pytest.param(GithubEvents.PULL_REQUEST, "someone/airflow", False, id="fork-pr"), + ], +) +def test_is_fork_push(event, repository, expected): + assert is_fork_push(event, repository) is expected + + [email protected]("airflow_breeze.utils.fork_compare.run_gh_command", autospec=True) [email protected]( + ("files", "expected"), + [ + pytest.param([], ForkChanges(files=(), truncated=False), id="empty"), + pytest.param([["a.py"], ["b.md"]], ForkChanges(files=("a.py", "b.md"), truncated=False), id="small"), + pytest.param( + [["docs/x.yml", ".github/workflows/x.yml"]], + ForkChanges(files=("docs/x.yml", ".github/workflows/x.yml"), truncated=False), + id="rename-keeps-both-paths", + ), + pytest.param( + [[f"f{i}.py"] for i in range(COMPARE_FILE_CAP)], + ForkChanges(files=tuple(f"f{i}.py" for i in range(COMPARE_FILE_CAP)), truncated=True), + id="at-cap", + ), + ], +) +def test_get_fork_changes(mock_gh, files, expected): + mock_gh.return_value = subprocess.CompletedProcess( + [], 0, stdout=json.dumps({"files": files, "merge_base": None}), stderr="" + ) + + assert get_fork_changes(fork_owner="someone", sha="abc", base="main") == expected + + assert mock_gh.call_args.args[0][:3] == ["gh", "api", "repos/apache/airflow/compare/main...someone:abc"] + + [email protected]("airflow_breeze.utils.fork_compare.run_gh_command", autospec=True) +def test_get_fork_changes_runs_everything_when_the_compare_fails(mock_gh): + mock_gh.return_value = subprocess.CompletedProcess([], 1, stdout="", stderr="Not Found") + + assert get_fork_changes(fork_owner="someone", sha="abc", base="main") == ForkChanges( + files=(), truncated=True + ) + + [email protected]("airflow_breeze.utils.fork_compare.run_gh_command", autospec=True) +def test_get_fork_changes_returns_the_merge_base(mock_gh): + mock_gh.return_value = subprocess.CompletedProcess( + [], 0, stdout=json.dumps({"files": [["a.py"]], "merge_base": "base-sha"}), stderr="" + ) + + assert get_fork_changes(fork_owner="someone", sha="abc", base="main").merge_base == "base-sha" + + [email protected]("airflow_breeze.utils.fork_compare.run_gh_command", autospec=True) [email protected]( + ("returncode", "stdout", "expected"), + [ + pytest.param(0, '["full tests needed"]', ("full tests needed",), id="pull-request"), + pytest.param(0, "[]", (), id="no-pull-request"), + pytest.param(1, "", (), id="call-fails"), + ], +) +def test_get_fork_pull_request_labels(mock_gh, returncode, stdout, expected): + mock_gh.return_value = subprocess.CompletedProcess([], returncode, stdout=stdout, stderr="boom") + + assert get_fork_pull_request_labels(fork_owner="someone", branch="fix", base="v3-1-test") == expected + + assert "head=someone:fix" in mock_gh.call_args.args[0] + assert "base=v3-1-test" in mock_gh.call_args.args[0] + + [email protected]("airflow_breeze.utils.fork_compare.run_gh_command", autospec=True) [email protected]( + ("returncode", "stderr", "expected"), + [ + pytest.param(0, "", "content", id="found"), + pytest.param(1, "gh: Not Found (HTTP 404)", None, id="missing"), + ], +) +def test_get_upstream_file(mock_gh, returncode, stderr, expected): + mock_gh.return_value = subprocess.CompletedProcess([], returncode, stdout="content", stderr=stderr) + + assert get_upstream_file(path="pyproject.toml", ref="base-sha") == expected + + assert mock_gh.call_args.args[0][-1] == "repos/apache/airflow/contents/pyproject.toml?ref=base-sha" + + [email protected]("airflow_breeze.utils.fork_compare.run_gh_command", autospec=True) +def test_get_upstream_file_fails_on_errors_other_than_absence(mock_gh): + mock_gh.return_value = subprocess.CompletedProcess([], 1, stdout="", stderr="HTTP 502: Bad Gateway") + + with pytest.raises(UpstreamFileError, match="502"): + get_upstream_file(path="pyproject.toml", ref="base-sha") diff --git a/dev/breeze/tests/test_selective_checks.py b/dev/breeze/tests/test_selective_checks.py index 174012ebaef..90ee9560f0d 100644 --- a/dev/breeze/tests/test_selective_checks.py +++ b/dev/breeze/tests/test_selective_checks.py @@ -2949,6 +2949,49 @@ def test_selective_tests_on_push(selective_tests_on_push: bool, expected_outputs assert_outputs_are_printed(expected_outputs, str(stderr)) [email protected]( + ("files", "fork_changes_truncated", "expected_outputs"), + [ + pytest.param( + ("airflow-core/docs/index.rst",), + False, + {"full-tests-needed": "false", "all-versions": "false", "is-committer-build": "false"}, + id="Fork push of a docs change does not run everything", + ), + pytest.param( + (), + False, + {"full-tests-needed": "false", "basic-checks-only": "true", "is-committer-build": "false"}, + id="Fork push without changes against upstream runs basic checks only", + ), + pytest.param( + ("airflow-core/docs/index.rst",), + True, + { + "full-tests-needed": "true", + "all-versions": "true", + "is-committer-build": "false", + "run-ui-e2e-tests": "true", + }, + id="Fork push with a truncated change set runs everything", + ), + ], +) +def test_expected_output_fork_push( + files: tuple[str, ...], fork_changes_truncated: bool, expected_outputs: dict[str, str] +): + stderr = SelectiveChecks( + files=files, + commit_ref=NEUTRAL_COMMIT, + github_event=GithubEvents.PUSH, + github_repository="potiuk/airflow", + github_actor="potiuk", + fork_push=True, + fork_changes_truncated=fork_changes_truncated, + ) + assert_outputs_are_printed(expected_outputs, str(stderr)) + + @pytest.mark.parametrize( ("files", "expected_outputs"), [ @@ -4140,6 +4183,62 @@ dependencies = [ ).provider_dependency_bump +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_provider_dependency_bump_check_fails_on_a_fork_push(mock_run_command): + """A push in a contributor's fork is checked like the pull request it stands for.""" + mock_run_command.side_effect = lambda *args, **kwargs: Mock( + returncode=0, + stdout=( + '[project]\ndependencies = ["apache-airflow-providers-common-sql>=1.0.0"]\n' + if "^:" in args[0][2] + else '[project]\ndependencies = ["apache-airflow-providers-common-sql>=1.1.0"]\n' + ), + ) + + with pytest.raises(SystemExit): + SelectiveChecks( + files=("providers/amazon/pyproject.toml",), + commit_ref=NEUTRAL_COMMIT, + pr_labels=(), + github_event=GithubEvents.PUSH, + default_branch="main", + fork_push=True, + ).provider_dependency_bump + + +@patch("airflow_breeze.utils.selective_checks.get_upstream_file", autospec=True) +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_provider_dependency_bump_on_a_fork_push_compares_with_the_merge_base( + mock_run_command, mock_upstream_file +): + """ + After the contributor merges upstream main, the pushed commit's parent is their own old tip, + so a release manager's bump from main would look like theirs. + """ + bumped = '[project]\ndependencies = ["apache-airflow-providers-common-sql>=1.1.0"]\n' + mock_run_command.side_effect = lambda *args, **kwargs: Mock( + returncode=0, + stdout=( + '[project]\ndependencies = ["apache-airflow-providers-common-sql>=1.0.0"]\n' + if "^:" in args[0][2] + else bumped + ), + ) + mock_upstream_file.return_value = bumped + + assert not SelectiveChecks( + files=("providers/amazon/pyproject.toml",), + commit_ref=NEUTRAL_COMMIT, + pr_labels=(), + github_event=GithubEvents.PUSH, + default_branch="main", + fork_push=True, + fork_merge_base="base-sha", + ).provider_dependency_bump + + mock_upstream_file.assert_called_once_with(path="providers/amazon/pyproject.toml", ref="base-sha") + + @patch("airflow_breeze.utils.selective_checks.run_command") def test_provider_dependency_bump_check_skipped_on_release_branch(mock_run_command): """Test that provider dependency bump check is a no-op on release branches (v3-X-test).""" @@ -4356,6 +4455,27 @@ dependencies = [ ).common_compat_changed_without_next_version +@patch("airflow_breeze.utils.selective_checks.run_command") +def test_common_compat_changed_without_next_version_fails_on_a_fork_push(mock_run_command): + mock_run_command.side_effect = lambda *args, **kwargs: Mock( + returncode=0, + stdout='[project]\ndependencies = ["apache-airflow-providers-common-compat>=1.8.0"]\n', + ) + + with pytest.raises(SystemExit): + _ = SelectiveChecks( + files=( + "providers/common/compat/src/airflow/providers/common/compat/file.py", + "providers/ftp/src/airflow/providers/ftp/hooks/ftp.py", + ), + commit_ref=NEUTRAL_COMMIT, + pr_labels=(), + github_event=GithubEvents.PUSH, + default_branch="main", + fork_push=True, + ).common_compat_changed_without_next_version + + @patch("airflow_breeze.utils.selective_checks.run_command") def test_common_compat_only_changed_passes(mock_run_command): """Test that check passes when only common.compat provider changes."""
