From a4b55985fafabf28c2eded19eca56e75367e8f15 Mon Sep 17 00:00:00 2001
From: "Mikhail f. Shiryaev" <felixoid@clickhouse.com>
Date: Thu, 6 Jun 2024 07:53:41 +0200
Subject: [PATCH 1/2] Fix issue with overwriting a lack of documentation
 failure

---
 tests/ci/run_check.py | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/ci/run_check.py b/tests/ci/run_check.py
index 9d9d1433073..064c1563913 100644
--- a/tests/ci/run_check.py
+++ b/tests/ci/run_check.py
@@ -23,7 +23,7 @@ from lambda_shared_package.lambda_shared.pr import (
     check_pr_description,
 )
 from pr_info import PRInfo
-from report import FAILURE, PENDING, SUCCESS
+from report import FAILURE, PENDING, SUCCESS, StatusType
 
 TRUSTED_ORG_IDS = {
     54801242,  # clickhouse
@@ -93,6 +93,7 @@ def main():
     description = format_description(description)
     gh = Github(get_best_robot_token(), per_page=100)
     commit = get_commit(gh, pr_info.sha)
+    status = SUCCESS  # type: StatusType
 
     description_error, category = check_pr_description(pr_info.body, GITHUB_REPOSITORY)
     pr_labels_to_add = []
@@ -132,6 +133,7 @@ def main():
     if pr_labels_to_remove:
         remove_labels(gh, pr_info, pr_labels_to_remove)
 
+    # 1. Next three IFs are in a correct order. First - fatal error
     if description_error:
         print(
             "::error ::Cannot run, PR description does not match the template: "
@@ -146,9 +148,10 @@ def main():
             f"{GITHUB_SERVER_URL}/{GITHUB_REPOSITORY}/"
             "blob/master/.github/PULL_REQUEST_TEMPLATE.md?plain=1"
         )
+        status = FAILURE
         post_commit_status(
             commit,
-            FAILURE,
+            status,
             url,
             format_description(description_error),
             PR_CHECK,
@@ -156,6 +159,7 @@ def main():
         )
         sys.exit(1)
 
+    # 2. Then we check if the documentation is not created to fail the Mergeable check
     if (
         Labels.PR_FEATURE in pr_info.labels
         and not pr_info.has_changes_in_documentation()
@@ -164,20 +168,15 @@ def main():
             f"The '{Labels.PR_FEATURE}' in the labels, "
             "but there's no changed documentation"
         )
-        post_commit_status(
-            commit,
-            FAILURE,
-            "",
-            f"expect adding docs for {Labels.PR_FEATURE}",
-            PR_CHECK,
-            pr_info,
-        )
-        # allow the workflow to continue
+        status = FAILURE
+        description = f"expect adding docs for {Labels.PR_FEATURE}"
+    # 3. But we allow the workflow to continue
 
+    # 4. And post only a single commit status on a failure
     if not can_run:
         post_commit_status(
             commit,
-            FAILURE,
+            status,
             "",
             description,
             PR_CHECK,
@@ -186,11 +185,12 @@ def main():
         print("::notice ::Cannot run")
         sys.exit(1)
 
+    # The status for continue can be posted only one time, not more.
     post_commit_status(
         commit,
-        SUCCESS,
+        status,
         "",
-        "ok",
+        description,
         PR_CHECK,
         pr_info,
     )

From 61582a9bee43d75ab24eb357cc30045dd03b6b36 Mon Sep 17 00:00:00 2001
From: "Mikhail f. Shiryaev" <felixoid@clickhouse.com>
Date: Thu, 6 Jun 2024 07:54:38 +0200
Subject: [PATCH 2/2] Use logging where no special strings are required

---
 tests/ci/run_check.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/ci/run_check.py b/tests/ci/run_check.py
index 064c1563913..131cbeef786 100644
--- a/tests/ci/run_check.py
+++ b/tests/ci/run_check.py
@@ -58,7 +58,7 @@ def pr_is_by_trusted_user(pr_user_login, pr_user_orgs):
 # Returns can_run, description
 def should_run_ci_for_pr(pr_info: PRInfo) -> Tuple[bool, str]:
     # Consider the labels and whether the user is trusted.
-    print("Got labels", pr_info.labels)
+    logging.info("Got labels: %s", pr_info.labels)
 
     if OK_SKIP_LABELS.intersection(pr_info.labels):
         return True, "Don't try new checks for release/backports/cherry-picks"
@@ -66,9 +66,10 @@ def should_run_ci_for_pr(pr_info: PRInfo) -> Tuple[bool, str]:
     if Labels.CAN_BE_TESTED not in pr_info.labels and not pr_is_by_trusted_user(
         pr_info.user_login, pr_info.user_orgs
     ):
-        print(
-            f"PRs by untrusted users need the '{Labels.CAN_BE_TESTED}' label - "
-            "please contact a member of the core team"
+        logging.info(
+            "PRs by untrusted users need the '%s' label - "
+            "please contact a member of the core team",
+            Labels.CAN_BE_TESTED,
         )
         return False, "Needs 'can be tested' label"
 
@@ -126,7 +127,9 @@ def main():
             f"::notice :: Add backport labels [{backport_labels}] for a given PR category"
         )
 
-    print(f"Change labels: add {pr_labels_to_add}, remove {pr_labels_to_remove}")
+    logging.info(
+        "Change labels: add %s, remove %s", pr_labels_to_add, pr_labels_to_remove
+    )
     if pr_labels_to_add:
         post_labels(gh, pr_info, pr_labels_to_add)
 
@@ -165,7 +168,7 @@ def main():
         and not pr_info.has_changes_in_documentation()
     ):
         print(
-            f"The '{Labels.PR_FEATURE}' in the labels, "
+            f"::error ::The '{Labels.PR_FEATURE}' in the labels, "
             "but there's no changed documentation"
         )
         status = FAILURE
@@ -182,7 +185,7 @@ def main():
             PR_CHECK,
             pr_info,
         )
-        print("::notice ::Cannot run")
+        print("::error ::Cannot run")
         sys.exit(1)
 
     # The status for continue can be posted only one time, not more.