pull-requests: use proper diff calculation for versioning of PRs.

rhodecode/apps/repository/tests/test_repo_pullrequests.py

@@ -116,6 +116,223 b' 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(

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -214,9 +214,12 b' 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 b' 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 b' 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:

rhodecode/tests/plugin.py

@@ -560,6 +560,7 b' 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 b' 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 b' 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