From dec35513e234d9c109628bc4993a0ec3ca6a0631 Mon Sep 17 00:00:00 2001 From: Ivan Razumov Date: Tue, 10 Dec 2024 18:24:21 +0100 Subject: [PATCH] Update draft handling and tests --- .gitignore | 5 +- process_pr.py | 94 ++++++++++++------- .../TestProcessPr.test_assign.json | 2 +- .../TestProcessPr.test_close.json | 8 +- .../TestProcessPr.test_draft_pr_updated.json | 4 + .../TestProcessPr.test_reopen.json | 8 +- tests/run_pr_tests.sh | 1 + 7 files changed, 79 insertions(+), 43 deletions(-) diff --git a/.gitignore b/.gitignore index b32d8aa7acca..dd96c08ca9ec 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,5 @@ *.pyc -.idea \ No newline at end of file +.idea +*.properties +tests/Framework.py +venv diff --git a/process_pr.py b/process_pr.py index 9130b39244a9..b97624d3004b 100644 --- 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 here\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 here\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 here\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 here\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 index 18535f999efa..4ba33c6fed47 100644 --- a/tests/PRActionData/TestProcessPr.test_assign.json +++ b/tests/PRActionData/TestProcessPr.test_assign.json @@ -107,4 +107,4 @@ "context": "bot/17/ack" } } -] +] \ No newline at end of file diff --git a/tests/PRActionData/TestProcessPr.test_close.json b/tests/PRActionData/TestProcessPr.test_close.json index 30a05265dbae..3b99c6ff4a42 100644 --- 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" diff --git a/tests/PRActionData/TestProcessPr.test_draft_pr_updated.json b/tests/PRActionData/TestProcessPr.test_draft_pr_updated.json index b2fd5a10e10f..95d3dd883a5d 100644 --- 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" diff --git a/tests/PRActionData/TestProcessPr.test_reopen.json b/tests/PRActionData/TestProcessPr.test_reopen.json index 430c32382e78..7cea3e6ad936 100644 --- 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" diff --git a/tests/run_pr_tests.sh b/tests/run_pr_tests.sh index f0d424c229a1..60405ae36150 100755 --- 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