You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/09/12 22:05:19 UTC
[GitHub] [incubator-mxnet-ci] szha commented on a change in pull request #27: Serverless based Lambda function for Labelling PRs based on CI & PR Review status
szha commented on a change in pull request #27:
URL: https://github.com/apache/incubator-mxnet-ci/pull/27#discussion_r487449601
##########
File path: services/lambda-pr-status-labeler/pr_status_bot/test_mock.py
##########
@@ -0,0 +1,112 @@
+import json
+import os
+from PRStatusBot import PRStatusBot, GithubObj, FAILURE_STATE, PENDING_STATE, SUCCESS_STATE, PR_WORK_IN_PROGRESS_LABEL, PR_AWAITING_TESTING_LABEL, PR_AWAITING_REVIEW_LABEL, PR_AWAITING_RESPONSE_LABEL, PR_AWAITING_MERGE_LABEL, WORK_IN_PROGRESS_TITLE_SUBSTRING
+import handler
+
+
+def test_if_payload_is_non_PR(mocker):
+ payload = {'target_url': 'master'}
+ prsb = PRStatusBot(None, None, False)
+ actual = prsb.parse_payload(payload)
+ expected = 1
+ assert actual == expected
+
+
+def test_if_pr_closed(mocker):
+ payload = {'target_url': 'PR-1'}
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_get_pull_request_object', return_value=MockPR(state='closed'))
+ actual = prsb.parse_payload(payload)
+ expected = 2
+ assert actual == expected
+
+
+def test_if_pr_wip_title(mocker):
+ mockpr = MockPR(title=WORK_IN_PROGRESS_TITLE_SUBSTRING)
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(FAILURE_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_pr_draft(mocker):
+ mockpr = MockPR(draft=True)
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_ci_status_failure(mocker):
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(FAILURE_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_ci_status_pending(mocker):
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(PENDING_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_TESTING_LABEL)
+
+
+def test_if_pr_no_reviews(mocker):
+ def mock_no_review_counts(self, pr):
+ # approves, request_changes, comments
+ return 0, 0, 0
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ mocker.patch.object(PRStatusBot, '_parse_reviews', mock_no_review_counts)
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_REVIEW_LABEL)
+
+
+def test_if_pr_reviews_requested_changes(mocker):
+ def mock_no_review_counts(self, pr):
+ # approves, request_changes, comments
+ return 0, 1, 0
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ mocker.patch.object(PRStatusBot, '_parse_reviews', mock_no_review_counts)
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_RESPONSE_LABEL)
+
+
+class MockRepo:
+ def __init__(self, name=None):
+ self.name = name
+
+ def get_commit(self, commit_sha):
+ return 'abc'
+
+ def get_pull(self, pr_number):
+ return MockPR(pr_number)
+
+
+class MockGithub:
+ def __init__(self):
+ pass
+
+ def get_repo(self, name=None):
+ return MockRepo(name)
+
+
+class MockLabel:
+ def __init__(self, name=None):
+ self.name = name
+
+
+class MockPR:
+ def __init__(self, pr_number=1, state='open', title='abc', draft=False):
+ self.number = pr_number
+ self.state = state
+ self.title = title
+ self.draft = draft
+
+ def get_labels(self):
+ return [MockLabel('lab1'), MockLabel('lab2')]
Review comment:
use the real classes https://pygithub.readthedocs.io/en/latest/reference.html
##########
File path: services/lambda-pr-status-labeler/pr_status_bot/test.py
##########
@@ -0,0 +1,92 @@
+import json
+import os
+from PRStatusBot import PRStatusBot
+
+
+def load_and_test(data):
+ payload_json = json.loads(data)
+ os.environ["AWS_PROFILE"] = "mxnet-ci"
+ # set secret_name [commented since it is to be redacted]
+ # os.environ["secret_name"]
+ os.environ["region_name"] = "us-west-2"
+ pr_status_bot = PRStatusBot("apache/incubator-mxnet", apply_secret=True)
+ pr_status_bot.parse_payload(payload_json)
+
+
+def prepare_input(pr_num, context, state, sha):
+ data = {
+ "target_url": "PR-" + str(pr_num),
+ "context": "ci/jenkins/mxnet-validation/" + context,
+ "state": state,
+ "commit": {
+ "sha": sha
+ }
+ }
+ # return serialized data dictionary
+ return json.dumps(data)
+
+
+def check_ci_failure():
+ data = prepare_input(18984, "website", "failed", "6fbfa3c020e566c0d54825cbfb67abca1d70b4fa")
+ load_and_test(data)
Review comment:
+1 this still needs to be addressed.
##########
File path: services/lambda-pr-status-labeler/pr_status_bot/test_mock.py
##########
@@ -0,0 +1,112 @@
+import json
+import os
+from PRStatusBot import PRStatusBot, GithubObj, FAILURE_STATE, PENDING_STATE, SUCCESS_STATE, PR_WORK_IN_PROGRESS_LABEL, PR_AWAITING_TESTING_LABEL, PR_AWAITING_REVIEW_LABEL, PR_AWAITING_RESPONSE_LABEL, PR_AWAITING_MERGE_LABEL, WORK_IN_PROGRESS_TITLE_SUBSTRING
+import handler
+
+
+def test_if_payload_is_non_PR(mocker):
+ payload = {'target_url': 'master'}
+ prsb = PRStatusBot(None, None, False)
+ actual = prsb.parse_payload(payload)
+ expected = 1
+ assert actual == expected
+
+
+def test_if_pr_closed(mocker):
+ payload = {'target_url': 'PR-1'}
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_get_pull_request_object', return_value=MockPR(state='closed'))
+ actual = prsb.parse_payload(payload)
+ expected = 2
+ assert actual == expected
+
+
+def test_if_pr_wip_title(mocker):
+ mockpr = MockPR(title=WORK_IN_PROGRESS_TITLE_SUBSTRING)
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(FAILURE_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_pr_draft(mocker):
+ mockpr = MockPR(draft=True)
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_ci_status_failure(mocker):
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(FAILURE_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_ci_status_pending(mocker):
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(PENDING_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_TESTING_LABEL)
+
+
+def test_if_pr_no_reviews(mocker):
+ def mock_no_review_counts(self, pr):
+ # approves, request_changes, comments
+ return 0, 0, 0
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ mocker.patch.object(PRStatusBot, '_parse_reviews', mock_no_review_counts)
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_REVIEW_LABEL)
+
+
+def test_if_pr_reviews_requested_changes(mocker):
+ def mock_no_review_counts(self, pr):
+ # approves, request_changes, comments
+ return 0, 1, 0
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ mocker.patch.object(PRStatusBot, '_parse_reviews', mock_no_review_counts)
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_RESPONSE_LABEL)
+
+
+class MockRepo:
+ def __init__(self, name=None):
+ self.name = name
+
+ def get_commit(self, commit_sha):
+ return 'abc'
+
+ def get_pull(self, pr_number):
+ return MockPR(pr_number)
+
+
+class MockGithub:
+ def __init__(self):
+ pass
+
+ def get_repo(self, name=None):
+ return MockRepo(name)
+
+
+class MockLabel:
+ def __init__(self, name=None):
+ self.name = name
+
+
+class MockPR:
+ def __init__(self, pr_number=1, state='open', title='abc', draft=False):
+ self.number = pr_number
+ self.state = state
+ self.title = title
+ self.draft = draft
+
+ def get_labels(self):
+ return [MockLabel('lab1'), MockLabel('lab2')]
Review comment:
use the real classes https://pygithub.readthedocs.io/en/latest/reference.html
##########
File path: services/lambda-pr-status-labeler/pr_status_bot/test_mock.py
##########
@@ -0,0 +1,112 @@
+import json
+import os
+from PRStatusBot import PRStatusBot, GithubObj, FAILURE_STATE, PENDING_STATE, SUCCESS_STATE, PR_WORK_IN_PROGRESS_LABEL, PR_AWAITING_TESTING_LABEL, PR_AWAITING_REVIEW_LABEL, PR_AWAITING_RESPONSE_LABEL, PR_AWAITING_MERGE_LABEL, WORK_IN_PROGRESS_TITLE_SUBSTRING
+import handler
+
+
+def test_if_payload_is_non_PR(mocker):
+ payload = {'target_url': 'master'}
+ prsb = PRStatusBot(None, None, False)
+ actual = prsb.parse_payload(payload)
+ expected = 1
+ assert actual == expected
+
+
+def test_if_pr_closed(mocker):
+ payload = {'target_url': 'PR-1'}
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_get_pull_request_object', return_value=MockPR(state='closed'))
+ actual = prsb.parse_payload(payload)
+ expected = 2
+ assert actual == expected
+
+
+def test_if_pr_wip_title(mocker):
+ mockpr = MockPR(title=WORK_IN_PROGRESS_TITLE_SUBSTRING)
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(FAILURE_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_pr_draft(mocker):
+ mockpr = MockPR(draft=True)
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_ci_status_failure(mocker):
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(FAILURE_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_ci_status_pending(mocker):
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(PENDING_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_TESTING_LABEL)
+
+
+def test_if_pr_no_reviews(mocker):
+ def mock_no_review_counts(self, pr):
+ # approves, request_changes, comments
+ return 0, 0, 0
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ mocker.patch.object(PRStatusBot, '_parse_reviews', mock_no_review_counts)
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_REVIEW_LABEL)
+
+
+def test_if_pr_reviews_requested_changes(mocker):
+ def mock_no_review_counts(self, pr):
+ # approves, request_changes, comments
+ return 0, 1, 0
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ mocker.patch.object(PRStatusBot, '_parse_reviews', mock_no_review_counts)
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_RESPONSE_LABEL)
+
+
+class MockRepo:
+ def __init__(self, name=None):
+ self.name = name
+
+ def get_commit(self, commit_sha):
+ return 'abc'
+
+ def get_pull(self, pr_number):
+ return MockPR(pr_number)
+
+
+class MockGithub:
+ def __init__(self):
+ pass
+
+ def get_repo(self, name=None):
+ return MockRepo(name)
+
+
+class MockLabel:
+ def __init__(self, name=None):
+ self.name = name
+
+
+class MockPR:
+ def __init__(self, pr_number=1, state='open', title='abc', draft=False):
+ self.number = pr_number
+ self.state = state
+ self.title = title
+ self.draft = draft
+
+ def get_labels(self):
+ return [MockLabel('lab1'), MockLabel('lab2')]
Review comment:
use the real classes https://pygithub.readthedocs.io/en/latest/reference.html
##########
File path: services/lambda-pr-status-labeler/pr_status_bot/test.py
##########
@@ -0,0 +1,92 @@
+import json
+import os
+from PRStatusBot import PRStatusBot
+
+
+def load_and_test(data):
+ payload_json = json.loads(data)
+ os.environ["AWS_PROFILE"] = "mxnet-ci"
+ # set secret_name [commented since it is to be redacted]
+ # os.environ["secret_name"]
+ os.environ["region_name"] = "us-west-2"
+ pr_status_bot = PRStatusBot("apache/incubator-mxnet", apply_secret=True)
+ pr_status_bot.parse_payload(payload_json)
+
+
+def prepare_input(pr_num, context, state, sha):
+ data = {
+ "target_url": "PR-" + str(pr_num),
+ "context": "ci/jenkins/mxnet-validation/" + context,
+ "state": state,
+ "commit": {
+ "sha": sha
+ }
+ }
+ # return serialized data dictionary
+ return json.dumps(data)
+
+
+def check_ci_failure():
+ data = prepare_input(18984, "website", "failed", "6fbfa3c020e566c0d54825cbfb67abca1d70b4fa")
+ load_and_test(data)
Review comment:
+1 this still needs to be addressed.
##########
File path: services/lambda-pr-status-labeler/pr_status_bot/test_mock.py
##########
@@ -0,0 +1,112 @@
+import json
+import os
+from PRStatusBot import PRStatusBot, GithubObj, FAILURE_STATE, PENDING_STATE, SUCCESS_STATE, PR_WORK_IN_PROGRESS_LABEL, PR_AWAITING_TESTING_LABEL, PR_AWAITING_REVIEW_LABEL, PR_AWAITING_RESPONSE_LABEL, PR_AWAITING_MERGE_LABEL, WORK_IN_PROGRESS_TITLE_SUBSTRING
+import handler
+
+
+def test_if_payload_is_non_PR(mocker):
+ payload = {'target_url': 'master'}
+ prsb = PRStatusBot(None, None, False)
+ actual = prsb.parse_payload(payload)
+ expected = 1
+ assert actual == expected
+
+
+def test_if_pr_closed(mocker):
+ payload = {'target_url': 'PR-1'}
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_get_pull_request_object', return_value=MockPR(state='closed'))
+ actual = prsb.parse_payload(payload)
+ expected = 2
+ assert actual == expected
+
+
+def test_if_pr_wip_title(mocker):
+ mockpr = MockPR(title=WORK_IN_PROGRESS_TITLE_SUBSTRING)
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(FAILURE_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_pr_draft(mocker):
+ mockpr = MockPR(draft=True)
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_ci_status_failure(mocker):
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(FAILURE_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_WORK_IN_PROGRESS_LABEL)
+
+
+def test_if_ci_status_pending(mocker):
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ prsb._label_pr_based_on_status(PENDING_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_TESTING_LABEL)
+
+
+def test_if_pr_no_reviews(mocker):
+ def mock_no_review_counts(self, pr):
+ # approves, request_changes, comments
+ return 0, 0, 0
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ mocker.patch.object(PRStatusBot, '_parse_reviews', mock_no_review_counts)
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_REVIEW_LABEL)
+
+
+def test_if_pr_reviews_requested_changes(mocker):
+ def mock_no_review_counts(self, pr):
+ # approves, request_changes, comments
+ return 0, 1, 0
+ mockpr = MockPR()
+ prsb = PRStatusBot(None, None, False)
+ mocker.patch.object(PRStatusBot, '_add_label')
+ mocker.patch.object(PRStatusBot, '_parse_reviews', mock_no_review_counts)
+ prsb._label_pr_based_on_status(SUCCESS_STATE, mockpr)
+ PRStatusBot._add_label.assert_called_with(mockpr, PR_AWAITING_RESPONSE_LABEL)
+
+
+class MockRepo:
+ def __init__(self, name=None):
+ self.name = name
+
+ def get_commit(self, commit_sha):
+ return 'abc'
+
+ def get_pull(self, pr_number):
+ return MockPR(pr_number)
+
+
+class MockGithub:
+ def __init__(self):
+ pass
+
+ def get_repo(self, name=None):
+ return MockRepo(name)
+
+
+class MockLabel:
+ def __init__(self, name=None):
+ self.name = name
+
+
+class MockPR:
+ def __init__(self, pr_number=1, state='open', title='abc', draft=False):
+ self.number = pr_number
+ self.state = state
+ self.title = title
+ self.draft = draft
+
+ def get_labels(self):
+ return [MockLabel('lab1'), MockLabel('lab2')]
Review comment:
use the real classes https://pygithub.readthedocs.io/en/latest/reference.html
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org