Update draft handling and tests

cms-sw · Dec 10, 2024 · dec3551 · dec3551
1 parent 3a04898
commit dec3551
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 43 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,5 @@
 *.pyc
-.idea
+.idea
+*.properties
+tests/Framework.py
+venv
diff --git a/process_pr.py b/process_pr.py
@@ -16,6 +16,7 @@
     NEW_ISSUE_PREFIX,
     NEW_PR_PREFIX,
     ISSUE_SEEN_MSG,
+    ISSUE_UPDATED_MSG,
     BUILD_REL,
     GH_CMSSW_REPO,
     GH_CMSDIST_REPO,
@@ -1487,7 +1488,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
                 IGNORING_TESTS_MSG, first_line
             ):
                 signatures["tests"] = "pending"
-            elif re.match("Pull request ([^ #]+|)[#][0-9]+ was updated[.].*", first_line):
+            elif re.match(ISSUE_UPDATED_MSG, first_line):
                 pull_request_updated = False
             elif re.match(TRIGERING_TESTS_MSG, first_line) or re.match(
                 TRIGERING_TESTS_MSG1, first_line
@@ -1672,6 +1673,13 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
 
         print("Processing commits")
 
+        if not pull_request_updated:
+            _bot_cache_shas = set(bot_cache["commits"].keys())
+            diff = all_commit_shas.difference(_bot_cache_shas)
+            if diff:
+                print("Setting pull-request-update to True:", len(diff), "new commit(s)")
+                pull_request_updated = True
+
         # Make sure to mark squashed=False if a cached/squashed commit is added back
         for commit_sha in [
             sha
@@ -2478,40 +2486,62 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
                 pkg_msg.append("- %s (**%s**)" % (pkg, ", ".join(package_categories[pkg])))
             else:
                 pkg_msg.append("- %s (**new**)" % pkg)
-        pkg_msg_s = "\n".join(pkg_msg)
-        messageNewPR = (
-            f"{NEW_PR_PREFIX} {gh_user_char}{pr.user.login} for {pr.base.ref}.\n\n"
-            f"It involves the following packages:\n\n"
-            f"{pkg_msg_s}\n\n"
-            f"{new_package_message}\n"
-            f"{', '.join(missing_notifications)} can you please review it and eventually sign?"
-            f" Thanks.\n"
-            f"{watchersMsg}"
-            f"{releaseManagersMsg}"
-            f"{warning_msg}\n"
-            'cms-bot commands are listed <a href="http://cms-sw.github.io/cms-bot-cmssw-cmds.html">here</a>\n'
+        messageNewPR = format(
+            "%(msgPrefix)s %(gh_user_char)s%(user)s"
+            " for %(branch)s.\n\n"
+            "It involves the following packages:\n\n"
+            "%(packages)s\n\n"
+            "%(new_package_message)s\n"
+            "%(l2s)s can you please review it and eventually sign?"
+            " Thanks.\n"
+            "%(watchers)s"
+            "%(releaseManagers)s"
+            "%(patch_branch_warning)s\n"
+            'cms-bot commands are listed <a href="http://cms-sw.github.io/cms-bot-cmssw-cmds.html">here</a>\n',
+            msgPrefix=NEW_PR_PREFIX,
+            user=pr.user.login,
+            gh_user_char=gh_user_char,
+            branch=pr.base.ref,
+            l2s=", ".join(missing_notifications),
+            packages="\n".join(pkg_msg),
+            new_package_message=new_package_message,
+            watchers=watchersMsg,
+            releaseManagers=releaseManagersMsg,
+            patch_branch_warning=warning_msg,
+        )
+
+        messageUpdatedPR = format(
+            "Pull request #%(pr)s was updated.",
+            pr=pr.number,
         )
 
-        messageUpdatedPR = f"Pull request #{pr.number} was updated. "
         if not is_draft_pr:
-            messageUpdatedPR += (
-                f"{', '.join(missing_notifications)} can you please check and sign again.\n"
+            messageUpdatedPR += format(
+                " %(signers)s can you please check and sign again.\n",
+                signers=", ".join(missing_notifications),
             )
     else:
-        messageNewPR = (
-            f"{NEW_PR_PREFIX} {gh_user_char}{pr.user.login} "
-            f"for branch {pr.base.ref}.\n\n"
-            f"{', '.join(missing_notifications)} can you please review it and eventually sign?"
-            f" Thanks.\n"
-            f"{watchersMsg}"
-            f"{releaseManagersMsg}"
-            'cms-bot commands are listed <a href="http://cms-sw.github.io/cms-bot-cmssw-cmds.html">here</a>\n'
+        messageNewPR = format(
+            "%(msgPrefix)s %(gh_user_char)s%(user)s"
+            " for branch %(branch)s.\n\n"
+            "%(l2s)s can you please review it and eventually sign?"
+            " Thanks.\n"
+            "%(watchers)s"
+            "%(releaseManagers)s"
+            'cms-bot commands are listed <a href="http://cms-sw.github.io/cms-bot-cmssw-cmds.html">here</a>\n',
+            msgPrefix=NEW_PR_PREFIX,
+            user=pr.user.login,
+            gh_user_char=gh_user_char,
+            branch=pr.base.ref,
+            l2s=", ".join(missing_notifications),
+            releaseManagers=releaseManagersMsg,
+            watchers=watchersMsg,
         )
 
-        messageUpdatedPR = f"Pull request #{pr.number} was updated."
+        messageUpdatedPR = format("Pull request #%(pr)s was updated.", pr=pr.number)
 
     # Finally decide whether or not we should close the pull request:
-    messageBranchClosed = (
+    messageBranchClosed = format(
         "This branch is closed for updates."
         " Closing this pull request.\n"
         " Please bring this up in the ORP"
@@ -2522,12 +2552,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:
-            if not is_draft_pr:
-                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:
@@ -2537,7 +2565,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_updated.json b/tests/PRActionData/TestProcessPr.test_draft_pr_updated.json
@@ -61,6 +61,10 @@
             "context": "cms/21/code-checks"
         }
     },
+    {
+        "type": "create-comment",
+        "data": "Pull request #21 was updated."
+    },
     {
         "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\"},\"last_seen_sha\":\"28ce3f5888b5433dcae409ee336b2ad422595246\",\"signatures\":{}} -->"

diff --git a/tests/PRActionData/TestProcessPr.test_reopen.json b/tests/PRActionData/TestProcessPr.test_reopen.json
@@ -100,6 +100,10 @@
             true
         ]
     },
+    {
+        "type": "open",
+        "data": null
+    },
     {
         "type": "emoji",
         "data": [
@@ -116,10 +120,6 @@
         "type": "remove-label",
         "data": []
     },
-    {
-        "type": "open",
-        "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\",\"2056935714\":\"+1\"},\"last_seen_sha\":\"dae848e38b8e387d7283a3e35818121487d9d76b\",\"signatures\":{\"2049242908\":\"2a9454e30606b17e52000110972998326ce9e428\"}} -->"

diff --git a/tests/run_pr_tests.sh b/tests/run_pr_tests.sh
@@ -10,6 +10,7 @@ if [ $INSTALL_REQS -eq 1 ]; then
   python3 -m pip install --upgrade pip
   pip install -r test-requirements.txt
 fi
+
 if [ ! -e Framework.py ]; then
   curl -L https://github.com/PyGithub/PyGithub/raw/v1.54/tests/Framework.py > Framework.py
   sed -i -e 's/self\.retry/self.retry, per_page=100/g' Framework.py
-Original file line number
+Diff line change
@@ Expand Up / @@ -107,4 +107,4 @@ @@
                 "context": "bot/17/ack"
             }
         }
-    ]
+    ]