From e3329a10f92f429d70ba19ba9bf72e0279d43668 Mon Sep 17 00:00:00 2001 From: Max K Date: Sat, 18 May 2024 18:00:32 +0200 Subject: [PATCH 1/6] CI: mergeable check redesign --- .github/workflows/pull_request.yml | 27 ++++++++++++++++++++++++++- tests/ci/ci_config.py | 4 +++- tests/ci/commit_status_helper.py | 9 ++++++++- tests/ci/merge_pr.py | 19 ++++++++++++++++++- 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 9f16e32707e..e0eda476d19 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -130,6 +130,14 @@ jobs: with: stage: Tests_2 data: ${{ needs.RunConfig.outputs.data }} + # stage for jobs that do not prohibit merge + Tests_3: + needs: [RunConfig, Tests_1, Tests_2] + if: ${{ !failure() && !cancelled() && contains(fromJson(needs.RunConfig.outputs.data).stages_data.stages_to_do, 'Tests_3') }} + uses: ./.github/workflows/reusable_test_stage.yml + with: + stage: Tests_3 + data: ${{ needs.RunConfig.outputs.data }} ################################# Reports ################################# # Reports should by run even if Builds_1/2 fail, so put them separatly in wf (not in Tests_1/2) @@ -156,15 +164,32 @@ jobs: runner_type: style-checker-aarch64 data: ${{ needs.RunConfig.outputs.data }} + CheckReadyForMerge: + if: ${{ !failure() && !cancelled() }} + needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2] + runs-on: [self-hosted, style-checker-aarch64] + steps: + - name: Check out repository code + uses: ClickHouse/checkout@v1 + with: + filter: tree:0 + - name: Check and set merge status + if: ${{ github.event_name != 'merge_group' }} + run: | + cd "$GITHUB_WORKSPACE/tests/ci" + python3 merge_pr.py --set-status + ################################# Stage Final ################################# # FinishCheck: if: ${{ !failure() && !cancelled() }} - needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2] + needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2, Tests_3, CheckReadyForMerge] runs-on: [self-hosted, style-checker] steps: - name: Check out repository code uses: ClickHouse/checkout@v1 + with: + filter: tree:0 - name: Finish label run: | cd "$GITHUB_WORKSPACE/tests/ci" diff --git a/tests/ci/ci_config.py b/tests/ci/ci_config.py index 588f4934125..6d450a79a69 100644 --- a/tests/ci/ci_config.py +++ b/tests/ci/ci_config.py @@ -25,6 +25,7 @@ class CIStages(metaclass=WithIter): BUILDS_2 = "Builds_2" TESTS_1 = "Tests_1" TESTS_2 = "Tests_2" + TESTS_3 = "Tests_3" class Runners(metaclass=WithIter): @@ -579,7 +580,6 @@ class CIConfig: elif job_name == JobNames.BUILD_CHECK_SPECIAL: stage_type = CIStages.TESTS_2 elif self.is_test_job(job_name): - stage_type = CIStages.TESTS_1 if job_name in CI_CONFIG.test_configs: required_build = CI_CONFIG.test_configs[job_name].required_build assert required_build @@ -591,6 +591,8 @@ class CIConfig: stage_type = CIStages.TESTS_2 else: stage_type = CIStages.TESTS_1 + if job_name not in REQUIRED_CHECKS: + stage_type = CIStages.TESTS_3 assert stage_type, f"BUG [{job_name}]" return stage_type diff --git a/tests/ci/commit_status_helper.py b/tests/ci/commit_status_helper.py index 0b51d98b479..0ca25f39976 100644 --- a/tests/ci/commit_status_helper.py +++ b/tests/ci/commit_status_helper.py @@ -469,7 +469,10 @@ def update_mergeable_check( def trigger_mergeable_check( - commit: Commit, statuses: CommitStatuses, hide_url: bool = False + commit: Commit, + statuses: CommitStatuses, + hide_url: bool = False, + set_if_green: bool = False, ) -> CommitStatus: """calculate and update StatusNames.MERGEABLE""" required_checks = [ @@ -502,6 +505,10 @@ def trigger_mergeable_check( state = FAILURE description = format_description(description) + if not set_if_green and state == SUCCESS: + # do not set green Mergeable Check status + return SUCCESS + if mergeable_status is None or mergeable_status.description != description: return set_mergeable_check(commit, description, state, hide_url) diff --git a/tests/ci/merge_pr.py b/tests/ci/merge_pr.py index 450ece62d4b..519fa5fcebb 100644 --- a/tests/ci/merge_pr.py +++ b/tests/ci/merge_pr.py @@ -13,7 +13,11 @@ from github.PaginatedList import PaginatedList from github.PullRequestReview import PullRequestReview from github.WorkflowRun import WorkflowRun -from commit_status_helper import get_commit_filtered_statuses +from commit_status_helper import ( + get_commit_filtered_statuses, + get_commit, + trigger_mergeable_check, +) from get_robot_token import get_best_robot_token from github_helper import GitHub, NamedUser, PullRequest, Repository from pr_info import PRInfo @@ -173,6 +177,11 @@ def parse_args() -> argparse.Namespace: action="store_true", help="if set, the script won't merge the PR, just check the conditions", ) + parser.add_argument( + "--set-ci-status", + action="store_true", + help="if set, only update/set Mergeable Check status", + ) parser.add_argument( "--check-approved", action="store_true", @@ -226,6 +235,14 @@ def main(): token = args.token or get_best_robot_token() gh = GitHub(token) repo = gh.get_repo(args.repo) + + if args.set_ci_status: + # set mergeable check status and exit + commit = get_commit(gh, args.pr_info.sha) + statuses = get_commit_filtered_statuses(commit) + trigger_mergeable_check(commit, statuses, hide_url=False, set_if_green=True) + return + # An ugly and not nice fix to patch the wrong organization URL, # see https://github.com/PyGithub/PyGithub/issues/2395#issuecomment-1378629710 # pylint: disable=protected-access From d5eac97d458c0177a6d3d4bb2b603ef1d14feed1 Mon Sep 17 00:00:00 2001 From: Max K Date: Sat, 18 May 2024 19:13:34 +0200 Subject: [PATCH 2/6] remove update_mergeable_check_from_ci.py --- .github/workflows/pull_request.yml | 1 - tests/ci/ci.py | 40 +----------------------------- tests/ci/commit_status_helper.py | 16 +++++------- 3 files changed, 7 insertions(+), 50 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index e0eda476d19..a9570bc2674 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -174,7 +174,6 @@ jobs: with: filter: tree:0 - name: Check and set merge status - if: ${{ github.event_name != 'merge_group' }} run: | cd "$GITHUB_WORKSPACE/tests/ci" python3 merge_pr.py --set-status diff --git a/tests/ci/ci.py b/tests/ci/ci.py index 08048564383..3aa8f1bb813 100644 --- a/tests/ci/ci.py +++ b/tests/ci/ci.py @@ -17,7 +17,7 @@ from typing import Any, Dict, List, Optional, Sequence, Set, Tuple, Union import docker_images_helper import upload_result_helper from build_check import get_release_or_pr -from ci_config import CI_CONFIG, Build, CILabels, CIStages, JobNames, StatusNames +from ci_config import CI_CONFIG, Build, CILabels, CIStages, JobNames from ci_utils import GHActions, is_hex, normalize_string from clickhouse_helper import ( CiLogsCredentials, @@ -34,16 +34,12 @@ from commit_status_helper import ( get_commit, post_commit_status, set_status_comment, - update_mergeable_check, - update_upstream_sync_status, ) from digest_helper import DockerDigester, JobDigester from env_helper import ( CI, GITHUB_JOB_API_URL, - GITHUB_REPOSITORY, GITHUB_RUN_URL, - GITHUB_UPSTREAM_REPOSITORY, REPO_COPY, REPORT_PATH, S3_BUILDS_BUCKET, @@ -56,7 +52,6 @@ from github_helper import GitHub from pr_info import PRInfo from report import ERROR, SUCCESS, BuildResult, JobReport from s3_helper import S3Helper -from synchronizer_utils import SYNC_BRANCH_PREFIX from version_helper import get_version_from_repo # pylint: disable=too-many-lines @@ -2189,39 +2184,6 @@ def main() -> int: pr_info, dump_to_file=True, ) - if not pr_info.is_merge_queue: - # in the merge queue mergeable status must be set only in FinishCheck (last job in wf) - mergeable_status = update_mergeable_check( - commit, - pr_info, - job_report.check_name or _get_ext_check_name(args.job_name), - ) - - # Process upstream StatusNames.SYNC - if ( - pr_info.head_ref.startswith(f"{SYNC_BRANCH_PREFIX}/pr/") - and mergeable_status - and GITHUB_REPOSITORY != GITHUB_UPSTREAM_REPOSITORY - ): - upstream_pr_number = int( - pr_info.head_ref.split("/pr/", maxsplit=1)[1] - ) - update_upstream_sync_status( - upstream_pr_number, pr_info.number, gh, mergeable_status - ) - prepared_events = prepare_tests_results_for_clickhouse( - pr_info, - [], - job_report.status, - 0, - job_report.start_time, - f"https://github.com/ClickHouse/ClickHouse/pull/{upstream_pr_number}", - StatusNames.SYNC, - ) - prepared_events[0]["test_context_raw"] = args.job_name - ch_helper.insert_events_into( - db="default", table="checks", events=prepared_events - ) print(f"Job report url: [{check_url}]") prepared_events = prepare_tests_results_for_clickhouse( diff --git a/tests/ci/commit_status_helper.py b/tests/ci/commit_status_helper.py index 0ca25f39976..2ba6fba8b83 100644 --- a/tests/ci/commit_status_helper.py +++ b/tests/ci/commit_status_helper.py @@ -447,9 +447,7 @@ def set_mergeable_check( ) -def update_mergeable_check( - commit: Commit, pr_info: PRInfo, check_name: str -) -> Optional[CommitStatus]: +def update_mergeable_check(commit: Commit, pr_info: PRInfo, check_name: str) -> None: "check if the check_name in REQUIRED_CHECKS and then trigger update" not_run = ( pr_info.labels.intersection({Labels.SKIP_MERGEABLE_CHECK, Labels.RELEASE}) @@ -460,12 +458,12 @@ def update_mergeable_check( if not_run: # Let's avoid unnecessary work - return None + return logging.info("Update Mergeable Check by %s", check_name) statuses = get_commit_filtered_statuses(commit) - return trigger_mergeable_check(commit, statuses) + trigger_mergeable_check(commit, statuses) def trigger_mergeable_check( @@ -473,7 +471,7 @@ def trigger_mergeable_check( statuses: CommitStatuses, hide_url: bool = False, set_if_green: bool = False, -) -> CommitStatus: +) -> None: """calculate and update StatusNames.MERGEABLE""" required_checks = [ status for status in statuses if status.context in REQUIRED_CHECKS @@ -507,12 +505,10 @@ def trigger_mergeable_check( if not set_if_green and state == SUCCESS: # do not set green Mergeable Check status - return SUCCESS + return if mergeable_status is None or mergeable_status.description != description: - return set_mergeable_check(commit, description, state, hide_url) - - return mergeable_status + set_mergeable_check(commit, description, state, hide_url) def update_upstream_sync_status( From 4e4d078786a6a7a22b1c6d2190c8a414614cafd2 Mon Sep 17 00:00:00 2001 From: Max K Date: Sat, 18 May 2024 20:36:04 +0200 Subject: [PATCH 3/6] return sync pr status to upstream from FinishCheck job --- .github/workflows/pull_request.yml | 4 ++-- tests/ci/commit_status_helper.py | 18 ++++++++++-------- tests/ci/finish_check.py | 13 ++++++++++++- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index a9570bc2674..f2e4b5f328d 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -165,7 +165,7 @@ jobs: data: ${{ needs.RunConfig.outputs.data }} CheckReadyForMerge: - if: ${{ !failure() && !cancelled() }} + if: ${{ !cancelled() }} needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2] runs-on: [self-hosted, style-checker-aarch64] steps: @@ -176,7 +176,7 @@ jobs: - name: Check and set merge status run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 merge_pr.py --set-status + python3 merge_pr.py --set-ci-status ################################# Stage Final ################################# # diff --git a/tests/ci/commit_status_helper.py b/tests/ci/commit_status_helper.py index 2ba6fba8b83..ec9746e9af9 100644 --- a/tests/ci/commit_status_helper.py +++ b/tests/ci/commit_status_helper.py @@ -471,7 +471,7 @@ def trigger_mergeable_check( statuses: CommitStatuses, hide_url: bool = False, set_if_green: bool = False, -) -> None: +) -> StatusType: """calculate and update StatusNames.MERGEABLE""" required_checks = [ status for status in statuses if status.context in REQUIRED_CHECKS @@ -505,17 +505,19 @@ def trigger_mergeable_check( if not set_if_green and state == SUCCESS: # do not set green Mergeable Check status - return + pass + else: + if mergeable_status is None or mergeable_status.description != description: + set_mergeable_check(commit, description, state, hide_url) - if mergeable_status is None or mergeable_status.description != description: - set_mergeable_check(commit, description, state, hide_url) + return state def update_upstream_sync_status( upstream_pr_number: int, sync_pr_number: int, gh: Github, - mergeable_status: CommitStatus, + state: StatusType, ) -> None: upstream_repo = gh.get_repo(GITHUB_UPSTREAM_REPOSITORY) upstream_pr = upstream_repo.get_pull(upstream_pr_number) @@ -546,19 +548,19 @@ def update_upstream_sync_status( ) return - sync_status = get_status(mergeable_status.state) + sync_status = get_status(state) logging.info( "Using commit %s to post the %s status `%s`: [%s]", upstream_commit.sha, sync_status, StatusNames.SYNC, - mergeable_status.description, + "", ) post_commit_status( upstream_commit, sync_status, "", # let's won't expose any urls from cloud - mergeable_status.description, + "", StatusNames.SYNC, ) trigger_mergeable_check( diff --git a/tests/ci/finish_check.py b/tests/ci/finish_check.py index a66ebbeadf4..615b26b51f0 100644 --- a/tests/ci/finish_check.py +++ b/tests/ci/finish_check.py @@ -11,10 +11,13 @@ from commit_status_helper import ( post_commit_status, set_mergeable_check, trigger_mergeable_check, + update_upstream_sync_status, ) from get_robot_token import get_best_robot_token from pr_info import PRInfo from report import PENDING, SUCCESS +from synchronizer_utils import SYNC_BRANCH_PREFIX +from tests.ci.env_helper import GITHUB_REPOSITORY, GITHUB_UPSTREAM_REPOSITORY def main(): @@ -40,7 +43,15 @@ def main(): set_mergeable_check(commit, "workflow passed", "success") else: statuses = get_commit_filtered_statuses(commit) - trigger_mergeable_check(commit, statuses) + state = trigger_mergeable_check(commit, statuses, set_if_green=True) + + # Process upstream StatusNames.SYNC + if ( + pr_info.head_ref.startswith(f"{SYNC_BRANCH_PREFIX}/pr/") + and GITHUB_REPOSITORY != GITHUB_UPSTREAM_REPOSITORY + ): + upstream_pr_number = int(pr_info.head_ref.split("/pr/", maxsplit=1)[1]) + update_upstream_sync_status(upstream_pr_number, pr_info.number, gh, state) statuses = [s for s in statuses if s.context == StatusNames.CI] if not statuses: From 5698ef698d20c12d83fa7f685cbfee9352b4583d Mon Sep 17 00:00:00 2001 From: Max K Date: Sat, 18 May 2024 21:17:22 +0200 Subject: [PATCH 4/6] check overall wf status in mergeable check --- .github/workflows/pull_request.yml | 18 +++++++----------- tests/ci/ci.py | 8 ++++---- tests/ci/commit_status_helper.py | 4 ++++ tests/ci/finish_check.py | 2 +- tests/ci/merge_pr.py | 15 ++++++++++++++- 5 files changed, 30 insertions(+), 17 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index f2e4b5f328d..21c2e48677d 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -140,13 +140,11 @@ jobs: data: ${{ needs.RunConfig.outputs.data }} ################################# Reports ################################# - # Reports should by run even if Builds_1/2 fail, so put them separatly in wf (not in Tests_1/2) + # Reports should by run even if Builds_1/2 fail, so put them separately in wf (not in Tests_1/2) Builds_1_Report: # run report check for failed builds to indicate the CI error - if: ${{ !cancelled() && contains(fromJson(needs.RunConfig.outputs.data).jobs_data.jobs_to_do, 'ClickHouse build check') }} - needs: - - RunConfig - - Builds_1 + if: ${{ !cancelled() && needs.StyleCheck.result == 'success' && contains(fromJson(needs.RunConfig.outputs.data).jobs_data.jobs_to_do, 'ClickHouse build check') }} + needs: [RunConfig, StyleCheck, Builds_1] uses: ./.github/workflows/reusable_test.yml with: test_name: ClickHouse build check @@ -154,10 +152,8 @@ jobs: data: ${{ needs.RunConfig.outputs.data }} Builds_2_Report: # run report check for failed builds to indicate the CI error - if: ${{ !cancelled() && contains(fromJson(needs.RunConfig.outputs.data).jobs_data.jobs_to_do, 'ClickHouse special build check') }} - needs: - - RunConfig - - Builds_2 + if: ${{ !cancelled() && needs.StyleCheck.result == 'success' && contains(fromJson(needs.RunConfig.outputs.data).jobs_data.jobs_to_do, 'ClickHouse special build check') }} + needs: [RunConfig, StyleCheck, Builds_2] uses: ./.github/workflows/reusable_test.yml with: test_name: ClickHouse special build check @@ -165,7 +161,7 @@ jobs: data: ${{ needs.RunConfig.outputs.data }} CheckReadyForMerge: - if: ${{ !cancelled() }} + if: ${{ !cancelled() && needs.StyleCheck.result == 'success' }} needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2] runs-on: [self-hosted, style-checker-aarch64] steps: @@ -176,7 +172,7 @@ jobs: - name: Check and set merge status run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 merge_pr.py --set-ci-status + python3 merge_pr.py --set-ci-status --wf-status ${{ contains(needs.*.result, 'failure') && 'failure' || 'success' }} ################################# Stage Final ################################# # diff --git a/tests/ci/ci.py b/tests/ci/ci.py index 3aa8f1bb813..3a616c8aad6 100644 --- a/tests/ci/ci.py +++ b/tests/ci/ci.py @@ -886,9 +886,9 @@ class CiOptions: for job in job_with_parents: if job in jobs_to_do and job not in jobs_to_do_requested: jobs_to_do_requested.append(job) - assert ( - jobs_to_do_requested - ), f"Include tags are set but no job configured - Invalid tags, probably [{self.include_keywords}]" + print( + f"WARNING: Include tags are set but no job configured - Invalid tags, probably [{self.include_keywords}]" + ) if JobNames.STYLE_CHECK not in jobs_to_do_requested: # Style check must not be omitted jobs_to_do_requested.append(JobNames.STYLE_CHECK) @@ -898,7 +898,7 @@ class CiOptions: if self.ci_sets: for tag in self.ci_sets: label_config = CI_CONFIG.get_label_config(tag) - assert label_config, f"Unknonwn tag [{tag}]" + assert label_config, f"Unknown tag [{tag}]" print( f"NOTE: CI Set's tag: [{tag}], add jobs: [{label_config.run_jobs}]" ) diff --git a/tests/ci/commit_status_helper.py b/tests/ci/commit_status_helper.py index ec9746e9af9..733b07813a5 100644 --- a/tests/ci/commit_status_helper.py +++ b/tests/ci/commit_status_helper.py @@ -471,6 +471,7 @@ def trigger_mergeable_check( statuses: CommitStatuses, hide_url: bool = False, set_if_green: bool = False, + workflow_failed: bool = False, ) -> StatusType: """calculate and update StatusNames.MERGEABLE""" required_checks = [ @@ -501,6 +502,9 @@ def trigger_mergeable_check( if fail: description = "failed: " + ", ".join(fail) state = FAILURE + elif workflow_failed: + description = "check workflow failures" + state = FAILURE description = format_description(description) if not set_if_green and state == SUCCESS: diff --git a/tests/ci/finish_check.py b/tests/ci/finish_check.py index 615b26b51f0..b31be7654d3 100644 --- a/tests/ci/finish_check.py +++ b/tests/ci/finish_check.py @@ -17,7 +17,7 @@ from get_robot_token import get_best_robot_token from pr_info import PRInfo from report import PENDING, SUCCESS from synchronizer_utils import SYNC_BRANCH_PREFIX -from tests.ci.env_helper import GITHUB_REPOSITORY, GITHUB_UPSTREAM_REPOSITORY +from env_helper import GITHUB_REPOSITORY, GITHUB_UPSTREAM_REPOSITORY def main(): diff --git a/tests/ci/merge_pr.py b/tests/ci/merge_pr.py index 519fa5fcebb..500de4eb718 100644 --- a/tests/ci/merge_pr.py +++ b/tests/ci/merge_pr.py @@ -182,6 +182,12 @@ def parse_args() -> argparse.Namespace: action="store_true", help="if set, only update/set Mergeable Check status", ) + parser.add_argument( + "--wf-status", + type=str, + default="", + help="overall workflow status [success|failure]. used with --set-ci-status only", + ) parser.add_argument( "--check-approved", action="store_true", @@ -237,10 +243,17 @@ def main(): repo = gh.get_repo(args.repo) if args.set_ci_status: + assert args.wf_status in ("failure", "success") # set mergeable check status and exit commit = get_commit(gh, args.pr_info.sha) statuses = get_commit_filtered_statuses(commit) - trigger_mergeable_check(commit, statuses, hide_url=False, set_if_green=True) + trigger_mergeable_check( + commit, + statuses, + hide_url=False, + set_if_green=True, + workflow_failed=(args.wf_status != "success"), + ) return # An ugly and not nice fix to patch the wrong organization URL, From c4f47b907182e58c12dcf7934e4745ad8048788d Mon Sep 17 00:00:00 2001 From: Max Kainov Date: Mon, 20 May 2024 14:09:17 +0000 Subject: [PATCH 5/6] set greem mergeable status in upstream pr from finish check in sync --- tests/ci/commit_status_helper.py | 2 ++ tests/ci/finish_check.py | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/ci/commit_status_helper.py b/tests/ci/commit_status_helper.py index 733b07813a5..34e2d9f8085 100644 --- a/tests/ci/commit_status_helper.py +++ b/tests/ci/commit_status_helper.py @@ -522,6 +522,7 @@ def update_upstream_sync_status( sync_pr_number: int, gh: Github, state: StatusType, + can_set_green_mergeable_status: bool = False, ) -> None: upstream_repo = gh.get_repo(GITHUB_UPSTREAM_REPOSITORY) upstream_pr = upstream_repo.get_pull(upstream_pr_number) @@ -571,4 +572,5 @@ def update_upstream_sync_status( upstream_commit, get_commit_filtered_statuses(upstream_commit), True, + set_if_green=can_set_green_mergeable_status, ) diff --git a/tests/ci/finish_check.py b/tests/ci/finish_check.py index b31be7654d3..1a7000f5353 100644 --- a/tests/ci/finish_check.py +++ b/tests/ci/finish_check.py @@ -51,7 +51,13 @@ def main(): and GITHUB_REPOSITORY != GITHUB_UPSTREAM_REPOSITORY ): upstream_pr_number = int(pr_info.head_ref.split("/pr/", maxsplit=1)[1]) - update_upstream_sync_status(upstream_pr_number, pr_info.number, gh, state) + update_upstream_sync_status( + upstream_pr_number, + pr_info.number, + gh, + state, + can_set_green_mergeable_status=True, + ) statuses = [s for s in statuses if s.context == StatusNames.CI] if not statuses: From a735ab7dd1f46e4e6fbcd529efafaada5b70afa5 Mon Sep 17 00:00:00 2001 From: Max Kainov Date: Mon, 20 May 2024 18:58:56 +0000 Subject: [PATCH 6/6] fix upstream commit status update in sync pr --- .github/workflows/pull_request.yml | 2 +- tests/ci/commit_status_helper.py | 36 +++++++++++++----------------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 21c2e48677d..f20e987db97 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -178,7 +178,7 @@ jobs: # FinishCheck: if: ${{ !failure() && !cancelled() }} - needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2, Tests_3, CheckReadyForMerge] + needs: [RunConfig, BuildDockers, StyleCheck, FastTest, Builds_1, Builds_2, Builds_1_Report, Builds_2_Report, Tests_1, Tests_2, Tests_3] runs-on: [self-hosted, style-checker] steps: - name: Check out repository code diff --git a/tests/ci/commit_status_helper.py b/tests/ci/commit_status_helper.py index 34e2d9f8085..bbda97b9084 100644 --- a/tests/ci/commit_status_helper.py +++ b/tests/ci/commit_status_helper.py @@ -530,47 +530,41 @@ def update_upstream_sync_status( sync_pr = sync_repo.get_pull(sync_pr_number) # Find the commit that is in both repos, upstream and cloud sync_commits = sync_pr.get_commits().reversed - upstream_commits = upstream_pr.get_commits() + upstream_commits = upstream_pr.get_commits().reversed # Github objects are compared by _url attribute. We can't compare them directly and # should compare commits by SHA1 - upstream_shas = [uc.sha for uc in upstream_commits] + upstream_shas = [c.sha for c in upstream_commits] logging.info("Commits in upstream PR:\n %s", ", ".join(upstream_shas)) - sync_shas = [uc.sha for uc in upstream_commits] + sync_shas = [c.sha for c in sync_commits] logging.info("Commits in sync PR:\n %s", ", ".join(reversed(sync_shas))) - found = False - for commit in sync_commits: - try: - idx = upstream_shas.index(commit.sha) - found = True - upstream_commit = upstream_commits[idx] - break - except ValueError: - continue - if not found: - logging.info( - "There's no same commits in upstream and sync PRs, probably force-push" - ) - return + # find latest synced commit + last_synced_upstream_commit = None + for commit in upstream_commits: + if commit.sha in sync_shas: + last_synced_upstream_commit = commit + break + + assert last_synced_upstream_commit sync_status = get_status(state) logging.info( "Using commit %s to post the %s status `%s`: [%s]", - upstream_commit.sha, + last_synced_upstream_commit.sha, sync_status, StatusNames.SYNC, "", ) post_commit_status( - upstream_commit, + last_synced_upstream_commit, sync_status, "", # let's won't expose any urls from cloud "", StatusNames.SYNC, ) trigger_mergeable_check( - upstream_commit, - get_commit_filtered_statuses(upstream_commit), + last_synced_upstream_commit, + get_commit_filtered_statuses(last_synced_upstream_commit), True, set_if_green=can_set_green_mergeable_status, )