# HG changeset patch # User Marcin Kuzminski # Date 2020-07-03 09:29:52 # Node ID f4c1c6eb26f22512b29e7f7bcb5caf74d8e67d9b # Parent 987d4ca0c548d152d29e5e32c6e16fb5b0ab0ef4 pull-requests: use proper diff calculation for versioning of PRs. diff --git a/rhodecode/apps/repository/tests/test_repo_pullrequests.py b/rhodecode/apps/repository/tests/test_repo_pullrequests.py --- a/rhodecode/apps/repository/tests/test_repo_pullrequests.py +++ b/rhodecode/apps/repository/tests/test_repo_pullrequests.py @@ -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">LINE2') + 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">LINE2') + response.mustcontain('cb-addition">LINE3') + response.mustcontain('cb-addition">LINE4') + response.mustcontain('cb-addition">LINE5') + + # 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">{}'.format(text) + + def cb_context(text): + return '' \ + '{}'.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( diff --git a/rhodecode/apps/repository/views/repo_pull_requests.py b/rhodecode/apps/repository/views/repo_pull_requests.py --- a/rhodecode/apps/repository/views/repo_pull_requests.py +++ b/rhodecode/apps/repository/views/repo_pull_requests.py @@ -214,9 +214,12 @@ class RepoPullRequestsView(RepoAppView, 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): - target_ref_id = ancestor_commit.raw_id + if use_ancestor: + # we might want to not use it for versions + 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, 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, 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: diff --git a/rhodecode/tests/plugin.py b/rhodecode/tests/plugin.py --- a/rhodecode/tests/plugin.py +++ b/rhodecode/tests/plugin.py @@ -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