pull-requests: use proper diff calculation for versioning of PRs.
This commit is contained in:
Marcin Kuzminski
parent
8d9e71fee2
commit
190b145363
3 changed files with 233 additions and 5 deletions
|
|
@ -116,6 +116,223 @@ class TestPullrequestsView(object):
|
|||
if range_diff == "1":
|
||||
response.mustcontain('Turn off: Show the diff as commit range')
|
||||
|
||||
def test_show_versions_of_pr(self, backend, csrf_token):
|
||||
commits = [
|
||||
{'message': 'initial-commit',
|
||||
'added': [FileNode('test-file.txt', 'LINE1\n')]},
|
||||
|
||||
{'message': 'commit-1',
|
||||
'changed': [FileNode('test-file.txt', 'LINE1\nLINE2\n')]},
|
||||
# Above is the initial version of PR that changes a single line
|
||||
|
||||
# from now on we'll add 3x commit adding a nother line on each step
|
||||
{'message': 'commit-2',
|
||||
'changed': [FileNode('test-file.txt', 'LINE1\nLINE2\nLINE3\n')]},
|
||||
|
||||
{'message': 'commit-3',
|
||||
'changed': [FileNode('test-file.txt', 'LINE1\nLINE2\nLINE3\nLINE4\n')]},
|
||||
|
||||
{'message': 'commit-4',
|
||||
'changed': [FileNode('test-file.txt', 'LINE1\nLINE2\nLINE3\nLINE4\nLINE5\n')]},
|
||||
]
|
||||
|
||||
commit_ids = backend.create_master_repo(commits)
|
||||
target = backend.create_repo(heads=['initial-commit'])
|
||||
source = backend.create_repo(heads=['commit-1'])
|
||||
source_repo_name = source.repo_name
|
||||
target_repo_name = target.repo_name
|
||||
|
||||
target_ref = 'branch:{branch}:{commit_id}'.format(
|
||||
branch=backend.default_branch_name, commit_id=commit_ids['initial-commit'])
|
||||
source_ref = 'branch:{branch}:{commit_id}'.format(
|
||||
branch=backend.default_branch_name, commit_id=commit_ids['commit-1'])
|
||||
|
||||
response = self.app.post(
|
||||
route_path('pullrequest_create', repo_name=source.repo_name),
|
||||
[
|
||||
('source_repo', source.repo_name),
|
||||
('source_ref', source_ref),
|
||||
('target_repo', target.repo_name),
|
||||
('target_ref', target_ref),
|
||||
('common_ancestor', commit_ids['initial-commit']),
|
||||
('pullrequest_title', 'Title'),
|
||||
('pullrequest_desc', 'Description'),
|
||||
('description_renderer', 'markdown'),
|
||||
('__start__', 'review_members:sequence'),
|
||||
('__start__', 'reviewer:mapping'),
|
||||
('user_id', '1'),
|
||||
('__start__', 'reasons:sequence'),
|
||||
('reason', 'Some reason'),
|
||||
('__end__', 'reasons:sequence'),
|
||||
('__start__', 'rules:sequence'),
|
||||
('__end__', 'rules:sequence'),
|
||||
('mandatory', 'False'),
|
||||
('__end__', 'reviewer:mapping'),
|
||||
('__end__', 'review_members:sequence'),
|
||||
('__start__', 'revisions:sequence'),
|
||||
('revisions', commit_ids['commit-1']),
|
||||
('__end__', 'revisions:sequence'),
|
||||
('user', ''),
|
||||
('csrf_token', csrf_token),
|
||||
],
|
||||
status=302)
|
||||
|
||||
location = response.headers['Location']
|
||||
|
||||
pull_request_id = location.rsplit('/', 1)[1]
|
||||
assert pull_request_id != 'new'
|
||||
pull_request = PullRequest.get(int(pull_request_id))
|
||||
|
||||
pull_request_id = pull_request.pull_request_id
|
||||
|
||||
# Show initial version of PR
|
||||
response = self.app.get(
|
||||
route_path('pullrequest_show',
|
||||
repo_name=target_repo_name,
|
||||
pull_request_id=pull_request_id))
|
||||
|
||||
response.mustcontain('commit-1')
|
||||
response.mustcontain(no=['commit-2'])
|
||||
response.mustcontain(no=['commit-3'])
|
||||
response.mustcontain(no=['commit-4'])
|
||||
|
||||
response.mustcontain('cb-addition"></span><span>LINE2</span>')
|
||||
response.mustcontain(no=['LINE3'])
|
||||
response.mustcontain(no=['LINE4'])
|
||||
response.mustcontain(no=['LINE5'])
|
||||
|
||||
# update PR #1
|
||||
source_repo = Repository.get_by_repo_name(source_repo_name)
|
||||
backend.pull_heads(source_repo, heads=['commit-2'])
|
||||
response = self.app.post(
|
||||
route_path('pullrequest_update',
|
||||
repo_name=target_repo_name, pull_request_id=pull_request_id),
|
||||
params={'update_commits': 'true', 'csrf_token': csrf_token})
|
||||
|
||||
# update PR #2
|
||||
source_repo = Repository.get_by_repo_name(source_repo_name)
|
||||
backend.pull_heads(source_repo, heads=['commit-3'])
|
||||
response = self.app.post(
|
||||
route_path('pullrequest_update',
|
||||
repo_name=target_repo_name, pull_request_id=pull_request_id),
|
||||
params={'update_commits': 'true', 'csrf_token': csrf_token})
|
||||
|
||||
# update PR #3
|
||||
source_repo = Repository.get_by_repo_name(source_repo_name)
|
||||
backend.pull_heads(source_repo, heads=['commit-4'])
|
||||
response = self.app.post(
|
||||
route_path('pullrequest_update',
|
||||
repo_name=target_repo_name, pull_request_id=pull_request_id),
|
||||
params={'update_commits': 'true', 'csrf_token': csrf_token})
|
||||
|
||||
# Show final version !
|
||||
response = self.app.get(
|
||||
route_path('pullrequest_show',
|
||||
repo_name=target_repo_name,
|
||||
pull_request_id=pull_request_id))
|
||||
|
||||
# 3 updates, and the latest == 4
|
||||
response.mustcontain('4 versions available for this pull request')
|
||||
response.mustcontain(no=['rhodecode diff rendering error'])
|
||||
|
||||
# initial show must have 3 commits, and 3 adds
|
||||
response.mustcontain('commit-1')
|
||||
response.mustcontain('commit-2')
|
||||
response.mustcontain('commit-3')
|
||||
response.mustcontain('commit-4')
|
||||
|
||||
response.mustcontain('cb-addition"></span><span>LINE2</span>')
|
||||
response.mustcontain('cb-addition"></span><span>LINE3</span>')
|
||||
response.mustcontain('cb-addition"></span><span>LINE4</span>')
|
||||
response.mustcontain('cb-addition"></span><span>LINE5</span>')
|
||||
|
||||
# fetch versions
|
||||
pr = PullRequest.get(pull_request_id)
|
||||
versions = [x.pull_request_version_id for x in pr.versions.all()]
|
||||
assert len(versions) == 3
|
||||
|
||||
# show v1,v2,v3,v4
|
||||
def cb_line(text):
|
||||
return 'cb-addition"></span><span>{}</span>'.format(text)
|
||||
|
||||
def cb_context(text):
|
||||
return '<span class="cb-code"><span class="cb-action cb-context">' \
|
||||
'</span><span>{}</span></span>'.format(text)
|
||||
|
||||
commit_tests = {
|
||||
# in response, not in response
|
||||
1: (['commit-1'], ['commit-2', 'commit-3', 'commit-4']),
|
||||
2: (['commit-1', 'commit-2'], ['commit-3', 'commit-4']),
|
||||
3: (['commit-1', 'commit-2', 'commit-3'], ['commit-4']),
|
||||
4: (['commit-1', 'commit-2', 'commit-3', 'commit-4'], []),
|
||||
}
|
||||
diff_tests = {
|
||||
1: (['LINE2'], ['LINE3', 'LINE4', 'LINE5']),
|
||||
2: (['LINE2', 'LINE3'], ['LINE4', 'LINE5']),
|
||||
3: (['LINE2', 'LINE3', 'LINE4'], ['LINE5']),
|
||||
4: (['LINE2', 'LINE3', 'LINE4', 'LINE5'], []),
|
||||
}
|
||||
for idx, ver in enumerate(versions, 1):
|
||||
|
||||
response = self.app.get(
|
||||
route_path('pullrequest_show',
|
||||
repo_name=target_repo_name,
|
||||
pull_request_id=pull_request_id,
|
||||
params={'version': ver}))
|
||||
|
||||
response.mustcontain(no=['rhodecode diff rendering error'])
|
||||
response.mustcontain('Showing changes at v{}'.format(idx))
|
||||
|
||||
yes, no = commit_tests[idx]
|
||||
for y in yes:
|
||||
response.mustcontain(y)
|
||||
for n in no:
|
||||
response.mustcontain(no=n)
|
||||
|
||||
yes, no = diff_tests[idx]
|
||||
for y in yes:
|
||||
response.mustcontain(cb_line(y))
|
||||
for n in no:
|
||||
response.mustcontain(no=n)
|
||||
|
||||
# show diff between versions
|
||||
diff_compare_tests = {
|
||||
1: (['LINE3'], ['LINE1', 'LINE2']),
|
||||
2: (['LINE3', 'LINE4'], ['LINE1', 'LINE2']),
|
||||
3: (['LINE3', 'LINE4', 'LINE5'], ['LINE1', 'LINE2']),
|
||||
}
|
||||
for idx, ver in enumerate(versions, 1):
|
||||
adds, context = diff_compare_tests[idx]
|
||||
|
||||
to_ver = ver+1
|
||||
if idx == 3:
|
||||
to_ver = 'latest'
|
||||
|
||||
response = self.app.get(
|
||||
route_path('pullrequest_show',
|
||||
repo_name=target_repo_name,
|
||||
pull_request_id=pull_request_id,
|
||||
params={'from_version': versions[0], 'version': to_ver}))
|
||||
|
||||
response.mustcontain(no=['rhodecode diff rendering error'])
|
||||
|
||||
for a in adds:
|
||||
response.mustcontain(cb_line(a))
|
||||
for c in context:
|
||||
response.mustcontain(cb_context(c))
|
||||
|
||||
# test version v2 -> v3
|
||||
response = self.app.get(
|
||||
route_path('pullrequest_show',
|
||||
repo_name=target_repo_name,
|
||||
pull_request_id=pull_request_id,
|
||||
params={'from_version': versions[1], 'version': versions[2]}))
|
||||
|
||||
response.mustcontain(cb_context('LINE1'))
|
||||
response.mustcontain(cb_context('LINE2'))
|
||||
response.mustcontain(cb_context('LINE3'))
|
||||
response.mustcontain(cb_line('LINE4'))
|
||||
|
||||
def test_close_status_visibility(self, pr_util, user_util, csrf_token):
|
||||
# Logout
|
||||
response = self.app.post(
|
||||
|
|
|
|||
|
|
@ -214,9 +214,12 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
|
|||
ancestor_commit,
|
||||
source_ref_id, target_ref_id,
|
||||
target_commit, source_commit, diff_limit, file_limit,
|
||||
fulldiff, hide_whitespace_changes, diff_context):
|
||||
fulldiff, hide_whitespace_changes, diff_context, use_ancestor=True):
|
||||
|
||||
if use_ancestor:
|
||||
# we might want to not use it for versions
|
||||
target_ref_id = ancestor_commit.raw_id
|
||||
|
||||
target_ref_id = ancestor_commit.raw_id
|
||||
vcs_diff = PullRequestModel().get_diff(
|
||||
source_repo, source_ref_id, target_ref_id,
|
||||
hide_whitespace_changes, diff_context)
|
||||
|
|
@ -593,6 +596,10 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
|
|||
else:
|
||||
c.inline_comments = display_inline_comments
|
||||
|
||||
use_ancestor = True
|
||||
if from_version_normalized != version_normalized:
|
||||
use_ancestor = False
|
||||
|
||||
has_proper_diff_cache = cached_diff and cached_diff.get('commits')
|
||||
if not force_recache and has_proper_diff_cache:
|
||||
c.diffset = cached_diff['diff']
|
||||
|
|
@ -603,7 +610,9 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
|
|||
source_ref_id, target_ref_id,
|
||||
target_commit, source_commit,
|
||||
diff_limit, file_limit, c.fulldiff,
|
||||
hide_whitespace_changes, diff_context)
|
||||
hide_whitespace_changes, diff_context,
|
||||
use_ancestor=use_ancestor
|
||||
)
|
||||
|
||||
# save cached diff
|
||||
if caching_enabled:
|
||||
|
|
|
|||
|
|
@ -560,6 +560,7 @@ class Backend(object):
|
|||
|
||||
invalid_repo_name = re.compile(r'[^0-9a-zA-Z]+')
|
||||
_master_repo = None
|
||||
_master_repo_path = ''
|
||||
_commit_ids = {}
|
||||
|
||||
def __init__(self, alias, repo_name, test_name, test_repo_container):
|
||||
|
|
@ -624,6 +625,8 @@ class Backend(object):
|
|||
Returns a commit map which maps from commit message to raw_id.
|
||||
"""
|
||||
self._master_repo = self.create_repo(commits=commits)
|
||||
self._master_repo_path = self._master_repo.repo_full_path
|
||||
|
||||
return self._commit_ids
|
||||
|
||||
def create_repo(
|
||||
|
|
@ -661,11 +664,10 @@ class Backend(object):
|
|||
"""
|
||||
Make sure that repo contains all commits mentioned in `heads`
|
||||
"""
|
||||
vcsmaster = self._master_repo.scm_instance()
|
||||
vcsrepo = repo.scm_instance()
|
||||
vcsrepo.config.clear_section('hooks')
|
||||
commit_ids = [self._commit_ids[h] for h in heads]
|
||||
vcsrepo.pull(vcsmaster.path, commit_ids=commit_ids)
|
||||
vcsrepo.pull(self._master_repo_path, commit_ids=commit_ids)
|
||||
|
||||
def create_fork(self):
|
||||
repo_to_fork = self.repo_name
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue