engineering/unrhodecode
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

pull-requests: ensure merge response provide more details

Browse source 
- refactor merge responses with detailed messages
- fixes #5520
- code cleanups
This commit is contained in:
Daniel Dourvaris 2018-12-30 11:00:09 +01:00
parent a755afe06e
commit 599f9b7964
11 changed files with 271 additions and 176 deletions
Download patch file Download diff file Expand all files Collapse all files

2
rhodecode/api/views/pull_request_api.py
View file

@ -313,7 +313,7 @@ def merge_pull_request(
# In previous versions the merge response directly contained the merge
# commit id. It is now contained in the merge reference object. To be
# backwards compatible we have to extract it again.
merge_response = merge_response._asdict()
merge_response = merge_response.asdict()
merge_response['merge_commit_id'] = merge_response['merge_ref'].commit_id
return merge_response

80
rhodecode/apps/repository/tests/test_repo_pullrequests.py
View file

@ -32,7 +32,6 @@ from rhodecode.model.pull_request import PullRequestModel
from rhodecode.model.user import UserModel
from rhodecode.tests import (
assert_session_flash, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN)
from rhodecode.tests.utils import AssertResponse
def route_path(name, params=None, **kwargs):
@ -233,8 +232,7 @@ class TestPullrequestsView(object):
route_path('pullrequest_update',
repo_name=pull_request.target_repo.repo_name,
pull_request_id=pull_request_id),
params={'update_commits': 'true',
'csrf_token': csrf_token})
params={'update_commits': 'true', 'csrf_token': csrf_token})
expected_msg = str(PullRequestModel.UPDATE_STATUS_MESSAGES[
UpdateFailureReason.MISSING_SOURCE_REF])
@ -244,7 +242,8 @@ class TestPullrequestsView(object):
from rhodecode.lib.vcs.backends.base import MergeFailureReason
pull_request = pr_util.create_pull_request(
approved=True, mergeable=True)
pull_request.target_ref = 'branch:invalid-branch:invalid-commit-id'
unicode_reference = u'branch:invalid-branch:invalid-commit-id'
pull_request.target_ref = unicode_reference
Session().add(pull_request)
Session().commit()
@ -255,12 +254,12 @@ class TestPullrequestsView(object):
pull_request_id=pull_request_id)
response = self.app.get(pull_request_url)
assertr = AssertResponse(response)
expected_msg = PullRequestModel.MERGE_STATUS_MESSAGES[
MergeFailureReason.MISSING_TARGET_REF]
assertr.element_contains(
'span[data-role="merge-message"]', str(expected_msg))
target_ref_id = 'invalid-branch'
merge_resp = MergeResponse(
True, True, '', MergeFailureReason.MISSING_TARGET_REF,
metadata={'target_ref': PullRequest.unicode_to_reference(unicode_reference)})
response.assert_response().element_contains(
'span[data-role="merge-message"]', merge_resp.merge_status_message)
def test_comment_and_close_pull_request_custom_message_approved(
self, pr_util, csrf_token, xhr_header):
@ -272,8 +271,8 @@ class TestPullrequestsView(object):
self.app.post(
route_path('pullrequest_comment_create',
repo_name=pull_request.target_repo.scm_instance().name,
pull_request_id=pull_request_id),
repo_name=pull_request.target_repo.scm_instance().name,
pull_request_id=pull_request_id),
params={
'close_pull_request': '1',
'text': 'Closing a PR',
@ -608,8 +607,7 @@ class TestPullrequestsView(object):
response = self.app.post(
route_path('pullrequest_merge',
repo_name=repo_name,
pull_request_id=pull_request_id),
repo_name=repo_name, pull_request_id=pull_request_id),
params={'csrf_token': csrf_token}).follow()
assert response.status_int == 200
@ -624,10 +622,13 @@ class TestPullrequestsView(object):
pull_request_id = pull_request.pull_request_id
repo_name = pull_request.target_repo.scm_instance().name
merge_resp = MergeResponse(True, False, 'STUB_COMMIT_ID',
MergeFailureReason.PUSH_FAILED,
metadata={'target': 'shadow repo',
'merge_commit': 'xxx'})
model_patcher = mock.patch.multiple(
PullRequestModel,
merge_repo=mock.Mock(return_value=MergeResponse(
True, False, 'STUB_COMMIT_ID', MergeFailureReason.PUSH_FAILED)),
merge_repo=mock.Mock(return_value=merge_resp),
merge_status=mock.Mock(return_value=(True, 'WRONG_MESSAGE')))
with model_patcher:
@ -637,8 +638,10 @@ class TestPullrequestsView(object):
pull_request_id=pull_request_id),
params={'csrf_token': csrf_token}, status=302)
assert_session_flash(response, PullRequestModel.MERGE_STATUS_MESSAGES[
MergeFailureReason.PUSH_FAILED])
merge_resp = MergeResponse(True, True, '', MergeFailureReason.PUSH_FAILED,
metadata={'target': 'shadow repo',
'merge_commit': 'xxx'})
assert_session_flash(response, merge_resp.merge_status_message)
def test_update_source_revision(self, backend, csrf_token):
commits = [
@ -909,11 +912,11 @@ class TestPullrequestsView(object):
pull_request_id=pull_request.pull_request_id))
assert response.status_int == 200
assert_response = AssertResponse(response)
assert_response.element_contains(
response.assert_response().element_contains(
'#changeset_compare_view_content .alert strong',
'Missing commits')
assert_response.element_contains(
response.assert_response().element_contains(
'#changeset_compare_view_content .alert',
'This pull request cannot be displayed, because one or more'
' commits no longer exist in the source repository.')
@ -941,15 +944,15 @@ class TestPullrequestsView(object):
pull_request_id=pull_request.pull_request_id))
assert response.status_int == 200
assert_response = AssertResponse(response)
assert_response.element_contains(
response.assert_response().element_contains(
'#changeset_compare_view_content .alert strong',
'Missing commits')
assert_response.element_contains(
response.assert_response().element_contains(
'#changeset_compare_view_content .alert',
'This pull request cannot be displayed, because one or more'
' commits no longer exist in the source repository.')
assert_response.element_contains(
response.assert_response().element_contains(
'#update_commits',
'Update commits')
@ -987,8 +990,7 @@ class TestPullrequestsView(object):
pull_request_id=pull_request.pull_request_id))
assert response.status_int == 200
assert_response = AssertResponse(response)
assert_response.element_contains(
response.assert_response().element_contains(
'#changeset_compare_view_content .alert strong',
'Missing commits')
@ -1004,12 +1006,11 @@ class TestPullrequestsView(object):
repo_name=pull_request.target_repo.scm_instance().name,
pull_request_id=pull_request.pull_request_id))
assert response.status_int == 200
assert_response = AssertResponse(response)
origin = assert_response.get_element('.pr-origininfo .tag')
origin = response.assert_response().get_element('.pr-origininfo .tag')
origin_children = origin.getchildren()
assert len(origin_children) == 1
target = assert_response.get_element('.pr-targetinfo .tag')
target = response.assert_response().get_element('.pr-targetinfo .tag')
target_children = target.getchildren()
assert len(target_children) == 1
@ -1038,13 +1039,12 @@ class TestPullrequestsView(object):
repo_name=pull_request.target_repo.scm_instance().name,
pull_request_id=pull_request.pull_request_id))
assert response.status_int == 200
assert_response = AssertResponse(response)
origin = assert_response.get_element('.pr-origininfo .tag')
origin = response.assert_response().get_element('.pr-origininfo .tag')
assert origin.text.strip() == 'bookmark: origin'
assert origin.getchildren() == []
target = assert_response.get_element('.pr-targetinfo .tag')
target = response.assert_response().get_element('.pr-targetinfo .tag')
assert target.text.strip() == 'bookmark: target'
assert target.getchildren() == []
@ -1060,13 +1060,12 @@ class TestPullrequestsView(object):
repo_name=pull_request.target_repo.scm_instance().name,
pull_request_id=pull_request.pull_request_id))
assert response.status_int == 200
assert_response = AssertResponse(response)
origin = assert_response.get_element('.pr-origininfo .tag')
origin = response.assert_response().get_element('.pr-origininfo .tag')
assert origin.text.strip() == 'tag: origin'
assert origin.getchildren() == []
target = assert_response.get_element('.pr-targetinfo .tag')
target = response.assert_response().get_element('.pr-targetinfo .tag')
assert target.text.strip() == 'tag: target'
assert target.getchildren() == []
@ -1090,12 +1089,13 @@ class TestPullrequestsView(object):
repo_name=target_repo.name,
pull_request_id=pr_id))
assertr = AssertResponse(response)
if mergeable:
assertr.element_value_contains('input.pr-mergeinfo', shadow_url)
assertr.element_value_contains('input.pr-mergeinfo ', 'pr-merge')
response.assert_response().element_value_contains(
'input.pr-mergeinfo', shadow_url)
response.assert_response().element_value_contains(
'input.pr-mergeinfo ', 'pr-merge')
else:
assertr.no_element_exists('.pr-mergeinfo')
response.assert_response().no_element_exists('.pr-mergeinfo')
@pytest.mark.usefixtures('app')

6
rhodecode/apps/repository/views/repo_pull_requests.py
View file

@ -1181,10 +1181,8 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
h.flash(msg, category='success')
else:
log.debug(
"The merge was not successful. Merge response: %s",
merge_resp)
msg = PullRequestModel().merge_status_message(
merge_resp.failure_reason)
"The merge was not successful. Merge response: %s", merge_resp)
msg = merge_resp.merge_status_message
h.flash(msg, category='error')
def _update_reviewers(self, pull_request, review_members, reviewer_rules):

110
rhodecode/lib/vcs/backends/base.py
View file

@ -21,20 +21,20 @@
"""
Base module for all VCS systems
"""
import collections
import os
import re
import time
import shutil
import datetime
import fnmatch
import itertools
import logging
import os
import re
import time
import collections
import warnings
import shutil
from zope.cachedescriptors.property import Lazy as LazyProperty
from rhodecode.translation import lazy_ugettext
from rhodecode.lib.utils2 import safe_str, safe_unicode
from rhodecode.lib.vcs import connection
from rhodecode.lib.vcs.utils import author_name, author_email
@ -54,9 +54,6 @@ FILEMODE_DEFAULT = 0o100644
FILEMODE_EXECUTABLE = 0o100755
Reference = collections.namedtuple('Reference', ('type', 'name', 'commit_id'))
MergeResponse = collections.namedtuple(
'MergeResponse',
('possible', 'executed', 'merge_ref', 'failure_reason'))
class MergeFailureReason(object):
@ -142,6 +139,92 @@ class UpdateFailureReason(object):
MISSING_SOURCE_REF = 5
class MergeResponse(object):
# uses .format(**metadata) for variables
MERGE_STATUS_MESSAGES = {
MergeFailureReason.NONE: lazy_ugettext(
u'This pull request can be automatically merged.'),
MergeFailureReason.UNKNOWN: lazy_ugettext(
u'This pull request cannot be merged because of an unhandled exception. '
u'{exception}'),
MergeFailureReason.MERGE_FAILED: lazy_ugettext(
u'This pull request cannot be merged because of merge conflicts.'),
MergeFailureReason.PUSH_FAILED: lazy_ugettext(
u'This pull request could not be merged because push to '
u'target:`{target}@{merge_commit}` failed.'),
MergeFailureReason.TARGET_IS_NOT_HEAD: lazy_ugettext(
u'This pull request cannot be merged because the target '
u'`{target_ref.name}` is not a head.'),
MergeFailureReason.HG_SOURCE_HAS_MORE_BRANCHES: lazy_ugettext(
u'This pull request cannot be merged because the source contains '
u'more branches than the target.'),
MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS: lazy_ugettext(
u'This pull request cannot be merged because the target '
u'has multiple heads: `{heads}`.'),
MergeFailureReason.TARGET_IS_LOCKED: lazy_ugettext(
u'This pull request cannot be merged because the target repository is '
u'locked by {locked_by}.'),
MergeFailureReason.MISSING_TARGET_REF: lazy_ugettext(
u'This pull request cannot be merged because the target '
u'reference `{target_ref.name}` is missing.'),
MergeFailureReason.MISSING_SOURCE_REF: lazy_ugettext(
u'This pull request cannot be merged because the source '
u'reference `{source_ref.name}` is missing.'),
MergeFailureReason.SUBREPO_MERGE_FAILED: lazy_ugettext(
u'This pull request cannot be merged because of conflicts related '
u'to sub repositories.'),
# Deprecations
MergeFailureReason._DEPRECATED_MISSING_COMMIT: lazy_ugettext(
u'This pull request cannot be merged because the target or the '
u'source reference is missing.'),
}
def __init__(self, possible, executed, merge_ref, failure_reason, metadata=None):
self.possible = possible
self.executed = executed
self.merge_ref = merge_ref
self.failure_reason = failure_reason
self.metadata = metadata or {}
def __repr__(self):
return '<MergeResponse:{} {}>'.format(self.label, self.failure_reason)
def __eq__(self, other):
same_instance = isinstance(other, self.__class__)
return same_instance \
and self.possible == other.possible \
and self.executed == other.executed \
and self.failure_reason == other.failure_reason
@property
def label(self):
label_dict = dict((v, k) for k, v in MergeFailureReason.__dict__.items() if
not k.startswith('_'))
return label_dict.get(self.failure_reason)
@property
def merge_status_message(self):
"""
Return a human friendly error message for the given merge status code.
"""
msg = safe_unicode(self.MERGE_STATUS_MESSAGES[self.failure_reason])
try:
return msg.format(**self.metadata)
except Exception:
log.exception('Failed to format %s message', self)
return msg
def asdict(self):
data = {}
for k in ['possible', 'executed', 'merge_ref', 'failure_reason']:
data[k] = getattr(self, k)
return data
class BaseRepository(object):
"""
Base Repository for final backends
@ -501,12 +584,11 @@ class BaseRepository(object):
repo_id, workspace_id, target_ref, source_repo,
source_ref, message, user_name, user_email, dry_run=dry_run,
use_rebase=use_rebase, close_branch=close_branch)
except RepositoryError:
log.exception(
'Unexpected failure when running merge, dry-run=%s',
dry_run)
except RepositoryError as exc:
log.exception('Unexpected failure when running merge, dry-run=%s', dry_run)
return MergeResponse(
False, False, None, MergeFailureReason.UNKNOWN)
False, False, None, MergeFailureReason.UNKNOWN,
metadata={'exception': str(exc)})
def _merge_repo(self, repo_id, workspace_id, target_ref,
source_repo, source_ref, merge_message,

25
rhodecode/lib/vcs/backends/git/repository.py
View file

@ -911,11 +911,15 @@ class GitRepository(BaseRepository):
source_repo, source_ref, merge_message,
merger_name, merger_email, dry_run=False,
use_rebase=False, close_branch=False):
log.debug('Executing merge_repo with %s strategy, dry_run mode:%s',
'rebase' if use_rebase else 'merge', dry_run)
if target_ref.commit_id != self.branches[target_ref.name]:
log.warning('Target ref %s commit mismatch %s vs %s', target_ref,
target_ref.commit_id, self.branches[target_ref.name])
return MergeResponse(
False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD)
False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD,
metadata={'target_ref': target_ref})
shadow_repository_path = self._maybe_prepare_merge_workspace(
repo_id, workspace_id, target_ref, source_ref)
@ -943,7 +947,8 @@ class GitRepository(BaseRepository):
target_ref, target_ref.commit_id,
shadow_repo.branches[target_ref.name])
return MergeResponse(
False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD)
False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD,
metadata={'target_ref': target_ref})
# calculate new branch
pr_branch = shadow_repo._get_new_pr_branch(
@ -954,12 +959,15 @@ class GitRepository(BaseRepository):
try:
shadow_repo._local_fetch(source_repo.path, source_ref.name)
except RepositoryError:
log.exception('Failure when doing local fetch on git shadow repo')
log.exception('Failure when doing local fetch on '
'shadow repo: %s', shadow_repo)
return MergeResponse(
False, False, None, MergeFailureReason.MISSING_SOURCE_REF)
False, False, None, MergeFailureReason.MISSING_SOURCE_REF,
metadata={'source_ref': source_ref})
merge_ref = None
merge_failure_reason = MergeFailureReason.NONE
metadata = {}
try:
shadow_repo._local_merge(merge_message, merger_name, merger_email,
[source_ref.commit_id])
@ -988,12 +996,15 @@ class GitRepository(BaseRepository):
merge_succeeded = True
except RepositoryError:
log.exception(
'Failure when doing local push on git shadow repo')
'Failure when doing local push from the shadow '
'repository to the target repository at %s.', self.path)
merge_succeeded = False
merge_failure_reason = MergeFailureReason.PUSH_FAILED
metadata['target'] = 'git shadow repo'
metadata['merge_commit'] = pr_branch
else:
merge_succeeded = False
return MergeResponse(
merge_possible, merge_succeeded, merge_ref,
merge_failure_reason)
merge_possible, merge_succeeded, merge_ref, merge_failure_reason,
metadata=metadata)

36
rhodecode/lib/vcs/backends/hg/repository.py
View file

@ -610,7 +610,7 @@ class MercurialRepository(BaseRepository):
Returns the commit id of the merge and a boolean indicating if the
commit needs to be pushed.
"""
self._update(target_ref.commit_id)
self._update(target_ref.commit_id, clean=True)
ancestor = self._ancestor(target_ref.commit_id, source_ref.commit_id)
is_the_same_branch = self._is_the_same_branch(target_ref, source_ref)
@ -631,7 +631,7 @@ class MercurialRepository(BaseRepository):
self._remote.rebase(
source=source_ref.commit_id, dest=target_ref.commit_id)
self._remote.invalidate_vcs_cache()
self._update(bookmark_name)
self._update(bookmark_name, clean=True)
return self._identify(), True
except RepositoryError:
# The rebase-abort may raise another exception which 'hides'
@ -710,18 +710,21 @@ class MercurialRepository(BaseRepository):
'rebase' if use_rebase else 'merge', dry_run)
if target_ref.commit_id not in self._heads():
return MergeResponse(
False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD)
False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD,
metadata={'target_ref': target_ref})
try:
if (target_ref.type == 'branch' and
len(self._heads(target_ref.name)) != 1):
if target_ref.type == 'branch' and len(self._heads(target_ref.name)) != 1:
heads = ','.join(self._heads(target_ref.name))
return MergeResponse(
False, False, None,
MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS)
MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS,
metadata={'heads': heads})
except CommitDoesNotExistError:
log.exception('Failure when looking up branch heads on hg target')
return MergeResponse(
False, False, None, MergeFailureReason.MISSING_TARGET_REF)
False, False, None, MergeFailureReason.MISSING_TARGET_REF,
metadata={'target_ref': target_ref})
shadow_repository_path = self._maybe_prepare_merge_workspace(
repo_id, workspace_id, target_ref, source_ref)
@ -730,6 +733,7 @@ class MercurialRepository(BaseRepository):
log.debug('Pulling in target reference %s', target_ref)
self._validate_pull_reference(target_ref)
shadow_repo._local_pull(self.path, target_ref)
try:
log.debug('Pulling in source reference %s', source_ref)
source_repo._validate_pull_reference(source_ref)
@ -737,12 +741,14 @@ class MercurialRepository(BaseRepository):
except CommitDoesNotExistError:
log.exception('Failure when doing local pull on hg shadow repo')
return MergeResponse(
False, False, None, MergeFailureReason.MISSING_SOURCE_REF)
False, False, None, MergeFailureReason.MISSING_SOURCE_REF,
metadata={'source_ref': source_ref})
merge_ref = None
merge_commit_id = None
close_commit_id = None
merge_failure_reason = MergeFailureReason.NONE
metadata = {}
# enforce that close branch should be used only in case we source from
# an actual Branch
@ -758,8 +764,8 @@ class MercurialRepository(BaseRepository):
target_ref, merger_name, merger_email, source_ref)
merge_possible = True
except RepositoryError:
log.exception(
'Failure when doing close branch on hg shadow repo')
log.exception('Failure when doing close branch on '
'shadow repo: %s', shadow_repo)
merge_possible = False
merge_failure_reason = MergeFailureReason.MERGE_FAILED
else:
@ -824,19 +830,21 @@ class MercurialRepository(BaseRepository):
except RepositoryError:
log.exception(
'Failure when doing local push from the shadow '
'repository to the target repository.')
'repository to the target repository at %s.', self.path)
merge_succeeded = False
merge_failure_reason = MergeFailureReason.PUSH_FAILED
metadata['target'] = 'hg shadow repo'
metadata['merge_commit'] = merge_commit_id
else:
merge_succeeded = True
else:
merge_succeeded = False
return MergeResponse(
merge_possible, merge_succeeded, merge_ref, merge_failure_reason)
merge_possible, merge_succeeded, merge_ref, merge_failure_reason,
metadata=metadata)
def _get_shadow_instance(
self, shadow_repository_path, enable_hooks=False):
def _get_shadow_instance(self, shadow_repository_path, enable_hooks=False):
config = self.config.copy()
if not enable_hooks:
config.clear_section('hooks')

61
rhodecode/model/pull_request.py
View file

@ -33,7 +33,7 @@ import collections
from pyramid.threadlocal import get_current_request
from rhodecode import events
from rhodecode.translation import lazy_ugettext#, _
from rhodecode.translation import lazy_ugettext
from rhodecode.lib import helpers as h, hooks_utils, diffs
from rhodecode.lib import audit_logger
from rhodecode.lib.compat import OrderedDict
@ -75,43 +75,6 @@ class PullRequestModel(BaseModel):
DIFF_CONTEXT = diffs.DEFAULT_CONTEXT
MERGE_STATUS_MESSAGES = {
MergeFailureReason.NONE: lazy_ugettext(
'This pull request can be automatically merged.'),
MergeFailureReason.UNKNOWN: lazy_ugettext(
'This pull request cannot be merged because of an unhandled'
' exception.'),
MergeFailureReason.MERGE_FAILED: lazy_ugettext(
'This pull request cannot be merged because of merge conflicts.'),
MergeFailureReason.PUSH_FAILED: lazy_ugettext(
'This pull request could not be merged because push to target'
' failed.'),
MergeFailureReason.TARGET_IS_NOT_HEAD: lazy_ugettext(
'This pull request cannot be merged because the target is not a'
' head.'),
MergeFailureReason.HG_SOURCE_HAS_MORE_BRANCHES: lazy_ugettext(
'This pull request cannot be merged because the source contains'
' more branches than the target.'),
MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS: lazy_ugettext(
'This pull request cannot be merged because the target has'
' multiple heads.'),
MergeFailureReason.TARGET_IS_LOCKED: lazy_ugettext(
'This pull request cannot be merged because the target repository'
' is locked.'),
MergeFailureReason._DEPRECATED_MISSING_COMMIT: lazy_ugettext(
'This pull request cannot be merged because the target or the '
'source reference is missing.'),
MergeFailureReason.MISSING_TARGET_REF: lazy_ugettext(
'This pull request cannot be merged because the target '
'reference is missing.'),
MergeFailureReason.MISSING_SOURCE_REF: lazy_ugettext(
'This pull request cannot be merged because the source '
'reference is missing.'),
MergeFailureReason.SUBREPO_MERGE_FAILED: lazy_ugettext(
'This pull request cannot be merged because of conflicts related '
'to sub repositories.'),
}
UPDATE_STATUS_MESSAGES = {
UpdateFailureReason.NONE: lazy_ugettext(
'Pull request update successful.'),
@ -593,8 +556,7 @@ class PullRequestModel(BaseModel):
extras['user_agent'] = 'internal-merge'
merge_state = self._merge_pull_request(pull_request, user, extras)
if merge_state.executed:
log.debug(
"Merge was successful, updating the pull request comments.")
log.debug("Merge was successful, updating the pull request comments.")
self._comment_and_close_pr(pull_request, user, merge_state)
self._log_audit_action(
@ -1254,8 +1216,7 @@ class PullRequestModel(BaseModel):
pull_request,
force_shadow_repo_refresh=force_shadow_repo_refresh)
log.debug("Merge response: %s", resp)
status = resp.possible, self.merge_status_message(
resp.failure_reason)
status = resp.possible, resp.merge_status_message
except NotImplementedError:
status = False, _('Pull request merging is not supported.')
@ -1297,21 +1258,23 @@ class PullRequestModel(BaseModel):
"Trying out if the pull request %s can be merged. Force_refresh=%s",
pull_request.pull_request_id, force_shadow_repo_refresh)
target_vcs = pull_request.target_repo.scm_instance()
# Refresh the target reference.
try:
target_ref = self._refresh_reference(
pull_request.target_ref_parts, target_vcs)
except CommitDoesNotExistError:
merge_state = MergeResponse(
False, False, None, MergeFailureReason.MISSING_TARGET_REF)
False, False, None, MergeFailureReason.MISSING_TARGET_REF,
metadata={'target_ref': pull_request.target_ref_parts})
return merge_state
target_locked = pull_request.target_repo.locked
if target_locked and target_locked[0]:
log.debug("The target repository is locked.")
locked_by = 'user:{}'.format(target_locked[0])
log.debug("The target repository is locked by %s.", locked_by)
merge_state = MergeResponse(
False, False, None, MergeFailureReason.TARGET_IS_LOCKED)
False, False, None, MergeFailureReason.TARGET_IS_LOCKED,
metadata={'locked_by': locked_by})
elif force_shadow_repo_refresh or self._needs_merge_state_refresh(
pull_request, target_ref):
log.debug("Refreshing the merge status of the repository.")
@ -1369,12 +1332,6 @@ class PullRequestModel(BaseModel):
workspace_id = 'pr-%s' % pull_request.pull_request_id
return workspace_id
def merge_status_message(self, status_code):
"""
Return a human friendly error message for the given merge status code.
"""
return self.MERGE_STATUS_MESSAGES[status_code]
def generate_repo_data(self, repo, commit_id=None, branch=None,
bookmark=None, translator=None):
from rhodecode.model.repo import RepoModel

79
rhodecode/tests/models/test_pullrequest.py
View file

@ -50,9 +50,11 @@ class TestPullRequestModel(object):
A pull request combined with multiples patches.
"""
BackendClass = get_backend(backend.alias)
merge_resp = MergeResponse(
False, False, None, MergeFailureReason.UNKNOWN,
metadata={'exception': 'MockError'})
self.merge_patcher = mock.patch.object(
BackendClass, 'merge', return_value=MergeResponse(
False, False, None, MergeFailureReason.UNKNOWN))
BackendClass, 'merge', return_value=merge_resp)
self.workspace_remove_patcher = mock.patch.object(
BackendClass, 'cleanup_merge_workspace')
@ -162,7 +164,7 @@ class TestPullRequestModel(object):
status, msg = PullRequestModel().merge_status(pull_request)
assert status is True
assert msg.eval() == 'This pull request can be automatically merged.'
assert msg == 'This pull request can be automatically merged.'
self.merge_mock.assert_called_with(
self.repo_id, self.workspace_id,
pull_request.target_ref_parts,
@ -177,7 +179,7 @@ class TestPullRequestModel(object):
self.merge_mock.reset_mock()
status, msg = PullRequestModel().merge_status(pull_request)
assert status is True
assert msg.eval() == 'This pull request can be automatically merged.'
assert msg == 'This pull request can be automatically merged.'
assert self.merge_mock.called is False
def test_merge_status_known_failure(self, pull_request):
@ -190,9 +192,7 @@ class TestPullRequestModel(object):
status, msg = PullRequestModel().merge_status(pull_request)
assert status is False
assert (
msg.eval() ==
'This pull request cannot be merged because of merge conflicts.')
assert msg == 'This pull request cannot be merged because of merge conflicts.'
self.merge_mock.assert_called_with(
self.repo_id, self.workspace_id,
pull_request.target_ref_parts,
@ -208,14 +208,13 @@ class TestPullRequestModel(object):
self.merge_mock.reset_mock()
status, msg = PullRequestModel().merge_status(pull_request)
assert status is False
assert (
msg.eval() ==
'This pull request cannot be merged because of merge conflicts.')
assert msg == 'This pull request cannot be merged because of merge conflicts.'
assert self.merge_mock.called is False
def test_merge_status_unknown_failure(self, pull_request):
self.merge_mock.return_value = MergeResponse(
False, False, None, MergeFailureReason.UNKNOWN)
False, False, None, MergeFailureReason.UNKNOWN,
metadata={'exception': 'MockError'})
assert pull_request._last_merge_source_rev is None
assert pull_request._last_merge_target_rev is None
@ -223,9 +222,9 @@ class TestPullRequestModel(object):
status, msg = PullRequestModel().merge_status(pull_request)
assert status is False
assert msg.eval() == (
'This pull request cannot be merged because of an unhandled'
' exception.')
assert msg == (
'This pull request cannot be merged because of an unhandled exception. '
'MockError')
self.merge_mock.assert_called_with(
self.repo_id, self.workspace_id,
pull_request.target_ref_parts,
@ -240,18 +239,18 @@ class TestPullRequestModel(object):
self.merge_mock.reset_mock()
status, msg = PullRequestModel().merge_status(pull_request)
assert status is False
assert msg.eval() == (
'This pull request cannot be merged because of an unhandled'
' exception.')
assert msg == (
'This pull request cannot be merged because of an unhandled exception. '
'MockError')
assert self.merge_mock.called is True
def test_merge_status_when_target_is_locked(self, pull_request):
pull_request.target_repo.locked = [1, u'12345.50', 'lock_web']
status, msg = PullRequestModel().merge_status(pull_request)
assert status is False
assert msg.eval() == (
'This pull request cannot be merged because the target repository'
' is locked.')
assert msg == (
'This pull request cannot be merged because the target repository '
'is locked by user:1.')
def test_merge_status_requirements_check_target(self, pull_request):
@ -460,6 +459,46 @@ def test_outdated_comments(
outdated_comment_mock.assert_called_with(pull_request)
@pytest.mark.parametrize('mr_type, expected_msg', [
(MergeFailureReason.NONE,
'This pull request can be automatically merged.'),
(MergeFailureReason.UNKNOWN,
'This pull request cannot be merged because of an unhandled exception. CRASH'),
(MergeFailureReason.MERGE_FAILED,
'This pull request cannot be merged because of merge conflicts.'),
(MergeFailureReason.PUSH_FAILED,
'This pull request could not be merged because push to target:`some-repo@merge_commit` failed.'),
(MergeFailureReason.TARGET_IS_NOT_HEAD,
'This pull request cannot be merged because the target `ref_name` is not a head.'),
(MergeFailureReason.HG_SOURCE_HAS_MORE_BRANCHES,
'This pull request cannot be merged because the source contains more branches than the target.'),
(MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS,
'This pull request cannot be merged because the target has multiple heads: `a,b,c`.'),
(MergeFailureReason.TARGET_IS_LOCKED,
'This pull request cannot be merged because the target repository is locked by user:123.'),
(MergeFailureReason.MISSING_TARGET_REF,
'This pull request cannot be merged because the target reference `ref_name` is missing.'),
(MergeFailureReason.MISSING_SOURCE_REF,
'This pull request cannot be merged because the source reference `ref_name` is missing.'),
(MergeFailureReason.SUBREPO_MERGE_FAILED,
'This pull request cannot be merged because of conflicts related to sub repositories.'),
])
def test_merge_response_message(mr_type, expected_msg):
merge_ref = Reference('type', 'ref_name', '6126b7bfcc82ad2d3deaee22af926b082ce54cc6')
metadata = {
'exception': "CRASH",
'target': 'some-repo',
'merge_commit': 'merge_commit',
'target_ref': merge_ref,
'source_ref': merge_ref,
'heads': ','.join(['a', 'b', 'c']),
'locked_by': 'user:123'}
merge_response = MergeResponse(True, True, merge_ref, mr_type, metadata=metadata)
assert merge_response.merge_status_message == expected_msg
@pytest.fixture
def merge_extras(user_regular):
"""

3
rhodecode/tests/plugin.py
View file

@ -661,8 +661,7 @@ class Backend(object):
def _next_repo_name(self):
return u"%s_%s" % (
self.invalid_repo_name.sub(u'_', self._test_name),
len(self._cleanup_repos))
self.invalid_repo_name.sub(u'_', self._test_name), len(self._cleanup_repos))
def ensure_file(self, filename, content='Test content\n'):
assert self._cleanup_repos, "Avoid writing into vcs_test repos"

4
rhodecode/tests/vcs/test_hg.py
View file

@ -681,9 +681,11 @@ TODO: To be written...
workspace_id = 'test-merge'
assert len(target_repo._heads(branch='default')) == 2
heads = target_repo._heads(branch='default')
expected_merge_response = MergeResponse(
False, False, None,
MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS)
MergeFailureReason.HG_TARGET_HAS_MULTIPLE_HEADS,
metadata={'heads': heads})
repo_id = repo_id_generator(target_repo.path)
merge_response = target_repo.merge(
repo_id, workspace_id, target_ref, source_repo, source_ref,

41
rhodecode/tests/vcs/test_repository.py
View file

@ -284,11 +284,9 @@ class TestRepositoryMerge(object):
self.source_commit = self.source_repo.get_commit()
# This only works for Git and Mercurial
default_branch = self.target_repo.DEFAULT_BRANCH_NAME
self.target_ref = Reference(
'branch', default_branch, self.target_commit.raw_id)
self.source_ref = Reference(
'branch', default_branch, self.source_commit.raw_id)
self.workspace_id = 'test-merge'
self.target_ref = Reference('branch', default_branch, self.target_commit.raw_id)
self.source_ref = Reference('branch', default_branch, self.source_commit.raw_id)
self.workspace_id = 'test-merge-{}'.format(vcsbackend.alias)
self.repo_id = repo_id_generator(self.target_repo.path)
def prepare_for_conflict(self, vcsbackend):
@ -300,11 +298,9 @@ class TestRepositoryMerge(object):
self.source_commit = self.source_repo.get_commit()
# This only works for Git and Mercurial
default_branch = self.target_repo.DEFAULT_BRANCH_NAME
self.target_ref = Reference(
'branch', default_branch, self.target_commit.raw_id)
self.source_ref = Reference(
'branch', default_branch, self.source_commit.raw_id)
self.workspace_id = 'test-merge'
self.target_ref = Reference('branch', default_branch, self.target_commit.raw_id)
self.source_ref = Reference('branch', default_branch, self.source_commit.raw_id)
self.workspace_id = 'test-merge-{}'.format(vcsbackend.alias)
self.repo_id = repo_id_generator(self.target_repo.path)
def test_merge_success(self, vcsbackend):
@ -367,10 +363,11 @@ class TestRepositoryMerge(object):
# Multiple merges may differ in their commit id. Therefore we set the
# commit id to `None` before comparing the merge responses.
merge_response = merge_response._replace(
merge_ref=merge_response.merge_ref._replace(commit_id=None))
merge_response_update = merge_response_update._replace(
merge_ref=merge_response_update.merge_ref._replace(commit_id=None))
new_merge_ref = merge_response.merge_ref._replace(commit_id=None)
merge_response.merge_ref = new_merge_ref
new_update_merge_ref = merge_response_update.merge_ref._replace(commit_id=None)
merge_response_update.merge_ref = new_update_merge_ref
assert merge_response == merge_response_update
assert merge_response.possible is True
@ -381,6 +378,7 @@ class TestRepositoryMerge(object):
@pytest.mark.parametrize('dry_run', [True, False])
def test_merge_conflict(self, vcsbackend, dry_run):
self.prepare_for_conflict(vcsbackend)
expected_merge_response = MergeResponse(
False, False, None, MergeFailureReason.MERGE_FAILED)
@ -399,12 +397,11 @@ class TestRepositoryMerge(object):
def test_merge_target_is_not_head(self, vcsbackend):
self.prepare_for_success(vcsbackend)
expected_merge_response = MergeResponse(
False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD)
target_ref = Reference(
self.target_ref.type, self.target_ref.name, '0' * 40)
expected_merge_response = MergeResponse(
False, False, None, MergeFailureReason.TARGET_IS_NOT_HEAD,
metadata={'target_ref': target_ref})
merge_response = self.target_repo.merge(
self.repo_id, self.workspace_id, target_ref, self.source_repo,
self.source_ref, dry_run=True)
@ -413,11 +410,12 @@ class TestRepositoryMerge(object):
def test_merge_missing_source_reference(self, vcsbackend):
self.prepare_for_success(vcsbackend)
expected_merge_response = MergeResponse(
False, False, None, MergeFailureReason.MISSING_SOURCE_REF)
source_ref = Reference(
self.source_ref.type, 'not_existing', self.source_ref.commit_id)
expected_merge_response = MergeResponse(
False, False, None, MergeFailureReason.MISSING_SOURCE_REF,
metadata={'source_ref': source_ref})
merge_response = self.target_repo.merge(
self.repo_id, self.workspace_id, self.target_ref,
@ -429,7 +427,8 @@ class TestRepositoryMerge(object):
def test_merge_raises_exception(self, vcsbackend):
self.prepare_for_success(vcsbackend)
expected_merge_response = MergeResponse(
False, False, None, MergeFailureReason.UNKNOWN)
False, False, None, MergeFailureReason.UNKNOWN,
metadata={'exception': 'ErrorForTest'})
with mock.patch.object(self.target_repo, '_merge_repo',
side_effect=RepositoryError()):