Merge pull request #2395 from cms-sw/draft-prs

Implement special handling for draft PRs (and add tests)
cms-sw · Dec 16, 2024 · 7d09b46 · 7d09b46
2 parents 5ffdb64 + c25291f
commit 7d09b46
Show file tree

Hide file tree

Showing 23 changed files with 1,378 additions and 33 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,6 @@
 *.pyc
-.idea
+.idea
+*.properties
+tests/Framework.py
+venv
+GithubCredentials.py
diff --git a/githublabels.py b/githublabels.py
@@ -64,6 +64,7 @@ def get_dpg_pog():
     "hold": LABEL_COLORS["hold"],
     "compilation-warnings": LABEL_COLORS["hold"],
     "requires-external": LABEL_COLORS["info"],
+    "fully-signed-draft": LABEL_COLORS["hold"],
 }
 
 for lab in TYPE_COMMANDS:

diff --git a/process_pr.py b/process_pr.py
@@ -884,8 +884,10 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
         return
 
     gh_user_char = "@"
+
     if not notify_user(issue):
         gh_user_char = ""
+
     api_rate_limits(gh)
     prId = issue.number
     repository = repo.full_name
@@ -964,12 +966,17 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
     warned_too_many_commits = False
     ok_too_many_files = False
     warned_too_many_files = False
+    is_draft_pr = False
 
     if issue.pull_request:
         pr = repo.get_pull(prId)
         if pr.changed_files == 0:
             print("Ignoring: PR with no files changed")
             return
+        if pr.draft:
+            print("Draft PR, mentions turned off")
+            is_draft_pr = True
+            gh_user_char = ""
         if cmssw_repo and cms_repo and (pr.base.ref == CMSSW_DEVEL_BRANCH):
             if pr.state != "closed":
                 print("This pull request must go in to master branch")
@@ -1113,6 +1120,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
         print("PR Statuses:", commit_statuses)
         print(len(commit_statuses))
         last_commit_date = last_commit.committer.date
+
         print(
             "Latest commit by ",
             last_commit.committer.name.encode("ascii", "ignore").decode(),
@@ -1216,6 +1224,8 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
             technical_comments.append(comment)
 
     if technical_comments:
+        if is_draft_pr:
+            pull_request_updated = technical_comments[0].created_at < last_commit_date
         bot_cache = extract_bot_cache(technical_comments)
 
     # Make sure bot cache has the needed keys
@@ -2186,7 +2196,10 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
     if missingApprovals:
         labels.append("pending-signatures")
     elif not "pending-assignment" in labels:
-        labels.append("fully-signed")
+        if not is_draft_pr:
+            labels.append("fully-signed")
+        else:
+            labels.append("fully-signed-draft")
     if need_external:
         labels.append("requires-external")
     labels = set(labels)
@@ -2200,6 +2213,9 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
         new_categories.add(nc_lab)
 
     if new_assign_cats:
+        tmp_gh_user_char = gh_user_char
+        if notify_user(issue):
+            gh_user_char = "@"
         new_l2s = [
             gh_user_char + name
             for name, l2_categories in list(CMSSW_L2.items())
@@ -2215,6 +2231,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
                 + ",".join(new_l2s)
                 + " you have been requested to review this Pull request/Issue and eventually sign? Thanks"
             )
+        gh_user_char = tmp_gh_user_char
 
     # update blocker massge
     if new_blocker:
@@ -2353,11 +2370,19 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
                 managers=releaseManagersList,
             )
         elif ("orp" in signatures) and (signatures["orp"] != "approved"):
-            autoMergeMsg = format(
-                "This pull request will now be reviewed by the release team"
-                " before it's merged. %(managers)s (and backports should be raised in the release meeting by the corresponding L2)",
-                managers=releaseManagersList,
-            )
+            if not is_draft_pr:
+                autoMergeMsg = format(
+                    "This pull request will now be reviewed by the release team"
+                    " before it's merged. %(managers)s (and backports should be raised in the release meeting by the corresponding L2)",
+                    managers=releaseManagersList,
+                )
+            else:
+                if ("fully-signed-draft" in labels) and (not "fully-signed-draft" in old_labels):
+                    messageFullySignedDraft = f'@{pr.user.login} if this PR is ready to be reviewed by the release team, please remove the "Draft" status.'
+                    if not dryRun:
+                        issue.create_comment(messageFullySignedDraft)
+                    else:
+                        print("DRY-RUN: not posting comment", messageFullySignedDraft)
 
     devReleaseRelVal = ""
     if (pr.base.ref in RELEASE_BRANCH_PRODUCTION) and (pr.base.ref != "master"):
@@ -2498,11 +2523,15 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
         )
 
         messageUpdatedPR = format(
-            "Pull request #%(pr)s was updated."
-            " %(signers)s can you please check and sign again.\n",
+            "Pull request #%(pr)s was updated.",
             pr=pr.number,
-            signers=", ".join(missing_notifications),
         )
+
+        if not is_draft_pr:
+            messageUpdatedPR += format(
+                " %(signers)s can you please check and sign again.\n",
+                signers=", ".join(missing_notifications),
+            )
     else:
         messageNewPR = format(
             "%(msgPrefix)s %(gh_user_char)s%(user)s"
@@ -2535,11 +2564,10 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
     print("Status: Not see= %s, Updated: %s" % (already_seen, pull_request_updated))
     if is_closed_branch(pr.base.ref) and (pr.state != "closed"):
         commentMsg = messageBranchClosed
-    elif (not already_seen) or pull_request_updated:
-        if not already_seen:
-            commentMsg = messageNewPR
-        else:
-            commentMsg = messageUpdatedPR
+    elif (already_seen or is_draft_pr) and pull_request_updated:
+        commentMsg = messageUpdatedPR
+    elif not already_seen and not is_draft_pr:
+        commentMsg = messageNewPR
     elif new_categories:
         commentMsg = messageUpdatedPR
     elif not missingApprovals:
@@ -2549,7 +2577,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
     if commentMsg and dryRun:
         print("The following comment will be made:")
         try:
-            print(commentMsg.decode("ascii", "replace"))
+            print(commentMsg.encode("ascii", "replace").decode("ascii", "replace"))
         except:
             pass
     for pre_check in pre_checks + extra_pre_checks:

diff --git a/tests/PRActionData/TestProcessPr.test_assign.json b/tests/PRActionData/TestProcessPr.test_assign.json
@@ -107,4 +107,4 @@
             "context": "bot/17/ack"
         }
     }
-]
+]
diff --git a/tests/PRActionData/TestProcessPr.test_close.json b/tests/PRActionData/TestProcessPr.test_close.json
@@ -91,6 +91,10 @@
             true
         ]
     },
+    {
+        "type": "close",
+        "data": null
+    },
     {
         "type": "emoji",
         "data": [
@@ -107,10 +111,6 @@
         "type": "remove-label",
         "data": []
     },
-    {
-        "type": "close",
-        "data": null
-    },
     {
         "type": "edit-comment",
         "data": "cms-bot internal usage<!-- bot cache: {\"commits\":{\"2a9454e30606b17e52000110972998326ce9e428\":{\"files\":[\"Utilities/ReleaseScripts/test/BuildFile.xml\"],\"squashed\":false,\"time\":1711538467},\"79752f053efecad55dde17732259737e621a1f3f\":{\"files\":[\"Utilities/ReleaseScripts/test/BuildFile.xml\"],\"squashed\":false,\"time\":1712828239},\"dae848e38b8e387d7283a3e35818121487d9d76b\":{\"files\":[\"DQMServices/Components/test/dqmiofilecopy.sh\"],\"squashed\":false,\"time\":1712829250},\"e4d069b76c464274bf6e7d7cf9bac2153ed9a903\":{\"files\":[\"DQMServices/Components/test/dqmiofilecopy.sh\"],\"squashed\":false,\"time\":1712819089}},\"emoji\":{\"2049242908\":\"+1\",\"2049536626\":\"+1\",\"2056736344\":\"+1\",\"2056739513\":\"+1\",\"2056740892\":\"+1\",\"2056796593\":\"+1\",\"2056801055\":\"+1\",\"2056820593\":\"+1\",\"2056903278\":\"+1\",\"2056930228\":\"+1\",\"2056934192\":\"+1\"},\"last_seen_sha\":\"dae848e38b8e387d7283a3e35818121487d9d76b\",\"signatures\":{\"2049242908\":\"2a9454e30606b17e52000110972998326ce9e428\"}} -->"

diff --git a/tests/PRActionData/TestProcessPr.test_draft_pr_ask_ready.json b/tests/PRActionData/TestProcessPr.test_draft_pr_ask_ready.json
@@ -0,0 +1,90 @@
+[
+    {
+        "type": "load-bot-cache",
+        "data": {
+            "commits": {
+                "28ce3f5888b5433dcae409ee336b2ad422595246": {
+                    "files": [
+                        "FWCore/Utilities/BuildFile.xml"
+                    ],
+                    "squashed": false,
+                    "time": 1733495641
+                },
+                "9138474754099798ff50cb07287e7caa4786f247": {
+                    "files": [
+                        "FWCore/Utilities/BuildFile.xml"
+                    ],
+                    "squashed": false,
+                    "time": 1733482192
+                }
+            },
+            "emoji": {
+                "2523442237": "+1",
+                "2532358732": "+1",
+                "2545714988": "+1",
+                "2545738731": "+1"
+            },
+            "last_seen_sha": "28ce3f5888b5433dcae409ee336b2ad422595246",
+            "signatures": {
+                "2545714988": "28ce3f5888b5433dcae409ee336b2ad422595246",
+                "2545738731": "28ce3f5888b5433dcae409ee336b2ad422595246"
+            }
+        }
+    },
+    {
+        "type": "emoji",
+        "data": [
+            2523442237,
+            "+1",
+            true
+        ]
+    },
+    {
+        "type": "emoji",
+        "data": [
+            2545714988,
+            "+1",
+            true
+        ]
+    },
+    {
+        "type": "emoji",
+        "data": [
+            2545738731,
+            "+1",
+            true
+        ]
+    },
+    {
+        "type": "add-label",
+        "data": [
+            "fully-signed-draft",
+            "l1-approved"
+        ]
+    },
+    {
+        "type": "remove-label",
+        "data": [
+            "l1-pending",
+            "pending-signatures"
+        ]
+    },
+    {
+        "type": "create-comment",
+        "data": "@iarspider if this PR is ready to be reviewed by the release team, please remove the \"Draft\" status."
+    },
+    {
+        "type": "edit-comment",
+        "data": "cms-bot internal usage<!-- bot cache: {\"commits\":{\"28ce3f5888b5433dcae409ee336b2ad422595246\":{\"files\":[\"FWCore/Utilities/BuildFile.xml\"],\"squashed\":false,\"time\":1733495641},\"9138474754099798ff50cb07287e7caa4786f247\":{\"files\":[\"FWCore/Utilities/BuildFile.xml\"],\"squashed\":false,\"time\":1733482192}},\"emoji\":{\"2523442237\":\"+1\",\"2532358732\":\"+1\",\"2545714988\":\"+1\",\"2545738731\":\"+1\"},\"last_seen_sha\":\"28ce3f5888b5433dcae409ee336b2ad422595246\",\"signatures\":{\"2545714988\":\"28ce3f5888b5433dcae409ee336b2ad422595246\",\"2545738731\":\"28ce3f5888b5433dcae409ee336b2ad422595246\"}} -->"
+    },
+    {
+        "type": "status",
+        "data": {
+            "commit": "28ce3f5888b5433dcae409ee336b2ad422595246",
+            "state": "success",
+            "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/21#issuecomment-2545738731",
+            "description": "Comment by iarspider at 2024-12-16 14:13:40 UTC processed.",
+            "context": "bot/21/ack"
+        }
+    }
+]
diff --git a/tests/PRActionData/TestProcessPr.test_draft_pr_assign.json b/tests/PRActionData/TestProcessPr.test_draft_pr_assign.json
@@ -0,0 +1,57 @@
+[
+    {
+        "type": "load-bot-cache",
+        "data": {
+            "commits": {
+                "9138474754099798ff50cb07287e7caa4786f247": {
+                    "files": [
+                        "FWCore/Utilities/BuildFile.xml"
+                    ],
+                    "squashed": false,
+                    "time": 1733482192
+                }
+            },
+            "emoji": {
+                "2523442237": "+1"
+            },
+            "last_seen_sha": "9138474754099798ff50cb07287e7caa4786f247",
+            "signatures": {}
+        }
+    },
+    {
+        "type": "emoji",
+        "data": [
+            2523442237,
+            "+1",
+            true
+        ]
+    },
+    {
+        "type": "create-comment",
+        "data": "New categories assigned: l1\n\n@aloeliger,@epalencia you have been requested to review this Pull request/Issue and eventually sign? Thanks"
+    },
+    {
+        "type": "add-label",
+        "data": [
+            "l1-pending"
+        ]
+    },
+    {
+        "type": "remove-label",
+        "data": []
+    },
+    {
+        "type": "edit-comment",
+        "data": "cms-bot internal usage<!-- bot cache: {\"commits\":{\"9138474754099798ff50cb07287e7caa4786f247\":{\"files\":[\"FWCore/Utilities/BuildFile.xml\"],\"squashed\":false,\"time\":1733482192}},\"emoji\":{\"2523442237\":\"+1\"},\"last_seen_sha\":\"9138474754099798ff50cb07287e7caa4786f247\",\"signatures\":{}} -->"
+    },
+    {
+        "type": "status",
+        "data": {
+            "commit": "9138474754099798ff50cb07287e7caa4786f247",
+            "state": "success",
+            "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/21#issuecomment-2523442237",
+            "description": "Comment by iarspider at 2024-12-06 15:01:51 UTC processed.",
+            "context": "bot/21/ack"
+        }
+    }
+]
-Original file line number
+Diff line change
@@ Expand Up / @@ -107,4 +107,4 @@ @@
                 "context": "bot/17/ack"
             }
         }
-    ]
+    ]