From c60ca167527ed704f3ff1a03b071e0e6e13e4062 Mon Sep 17 00:00:00 2001 From: Jude N Date: Sun, 28 Apr 2019 13:28:42 -0400 Subject: [PATCH] replace pyapi-gitlab with python-gitlab / start using httmock in the unit tests --- .pylintrc | 2 +- plugins/marge.py | 190 ++++++-------- requirements.txt | 2 +- setup.cfg | 2 +- test-requirements.txt | 1 + tests/test_marge.py | 563 ++++++++++++++++++++++-------------------- 6 files changed, 384 insertions(+), 376 deletions(-) diff --git a/.pylintrc b/.pylintrc index 694a07c..76b0642 100644 --- a/.pylintrc +++ b/.pylintrc @@ -59,7 +59,7 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use"--disable=all --enable=classes # --disable=W" -disable=import-star-module-level,old-octal-literal,oct-method,print-statement,unpacking-in-except,parameter-unpacking,backtick,old-raise-syntax,old-ne-operator,long-suffix,dict-view-method,dict-iter-method,metaclass-assignment,next-method-called,raising-string,indexing-exception,raw_input-builtin,long-builtin,file-builtin,execfile-builtin,coerce-builtin,cmp-builtin,buffer-builtin,basestring-builtin,apply-builtin,filter-builtin-not-iterating,using-cmp-argument,useless-suppression,range-builtin-not-iterating,suppressed-message,no-absolute-import,old-division,cmp-method,reload-builtin,zip-builtin-not-iterating,intern-builtin,unichr-builtin,reduce-builtin,standarderror-builtin,unicode-builtin,xrange-builtin,coerce-method,delslice-method,getslice-method,setslice-method,input-builtin,round-builtin,hex-method,nonzero-method,map-builtin-not-iterating,line-too-long,no-name-in-module,input-error,missing-super-argument,no-self-use,too-many-locals,too-many-branches,import-error,locally-disabled,locally-enabled,too-many-ancestors,useless-super-delegation,logging-format-interpolation,too-many-statements,too-many-arguments +disable=import-star-module-level,old-octal-literal,oct-method,print-statement,unpacking-in-except,parameter-unpacking,backtick,old-raise-syntax,old-ne-operator,long-suffix,dict-view-method,dict-iter-method,metaclass-assignment,next-method-called,raising-string,indexing-exception,raw_input-builtin,long-builtin,file-builtin,execfile-builtin,coerce-builtin,cmp-builtin,buffer-builtin,basestring-builtin,apply-builtin,filter-builtin-not-iterating,using-cmp-argument,useless-suppression,range-builtin-not-iterating,suppressed-message,no-absolute-import,old-division,cmp-method,reload-builtin,zip-builtin-not-iterating,intern-builtin,unichr-builtin,reduce-builtin,standarderror-builtin,unicode-builtin,xrange-builtin,coerce-method,delslice-method,getslice-method,setslice-method,input-builtin,round-builtin,hex-method,nonzero-method,map-builtin-not-iterating,line-too-long,no-name-in-module,input-error,missing-super-argument,no-self-use,too-many-locals,too-many-branches,import-error,locally-disabled,locally-enabled,too-many-ancestors,useless-super-delegation,logging-format-interpolation,too-many-statements,too-many-arguments,broad-except [REPORTS] diff --git a/plugins/marge.py b/plugins/marge.py index 86b66ec..d2bfdb6 100755 --- a/plugins/marge.py +++ b/plugins/marge.py @@ -9,57 +9,6 @@ from errbot import BotPlugin, botcmd, arg_botcmd, re_botcmd, webhook from errbot.templating import tenv from errcron.bot import CrontabMixin import gitlab -import requests - - -def addprojecthook_extra(self, project_id, url, push=False, issues=False, merge_requests=False, tag_push=False, extra_data=None): - """ - A copy parent addprojecthook with an extra_data field - """ - data = {"id": project_id, "url": url} - if extra_data: - for ed_key, ed_value in extra_data.items(): - data[ed_key] = ed_value - data['push_events'] = int(bool(push)) - data['issues_events'] = int(bool(issues)) - data['merge_requests_events'] = int(bool(merge_requests)) - data['tag_push_events'] = int(bool(tag_push)) - request = requests.post("{0}/{1}/hooks".format(self.projects_url, project_id), - headers=self.headers, data=data, verify=self.verify_ssl) - if request.status_code == 201: - return request.json() - return False -gitlab.Gitlab.addprojecthook_extra = addprojecthook_extra - - -def editprojecthook_extra(self, project_id, hook_id, url, push=False, issues=False, merge_requests=False, tag_push=False, extra_data=None): - """ - A copy of the parent editprojecthook with an extra_data field - """ - data = {"id": project_id, "hook_id": hook_id, "url": url} - if extra_data: - for ed_key, ed_value in extra_data.items(): - data[ed_key] = ed_value - data['push_events'] = int(bool(push)) - data['issues_events'] = int(bool(issues)) - data['merge_requests_events'] = int(bool(merge_requests)) - data['tag_push_events'] = int(bool(tag_push)) - request = requests.put("{0}/{1}/hooks/{2}".format(self.projects_url, project_id, hook_id), - headers=self.headers, data=data, verify=self.verify_ssl) - return request.status_code == 200 -gitlab.Gitlab.editprojecthook_extra = editprojecthook_extra - - -def getawardemoji(self, project_id, mr_iid): - """ - Give me the thumbs - """ - url = "{0}/{1}/merge_requests/{2}/award_emoji".format(self.projects_url, project_id, mr_iid) - request = requests.get(url, headers=self.headers, verify=self.verify_ssl) - if request.status_code == 200: - return request.json() - return False -gitlab.Gitlab.getawardemoji = getawardemoji def deltastr(any_delta): @@ -148,7 +97,7 @@ class Marge(BotPlugin, CrontabMixin): Marge.CRONTAB = ['{} .crontab_hook'.format(self.config['CRONTAB'])] gitlab_auth_token = self.config['GITLAB_ADMIN_TOKEN'] verify_ssl = self.config['VERIFY_SSL'] - self.gitlab = gitlab.Gitlab(self.git_host, gitlab_auth_token, verify_ssl=verify_ssl) + self.gitlab = gitlab.Gitlab('https://' + self.git_host, gitlab_auth_token, ssl_verify=verify_ssl) self.activate_crontab() self.soak_delta = relativedelta(hours=self.config['CRONTAB_SOAK_HOURS']) @@ -181,15 +130,15 @@ class Marge(BotPlugin, CrontabMixin): wip = "WIP " else: wip = "" + url = request['project']['homepage'] title = request['object_attributes']['title'] - author_id = request['object_attributes']['author_id'] # map this to user name ... - author = self.gitlab.getuser(author_id) - if author: - author_name = author['username'] - else: - self.log.info("unexpected author_id {}".format(author_id)) + try: + author = self.gitlab.users.get(author_id) + author_name = author.attributes['username'] + except Exception as exp: + self.log.info("unexpected author_id {}, exp={}".format(author_id, exp)) author_name = author_id target_project_id = request['object_attributes']['target_project_id'] @@ -211,7 +160,6 @@ class Marge(BotPlugin, CrontabMixin): open_mrs = self['OPEN_MRS'] - # jn: iid references in this block used to be mr_id if (target_project_id, iid, rooms) not in open_mrs: for a_room in rooms.split(','): if self.config: @@ -234,15 +182,16 @@ class Marge(BotPlugin, CrontabMixin): Create the merge request status message """ self.log.info("mr_status_msg: a_mr: {}".format(a_mr)) + mr_attrs = a_mr.attributes # Only weed out MRs less than the soak time for the crontab output (where author==None) now = datetime.now(timezone.utc) - creation_time = parser.parse(a_mr['created_at'], tzinfos=tzutc) + creation_time = parser.parse(mr_attrs['created_at'], tzinfos=tzutc) if not author: self.log.info("times: {}, {}, {}".format(creation_time, self.soak_delta, now)) if creation_time + self.soak_delta > now: - project_id = a_mr['target_project_id'] - mr_id = a_mr['id'] + project_id = mr_attrs['target_project_id'] + mr_id = mr_attrs['id'] soak_hours = self.config['CRONTAB_SOAK_HOURS'] info_template = "skipping: MR <{},{}> was opened less than {} hours ago" info_msg = info_template.format(project_id, mr_id, soak_hours) @@ -252,18 +201,18 @@ class Marge(BotPlugin, CrontabMixin): str_open_since = deltastr(now - creation_time) warning = "" - if a_mr['work_in_progress']: + if mr_attrs['work_in_progress']: warning = "still WIP" - elif a_mr['merge_status'] != 'can_be_merged': + elif mr_attrs['merge_status'] != 'can_be_merged': warning = "there are merge conflicts" if author: - authored = (a_mr['author']['id'] == author) + authored = (mr_attrs['author']['id'] == author) else: authored = False - msg = "{} (opened {})".format(a_mr['web_url'], str_open_since) - upvotes = a_mr['upvotes'] + msg = "{} (opened {})".format(mr_attrs['web_url'], str_open_since) + upvotes = mr_attrs['upvotes'] if upvotes >= 2: msg += ": Has 2+ upvotes and could be merged in now" if warning != "": @@ -272,12 +221,14 @@ class Marge(BotPlugin, CrontabMixin): msg += "." elif upvotes == 1: - award_emoji = self.gitlab.getawardemoji(a_mr['target_project_id'], a_mr['iid']) + self.log.error("pre-award a_mr: {}".format(dir(a_mr.awardemojis))) + award_emoji = a_mr.awardemojis.list() self.log.info("award_emoji: {}".format(award_emoji)) already_approved = "someone" for an_emoji in award_emoji: - if an_emoji["name"] == "thumbsup": - already_approved = an_emoji["user"]["username"] + emoji_attr = an_emoji.attributes + if emoji_attr["name"] == "thumbsup": + already_approved = emoji_attr["user"]["username"] if authored: msg += ": Your MR has been approved by " + already_approved + " and is waiting for another upvote" @@ -323,21 +274,27 @@ class Marge(BotPlugin, CrontabMixin): for (project_id, mr_id, notify_rooms) in open_mrs: + a_project = self.gitlab.projects.get(project_id) + if not a_project: + self.log.debug("Couldn't find project: {}".format(project_id)) + continue + # Lookup the MR from the project/id - a_mr = self.gitlab.getmergerequest(project_id, mr_id) + a_mr = a_project.mergerequests.get(mr_id) + mr_attrs = a_mr.attributes if not a_mr: 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 a_mr and 'never-close' in a_mr['labels']: + if 'labels' in mr_attrs and 'never-close' in mr_attrs['labels']: continue - self.log.info("a_mr: {} {} {} {}".format(project_id, mr_id, notify_rooms, a_mr['state'])) + self.log.info("a_mr: {} {} {} {}".format(project_id, mr_id, notify_rooms, mr_attrs['state'])) # If the MR is no longer open, skip to the next MR, # and don't include this MR in the next check - if 'opened' not in a_mr['state']: + if 'opened' not in mr_attrs['state']: continue else: still_open_mrs[(project_id, mr_id, notify_rooms)] = True @@ -391,12 +348,12 @@ class Marge(BotPlugin, CrontabMixin): return "No MRs to review" sender_gitlab_id = None - sender_users = self.gitlab.getusers(search=(('username', sender))) - if not sender_users: - self.log.error('problem mapping {} to a gitlab user'.format(sender)) + try: + sender_users = self.gitlab.users.list(username=sender) + sender_gitlab_id = sender_users[0].attributes['id'] + except Exception as exp: + self.log.error('problem mapping {} to a gitlab user, exp={}'.format(sender, exp)) sender_gitlab_id = None - else: - sender_gitlab_id = sender_users[0]['id'] # Walk through the MRs we've seen already: msg_list = [] @@ -406,22 +363,30 @@ class Marge(BotPlugin, CrontabMixin): self.log.info('open_mrs: {}'.format(open_mrs)) for (project, mr_id, notify_rooms) in open_mrs: + try: + a_project = self.gitlab.projects.get(project) + except Exception as exp: + self.log.debug("Couldn't find project: {}, exp: {}".format(project, exp)) + continue + # Lookup the MR from the project/id - a_mr = self.gitlab.getmergerequest(project, mr_id) - if not a_mr: - self.log.debug("Couldn't find project: {}, id: {}".format(project, id)) + try: + a_mr = a_project.mergerequests.get(mr_id) + mr_attrs = a_mr.attributes + except Exception as exp: + self.log.debug("Couldn't find project: {}, id: {}, exp: {}".format(project, mr_id, exp)) continue self.log.info('project: {}, id: {}, a_mr: {}'.format(project, id, a_mr)) # If the MR is tagged 'never-close' ignore it - if 'labels' in a_mr and 'never-close' in a_mr['labels']: + if 'labels' in mr_attrs and 'never-close' in mr_attrs['labels']: continue # If the MR is no longer open, skip to the next MR, # and don't include this MR in the next check - if 'opened' not in a_mr['state']: - self.log.info('state not opened: {}'.format(a_mr['state'])) + if 'opened' not in mr_attrs['state']: + self.log.info('state not opened: {}'.format(mr_attrs['state'])) continue else: still_open_mrs[(project, mr_id, notify_rooms)] = True @@ -447,7 +412,7 @@ class Marge(BotPlugin, CrontabMixin): self.log.info("rooms={}".format(rooms)) # get the group/repo repo, error out if it doesn't exist - project = self.gitlab.getproject(repo) + project = self.gitlab.projects.get(repo) if not project: msg = "Couldn't find repo {}".format(repo) self.log.info("watchrepo: {}".format(msg)) @@ -455,12 +420,12 @@ class Marge(BotPlugin, CrontabMixin): self.log.info('project: {}'.format(project)) - target_project_id = project['id'] + target_project_id = project.id # Check is the project already includes the margebot hook # If no hooks, will it return False or [] ? marge_hook = None - hooks = self.gitlab.getprojecthooks(target_project_id) + hooks = project.hooks.list() self.log.error("hooks = {}".format(hooks)) if hooks is False: msg = "Couldn't find {} hooks".format(repo) @@ -469,27 +434,38 @@ class Marge(BotPlugin, CrontabMixin): else: for a_hook in hooks: self.log.info('a_hook: {}'.format(a_hook)) - if a_hook['merge_requests_events'] and a_hook['url'].startswith(self.webhook_url): + hook_attributes = a_hook.attributes + if hook_attributes['merge_requests_events'] and hook_attributes['url'].startswith(self.webhook_url): marge_hook = a_hook break # If so replace it (or error out ?) url = "{}{}".format(self.webhook_url, rooms) # webhooks_url will end in '/' + hook_updated = True if marge_hook: - old_rooms = marge_hook['url'].split(self.webhook_url, 1)[1] + old_rooms = marge_hook.attributes['url'].split(self.webhook_url, 1)[1] if old_rooms == rooms: msg = "Already reporting {} MRs to the {} room(s)".format(repo, rooms) self.log.info('watchrepo: {}'.format(msg)) return msg else: - hook_updated = self.gitlab.editprojecthook_extra(target_project_id, marge_hook['id'], url, merge_requests=True, extra_data={'enable_ssl_verification': True}) - s_watch_msg = "Updating room list for {} MRs from {} to {}".format(repo, old_rooms, rooms) - s_action = "update" + try: + s_action = "update" + marge_hook.attributes['url'] = url + marge_hook.save() + s_watch_msg = "Updating room list for {} MRs from {} to {}".format(repo, old_rooms, rooms) + except Exception as exp: + hook_updated = False + self.log.error("watchrepo; update hook {} raised exception {}".format(repo, exp)) else: - hook_updated = self.gitlab.addprojecthook_extra(target_project_id, url, merge_requests=True, extra_data={'enable_ssl_verification': True}) - s_watch_msg = "Now watching for new MRs in the {} repo to the {} room(s)".format(repo, rooms) - s_action = "add" + try: + s_action = "add" + project.hooks.create({'url': url, 'merge_request_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 + self.log.error("watchrepo; create hook {} raised exception {}".format(repo, exp)) if not hook_updated: msg = "Couldn't {} hook: {}".format(s_action, repo) @@ -514,21 +490,13 @@ class Marge(BotPlugin, CrontabMixin): # If adding a new repo, check for existing opened/reopened MRs in the repo. else: - # For debugging the 'watchrepo didn't find my MR' issue. - # mr_list = self.gitlab.getmergerequests(target_project_id, page=1, per_page=100) - # self.log.error('juden: mr_list state: {}'.format(mr_list[0]['state'])) for state in ['opened', 'reopened']: - page = 1 - mr_list = self.gitlab.getmergerequests(target_project_id, page=page, per_page=100, state=state) - while (mr_list is not False) and (mr_list != []): - for an_mr in mr_list: - mr_count += 1 - self.log.info('watchrepo: an_mr WATS THE IID\n{}'.format(an_mr)) - mr_id = an_mr['iid'] - open_mrs[(target_project_id, mr_id, rooms)] = True - # Get the next page of MRs - page += 1 - mr_list = self.gitlab.getmergerequests(target_project_id, page=page, per_page=100) + a_project = self.gitlab.projects.get(target_project_id) + mr_list = a_project.mergerequests.list(state=state) + for an_mr in mr_list: + mr_count += 1 + mr_id = an_mr.attributes['iid'] + open_mrs[(target_project_id, mr_id, rooms)] = True self['OPEN_MRS'] = open_mrs @@ -537,7 +505,7 @@ class Marge(BotPlugin, CrontabMixin): elif mr_count == 1: mr_msg = "1 open MR was found in the repo. Run !reviews to see the updated MR list." else: - mr_msg = "{} open MRs were found in the repo. Run !reviews to see the updated MR list." + mr_msg = "{} open MRs were found in the repo. Run !reviews to see the updated MR list.".format(mr_count) return "{}\n{}".format(s_watch_msg, mr_msg) # pragma pylint: disable=unused-argument diff --git a/requirements.txt b/requirements.txt index 9723567..6e00525 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ errcron -pyapi-gitlab +python-gitlab python-dateutil errbot sleekxmpp diff --git a/setup.cfg b/setup.cfg index 6eb7cd1..5b1003c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,4 +4,4 @@ test = pytest [tool:pytest] addopts = --cov=plugins --cov-report term-missing --cov-report html --pep8 --pylint --pylint-rcfile=.pylintrc --ff -x pep8ignore = - *.py E501 \ No newline at end of file + *.py E501 diff --git a/test-requirements.txt b/test-requirements.txt index c35295c..37e388c 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,3 +1,4 @@ +httmock pytest pytest-cov pytest-mock diff --git a/tests/test_marge.py b/tests/test_marge.py index defbed9..9ab5d90 100644 --- a/tests/test_marge.py +++ b/tests/test_marge.py @@ -19,10 +19,186 @@ import pytest import requests import errbot from errbot.backends.test import testbot # pylint: disable=unused-import -import gitlab +from httmock import urlmatch, HTTMock from plugins.marge import deltastr +# HTTMock urlmatchers + + +@urlmatch(path=r'/api/v4/users/author_id$') +def mock_users_get_author_id(url, request): + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps({'username': 'author_id username', 'id': 3001})} + + +@urlmatch(path=r'/api/v4/users/[^/\?]+$') +def mock_users_get_unexpected(url, request): + return {'status_code': 404} + + +@urlmatch(path=r'/api/v4/users', query='username=gbin') +def mock_users_search_gbin(url, request): + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps([{'username': 'gbin', 'id': 3002}])} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/hooks$') +def mock_projects_hooks_list(url, request): + if request.method == "GET": + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': '[]'} + elif request.method == "POST": + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps({"id": "hook_id", + "merge_requests_events": True, + "url": "url"})} + elif request.method == "PUT": + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps({"id": "hook_id", + "merge_requests_events": True, + "url": "url"})} + else: + return {'status-code': 404} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+$') +def mock_projects_get(url, request): + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps({"id": "projectid"})} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/merge_requests') +def mock_projects_mergerequests_list_none(url, request): + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': '[]'} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/merge_requests/[^/]+') +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}, + 'created_at': 'Oct 29, 2017 2:37am', + 'merge_status': 'can_be_merged', + 'state': 'opened', + 'upvotes': 0, + 'web_url': 'http://gitlab.example.com/sample/mr/2001', + 'work_in_progress': False})} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/merge_requests/[^/]+') +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}, + 'created_at': 'Oct 29, 2017 2:37am', + 'merge_status': 'can_be_merged', + 'state': 'opened', + 'upvotes': 0, + 'web_url': 'http://gitlab.example.com/sample/mr/2001', + 'work_in_progress': True})} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/merge_requests/[^/]+') +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}, + 'created_at': 'Oct 29, 2017 2:37am', + 'merge_status': 'can_be_merged', + 'state': 'opened', + 'upvotes': 1, + 'web_url': 'http://gitlab.example.com/sample/mr/2001', + 'work_in_progress': False})} + + +@urlmatch(path=r'/api/v4/projects/[^/]+/merge_requests/[^/]+/award_emoji') +def mock_projects_mergerequests_awardemojis_list(url, request): + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps([{'id': 'id', + 'name': 'thumbsup', + 'user': {'username': 'ReviewerX'}}])} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/merge_requests/[^/]+') +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}, + 'created_at': 'Oct 29, 2017 2:37am', + 'merge_status': 'can_be_merged', + 'state': 'opened', + 'upvotes': 2, + 'web_url': 'http://gitlab.example.com/sample/mr/2001', + 'work_in_progress': False})} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/merge_requests/[^/]+') +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}, + 'created_at': 'Oct 29, 2017 2:37am', + 'merge_status': 'merge_conflicts', + 'state': 'opened', + 'upvotes': 2, + 'web_url': 'http://gitlab.example.com/sample/mr/2001', + 'work_in_progress': False})} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/merge_requests$', query='state=opened') +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}, + 'created_at': 'Oct 29, 2017 2:37am', + 'merge_status': 'can_be_merged', + 'state': 'opened', + 'upvotes': 0, + 'web_url': 'http://gitlab.example.com/sample/mr/2001', + 'work_in_progresso': False}])} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/merge_requests', query='state=reopened') +def mock_projects_mergerequests_list_reopened(url, request): + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps([])} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/hooks/[^/]+$') +def mock_projects_hooks_get_already_watching(url, request): + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps({'id': 'hook_id', + 'merge_requests_events': True, + 'url': 'https://webhook.errbot.com:3142/margebot/room1,room2,room3'})} + + +@urlmatch(path=r'^/api/v4/projects/[^/]+/hooks$') +def mock_projects_hooks_list_already_watching(url, request): + return {'status_code': 200, + 'headers': {'content-type': 'application/json'}, + 'content': json.dumps([{'id': 'hook_id', + 'merge_requests_events': True, + 'url': 'https://webhook.errbot.com:3142/margebot/room1,room2,room3'}])} + + class TestMarge(object): """ Margebot Tests @@ -54,10 +230,6 @@ class TestMarge(object): return testbot - @pytest.fixture - def margebot_no_reviews(self, margebot, monkeypatch): - return margebot - @pytest.fixture def margebot_one_review(self, margebot, monkeypatch): @@ -67,174 +239,6 @@ class TestMarge(object): monkeypatch.setattr(errbot.storage.StoreMixin, '__getitem__', mock_get, raising=False) return margebot - @pytest.fixture - def one_wip_review(self, margebot_one_review, monkeypatch): - - def mock_getmergerequest(self, project, iid): - return {'author': {'id': 2001}, - 'created_at': 'Oct 29, 2017 2:37am', - 'merge_status': 'can_be_merged', - 'state': 'opened', - 'upvotes': 0, - 'web_url': 'http://gitlab.example.com/sample/mr/2001', - 'work_in_progress': True} - monkeypatch.setattr(gitlab.Gitlab, 'getmergerequest', mock_getmergerequest) - return margebot_one_review - - @pytest.fixture - def one_waiting_review(self, margebot_one_review, monkeypatch): - - def mock_getmergerequest(self, project, iid): - return {'author': {'id': 2001}, - 'created_at': 'Oct 29, 2017 2:37am', - 'iid': 1, - 'merge_status': 'can_be_merged', - 'state': 'opened', - 'target_project_id': 1, - 'upvotes': 1, - 'web_url': 'http://gitlab.example.com/sample/mr/2001', - 'work_in_progress': False} - monkeypatch.setattr(gitlab.Gitlab, 'getmergerequest', mock_getmergerequest) - - def mock_getawardemoji(self, project, iid): - return [{'name': 'happycat', - 'user': {'username': 'ReviewerY'}}, - {'name': 'thumbsup', - 'user': {'username': 'ReviewerX'}}] - monkeypatch.setattr(gitlab.Gitlab, 'getawardemoji', mock_getawardemoji) - - return margebot_one_review - - @pytest.fixture - def one_conflicted_review(self, margebot_one_review, monkeypatch): - - def mock_getmergerequest(self, project, iid): - return {'author': {'id': 2001}, - 'created_at': 'Oct 29, 2017 2:37am', - 'merge_status': 'merge_conflicts', - 'state': 'opened', - 'upvotes': 2, - 'web_url': 'http://gitlab.example.com/sample/mr/2001', - 'work_in_progress': False} - monkeypatch.setattr(gitlab.Gitlab, 'getmergerequest', mock_getmergerequest) - return margebot_one_review - - @pytest.fixture - def one_mergable_review(self, margebot_one_review, monkeypatch): - - def mock_getmergerequest(self, project, iid): - return {'author': {'id': 2001}, - 'created_at': 'Oct 29, 2017 2:37am', - 'merge_status': 'can_be_merged', - 'state': 'opened', - 'upvotes': 2, - 'web_url': 'http://gitlab.example.com/sample/mr/2001', - 'work_in_progress': False} - monkeypatch.setattr(gitlab.Gitlab, 'getmergerequest', mock_getmergerequest) - return margebot_one_review - - @pytest.fixture - def gitlab(self, monkeypatch): - - def mock_getuser(self, user): - if user == '(missing)': - return False - else: - return {'username': user + ' username'} - monkeypatch.setattr(gitlab.Gitlab, 'getuser', mock_getuser) - - def mock_getusers(self, search=None): - if search: - ((_, val)) = search - if val == "(missing)": - return [] - else: - return [{'user': val, 'id': 3001}] - else: - return [{'user': 'user1', 'id': 3001}, {'user': 'user2', 'id': 3002}] - monkeypatch.setattr(gitlab.Gitlab, 'getusers', mock_getusers) - - def mock_getproject(self, repo): - return { - 'id': 'projectid' - } - monkeypatch.setattr(gitlab.Gitlab, 'getproject', mock_getproject) - - def mock_getprojecthooks(self, repo): - if repo == "(missing)": - return [] - else: - return [{'id': 'hook_id', 'merge_requests_events': True, 'url': 'url'}] - monkeypatch.setattr(gitlab.Gitlab, 'getprojecthooks', mock_getprojecthooks) - - # Waiting on Gitlab 8.9 or greater - # def mock_getapprovals(id): - # return [] - # monkeypatch.setattr(gitlab.Gitlab, 'getapprovals', mock_getapprovals) - - def mock_getmergerequests(self, project, page, per_page, state=None): - return [] - monkeypatch.setattr(gitlab.Gitlab, 'getmergerequests', mock_getmergerequests) - - def mock_addprojecthook_extra(self, project, url, push=False, issues=False, merge_requests=False, tag_push=False, extra_data=None): - return True - monkeypatch.setattr(gitlab.Gitlab, 'addprojecthook_extra', mock_addprojecthook_extra, raising=False) - - def mock_editprojecthook_extra(self, project, hook_id, url, push=False, issues=False, merge_requests=False, tag_push=False, extra_data=None): - return True - monkeypatch.setattr(gitlab.Gitlab, 'editprojecthook_extra', mock_editprojecthook_extra, raising=False) - - return gitlab.Gitlab - - -# @pytest.fixture -# def MargeGitlab(self, monkeypatch): -# def mock_addprojecthook_extra(self, project, url, push=False, issues=False, merge_requests=False, tag_push=False, extra_data=None): -# return True -# monkeypatch.setattr(plugins.marge.MargeGitlab, 'addprojecthook_extra') -# -# def mock_editprojecthook_extra(self, project, hook_id, url, push=False, issues=False, merge_requests=False, tag_push=False, extra_data=None): -# return True -# monkeypatch.setattr(plugins.marge.MargeGitlab, 'editprojecthook_extra') -# return plugins.marge.MargeGitlab - - @pytest.fixture - def gitlab_no_reviews(self, gitlab, monkeypatch): - def mock_getmergerequest(self, project, iid): - return {} - monkeypatch.setattr(gitlab, 'getuser', mock_getmergerequest) - return gitlab - - @pytest.fixture - def gitlab_one_review(self, gitlab, monkeypatch): - ret_mr = {'iid': 'mr_id', - 'author': {'id': 2001}, - 'created_at': 'Oct 29, 2017 2:37am', - 'merge_status': 'can_be_merged', - 'state': 'opened', - 'upvotes': 0, - 'web_url': 'http://gitlab.example.com/sample/mr/2001', - 'work_in_progress': False} - - def mock_getmergerequest(self, project, iid): - return ret_mr - monkeypatch.setattr(gitlab, 'getmergerequest', mock_getmergerequest) - - def mock_getmergerequests(self, project, page, per_page, state=None): - if state and state == 'opened': - return [ret_mr] - else: - return [] - monkeypatch.setattr(gitlab, 'getmergerequests', mock_getmergerequests) - return gitlab - - @pytest.fixture - def gitlab_already_watching(self, gitlab, monkeypatch): - def mock_getprojecthooks(self, project): - return [{'id': 'hook_id', 'merge_requests_events': True, 'url': 'https://webhook.errbot.com:3142/margebot/room1,room2,room3'}] - monkeypatch.setattr(gitlab, 'getprojecthooks', mock_getprojecthooks) - return gitlab - # ============================================================================ @pytest.mark.parametrize("rdelta,expected", [ @@ -263,29 +267,41 @@ class TestMarge(object): margebot.push_message('!webstatus') assert 'margebot/' in margebot.pop_message() - def test_watchrepo(self, margebot, gitlab): - margebot.push_message('!watchrepo group/new_repo room1,room2,room3') - pm = margebot.pop_message() - assert 'Now watching for new MRs in the group/new_repo repo to the room1,room2,room3 room(s)' in pm - assert 'No open MRs were found in the repo.' in pm - - def test_watchrepo_already_watching_repo(self, margebot, gitlab_already_watching): - margebot.push_message('!watchrepo group/new_repo room1,room2,room3') - pm = margebot.pop_message() - assert 'Already reporting group/new_repo MRs to the room1,room2,room3 room(s)' in pm - - def test_watchrepo_updating_roomlist(self, margebot, gitlab_already_watching): - margebot.push_message('!watchrepo group/new_repo room4,room5,room6') - pm = margebot.pop_message() - assert 'Updating room list for group/new_repo MRs from room1,room2,room3 to room4,room5,room6' in pm - - def test_watchrepo_existing_mr(self, margebot, gitlab_one_review): - margebot.push_message('!watchrepo sample/mr room1,room2,room3') - pm = margebot.pop_message() - assert 'Now watching for new MRs in the sample/mr repo to the room1,room2,room3 room(s)' in pm - assert '1 open MR was found in the repo.' in pm - - def test_gitlab_hook(self, margebot, gitlab): + def test_watchrepo(self, margebot): + with HTTMock(mock_projects_get, + mock_projects_hooks_list, + mock_projects_mergerequests_list_none): + margebot.push_message('!watchrepo group/new_repo room1,room2,room3') + pm = margebot.pop_message() + assert 'Now watching for new MRs in the group/new_repo repo to the room1,room2,room3 room(s)' in pm + assert 'No open MRs were found in the repo.' in pm + + def test_watchrepo_already_watching_repo(self, margebot): + with HTTMock(mock_projects_hooks_list_already_watching, + mock_projects_get): + margebot.push_message('!watchrepo group/new_repo room1,room2,room3') + pm = margebot.pop_message() + assert 'Already reporting group/new_repo MRs to the room1,room2,room3 room(s)' in pm + + def test_watchrepo_updating_roomlist(self, margebot): + with HTTMock(mock_projects_hooks_list_already_watching, + mock_projects_hooks_get_already_watching, + mock_projects_get): + margebot.push_message('!watchrepo group/new_repo room4,room5,room6') + pm = margebot.pop_message() + assert 'Updating room list for group/new_repo MRs from room1,room2,room3 to room4,room5,room6' in pm + + def test_watchrepo_existing_mr(self, margebot): + with HTTMock(mock_projects_get, + mock_projects_hooks_list, + mock_projects_mergerequests_list_unreviewed_review, + mock_projects_mergerequests_list_reopened): + margebot.push_message('!watchrepo sample/mr room1,room2,room3') + pm = margebot.pop_message() + assert 'Now watching for new MRs in the sample/mr repo to the room1,room2,room3 room(s)' in pm + assert '1 open MR was found in the repo.' in pm + + def test_gitlab_hook(self, margebot): request = json.dumps({'object_kind': 'merge_request', 'object_attributes': { 'state': 'opened', @@ -297,12 +313,13 @@ class TestMarge(object): 'iid': 'iid'}, 'project': { 'homepage': 'url'}}) - margebot.push_message("!webhook test /margebot/room1,room2 " + request) - assert 'Hi there ! author_id username has opened a new MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() - margebot.push_message('!reviews') - assert 'Hi there ! author_id username has opened a new MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() + with HTTMock(mock_users_get_author_id): + margebot.push_message("!webhook test /margebot/room1,room2 " + request) + assert 'Hi there ! author_id username has opened a new MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() + 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_wip(self, margebot, gitlab): + def test_gitlab_hook_wip(self, margebot): request = json.dumps({'object_kind': 'merge_request', 'object_attributes': { 'state': 'opened', @@ -314,12 +331,13 @@ class TestMarge(object): 'iid': 'iid'}, 'project': { 'homepage': 'url'}}) - margebot.push_message("!webhook test /margebot/room1,room2 " + request) - assert 'Hi there ! author_id username has opened a new WIP MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() - margebot.push_message('!reviews') - assert 'Hi there ! author_id username has opened a new WIP MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() + with HTTMock(mock_users_get_author_id): + margebot.push_message("!webhook test /margebot/room1,room2 " + request) + assert 'Hi there ! author_id username has opened a new WIP MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() + margebot.push_message('!reviews') + 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, gitlab): + def test_gitlab_hook_unexpected_user(self, margebot): request = json.dumps({'object_kind': 'merge_request', 'object_attributes': { 'state': 'opened', @@ -332,12 +350,13 @@ class TestMarge(object): 'project': { 'homepage': 'url'}}) - margebot.push_message("!webhook test /margebot/room1,room2 " + request) - assert 'Hi there ! (missing) has opened a new MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() - margebot.push_message('!reviews') - assert 'Hi there ! (missing) has opened a new MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() + with HTTMock(mock_users_get_unexpected): + margebot.push_message("!webhook test /margebot/room1,room2 " + request) + assert 'Hi there ! (missing) has opened a new MR: \"title\"\nurl/merge_requests/iid' in margebot.pop_message() + 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, gitlab, caplog): + def test_gitlab_hook_unexpected_object_kind(self, margebot, caplog): request = json.dumps({'object_kind': 'not_merge_request', 'object_attributes': { 'state': 'opened', @@ -350,62 +369,82 @@ class TestMarge(object): 'project': { 'homepage': 'url'}}) - margebot.push_message("!webhook test /margebot/room1,room2 " + request) - 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 - - def test_nothing_to_review(self, margebot, gitlab_no_reviews): - margebot.push_message('!reviews') - assert 'Hi gbin: I found no open MRs for you.' in margebot.pop_message() - - def test_get_one_open_mr(self, margebot_one_review, gitlab_one_review): - margebot_one_review.push_message('!reviews') - output = margebot_one_review.pop_message() - assert 'These MRs need some attention' in output - assert 'http://gitlab.example.com/sample/mr/2001' in output - assert 'No upvotes, please review.' in output - - def test_get_one_wip_mr(self, one_wip_review, gitlab): - one_wip_review.push_message('!reviews') - output = one_wip_review.pop_message() - assert 'These MRs need some attention' in output - assert 'http://gitlab.example.com/sample/mr/2001' in output - assert 'No upvotes, please review but still WIP.' in output - - def test_get_one_waiting_mr(self, margebot, one_waiting_review, gitlab): - one_waiting_review.push_message('!reviews') - output = margebot.pop_message() - assert 'These MRs need some attention' in output - assert 'http://gitlab.example.com/sample/mr/2001' in output - assert 'Approved by ReviewerX and waiting for another upvote.' in output - - def test_get_one_mergable_mr(self, margebot, one_mergable_review, gitlab): - one_mergable_review.push_message('!reviews') - output = margebot.pop_message() - assert 'These MRs need some attention' in output - assert 'http://gitlab.example.com/sample/mr/2001' in output - assert 'Has 2+ upvotes and could be merged in now.' in output - - def test_get_one_conflicted_mr(self, margebot, one_conflicted_review, gitlab): - one_conflicted_review.push_message('!reviews') - output = margebot.pop_message(timeout=1) - assert 'These MRs need some attention' in output - assert 'http://gitlab.example.com/sample/mr/2001' in output - assert 'Has 2+ upvotes and could be merged in now except there are merge conflicts.' in output - - def test_crontab_hook(self, one_waiting_review, gitlab, monkeypatch, mocker): - plugin = one_waiting_review._bot.plugin_manager.get_plugin_obj_by_name('Marge') + with HTTMock(mock_users_search_gbin): + margebot.push_message("!webhook test /margebot/room1,room2 " + request) + 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 + + def test_nothing_to_review(self, margebot): + with HTTMock(mock_users_search_gbin): + margebot.push_message('!reviews') + assert 'Hi gbin: I found no open MRs for you.' in margebot.pop_message() + + def test_get_one_open_mr(self, margebot_one_review): + with HTTMock(mock_users_search_gbin, + mock_projects_get, + mock_projects_mergerequests_get_unreviewed_review): + margebot_one_review.push_message('!reviews') + output = margebot_one_review.pop_message() + assert 'These MRs need some attention' in output + assert 'http://gitlab.example.com/sample/mr/2001' in output + assert 'No upvotes, please review.' in output + + def test_get_one_wip_mr(self, margebot_one_review): + with HTTMock(mock_users_search_gbin, + mock_projects_get, + mock_projects_mergerequests_get_wip_review): + margebot_one_review.push_message('!reviews') + output = margebot_one_review.pop_message() + assert 'These MRs need some attention' in output + assert 'http://gitlab.example.com/sample/mr/2001' in output + assert 'No upvotes, please review but still WIP.' in output + + def test_get_one_waiting_mr(self, margebot_one_review): + with HTTMock(mock_users_search_gbin, + mock_projects_get, + mock_projects_mergerequests_awardemojis_list, + mock_projects_mergerequests_get_waiting_review): + margebot_one_review.push_message('!reviews') + output = margebot_one_review.pop_message() + assert 'These MRs need some attention' in output + assert 'http://gitlab.example.com/sample/mr/2001' in output + assert 'Approved by ReviewerX and waiting for another upvote.' in output + + def test_get_one_mergable_mr(self, margebot_one_review): + with HTTMock(mock_users_search_gbin, + mock_projects_get, + mock_projects_mergerequests_get_mergable_review): + margebot_one_review.push_message('!reviews') + output = margebot_one_review.pop_message() + assert 'These MRs need some attention' in output + assert 'http://gitlab.example.com/sample/mr/2001' in output + assert 'Has 2+ upvotes and could be merged in now.' in output + + def test_get_one_conflicted_mr(self, margebot_one_review): + with HTTMock(mock_users_search_gbin, + mock_projects_get, + mock_projects_mergerequests_get_conflicted_review): + margebot_one_review.push_message('!reviews') + output = margebot_one_review.pop_message(timeout=1) + assert 'These MRs need some attention' in output + assert 'http://gitlab.example.com/sample/mr/2001' in output + assert 'Has 2+ upvotes and could be merged in now except there are merge conflicts.' in output + + def test_crontab_hook(self, margebot_one_review, monkeypatch, mocker): + plugin = margebot_one_review._bot.plugin_manager.get_plugin_obj_by_name('Marge') def mock_rooms(): return [mocker.MagicMock(node='room1'), mocker.MagicMock(node='room2')] monkeypatch.setattr(plugin, 'rooms', mock_rooms) - plugin.crontab_hook("unused") - output = one_waiting_review.pop_message() - assert 'These MRs need some attention' in output - assert 'http://gitlab.example.com/sample/mr/2001 (opened' in output + with HTTMock(mock_projects_get, + mock_projects_mergerequests_get_unreviewed_review): + plugin.crontab_hook("unused") + output = margebot_one_review.pop_message() + assert 'These MRs need some attention' in output + assert 'http://gitlab.example.com/sample/mr/2001 (opened' in output def test_margebot_unknown_command(self, margebot): margebot.push_message('Margebot, alacazam') -- 2.39.2