From 37ab8fc76b4d977914ed54fc392b14cdb478d9d1 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sun, 26 Jun 2022 12:11:52 +0200 Subject: [PATCH] Fix handling of GitHub Event types for new selective checks (#24665) One more finding after merging the selective checks in Python - I missed a case of "pull_request_target". Fixed and added more tests. --- .../airflow_breeze/commands/ci_commands.py | 2 +- .../src/airflow_breeze/global_constants.py | 1 + dev/breeze/tests/test_selective_checks.py | 90 ++++++++++++++- images/breeze/output-commands-hash.txt | 2 +- images/breeze/output-selective-check.svg | 104 +++++++++--------- 5 files changed, 143 insertions(+), 56 deletions(-) diff --git a/dev/breeze/src/airflow_breeze/commands/ci_commands.py b/dev/breeze/src/airflow_breeze/commands/ci_commands.py index c8260698d71aa..c65753e1a65e5 100644 --- a/dev/breeze/src/airflow_breeze/commands/ci_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/ci_commands.py @@ -205,7 +205,7 @@ def selective_check( from airflow_breeze.utils.selective_checks import SelectiveChecks github_event = GithubEvents(github_event_name) - if github_event == GithubEvents.PULL_REQUEST: + if commit_ref is not None: changed_files = get_changed_files(commit_ref=commit_ref, dry_run=dry_run, verbose=verbose) else: changed_files = () diff --git a/dev/breeze/src/airflow_breeze/global_constants.py b/dev/breeze/src/airflow_breeze/global_constants.py index 4e07591d5a8b4..5ba825e71f67a 100644 --- a/dev/breeze/src/airflow_breeze/global_constants.py +++ b/dev/breeze/src/airflow_breeze/global_constants.py @@ -304,6 +304,7 @@ class GithubEvents(Enum): PULL_REQUEST = "pull_request" PULL_REQUEST_REVIEW = "pull_request_review" PULL_REQUEST_TARGET = "pull_request_target" + PULL_REQUEST_WORKFLOW = "pull_request_workflow" PUSH = "push" SCHEDULE = "schedule" WORKFLOW_RUN = "workflow_run" diff --git a/dev/breeze/tests/test_selective_checks.py b/dev/breeze/tests/test_selective_checks.py index 492135ebd38b0..2d7e8fe83d88b 100644 --- a/dev/breeze/tests/test_selective_checks.py +++ b/dev/breeze/tests/test_selective_checks.py @@ -298,7 +298,7 @@ def test_expected_output_full_tests_needed( "upgrade-to-newer-dependencies": "false", "test-types": "", }, - id="Everything should run when full tests are needed even if no files are changed", + id="Nothing should run if only non-important files changed", ), pytest.param( ( @@ -371,6 +371,76 @@ def test_expected_output_pull_request_v2_3( assert_outputs_are_printed(expected_outputs, str(sc)) +@pytest.mark.parametrize( + "files, expected_outputs,", + [ + pytest.param( + ("INTHEWILD.md",), + { + "all-python-versions": "['3.7']", + "all-python-versions-list-as-string": "3.7", + "image-build": "false", + "needs-helm-tests": "false", + "run-tests": "false", + "docs-build": "false", + "upgrade-to-newer-dependencies": "false", + "test-types": "", + }, + id="Nothing should run if only non-important files changed", + ), + pytest.param( + ( + "airflow/cli/test.py", + "chart/aaaa.txt", + "tests/providers/google/file.py", + ), + { + "all-python-versions": "['3.7']", + "all-python-versions-list-as-string": "3.7", + "image-build": "true", + "needs-helm-tests": "true", + "run-tests": "true", + "docs-build": "true", + "run-kubernetes-tests": "true", + "upgrade-to-newer-dependencies": "false", + "test-types": "Always CLI", + }, + id="CLI tests and Kubernetes tests should run if cli/chart files changed", + ), + pytest.param( + ( + "airflow/file.py", + "tests/providers/google/file.py", + ), + { + "all-python-versions": "['3.7']", + "all-python-versions-list-as-string": "3.7", + "image-build": "true", + "needs-helm-tests": "false", + "run-tests": "true", + "docs-build": "true", + "run-kubernetes-tests": "false", + "upgrade-to-newer-dependencies": "false", + "test-types": "API Always CLI Core Integration Other Providers WWW", + }, + id="All tests except should run if core file changed", + ), + ], +) +def test_expected_output_pull_request_target( + files: Tuple[str, ...], + expected_outputs: Dict[str, str], +): + sc = SelectiveChecks( + files=files, + commit_ref="HEAD", + github_event=GithubEvents.PULL_REQUEST_TARGET, + pr_labels=(), + default_branch="main", + ) + assert_outputs_are_printed(expected_outputs, str(sc)) + + @pytest.mark.parametrize( "files, pr_labels, default_branch, expected_outputs,", [ @@ -441,11 +511,21 @@ def test_expected_output_push( assert_outputs_are_printed(expected_outputs, str(sc)) -def test_no_commit_provided(): +@pytest.mark.parametrize( + "github_event", + [ + GithubEvents.PUSH, + GithubEvents.PULL_REQUEST, + GithubEvents.PULL_REQUEST_TARGET, + GithubEvents.PULL_REQUEST_WORKFLOW, + GithubEvents.SCHEDULE, + ], +) +def test_no_commit_provided_trigger_full_build_for_any_event_type(github_event): sc = SelectiveChecks( files=(), commit_ref="", - github_event=GithubEvents.PULL_REQUEST, + github_event=github_event, pr_labels=(), default_branch="main", ) @@ -457,7 +537,9 @@ def test_no_commit_provided(): "needs-helm-tests": "true", "run-tests": "true", "docs-build": "true", - "upgrade-to-newer-dependencies": "false", + "upgrade-to-newer-dependencies": "true" + if github_event in [GithubEvents.PUSH, GithubEvents.SCHEDULE] + else "false", "test-types": "API Always CLI Core Integration Other Providers WWW", }, str(sc), diff --git a/images/breeze/output-commands-hash.txt b/images/breeze/output-commands-hash.txt index d5515f5d13aff..5562ae41f2f65 100644 --- a/images/breeze/output-commands-hash.txt +++ b/images/breeze/output-commands-hash.txt @@ -2,4 +2,4 @@ # This file is automatically generated by pre-commit. If you have a conflict with this file # Please do not solve it but run `breeze regenerate-command-images`. # This command should fix the conflict and regenerate help images that you have conflict with. -b669c8a210166579c58f904d9984d739 +db93ba3ceb45327f208f9ebb6e5d3644 diff --git a/images/breeze/output-selective-check.svg b/images/breeze/output-selective-check.svg index 67bc6c52533d9..45a774081903f 100644 --- a/images/breeze/output-selective-check.svg +++ b/images/breeze/output-selective-check.svg @@ -1,4 +1,4 @@ - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + + + + - Command: selective-check + Command: selective-check - + - - -Usage: breeze selective-check [OPTIONS] - -Checks what kind of tests should be run for an incoming commit. - -╭─ Selective check flags ──────────────────────────────────────────────────────────────────────────────────────────────╮ -│--commit-refCommit-ish reference to the commit that should be checked(TEXT)│ -│--pr-labelsSpace-separate list of labels which are valid for the PR(TEXT)│ -│--default-branchBranch against which the PR should be run(TEXT)[default: main]│ -│--github-event-nameName of the GitHub event that triggered the check                                          │ -│(pull_request | pull_request_review | pull_request_target | push | schedule | workflow_run)│ -│[default: pull_request]                                                                    │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ -╭─ Options ────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│--verbose-vPrint verbose information about performed steps.│ -│--dry-run-DIf dry-run is set, commands are only printed, not executed.│ -│--help-hShow this message and exit.│ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ + + +Usage: breeze selective-check [OPTIONS] + +Checks what kind of tests should be run for an incoming commit. + +╭─ Selective check flags ──────────────────────────────────────────────────────────────────────────────────────────────╮ +│--commit-refCommit-ish reference to the commit that should be checked(TEXT)│ +│--pr-labelsSpace-separate list of labels which are valid for the PR(TEXT)│ +│--default-branchBranch against which the PR should be run(TEXT)[default: main]│ +│--github-event-nameName of the GitHub event that triggered the check                                            │ +│(pull_request | pull_request_review | pull_request_target | pull_request_workflow | push |   │ +│schedule | workflow_run)                                                                     │ +│[default: pull_request]                                                                      │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─ Options ────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│--verbose-vPrint verbose information about performed steps.│ +│--dry-run-DIf dry-run is set, commands are only printed, not executed.│ +│--help-hShow this message and exit.│ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯