From 5b1a6970384b3bc18cde014175f2f280ff5bf99d Mon Sep 17 00:00:00 2001 From: Jude N Date: Sat, 31 Aug 2019 00:42:45 -0400 Subject: [PATCH] Backporting production changes / updating tests --- plugins/marge.py | 23 ++++++++--------- tests/test_marge.py | 62 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 61 insertions(+), 24 deletions(-) diff --git a/plugins/marge.py b/plugins/marge.py index 7be6ea6..0fe540f 100755 --- a/plugins/marge.py +++ b/plugins/marge.py @@ -122,8 +122,8 @@ class Marge(BotPlugin, CrontabMixin): # self.log.info("state: {}".format(request['object_attributes']['state'])) # verify it's a merge request - if request['object_kind'] != 'merge_request': - self.log.error('unexpecting object_kind: {}'.format(request['object_kind'])) + if request['event_type'] != 'merge_request': + self.log.error('unexpecting event_type: {}'.format(request['event_type'])) elif 'opened' in request['object_attributes']['state']: if request['object_attributes']['work_in_progress']: @@ -144,11 +144,11 @@ class Marge(BotPlugin, CrontabMixin): target_project_id = request['object_attributes']['target_project_id'] iid = request['object_attributes']['iid'] - # If the MR is tagged 'never-close' ignore it + # If the MR is tagged 'never-close' or 'abandoned' ignore it if 'labels' in request: for a_label in request['labels']: - if a_label['title'] == 'never-close': - self.log.info("Skipping never-close notice for {} MR".format(url)) + if a_label['title'] in ['never-close', 'abandoned']: + self.log.info("Skipping {} notice for {} MR".format(a_label['title'], url)) return "OK" msg_template = "Hi there ! {} has opened a new {}MR: \"{}\"\n{}/merge_requests/{}" @@ -211,7 +211,7 @@ class Marge(BotPlugin, CrontabMixin): else: authored = False - msg = "{} (opened {})".format(mr_attrs['web_url'], str_open_since) + msg = "{} (opened by {} {})".format(mr_attrs['web_url'], mr_attrs['author']['username'], str_open_since) upvotes = mr_attrs['upvotes'] if upvotes >= 2: msg += ": Has 2+ upvotes and could be merged in now" @@ -286,8 +286,8 @@ class Marge(BotPlugin, CrontabMixin): self.log.debug("Couldn't find project: {}, id: {}".format(project_id, mr_id)) continue - # If the MR is tagged 'never-close' ignore it - if 'labels' in mr_attrs and 'never-close' in mr_attrs['labels']: + # If the MR is tagged 'never-close' or 'abandoned', ignore it + if 'labels' in mr_attrs and ('never-close' in mr_attrs['labels'] or 'abandoned' in mr_attrs['labels']): continue self.log.info("a_mr: {} {} {} {}".format(project_id, mr_id, notify_rooms, mr_attrs['state'])) @@ -379,8 +379,8 @@ class Marge(BotPlugin, CrontabMixin): self.log.info('project: {}, id: {}, a_mr: {}'.format(project, id, a_mr)) - # If the MR is tagged 'never-close' ignore it - if 'labels' in mr_attrs and 'never-close' in mr_attrs['labels']: + # If the MR is tagged 'never-close' or 'abandoned', ignore it + if 'labels' in mr_attrs and ('never-close' in mr_attrs['labels'] or 'abandoned' in mr_attrs['labels']): continue # If the MR is no longer open, skip to the next MR, @@ -431,7 +431,6 @@ class Marge(BotPlugin, CrontabMixin): msg = "Couldn't find {} hooks".format(repo) self.log.error("watchrepo: {}".format(msg)) return msg - for a_hook in hooks: self.log.info('a_hook: {}'.format(a_hook)) hook_attributes = a_hook.attributes @@ -459,7 +458,7 @@ class Marge(BotPlugin, CrontabMixin): else: try: s_action = "add" - project.hooks.create({'url': url, 'merge_request_events': 1, 'enable_ssl_verification': True}) + project.hooks.create({'url': url, 'merge_requests_events': 1, 'enable_ssl_verification': True}) s_watch_msg = "Now watching for new MRs in the {} repo to the {} room(s)".format(repo, rooms) except Exception as exp: hook_updated = False diff --git a/tests/test_marge.py b/tests/test_marge.py index 6875007..d96fc60 100644 --- a/tests/test_marge.py +++ b/tests/test_marge.py @@ -86,7 +86,7 @@ def mock_projects_mergerequests_get_unreviewed_review(url, request): return {'status_code': 200, 'headers': {'content-type': 'application/json'}, 'content': json.dumps({'iid': 'mr_id', - 'author': {'id': 2001}, + 'author': {'username': 'ReviewerX', 'id': 2001}, 'created_at': 'Oct 29, 2017 2:37am', 'merge_status': 'can_be_merged', 'state': 'opened', @@ -100,7 +100,7 @@ def mock_projects_mergerequests_get_wip_review(url, request): return {'status_code': 200, 'headers': {'content-type': 'application/json'}, 'content': json.dumps({'iid': 'mr_id', - 'author': {'id': 2001}, + 'author': {'username': 'ReviewerX', 'id': 2001}, 'created_at': 'Oct 29, 2017 2:37am', 'merge_status': 'can_be_merged', 'state': 'opened', @@ -114,7 +114,7 @@ def mock_projects_mergerequests_get_waiting_review(url, request): return {'status_code': 200, 'headers': {'content-type': 'application/json'}, 'content': json.dumps({'iid': 'mr_id', - 'author': {'id': 2001}, + 'author': {'username': 'ReviewerX', 'id': 2001}, 'created_at': 'Oct 29, 2017 2:37am', 'merge_status': 'can_be_merged', 'state': 'opened', @@ -137,7 +137,7 @@ def mock_projects_mergerequests_get_mergable_review(url, request): return {'status_code': 200, 'headers': {'content-type': 'application/json'}, 'content': json.dumps({'iid': 'mr_id', - 'author': {'id': 2001}, + 'author': {'username': 'ReviewerX', 'id': 2001}, 'created_at': 'Oct 29, 2017 2:37am', 'merge_status': 'can_be_merged', 'state': 'opened', @@ -151,7 +151,7 @@ def mock_projects_mergerequests_get_conflicted_review(url, request): return {'status_code': 200, 'headers': {'content-type': 'application/json'}, 'content': json.dumps({'iid': 'mr_id', - 'author': {'id': 2001}, + 'author': {'username': 'ReviewerX', 'id': 2001}, 'created_at': 'Oct 29, 2017 2:37am', 'merge_status': 'merge_conflicts', 'state': 'opened', @@ -165,7 +165,7 @@ def mock_projects_mergerequests_list_unreviewed_review(url, request): return {'status_code': 200, 'headers': {'content-type': 'application/json'}, 'content': json.dumps([{'iid': 'mr_id', - 'author': {'id': 2001}, + 'author': {'username': 'ReviewerX', 'id': 2001}, 'created_at': 'Oct 29, 2017 2:37am', 'merge_status': 'can_be_merged', 'state': 'opened', @@ -302,7 +302,7 @@ class TestMarge: assert '1 open MR was found in the repo.' in pm def test_gitlab_hook(self, margebot): - request = json.dumps({'object_kind': 'merge_request', + request = json.dumps({'event_type': 'merge_request', 'object_attributes': { 'state': 'opened', 'work_in_progress': '', @@ -319,8 +319,46 @@ class TestMarge: margebot.push_message('!reviews') assert 'Hi there ! author_id username has opened a new MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() + def test_gitlab_hook_never_close(self, margebot): + request = json.dumps({'event_type': 'merge_request', + 'object_attributes': { + 'state': 'opened', + 'work_in_progress': '', + 'title': 'title', + 'author_id': 'author_id', + 'target_project_id': 'project_id', + 'id': 'id', + 'iid': 'iid'}, + 'project': { + 'homepage': 'url'}, + 'labels': [{'title': 'never-close'}]}) + with HTTMock(mock_users_get_author_id): + margebot.push_message("!webhook test /margebot/room1,room2 " + request) + assert 'Status code : 200' in margebot.pop_message() + margebot.push_message('!reviews') + assert 'Hi gbin: I found no open MRs for you.' in margebot.pop_message() + + def test_gitlab_hook_never_abandoned(self, margebot): + request = json.dumps({'event_type': 'merge_request', + 'object_attributes': { + 'state': 'opened', + 'work_in_progress': '', + 'title': 'title', + 'author_id': 'author_id', + 'target_project_id': 'project_id', + 'id': 'id', + 'iid': 'iid'}, + 'project': { + 'homepage': 'url'}, + 'labels': [{'title': 'abandoned'}]}) + with HTTMock(mock_users_get_author_id): + margebot.push_message("!webhook test /margebot/room1,room2 " + request) + assert 'Status code : 200' in margebot.pop_message() + margebot.push_message('!reviews') + assert 'Hi gbin: I found no open MRs for you.' in margebot.pop_message() + def test_gitlab_hook_wip(self, margebot): - request = json.dumps({'object_kind': 'merge_request', + request = json.dumps({'event_type': 'merge_request', 'object_attributes': { 'state': 'opened', 'work_in_progress': 'true', @@ -338,7 +376,7 @@ class TestMarge: assert 'Hi there ! author_id username has opened a new WIP MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() def test_gitlab_hook_unexpected_user(self, margebot): - request = json.dumps({'object_kind': 'merge_request', + request = json.dumps({'event_type': 'merge_request', 'object_attributes': { 'state': 'opened', 'work_in_progress': '', @@ -356,8 +394,8 @@ class TestMarge: margebot.push_message('!reviews') assert 'Hi there ! (missing) has opened a new MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() - def test_gitlab_hook_unexpected_object_kind(self, margebot, caplog): - request = json.dumps({'object_kind': 'not_merge_request', + def test_gitlab_hook_unexpected_event_type(self, margebot, caplog): + request = json.dumps({'event_type': 'not_merge_request', 'object_attributes': { 'state': 'opened', 'work_in_progress': '', @@ -374,7 +412,7 @@ class TestMarge: margebot.pop_message() margebot.push_message('!reviews') assert 'Hi gbin: I found no open MRs for you.' in margebot.pop_message() - assert 'unexpecting object_kind: not_merge_request' in caplog.text # Has to be at end of method + assert 'unexpecting event_type: not_merge_request' in caplog.text # Has to be at end of method def test_nothing_to_review(self, margebot): with HTTMock(mock_users_search_gbin): -- 2.39.2