diff --git a/.github/workflows/test-process-pr.yaml b/.github/workflows/test-process-pr.yaml new file mode 100644 index 000000000000..aa2389d8e3ab --- /dev/null +++ b/.github/workflows/test-process-pr.yaml @@ -0,0 +1,40 @@ +name: Test changes to process_pr.py + +on: + push: + paths: + - process_pr.py + - .github/workflows/test-process-pr.yaml + +jobs: + build: + runs-on: ubuntu-20.04 + strategy: + matrix: + python-version: ["3.6", "3.9", "3.10", "3.11"] + + steps: + - uses: MathRobin/timezone-action@v1.1 + with: + timezoneLinux: 'Europe/Paris' + - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + repository: PyGithub/PyGithub + ref: v1.56 + path: tmp + sparse-checkout: tests/Framework.py + sparse-checkout-cone-mode: false + - run: mv tmp/tests/Framework.py tests/ + - run: sed -i -e 's/pool_size=self.pool_size/per_page=100/g' tests/Framework.py + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r tests/test-requirements.txt + - name: Test with pytest + run: | + pytest -v -s tests/test_process_pr.py --auth_with_token diff --git a/github_utils.py b/github_utils.py index 4fe261c284fb..cb07c96d4211 100644 --- a/github_utils.py +++ b/github_utils.py @@ -725,6 +725,10 @@ def set_gh_user(user): GH_USER = user +def get_gh_user(): + return GH_USER + + def get_combined_statuses(commit, repository): get_gh_token(repository) return github_api("/repos/%s/commits/%s/status" % (repository, commit), method="GET") diff --git a/process_pr.py b/process_pr.py index 24245335b11e..9d4a85cb605a 100644 --- a/process_pr.py +++ b/process_pr.py @@ -1,3 +1,5 @@ +import copy + from categories import ( CMSSW_L2, CMSSW_L1, @@ -37,8 +39,7 @@ get_pr_commits_reversed, get_commit, ) -from github_utils import set_comment_emoji, get_comment_emojis, set_gh_user -from github_utils import set_issue_emoji, get_issue_emojis +from github_utils import set_gh_user, get_gh_user from socket import setdefaulttimeout from _py2with3compatibility import run_cmd from json import dumps, dump, load, loads @@ -237,7 +238,7 @@ def read_bot_cache(data): res = loads_maybe_decompress(data) for k, v in BOT_CACHE_TEMPLATE.items(): if k not in res: - res[k] = v + res[k] = copy.deepcopy(v) collect_commit_cache(res) return res @@ -439,6 +440,20 @@ def modify_comment(comment, match, replace, dryRun): return 0 +# github_utils.set_issue_emoji -> https://github.com/PyGithub/PyGithub/blob/v1.56/github/Issue.py#L569 +# github_utils.set_comment_emoji -> https://github.com/PyGithub/PyGithub/blob/v1.56/github/IssueComment.py#L149 +# github_utils.delete_issue_emoji -> https://github.com/PyGithub/PyGithub/blob/v1.56/github/Issue.py#L587 +# github_utils.delete_comment_emoji -> https://github.com/PyGithub/PyGithub/blob/v1.56/github/IssueComment.py#L168 +def set_emoji(repository, comment, emoji, reset_other): + if reset_other: + for e in comment.get_reactions(): + login = e.user.login.encode("ascii", "ignore").decode() + if login == get_gh_user() and e.content != emoji: + comment.delete_reaction(e.id) + + comment.create_reaction(emoji) + + def set_comment_emoji_cache(dryRun, bot_cache, comment, repository, emoji="+1", reset_other=True): if dryRun: return @@ -448,10 +463,7 @@ def set_comment_emoji_cache(dryRun, bot_cache, comment, repository, emoji="+1", or (bot_cache["emoji"][comment_id] != emoji) or (comment.reactions[emoji] == 0) ): - if "Issue.Issue" in str(type(comment)): - set_issue_emoji(comment.number, repository, emoji=emoji, reset_other=reset_other) - else: - set_comment_emoji(comment.id, repository, emoji=emoji, reset_other=reset_other) + set_emoji(repository, comment, emoji, reset_other) bot_cache["emoji"][comment_id] = emoji return @@ -462,11 +474,9 @@ def has_user_emoji(bot_cache, comment, repository, emoji, user): if (comment_id in bot_cache["emoji"]) and (comment.reactions[emoji] > 0): e = bot_cache["emoji"][comment_id] else: - emojis = None - if "Issue.Issue" in str(type(comment)): - emojis = get_issue_emojis(comment.number, repository) - else: - emojis = get_comment_emojis(comment.id, repository) + # github_utils.get_issue_emojis -> https://github.com/PyGithub/PyGithub/blob/v1.56/github/Issue.py#L556 + # github_utils.get_comment_emojis -> https://github.com/PyGithub/PyGithub/blob/v1.56/github/IssueComment.py#L135 + emojis = comment.get_reactions() for x in emojis: if x["user"]["login"].encode("ascii", "ignore").decode() == user: e = x["content"] @@ -831,10 +841,26 @@ def add_nonblocking_labels(chg_files, extra_labels): return +# TODO: remove once we update pygithub +def get_commit_files(repo_, commit): + return (x["filename"] for x in get_commit(repo_.full_name, commit.sha)["files"]) + + +def on_labels_changed(added_labels, removed_labels): + # Placeholder function replaced during testing + pass + + +def fetch_pr_result(url): + e, o = run_cmd("curl -k -s -L --max-time 60 %s" % url) + return e, o + + def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=False): global L2_DATA if (not force) and ignore_issue(repo_config, repo, issue): return + gh_user_char = "@" if not notify_user(issue): gh_user_char = "" @@ -1171,7 +1197,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F # Make sure bot cache has the needed keys for k, v in BOT_CACHE_TEMPLATE.items(): if k not in bot_cache: - bot_cache[k] = v + bot_cache[k] = copy.deepcopy(v) for comment in all_comments: ack_comment = comment @@ -1531,7 +1557,9 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F "{2}, to re-enable processing of this PR, you can write `+commit-count` in a comment. Thanks.".format( pr.commits, TOO_MANY_COMMITS_WARN_THRESHOLD, - ", ".join([gh_user_char + name for name in CMSSW_ISSUES_TRACKERS]), + ", ".join( + sorted([gh_user_char + name for name in CMSSW_ISSUES_TRACKERS]) + ), ) ) else: @@ -1628,7 +1656,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F bot_cache["commits"][commit.sha]["files"] = [] else: bot_cache["commits"][commit.sha]["files"] = sorted( - x["filename"] for x in get_commit(repo.full_name, commit.sha)["files"] + get_commit_files(repo, commit) ) elif len(commit.parents) > 1: @@ -1929,7 +1957,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F + "/pr-result" ) print("PR Result:", url) - e, o = run_cmd("curl -k -s -L --max-time 60 %s" % url) + e, o = fetch_pr_result(url) if e: print(o) raise Exception("System-error: unable to get PR result") @@ -2101,6 +2129,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F print("Blockers:", blockers) print("Changed Labels: added", labels - old_labels, "removed", old_labels - labels) + on_labels_changed(labels - old_labels, old_labels - labels) if old_labels == labels: print("Labels unchanged.") elif not dryRunOrig: @@ -2130,7 +2159,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F backport_msg = "" if backport_pr_num: backport_msg = "%s%s\n" % (BACKPORT_STR, backport_pr_num) - l2s = ", ".join([gh_user_char + name for name in CMSSW_ISSUES_TRACKERS]) + l2s = ", ".join(gh_user_char + name for name in sorted(CMSSW_ISSUES_TRACKERS)) issueMessage = format( "%(msgPrefix)s %(gh_user_char)s%(user)s.\n\n" "%(l2s)s can you please review it and eventually sign/assign?" @@ -2306,23 +2335,28 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F print("DRY-RUN: not posting comment", messageFullySigned) unsigned = [commit_sha for (commit_sha, v) in list(signatures.items()) if v == "pending"] - missing_notifications = [ - gh_user_char + name - for name, l2_categories in list(CMSSW_L2.items()) - for signature in signing_categories - if signature in l2_categories and signature in unsigned and signature not in ["orp"] - ] + missing_notifications = sorted( + list( + set( + gh_user_char + name + for name, l2_categories in list(CMSSW_L2.items()) + for signature in signing_categories + if signature in l2_categories + and signature in unsigned + and signature not in ["orp"] + ) + ) + ) - missing_notifications = set(missing_notifications) # Construct message for the watchers watchersMsg = "" if watchers: watchersMsg = format( "%(watchers)s this is something you requested to" " watch as well.\n", - watchers=", ".join(watchers), + watchers=", ".join(sorted(watchers)), ) # Construct message for the release managers. - managers = ", ".join([gh_user_char + x for x in releaseManagers]) + managers = ", ".join([gh_user_char + x for x in sorted(releaseManagers)]) releaseManagersMsg = "" if releaseManagers: diff --git a/pygithub_wrappers.py b/pygithub_wrappers.py deleted file mode 100644 index dd9159f5f393..000000000000 --- a/pygithub_wrappers.py +++ /dev/null @@ -1,72 +0,0 @@ -import github - -dryRun = False -actions = [] -extra_data = "" - -github__issuecomment__edit = github.IssueComment.IssueComment.edit - - -def comment__edit(self, body): - actions.append({"type": "edit-comment", "data": body}) - if dryRun: - print("DRY RUN: Updating existing comment with text") - print(body.encode("ascii", "ignore").decode()) - else: - return github__issuecomment__edit(self, body) - - -github.IssueComment.IssueComment.edit = comment__edit - - -github__issue__create_comment = github.Issue.Issue.create_comment - - -def issue__create_comment(self, body): - actions.append({"type": "create-comment", "data": body}) - if dryRun: - print("DRY RUN: Creating comment with text") - print(body.encode("ascii", "ignore").decode()) - else: - github__issue__create_comment(self, body) - - -github.Issue.Issue.create_comment = issue__create_comment - - -github__commit__create_status = github.Commit.Commit.create_status - - -def commit__create_status(self, state, target_url=None, description=None, context=None): - actions.append( - { - "type": "status", - "data": { - "commit": commit.sha, - "state": state, - "target_url": target_url, - "description": description, - "context": context, - }, - } - ) - - if target_url is None: - target_url = github.GithubObject.NotSet - - if description is None: - description = github.GithubObject.NotSet - - if context is None: - context = github.GithubObject.NotSet - - if not dryRun: - github__commit__create_status.create_status( - self, state, target_url=target_url, description=description, context=context - ) - else: - print( - "DRY RUN: set commit status state={0}, target_url={1}, description={2}, context={3}".format( - state, target_url, description, context - ) - ) diff --git a/tests/Framework.py b/tests/Framework.py deleted file mode 100644 index fd9ebca2213d..000000000000 --- a/tests/Framework.py +++ /dev/null @@ -1,376 +0,0 @@ -############################ Copyrights and license ############################ -# # -# Copyright 2012 Vincent Jacques # -# Copyright 2012 Zearin # -# Copyright 2013 AKFish # -# Copyright 2013 Vincent Jacques # -# Copyright 2014 Vincent Jacques # -# Copyright 2015 Uriel Corfa # -# Copyright 2016 Peter Buckley # -# Copyright 2017 Chris McBride # -# Copyright 2017 Hugo # -# Copyright 2017 Simon # -# Copyright 2018 Jacopo Notarstefano # -# Copyright 2018 Laurent Mazuel # -# Copyright 2018 Mike Miller # -# Copyright 2018 Wan Liuyang # -# Copyright 2018 sfdye # -# # -# This file is part of PyGithub. # -# http://pygithub.readthedocs.io/ # -# # -# PyGithub is free software: you can redistribute it and/or modify it under # -# the terms of the GNU Lesser General Public License as published by the Free # -# Software Foundation, either version 3 of the License, or (at your option) # -# any later version. # -# # -# PyGithub is distributed in the hope that it will be useful, but WITHOUT ANY # -# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS # -# FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more # -# details. # -# # -# You should have received a copy of the GNU Lesser General Public License # -# along with PyGithub. If not, see . # -# # -################################################################################ - -import difflib -import io -import json -import os -import re -import sys -import traceback -import unittest - -import github -import httpretty # type: ignore -import pytest -from requests.structures import CaseInsensitiveDict -from urllib3.util import Url # type: ignore - - -def readLine(file_): - line = file_.readline() - if isinstance(line, bytes): - line = line.decode("utf-8") - return line.strip() - - -class FakeHttpResponse: - def __init__(self, status, headers, output): - self.status = status - self.__headers = headers - self.__output = output - - def getheaders(self): - return self.__headers - - def read(self): - return self.__output - - -def fixAuthorizationHeader(headers): - if "Authorization" in headers: - if headers["Authorization"].endswith("ZmFrZV9sb2dpbjpmYWtlX3Bhc3N3b3Jk"): - # This special case is here to test the real Authorization header - # sent by PyGithub. It would have avoided issue https://github.com/jacquev6/PyGithub/issues/153 - # because we would have seen that Python 3 was not generating the same - # header as Python 2 - pass - elif headers["Authorization"].startswith("token "): - headers["Authorization"] = "token private_token_removed" - elif headers["Authorization"].startswith("Basic "): - headers["Authorization"] = "Basic login_and_password_removed" - elif headers["Authorization"].startswith("Bearer "): - headers["Authorization"] = "Bearer jwt_removed" - - -class RecordingConnection: - def __init__(self, file, protocol, host, port, *args, **kwds): - # write operations make the assumption that the file is not in binary mode - assert isinstance(file, io.TextIOBase) - self.__file = file - self.__protocol = protocol - self.__host = host - self.__port = port - self.__cnx = self._realConnection(host, port, *args, **kwds) - - def request(self, verb, url, input, headers): - self.__cnx.request(verb, url, input, headers) - # fixAuthorizationHeader changes the parameter directly to remove Authorization token. - # however, this is the real dictionary that *will be sent* by "requests", - # since we are writing here *before* doing the actual request. - # So we must avoid changing the real "headers" or this create this: - # https://github.com/PyGithub/PyGithub/pull/664#issuecomment-389964369 - # https://github.com/PyGithub/PyGithub/issues/822 - # Since it's dict[str, str], a simple copy is enough. - anonymous_headers = headers.copy() - fixAuthorizationHeader(anonymous_headers) - self.__writeLine(self.__protocol) - self.__writeLine(verb) - self.__writeLine(self.__host) - self.__writeLine(self.__port) - self.__writeLine(url) - self.__writeLine(anonymous_headers) - self.__writeLine(str(input).replace("\n", "").replace("\r", "")) - - def getresponse(self): - res = self.__cnx.getresponse() - - status = res.status - headers = res.getheaders() - output = res.read() - - self.__writeLine(status) - self.__writeLine(list(headers)) - self.__writeLine(output) - - return FakeHttpResponse(status, headers, output) - - def close(self): - self.__writeLine("") - return self.__cnx.close() - - def __writeLine(self, line): - self.__file.write(str(line) + "\n") - - -class RecordingHttpConnection(RecordingConnection): - _realConnection = github.Requester.HTTPRequestsConnectionClass - - def __init__(self, file, *args, **kwds): - super().__init__(file, "http", *args, **kwds) - - -class RecordingHttpsConnection(RecordingConnection): - _realConnection = github.Requester.HTTPSRequestsConnectionClass - - def __init__(self, file, *args, **kwds): - super().__init__(file, "https", *args, **kwds) - - -class ReplayingConnection: - def __init__(self, file, protocol, host, port, *args, **kwds): - self.__file = file - self.__protocol = protocol - self.__host = host - self.__port = port - self.response_headers = CaseInsensitiveDict() - - self.__cnx = self._realConnection(host, port, *args, **kwds) - - def request(self, verb, url, input, headers): - full_url = Url(scheme=self.__protocol, host=self.__host, port=self.__port, path=url) - - httpretty.register_uri(verb, full_url.url, body=self.__request_callback) - - self.__cnx.request(verb, url, input, headers) - - def __readNextRequest(self, verb, url, input, headers): - fixAuthorizationHeader(headers) - assert self.__protocol == readLine(self.__file) - assert verb == readLine(self.__file) - assert self.__host == readLine(self.__file) - assert str(self.__port) == readLine(self.__file) - assert self.__splitUrl(url) == self.__splitUrl(readLine(self.__file)) - assert headers == eval(readLine(self.__file)) - expectedInput = readLine(self.__file) - if isinstance(input, str): - trInput = input.replace("\n", "").replace("\r", "") - if input.startswith("{"): - assert json.loads(trInput) == json.loads(expectedInput) - else: - assert trInput == expectedInput - else: - # for non-string input (e.g. upload asset), let it pass. - pass - - def __splitUrl(self, url): - splitedUrl = url.split("?") - if len(splitedUrl) == 1: - return splitedUrl - assert len(splitedUrl) == 2 - base, qs = splitedUrl - return (base, sorted(qs.split("&"))) - - def __request_callback(self, request, uri, response_headers): - self.__readNextRequest( - self.__cnx.verb, self.__cnx.url, self.__cnx.input, self.__cnx.headers - ) - - status = int(readLine(self.__file)) - self.response_headers = CaseInsensitiveDict(eval(readLine(self.__file))) - output = bytearray(readLine(self.__file), "utf-8") - readLine(self.__file) - - # make a copy of the headers and remove the ones that interfere with the response handling - adding_headers = CaseInsensitiveDict(self.response_headers) - adding_headers.pop("content-length", None) - adding_headers.pop("transfer-encoding", None) - adding_headers.pop("content-encoding", None) - - response_headers.update(adding_headers) - return [status, response_headers, output] - - def getresponse(self): - # call original connection, this will go all the way down to the python socket and will be intercepted by httpretty - response = self.__cnx.getresponse() - - # restore original headers to the response - response.headers = self.response_headers - - return response - - def close(self): - self.__cnx.close() - - -class ReplayingHttpConnection(ReplayingConnection): - _realConnection = github.Requester.HTTPRequestsConnectionClass - - def __init__(self, file, *args, **kwds): - super().__init__(file, "http", *args, **kwds) - - -class ReplayingHttpsConnection(ReplayingConnection): - _realConnection = github.Requester.HTTPSRequestsConnectionClass - - def __init__(self, file, *args, **kwds): - super().__init__(file, "https", *args, **kwds) - - -class BasicTestCase(unittest.TestCase): - recordMode = False - tokenAuthMode = False - jwtAuthMode = False - retry = None - pool_size = None - replayDataFolder = os.path.join(os.path.dirname(__file__), "ReplayData") - - def setUp(self): - super().setUp() - self.__fileName = "" - self.__file = None - if ( - self.recordMode - ): # pragma no cover (Branch useful only when recording new tests, not used during automated tests) - github.Requester.Requester.injectConnectionClasses( - lambda ignored, *args, **kwds: RecordingHttpConnection( - self.__openFile("w"), *args, **kwds - ), - lambda ignored, *args, **kwds: RecordingHttpsConnection( - self.__openFile("w"), *args, **kwds - ), - ) - import GithubCredentials # type: ignore - - self.login = GithubCredentials.login - self.password = GithubCredentials.password - self.oauth_token = GithubCredentials.oauth_token - self.jwt = GithubCredentials.jwt - else: - github.Requester.Requester.injectConnectionClasses( - lambda ignored, *args, **kwds: ReplayingHttpConnection( - self.__openFile("r"), *args, **kwds - ), - lambda ignored, *args, **kwds: ReplayingHttpsConnection( - self.__openFile("r"), *args, **kwds - ), - ) - self.login = "login" - self.password = "password" - self.oauth_token = "oauth_token" - self.jwt = "jwt" - - httpretty.enable(allow_net_connect=False) - - def tearDown(self): - super().tearDown() - httpretty.disable() - httpretty.reset() - self.__closeReplayFileIfNeeded() - github.Requester.Requester.resetConnectionClasses() - - def __openFile(self, mode): - for _, _, functionName, _ in traceback.extract_stack(): - if ( - functionName.startswith("test") - or functionName == "setUp" - or functionName == "tearDown" - ): - if ( - functionName != "test" - ): # because in class Hook(Framework.TestCase), method testTest calls Hook.test - fileName = os.path.join( - self.replayDataFolder, - f"{self.__class__.__name__}.{functionName}.txt", - ) - if fileName != self.__fileName: - self.__closeReplayFileIfNeeded() - self.__fileName = fileName - self.__file = open(self.__fileName, mode, encoding="utf-8") - return self.__file - - def __closeReplayFileIfNeeded(self): - if self.__file is not None: - if ( - not self.recordMode - ): # pragma no branch (Branch useful only when recording new tests, not used during automated tests) - self.assertEqual(readLine(self.__file), "") - self.__file.close() - - def assertListKeyEqual(self, elements, key, expectedKeys): - realKeys = [key(element) for element in elements] - self.assertEqual(realKeys, expectedKeys) - - def assertListKeyBegin(self, elements, key, expectedKeys): - realKeys = [key(element) for element in elements[: len(expectedKeys)]] - self.assertEqual(realKeys, expectedKeys) - - -class TestCase(BasicTestCase): - def doCheckFrame(self, obj, frame): - if obj._headers == {} and frame is None: - return - if obj._headers is None and frame == {}: - return - self.assertEqual(obj._headers, frame[2]) - - def getFrameChecker(self): - return lambda requester, obj, frame: self.doCheckFrame(obj, frame) - - def setUp(self): - super().setUp() - - # Set up frame debugging - github.GithubObject.GithubObject.setCheckAfterInitFlag(True) - github.Requester.Requester.setDebugFlag(True) - github.Requester.Requester.setOnCheckMe(self.getFrameChecker()) - - if self.tokenAuthMode: - self.g = github.Github(self.oauth_token, retry=self.retry, per_page=100) - elif self.jwtAuthMode: - self.g = github.Github(jwt=self.jwt, retry=self.retry, per_page=100) - else: - self.g = github.Github(self.login, self.password, retry=self.retry, per_page=100) - - -def activateRecordMode(): # pragma no cover (Function useful only when recording new tests, not used during automated tests) - BasicTestCase.recordMode = True - - -def activateTokenAuthMode(): # pragma no cover (Function useful only when recording new tests, not used during automated tests) - BasicTestCase.tokenAuthMode = True - - -def activateJWTAuthMode(): # pragma no cover (Function useful only when recording new tests, not used during automated tests) - BasicTestCase.jwtAuthMode = True - - -def enableRetry(retry): - BasicTestCase.retry = retry - - -def setPoolSize(pool_size): - BasicTestCase.pool_size = pool_size diff --git a/tests/PRActionData/TestProcessPr.test_abort.json b/tests/PRActionData/TestProcessPr.test_abort.json index 67af22412aa6..5a0c691d6979 100644 --- a/tests/PRActionData/TestProcessPr.test_abort.json +++ b/tests/PRActionData/TestProcessPr.test_abort.json @@ -41,7 +41,8 @@ "2056796593": "+1", "2056801055": "+1", "2056820593": "+1", - "2056903278": "+1" + "2056903278": "+1", + "2056930228": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -132,6 +133,10 @@ "context": "bot/17/jenkins" } }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_assign.json b/tests/PRActionData/TestProcessPr.test_assign.json index 8582a519670e..512d9bb2bb6f 100644 --- a/tests/PRActionData/TestProcessPr.test_assign.json +++ b/tests/PRActionData/TestProcessPr.test_assign.json @@ -37,7 +37,8 @@ "2049536626": "+1", "2056736344": "+1", "2056739513": "+1", - "2056740892": "+1" + "2056740892": "+1", + "2056796593": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -92,6 +93,10 @@ "type": "remove-label", "data": [] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_clean_squash.json b/tests/PRActionData/TestProcessPr.test_clean_squash.json index 8d933df5c724..27d49f1c71ff 100644 --- a/tests/PRActionData/TestProcessPr.test_clean_squash.json +++ b/tests/PRActionData/TestProcessPr.test_clean_squash.json @@ -131,16 +131,6 @@ true ] }, - { - "type": "status", - "data": { - "commit": "35f9a4c06b006029da40ed8858e0dae4abd52cb3", - "state": "pending", - "target_url": null, - "description": "Waiting for authorized user to issue the test command.", - "context": "bot/17/jenkins" - } - }, { "type": "add-label", "data": [ @@ -175,36 +165,8 @@ "context": "cms/17/code-checks" } }, - { - "type": "status", - "data": { - "commit": "35f9a4c06b006029da40ed8858e0dae4abd52cb3", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2056820593", - "description": "2056820593:{\"PULL_REQUESTS\": \"cms-sw/cms-bot#2134\", \"SKIP_TESTS\": \"header,static\"}", - "context": "bot/17/test_parameters" - } - }, - { - "type": "emoji", - "data": [ - 2056820593, - "+1", - true - ] - }, { "type": "edit-comment", "data": "cms-bot internal usage" - }, - { - "type": "status", - "data": { - "commit": "35f9a4c06b006029da40ed8858e0dae4abd52cb3", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2056966759", - "description": "Comment by iarspider at 2024-04-15 14:14:24 UTC processed.", - "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 dc869e19a25c..30a05265dbae 100644 --- a/tests/PRActionData/TestProcessPr.test_close.json +++ b/tests/PRActionData/TestProcessPr.test_close.json @@ -42,7 +42,8 @@ "2056801055": "+1", "2056820593": "+1", "2056903278": "+1", - "2056930228": "+1" + "2056930228": "+1", + "2056934192": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -110,6 +111,10 @@ "type": "close", "data": null }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_code_check_approved.json b/tests/PRActionData/TestProcessPr.test_code_check_approved.json index df407db0822b..edd9cebff424 100644 --- a/tests/PRActionData/TestProcessPr.test_code_check_approved.json +++ b/tests/PRActionData/TestProcessPr.test_code_check_approved.json @@ -48,6 +48,10 @@ "context": "cms/17/code-checks" } }, + { + "type": "create-comment", + "data": "A new Pull Request was created by @iarspider for master.\n\nIt involves the following packages:\n\n- Utilities/ReleaseScripts (**core**)\n\n\n@Dr15Jones, @iarspider, @makortel, @smuzaffar can you please review it and eventually sign? Thanks.\n@wddgit this is something you requested to watch as well.\n@iarspider you are the release manager for this.\n\ncms-bot commands are listed here\n" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_dirty_squash.json b/tests/PRActionData/TestProcessPr.test_dirty_squash.json index fe4f83f7558c..388dcb68a6d5 100644 --- a/tests/PRActionData/TestProcessPr.test_dirty_squash.json +++ b/tests/PRActionData/TestProcessPr.test_dirty_squash.json @@ -138,16 +138,6 @@ true ] }, - { - "type": "status", - "data": { - "commit": "1d7419a436293c0337ca346fe868cb50cfdedc18", - "state": "pending", - "target_url": null, - "description": "Waiting for authorized user to issue the test command.", - "context": "bot/17/jenkins" - } - }, { "type": "add-label", "data": [] @@ -178,36 +168,8 @@ "context": "cms/17/code-checks" } }, - { - "type": "status", - "data": { - "commit": "1d7419a436293c0337ca346fe868cb50cfdedc18", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2056820593", - "description": "2056820593:{\"PULL_REQUESTS\": \"cms-sw/cms-bot#2134\", \"SKIP_TESTS\": \"header,static\"}", - "context": "bot/17/test_parameters" - } - }, - { - "type": "emoji", - "data": [ - 2056820593, - "+1", - true - ] - }, { "type": "edit-comment", "data": "cms-bot internal usage" - }, - { - "type": "status", - "data": { - "commit": "1d7419a436293c0337ca346fe868cb50cfdedc18", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2056966759", - "description": "Comment by iarspider at 2024-04-15 14:14:24 UTC processed.", - "context": "bot/17/ack" - } } ] \ No newline at end of file diff --git a/tests/PRActionData/TestProcessPr.test_hold.json b/tests/PRActionData/TestProcessPr.test_hold.json index 1713a8ce5824..75967d51a503 100644 --- a/tests/PRActionData/TestProcessPr.test_hold.json +++ b/tests/PRActionData/TestProcessPr.test_hold.json @@ -35,7 +35,8 @@ "emoji": { "2049242908": "+1", "2049536626": "+1", - "2056736344": "+1" + "2056736344": "+1", + "2056739513": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -73,6 +74,10 @@ "type": "remove-label", "data": [] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_invalid_type.json b/tests/PRActionData/TestProcessPr.test_invalid_type.json index 55b17a40d264..a3de1e8b069b 100644 --- a/tests/PRActionData/TestProcessPr.test_invalid_type.json +++ b/tests/PRActionData/TestProcessPr.test_invalid_type.json @@ -44,7 +44,8 @@ "2056903278": "+1", "2056930228": "+1", "2056934192": "+1", - "2056935714": "+1" + "2056935714": "+1", + "2056946596": "-1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -124,6 +125,10 @@ "type": "remove-label", "data": [] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_many_commits_ok.json b/tests/PRActionData/TestProcessPr.test_many_commits_ok.json index 63911a358c21..419832b48a8c 100644 --- a/tests/PRActionData/TestProcessPr.test_many_commits_ok.json +++ b/tests/PRActionData/TestProcessPr.test_many_commits_ok.json @@ -7,16 +7,6 @@ true ] }, - { - "type": "status", - "data": { - "commit": "fd99bd146e87a00eb90eccef12f487ecc8f4c496", - "state": "pending", - "target_url": null, - "description": "Waiting for authorized user to issue the test command.", - "context": "bot/18/jenkins" - } - }, { "type": "add-label", "data": [ @@ -56,16 +46,6 @@ }, { "type": "create-comment", - "data": "cms-bot internal usage" - }, - { - "type": "status", - "data": { - "commit": "fd99bd146e87a00eb90eccef12f487ecc8f4c496", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/18#issuecomment-2058816831", - "description": "Comment by iarspider at 2024-04-16 10:59:44 UTC processed.", - "context": "bot/18/ack" - } + "data": "cms-bot internal usage" } ] \ No newline at end of file diff --git a/tests/PRActionData/TestProcessPr.test_new_pr.json b/tests/PRActionData/TestProcessPr.test_new_pr.json index 23dfd9e4753d..f29ad4eb6a91 100644 --- a/tests/PRActionData/TestProcessPr.test_new_pr.json +++ b/tests/PRActionData/TestProcessPr.test_new_pr.json @@ -6,16 +6,6 @@ "title": "CMSSW_14_1_X" } }, - { - "type": "status", - "data": { - "commit": "2a9454e30606b17e52000110972998326ce9e428", - "state": "pending", - "target_url": null, - "description": "Waiting for authorized user to issue the test command.", - "context": "bot/17/jenkins" - } - }, { "type": "add-label", "data": [ @@ -55,15 +45,5 @@ { "type": "create-comment", "data": "cms-bot internal usage" - }, - { - "type": "status", - "data": { - "commit": "2a9454e30606b17e52000110972998326ce9e428", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17", - "description": "Comment by iarspider at 2024-03-27 12:22:45 UTC processed.", - "context": "bot/17/ack" - } } ] \ No newline at end of file diff --git a/tests/PRActionData/TestProcessPr.test_partial_reset.json b/tests/PRActionData/TestProcessPr.test_partial_reset.json index e98b97e36b43..250fb6aabc5c 100644 --- a/tests/PRActionData/TestProcessPr.test_partial_reset.json +++ b/tests/PRActionData/TestProcessPr.test_partial_reset.json @@ -35,16 +35,6 @@ true ] }, - { - "type": "status", - "data": { - "commit": "e4d069b76c464274bf6e7d7cf9bac2153ed9a903", - "state": "pending", - "target_url": null, - "description": "Waiting for authorized user to issue the test command.", - "context": "bot/17/jenkins" - } - }, { "type": "add-label", "data": [ @@ -85,15 +75,5 @@ { "type": "edit-comment", "data": "cms-bot internal usage" - }, - { - "type": "status", - "data": { - "commit": "e4d069b76c464274bf6e7d7cf9bac2153ed9a903", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2049247192", - "description": "Comment by iarspider at 2024-04-11 09:03:19 UTC processed.", - "context": "bot/17/ack" - } } ] \ No newline at end of file diff --git a/tests/PRActionData/TestProcessPr.test_reopen.json b/tests/PRActionData/TestProcessPr.test_reopen.json index b2878525a48e..430c32382e78 100644 --- a/tests/PRActionData/TestProcessPr.test_reopen.json +++ b/tests/PRActionData/TestProcessPr.test_reopen.json @@ -43,7 +43,8 @@ "2056820593": "+1", "2056903278": "+1", "2056930228": "+1", - "2056934192": "+1" + "2056934192": "+1", + "2056935714": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -119,6 +120,10 @@ "type": "open", "data": null }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_reset_signature.json b/tests/PRActionData/TestProcessPr.test_reset_signature.json index 54c5a13ebb23..231c16726926 100644 --- a/tests/PRActionData/TestProcessPr.test_reset_signature.json +++ b/tests/PRActionData/TestProcessPr.test_reset_signature.json @@ -42,16 +42,6 @@ true ] }, - { - "type": "status", - "data": { - "commit": "79752f053efecad55dde17732259737e621a1f3f", - "state": "pending", - "target_url": null, - "description": "Waiting for authorized user to issue the test command.", - "context": "bot/17/jenkins" - } - }, { "type": "add-label", "data": [ @@ -89,15 +79,5 @@ { "type": "edit-comment", "data": "cms-bot internal usage" - }, - { - "type": "status", - "data": { - "commit": "79752f053efecad55dde17732259737e621a1f3f", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2049247192", - "description": "Comment by iarspider at 2024-04-11 09:03:19 UTC processed.", - "context": "bot/17/ack" - } } ] \ No newline at end of file diff --git a/tests/PRActionData/TestProcessPr.test_revert.json b/tests/PRActionData/TestProcessPr.test_revert.json index 26488cc26579..b3aea4cacaff 100644 --- a/tests/PRActionData/TestProcessPr.test_revert.json +++ b/tests/PRActionData/TestProcessPr.test_revert.json @@ -49,16 +49,6 @@ true ] }, - { - "type": "status", - "data": { - "commit": "dae848e38b8e387d7283a3e35818121487d9d76b", - "state": "pending", - "target_url": null, - "description": "Waiting for authorized user to issue the test command.", - "context": "bot/17/jenkins" - } - }, { "type": "add-label", "data": [] @@ -94,15 +84,5 @@ { "type": "edit-comment", "data": "cms-bot internal usage" - }, - { - "type": "status", - "data": { - "commit": "dae848e38b8e387d7283a3e35818121487d9d76b", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2049247192", - "description": "Comment by iarspider at 2024-04-11 09:03:19 UTC processed.", - "context": "bot/17/ack" - } } ] \ No newline at end of file diff --git a/tests/PRActionData/TestProcessPr.test_run_test_params.json b/tests/PRActionData/TestProcessPr.test_run_test_params.json index 543a9212e480..3ebc060870c1 100644 --- a/tests/PRActionData/TestProcessPr.test_run_test_params.json +++ b/tests/PRActionData/TestProcessPr.test_run_test_params.json @@ -40,7 +40,8 @@ "2056740892": "+1", "2056796593": "+1", "2056801055": "+1", - "2056820593": "+1" + "2056820593": "+1", + "2056903278": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -135,6 +136,10 @@ true ] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_sign_core.json b/tests/PRActionData/TestProcessPr.test_sign_core.json index 11c5285c14fb..84244bdd6bac 100644 --- a/tests/PRActionData/TestProcessPr.test_sign_core.json +++ b/tests/PRActionData/TestProcessPr.test_sign_core.json @@ -11,7 +11,9 @@ "time": 1711538467 } }, - "emoji": {}, + "emoji": { + "2049242908": "+1" + }, "last_seen_sha": "2a9454e30606b17e52000110972998326ce9e428", "signatures": { "2049242908": "2a9454e30606b17e52000110972998326ce9e428" @@ -46,7 +48,7 @@ }, { "type": "edit-comment", - "data": "cms-bot internal usage" + "data": "cms-bot internal usage" }, { "type": "status", diff --git a/tests/PRActionData/TestProcessPr.test_sign_reject.json b/tests/PRActionData/TestProcessPr.test_sign_reject.json index 451d542922fd..1a86088ecfd5 100644 --- a/tests/PRActionData/TestProcessPr.test_sign_reject.json +++ b/tests/PRActionData/TestProcessPr.test_sign_reject.json @@ -58,7 +58,8 @@ "2056934192": "+1", "2056935714": "+1", "2056946596": "-1", - "2056966759": "+1" + "2056966759": "+1", + "2058758464": "+1" }, "last_seen_sha": "1d7419a436293c0337ca346fe868cb50cfdedc18", "signatures": { @@ -147,16 +148,6 @@ true ] }, - { - "type": "status", - "data": { - "commit": "1d7419a436293c0337ca346fe868cb50cfdedc18", - "state": "pending", - "target_url": null, - "description": "Waiting for authorized user to issue the test command.", - "context": "bot/17/jenkins" - } - }, { "type": "add-label", "data": [ @@ -169,36 +160,8 @@ "core-pending" ] }, - { - "type": "status", - "data": { - "commit": "1d7419a436293c0337ca346fe868cb50cfdedc18", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2056820593", - "description": "2056820593:{\"PULL_REQUESTS\": \"cms-sw/cms-bot#2134\", \"SKIP_TESTS\": \"header,static\"}", - "context": "bot/17/test_parameters" - } - }, - { - "type": "emoji", - "data": [ - 2056820593, - "+1", - true - ] - }, { "type": "edit-comment", - "data": "cms-bot internal usage" - }, - { - "type": "status", - "data": { - "commit": "1d7419a436293c0337ca346fe868cb50cfdedc18", - "state": "success", - "target_url": "https://github.com/iarspider-cmssw/cmssw/pull/17#issuecomment-2058758464", - "description": "Comment by iarspider at 2024-04-16 10:27:25 UTC processed.", - "context": "bot/17/ack" - } + "data": "cms-bot internal usage" } ] \ No newline at end of file diff --git a/tests/PRActionData/TestProcessPr.test_start_tests.json b/tests/PRActionData/TestProcessPr.test_start_tests.json index 9946dbfc7c8f..e0b8ba29ce7a 100644 --- a/tests/PRActionData/TestProcessPr.test_start_tests.json +++ b/tests/PRActionData/TestProcessPr.test_start_tests.json @@ -33,7 +33,8 @@ } }, "emoji": { - "2049242908": "+1" + "2049242908": "+1", + "2049536626": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -110,6 +111,14 @@ "context": "cms/17/code-checks" } }, + { + "type": "create-comment", + "data": "Pull request #17 was updated. @Dr15Jones, @makortel, @smuzaffar can you please check and sign again.\n" + }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_test_params.json b/tests/PRActionData/TestProcessPr.test_test_params.json index 76b731d03624..2ff5f2d0ffc7 100644 --- a/tests/PRActionData/TestProcessPr.test_test_params.json +++ b/tests/PRActionData/TestProcessPr.test_test_params.json @@ -39,7 +39,8 @@ "2056739513": "+1", "2056740892": "+1", "2056796593": "+1", - "2056801055": "+1" + "2056801055": "+1", + "2056820593": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -115,6 +116,10 @@ true ] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_tests_passed.json b/tests/PRActionData/TestProcessPr.test_tests_passed.json index e85e533ce7dd..f48c3585c1d6 100644 --- a/tests/PRActionData/TestProcessPr.test_tests_passed.json +++ b/tests/PRActionData/TestProcessPr.test_tests_passed.json @@ -34,7 +34,8 @@ }, "emoji": { "2049242908": "+1", - "2049536626": "+1" + "2049536626": "+1", + "2056736344": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -99,6 +100,10 @@ true ] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_unassign.json b/tests/PRActionData/TestProcessPr.test_unassign.json index d544c84393c0..34ea5045bd17 100644 --- a/tests/PRActionData/TestProcessPr.test_unassign.json +++ b/tests/PRActionData/TestProcessPr.test_unassign.json @@ -38,7 +38,8 @@ "2056736344": "+1", "2056739513": "+1", "2056740892": "+1", - "2056796593": "+1" + "2056796593": "+1", + "2056801055": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -96,6 +97,10 @@ "db-pending" ] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_unhold.json b/tests/PRActionData/TestProcessPr.test_unhold.json index ac5f59c464d0..5440353da65a 100644 --- a/tests/PRActionData/TestProcessPr.test_unhold.json +++ b/tests/PRActionData/TestProcessPr.test_unhold.json @@ -36,7 +36,8 @@ "2049242908": "+1", "2049536626": "+1", "2056736344": "+1", - "2056739513": "+1" + "2056739513": "+1", + "2056740892": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -78,6 +79,10 @@ "hold" ] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_valid_type.json b/tests/PRActionData/TestProcessPr.test_valid_type.json index 1d1722e02c03..378ad635269a 100644 --- a/tests/PRActionData/TestProcessPr.test_valid_type.json +++ b/tests/PRActionData/TestProcessPr.test_valid_type.json @@ -45,7 +45,8 @@ "2056930228": "+1", "2056934192": "+1", "2056935714": "+1", - "2056946596": "-1" + "2056946596": "-1", + "2056966759": "+1" }, "last_seen_sha": "dae848e38b8e387d7283a3e35818121487d9d76b", "signatures": { @@ -135,6 +136,10 @@ "type": "remove-label", "data": [] }, + { + "type": "edit-comment", + "data": "cms-bot internal usage" + }, { "type": "status", "data": { diff --git a/tests/PRActionData/TestProcessPr.test_many_commits.json b/tests/__init__.py similarity index 100% rename from tests/PRActionData/TestProcessPr.test_many_commits.json rename to tests/__init__.py diff --git a/tests/test-requirements.txt b/tests/test-requirements.txt index f87e113a4fa1..7b208eb2d000 100644 --- a/tests/test-requirements.txt +++ b/tests/test-requirements.txt @@ -2,3 +2,5 @@ cryptography httpretty>=1.0.3 pytest>=5.3 pytest-cov>=2.8 +PyGithub==1.56 +pyyaml diff --git a/tests/test_process_pr.py b/tests/test_process_pr.py index e8645d115b91..0ece7ad21ecd 100644 --- a/tests/test_process_pr.py +++ b/tests/test_process_pr.py @@ -1,29 +1,460 @@ +import functools import importlib import json import os +import os.path import sys import traceback +from unittest.mock import patch, PropertyMock import pytest +import github +import github.GithubObject from . import Framework from .Framework import readLine -from github.PaginatedList import PaginatedList +actions = [] +record_actions = False + + +# Utility function for recording calls and optionally calling the original function +def hook_and_call_original(hook, original_function, call_original, self, *args, **kwargs): + + if call_original: + res = original_function(self, *args, **kwargs) + else: + res = None + + res = hook(self, *args, **kwargs, res=res) + + return res + + +def on_issuecomment_edit(self, *args, **kwargs): + kwargs.pop("res") + assert len(kwargs) == 0, "IssueComment.edit signature has changed" + assert len(args) == 1, "IssueComment.edit signature has changed" + + body = args[0] + actions.append({"type": "edit-comment", "data": body}) + print("DRY RUN: Updating existing comment with text") + print(body.encode("ascii", "ignore").decode()) + + +def on_issuecomment_delete(self, *args, **kwargs): + kwargs.pop("res") + assert len(kwargs) == 0, "IssueComment.delete signature has changed" + assert len(args) == 0, "IssueComment.delete signature has changed" + actions.append({"type": "delete-comment", "data": None}) + + +def on_issue_create_comment(self, *args, **kwargs): + kwargs.pop("res") + assert len(kwargs) == 0, "Issue.create_comment signature has changed" + assert len(args) == 1, "Issue.create_comment signature has changed" + + body = args[0] + actions.append({"type": "create-comment", "data": body}) + print("DRY RUN: Creating comment with text") + print(body.encode("ascii", "ignore").decode()) + + +def on_issue_edit(self, *args, **kwargs): + if kwargs.get("state") == "closed": + actions.append({"type": "close", "data": None}) + + if kwargs.get("state") == "open": + actions.append({"type": "open", "data": None}) + + if kwargs.get("milestone", github.GithubObject.NotSet) != github.GithubObject.NotSet: + milestone = kwargs["milestone"] + actions.append( + { + "type": "update-milestone", + "data": {"id": milestone.number, "title": milestone.title}, + } + ) + + +def on_commit_create_status(self, *args, **kwargs): + assert len(args) == 1, "Commit.Commit.create_status signature has chagned" + + state = args[0] + target_url = kwargs.get("target_url") + description = kwargs.get("description") + context = kwargs.get("context") + + actions.append( + { + "type": "status", + "data": { + "commit": self.sha, + "state": state, + "target_url": target_url, + "description": description, + "context": context, + }, + } + ) + + print( + "DRY RUN: set commit status state={0}, target_url={1}, description={2}, context={3}".format( + state, target_url, description, context + ) + ) + + +# TODO: remove once we update pygithub +def get_commit_files(commit): + # noinspection PyProtectedMember + return github.PaginatedList.PaginatedList( + github.File.File, + commit._requester, + commit.url, + {}, + None, + "files", + ) + + +# TODO: remove once we update pygithub +def get_commit_files_pygithub(*args, **kwargs): + kwargs.pop("res") + assert len(args) == 2, "Signature of process_pr.get_commit_files changed" + assert len(kwargs) == 0, "Signature of process_pr.get_commit_files changed" + commit = args[1] + return (x.filename for x in get_commit_files(commit)) + + +# TODO: remove once we update pygithub +def github__issue__reactions(self): + """ + :type: dict + """ + self._completeIfNotSet(self._reactions) + return self._reactions.value + + +# TODO: remove once we update pygithub +def github__issuecomment__reactions(self): + """ + :type: dict + """ + self._completeIfNotSet(self._reactions) + return self._reactions.value + + +# TODO: remove once we update pygithub +def github__issue___initAttributes(self, *args, **kwargs): + res = kwargs.pop("res") + self._reactions = github.GithubObject.NotSet + + return res + + +# TODO: remove once we update pygithub +def github__issuecomment___initAttributes(self, *args, **kwargs): + res = kwargs.pop("res") + self._reactions = github.GithubObject.NotSet + + return res + + +# TODO: remove once we update pygithub +def github__issue___useAttributes(self, *args, **kwargs): + res = kwargs.pop("res") + attributes = args[0] + + if "reactions" in attributes: + self._reactions = self._makeDictAttribute(attributes["reactions"]) + + return res + + +# TODO: remove once we update pygithub +def github__issuecomment___useAttributes(self, *args, **kwargs): + res = kwargs.pop("res") + attributes = args[0] + + if "reactions" in attributes: + self._reactions = self._makeDictAttribute(attributes["reactions"]) + + return res + + +def on_read_bot_cache(*args, **kwargs): + assert "res" in kwargs + res = kwargs.pop("res") + assert len(args) == 1, "Signature of process_pr.read_bot_cache changed" + assert len(kwargs) == 0, "Signature of process_pr.read_bot_cache changed" + actions.append({"type": "load-bot-cache", "data": res}) + return res + + +def on_create_property_file(*args, **kwargs): + kwargs.pop("res") + assert len(args) == 3, "Signature of process_pr.create_property_file changed" + assert len(kwargs) == 0, "Signature of process_pr.create_property_file changed" + + out_file_name = args[0] + parameters = args[1] + + actions.append( + {"type": "create-property-file", "data": {"filename": out_file_name, "data": parameters}} + ) + + +def on_set_comment_emoji_cache(*args, **kwargs): + kwargs.pop("res") + assert len(args) == 4 + assert len(kwargs) <= 2 + comment = args[2] + + actions.append( + { + "type": "emoji", + "data": (comment.id, kwargs.get("emoji", "+1"), kwargs.get("reset_other", True)), + } + ) + + +def on_labels_changed(*args, **kwargs): + kwargs.pop("res") + assert len(args) == 2 + assert len(kwargs) == 0 + + added_labels = args[0] + removed_labels = args[1] + + actions.append({"type": "add-label", "data": sorted(list(added_labels))}) + actions.append({"type": "remove-label", "data": sorted(list(removed_labels))}) + + +def dummy_fetch_pr_result(*args, **kwargs): + assert len(args) == 1, "Signature of process_pr.fetch_pr_result changed" + assert len(kwargs) == 0, "Signature of process_pr.fetch_pr_result changed" + + return "", "ook" class TestProcessPr(Framework.TestCase): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.process_pr_module = None + self.prId = -1 + + def setUpReactions(self): + patcher_1 = patch( + "github.IssueComment.IssueComment.reactions", + new_callable=PropertyMock, + create=True, + ) + patcher_1.__get__ = github__issuecomment__reactions + + patcher_2 = patch("github.Issue.Issue.reactions", new_callable=PropertyMock, create=True) + patcher_2.__get__ = github__issue__reactions + + self.patchers.append(patcher_1) + self.patchers.append(patcher_2) + + patcher_1.start() + patcher_2.start() + + def setUpHooks(self): + self.patchers = [] + self.calls = {} + self.original_functions = {} + + functions_to_patch = [ + { + "module_path": "github.IssueComment", + "class_name": "IssueComment", + "function_name": "edit", + "hook_function": on_issuecomment_edit, + "call_original": False, + }, + { + "module_path": "github.IssueComment", + "class_name": "IssueComment", + "function_name": "delete", + "hook_function": on_issuecomment_delete, + "call_original": False, + }, + { + "module_path": "github.Issue", + "class_name": "Issue", + "function_name": "create_comment", + "hook_function": on_issue_create_comment, + "call_original": False, + }, + { + "module_path": "github.Issue", + "class_name": "Issue", + "function_name": "edit", + "hook_function": on_issue_edit, + "call_original": False, + }, + { + "module_path": "github.Commit", + "class_name": "Commit", + "function_name": "create_status", + "hook_function": on_commit_create_status, + "call_original": False, + }, + { + "module_path": "process_pr", + "class_name": None, + "function_name": "get_commit_files", + "hook_function": get_commit_files_pygithub, + "call_original": False, + }, + { + "module_path": "process_pr", + "class_name": None, + "function_name": "create_property_file", + "hook_function": on_create_property_file, + "call_original": True, + }, + # Make this function no-op + { + "module_path": "process_pr", + "class_name": None, + "function_name": "set_emoji", + "hook_function": lambda *args, **kwargs: None, + "call_original": False, + }, + # on_read_bot_cache + { + "module_path": "process_pr", + "class_name": None, + "function_name": "read_bot_cache", + "hook_function": on_read_bot_cache, + "call_original": True, + }, + # Actual handling of emoji cache + { + "module_path": "process_pr", + "class_name": None, + "function_name": "set_comment_emoji_cache", + "hook_function": on_set_comment_emoji_cache, + "call_original": True, + }, + { + "module_path": "process_pr", + "class_name": None, + "function_name": "on_labels_changed", + "hook_function": on_labels_changed, + "call_original": False, + }, + { + "module_path": "process_pr", + "class_name": None, + "function_name": "fetch_pr_result", + "hook_function": dummy_fetch_pr_result, + "call_original": False, + }, + # TODO: remove once we update PyGithub + { + "module_path": "github.IssueComment", + "class_name": "IssueComment", + "function_name": "_initAttributes", + "hook_function": github__issuecomment___initAttributes, + "call_original": True, + }, + # TODO: remove once we update PyGithub + { + "module_path": "github.Issue", + "class_name": "Issue", + "function_name": "_initAttributes", + "hook_function": github__issue___initAttributes, + "call_original": True, + }, + # TODO: remove once we update PyGithub + { + "module_path": "github.IssueComment", + "class_name": "IssueComment", + "function_name": "_useAttributes", + "hook_function": github__issuecomment___useAttributes, + "call_original": True, + }, + # TODO: remove once we update PyGithub + { + "module_path": "github.Issue", + "class_name": "Issue", + "function_name": "_useAttributes", + "hook_function": github__issue___useAttributes, + "call_original": True, + }, + ] + + for func in functions_to_patch: + module = __import__(func["module_path"], fromlist=func["class_name"]) + if func["class_name"] is not None: + original_class = getattr(module, func["class_name"]) + original_function = getattr(original_class, func["function_name"]) + fn_to_patch = "{}.{}.{}".format( + func["module_path"], func["class_name"], func["function_name"] + ) + else: + original_function = getattr(module, func["function_name"]) + fn_to_patch = "{}.{}".format(func["module_path"], func["function_name"]) + + side_effect = functools.partial( + hook_and_call_original, + func["hook_function"], + original_function, + func["call_original"], + ) + + self.original_functions[fn_to_patch] = original_function + + patcher = patch(fn_to_patch, side_effect=side_effect, autospec=True) + self.patchers.append(patcher) + + # Apply the patch + patcher.start() + + def tearDown(self): + for patcher in self.patchers: + patcher.stop() + + self.process_pr = None + self.process_pr_module = None + @staticmethod def compareActions(res_, expected_): - res = {json.dumps(x, sort_keys=True) for x in res_} - expected = {json.dumps(x, sort_keys=True) for x in expected_} + def normalize_data(data): + if isinstance(data, dict): + return frozenset((key, normalize_data(value)) for key, value in data.items()) + elif isinstance(data, list): + return tuple(normalize_data(item) for item in data) + else: + return data + + def normalize_action(action): + return (action["type"], normalize_data(action["data"])) + + def denormalize_data(data): + if isinstance(data, frozenset): + return {key: denormalize_data(value) for key, value in data} + elif isinstance(data, tuple): + return [denormalize_data(item) for item in data] + else: + return data + + def denormalize_action(normalized_action): + return {"type": normalized_action[0], "data": denormalize_data(normalized_action[1])} + + res = set([normalize_action(action) for action in res_]) + expected = set([normalize_action(action) for action in expected_]) if res.symmetric_difference(expected): for itm in res - expected: - print("New action", itm) + print("New action", denormalize_action(itm)) for itm in expected - res: - print("Missing action", itm) + print("Missing action", denormalize_action(itm)) pytest.fail("Actions mismatch") @@ -55,20 +486,16 @@ def __openEventFile(self, mode): def __closeEventReplayFileIfNeeded(self): if self.__eventFile is not None: if ( - not self.recordMode + not record_actions ): # pragma no branch (Branch useful only when recording new tests, not used during automated tests) self.assertEqual(readLine(self.__eventFile), "") self.__eventFile.close() - def setUp(self): - super().setUp() - - self.__eventFileName = "" - self.__eventFile = None - - self.actionDataFolder = "PRActionData" - if not os.path.exists(self.actionDataFolder): - os.mkdir(self.actionDataFolder) + @classmethod + def setUpClass(cls): + cls.actionDataFolder = "PRActionData" + if not os.path.exists(cls.actionDataFolder): + os.mkdir(cls.actionDataFolder) repo_config_dir = os.path.abspath( os.path.join(os.path.dirname(__file__), "..", "repos", "iarspider_cmssw", "cmssw") @@ -78,6 +505,7 @@ def setUp(self): sys.path.insert(0, repo_config_dir) if "repo_config" in sys.modules: + print("Reloading repo_config...") importlib.reload(sys.modules["repo_config"]) importlib.reload(sys.modules["milestones"]) importlib.reload(sys.modules["releases"]) @@ -85,35 +513,51 @@ def setUp(self): else: importlib.import_module("repo_config") - self.repo_config = sys.modules["repo_config"] - assert "iarspider_cmssw" in self.repo_config.__file__ + cls.repo_config = sys.modules["repo_config"] + assert "iarspider_cmssw" in cls.repo_config.__file__ + + def setUp(self): + super().setUp() + + self.__eventFileName = "" + self.__eventFile = None + + self.setUpHooks() + self.setUpReactions() + if "process_pr" not in sys.modules: + importlib.import_module("process_pr") - self.process_pr = importlib.import_module("process_pr").process_pr + self.process_pr_module = sys.modules["process_pr"] + self.process_pr = self.process_pr_module.process_pr - def runTest(self, prId=17): + global actions + actions = [] + + def runTest(self, prId=17, testName=""): repo = self.g.get_repo("iarspider-cmssw/cmssw") issue = repo.get_issue(prId) - if self.recordMode: + if record_actions: self.__openEventFile("w") self.replayData = None else: f = self.__openEventFile("r") self.replayData = json.load(f) - self.processPrData = self.process_pr( + self.process_pr( self.repo_config, self.g, repo, issue, - True, + False, self.repo_config.CMSBUILD_USER, ) + self.processPrData = actions self.checkOrSaveTest() self.__closeEventReplayFileIfNeeded() def checkOrSaveTest(self): - if self.recordMode: + if record_actions: json.dump(self.processPrData, self.__eventFile, indent=4) else: TestProcessPr.compareActions(self.processPrData, self.replayData) @@ -131,9 +575,9 @@ def mark_tests( comparision=True, ): repo = self.g.get_repo("iarspider-cmssw/cmssw") - pr = repo.get_pull(prId) + pr = repo.get_pull(self.prId) commit = pr.get_commits().reversed[0] - prefix = "cms/" + str(prId) + "/" + prefix = "cms/" + str(self.prId) + "/" if queue.endswith("_X"): queue = queue.rstrip("_X") prefix += (queue + "/" if queue else "") + arch @@ -176,7 +620,7 @@ def mark_tests( ) def test_new_pr(self): - self.runTest() + self.runTest(testName="new_pr") def test_code_check_approved(self): self.runTest() @@ -252,13 +696,13 @@ def test_sign_reject(self): self.runTest() def test_many_commits_warn(self): - self.runTest(18) + self.runTest(prId=18) def test_many_commits_ok(self): - self.runTest(18) + self.runTest(prId=18) def test_too_many_commits(self): - self.runTest(18) + self.runTest(prId=18) # Not yet implemented # def test_future_commit(self):