# HG changeset patch # User Mads Kiilerich # Date 2020-09-26 17:33:18 # Node ID b27584990c2c81278bfa53f5d89b29674aa5ee20 # Parent a2ec23356e6b504011c0eda069765896bcbeb308 tests: add coverage of Git pullrequests Git is so different than Mercurial that it is easier to use separate tests. Some of the tests that are relevant for Mercurial doesn't apply to the Git support. Also fix crash an odd corner case of creating PRs from a repo that has no branches at all. diff --git a/kallithea/controllers/pullrequests.py b/kallithea/controllers/pullrequests.py --- a/kallithea/controllers/pullrequests.py +++ b/kallithea/controllers/pullrequests.py @@ -161,8 +161,10 @@ class PullrequestsController(BaseRepoCon else: selected = 'tag:null:' + repo.EMPTY_CHANGESET tags.append((selected, 'null')) - else: - if 'master' in repo.branches: + else: # Git + if not repo.branches: + selected = '' # doesn't make sense, but better than nothing + elif 'master' in repo.branches: selected = 'branch:master:%s' % repo.branches['master'] else: k, v = list(repo.branches.items())[0] diff --git a/kallithea/tests/functional/test_pullrequests.py b/kallithea/tests/functional/test_pullrequests_git.py copy from kallithea/tests/functional/test_pullrequests.py copy to kallithea/tests/functional/test_pullrequests_git.py --- a/kallithea/tests/functional/test_pullrequests.py +++ b/kallithea/tests/functional/test_pullrequests_git.py @@ -3,7 +3,6 @@ import re import pytest from kallithea.controllers.pullrequests import PullrequestsController -from kallithea.model.db import PullRequest, User from kallithea.model.meta import Session from kallithea.tests import base from kallithea.tests.fixture import Fixture @@ -17,16 +16,16 @@ class TestPullrequestsController(base.Te def test_index(self): self.log_user() response = self.app.get(base.url(controller='pullrequests', action='index', - repo_name=base.HG_REPO)) + repo_name=base.GIT_REPO)) def test_create_trivial(self): self.log_user() response = self.app.post(base.url(controller='pullrequests', action='create', - repo_name=base.HG_REPO), - {'org_repo': base.HG_REPO, - 'org_ref': 'branch:stable:4f7e2131323e0749a740c0a56ab68ae9269c562a', - 'other_repo': base.HG_REPO, - 'other_ref': 'branch:default:96507bd11ecc815ebc6270fdf6db110928c09c1e', + repo_name=base.GIT_REPO), + {'org_repo': base.GIT_REPO, + 'org_ref': 'branch:master:5f2c6ee195929b0be80749243c18121c9864a3b3', + 'other_repo': base.GIT_REPO, + 'other_ref': 'tag:v0.2.2:137fea89f304a42321d40488091ee2ed419a3686', 'pullrequest_title': 'title', 'pullrequest_desc': 'description', '_session_csrf_secret_token': self.session_csrf_secret_token(), @@ -35,169 +34,33 @@ class TestPullrequestsController(base.Te response = response.follow() assert response.status == '200 OK' response.mustcontain('Successfully opened new pull request') - response.mustcontain('No additional changesets found for iterating on this pull request') - response.mustcontain('href="/vcs_test_hg/changeset/4f7e2131323e0749a740c0a56ab68ae9269c562a"') - - def test_available(self): - self.log_user() - response = self.app.post(base.url(controller='pullrequests', action='create', - repo_name=base.HG_REPO), - {'org_repo': base.HG_REPO, - 'org_ref': 'rev:94f45ed825a1:94f45ed825a113e61af7e141f44ca578374abef0', - 'other_repo': base.HG_REPO, - 'other_ref': 'branch:default:96507bd11ecc815ebc6270fdf6db110928c09c1e', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - '_session_csrf_secret_token': self.session_csrf_secret_token(), - }, - status=302) - response = response.follow() - assert response.status == '200 OK' - response.mustcontain(no='No additional changesets found for iterating on this pull request') - response.mustcontain('The following additional changes are available on stable:') - response.mustcontain('') - response.mustcontain('href="/vcs_test_hg/changeset/4f7e2131323e0749a740c0a56ab68ae9269c562a"') # as update - - def test_range(self): - self.log_user() - response = self.app.post(base.url(controller='pullrequests', action='create', - repo_name=base.HG_REPO), - {'org_repo': base.HG_REPO, - 'org_ref': 'branch:stable:4f7e2131323e0749a740c0a56ab68ae9269c562a', - 'other_repo': base.HG_REPO, - 'other_ref': 'rev:94f45ed825a1:94f45ed825a113e61af7e141f44ca578374abef0', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - '_session_csrf_secret_token': self.session_csrf_secret_token(), - }, - status=302) - response = response.follow() - assert response.status == '200 OK' - response.mustcontain('No additional changesets found for iterating on this pull request') - response.mustcontain('href="/vcs_test_hg/changeset/4f7e2131323e0749a740c0a56ab68ae9269c562a"') - - def test_update_reviewers(self): - self.log_user() - regular_user = User.get_by_username(base.TEST_USER_REGULAR_LOGIN) - regular_user2 = User.get_by_username(base.TEST_USER_REGULAR2_LOGIN) - admin_user = User.get_by_username(base.TEST_USER_ADMIN_LOGIN) + response.mustcontain('Git pull requests don't support iterating yet.') - # create initial PR - response = self.app.post(base.url(controller='pullrequests', action='create', - repo_name=base.HG_REPO), - {'org_repo': base.HG_REPO, - 'org_ref': 'rev:94f45ed825a1:94f45ed825a113e61af7e141f44ca578374abef0', - 'other_repo': base.HG_REPO, - 'other_ref': 'branch:default:96507bd11ecc815ebc6270fdf6db110928c09c1e', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - '_session_csrf_secret_token': self.session_csrf_secret_token(), - }, - status=302) - pull_request1_id = re.search(r'/pull-request/(\d+)/', response.location).group(1) - assert response.location == 'http://localhost/%s/pull-request/%s/_/stable' % (base.HG_REPO, pull_request1_id) - - # create new iteration - response = self.app.post(base.url(controller='pullrequests', action='post', - repo_name=base.HG_REPO, pull_request_id=pull_request1_id), - { - 'updaterev': '4f7e2131323e0749a740c0a56ab68ae9269c562a', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - 'owner': base.TEST_USER_ADMIN_LOGIN, - '_session_csrf_secret_token': self.session_csrf_secret_token(), - 'review_members': [regular_user.user_id], - }, - status=302) - pull_request2_id = re.search(r'/pull-request/(\d+)/', response.location).group(1) - assert pull_request2_id != pull_request1_id - assert response.location == 'http://localhost/%s/pull-request/%s/_/stable' % (base.HG_REPO, pull_request2_id) - response = response.follow() - # verify reviewer was added - response.mustcontain('' % regular_user.user_id) - - # update without creating new iteration - response = self.app.post(base.url(controller='pullrequests', action='post', - repo_name=base.HG_REPO, pull_request_id=pull_request2_id), - { - 'pullrequest_title': 'Title', - 'pullrequest_desc': 'description', - 'owner': base.TEST_USER_ADMIN_LOGIN, - '_session_csrf_secret_token': self.session_csrf_secret_token(), - 'org_review_members': [admin_user.user_id], # fake - just to get some 'meanwhile' warning ... but it is also added ... - 'review_members': [regular_user2.user_id, admin_user.user_id], - }, - status=302) - assert response.location == 'http://localhost/%s/pull-request/%s/_/stable' % (base.HG_REPO, pull_request2_id) - response = response.follow() - # verify reviewers were added / removed - response.mustcontain('Meanwhile, the following reviewers have been added: test_regular') - response.mustcontain('Meanwhile, the following reviewers have been removed: test_admin') - response.mustcontain('' % regular_user.user_id) - response.mustcontain('' % regular_user2.user_id) - response.mustcontain(no='' % admin_user.user_id) - - def test_update_with_invalid_reviewer(self): + def test_edit_with_invalid_reviewer(self): invalid_user_id = 99999 self.log_user() # create a valid pull request response = self.app.post(base.url(controller='pullrequests', action='create', - repo_name=base.HG_REPO), + repo_name=base.GIT_REPO), { - 'org_repo': base.HG_REPO, - 'org_ref': 'rev:94f45ed825a1:94f45ed825a113e61af7e141f44ca578374abef0', - 'other_repo': base.HG_REPO, - 'other_ref': 'branch:default:96507bd11ecc815ebc6270fdf6db110928c09c1e', + 'org_repo': base.GIT_REPO, + 'org_ref': 'branch:master:5f2c6ee195929b0be80749243c18121c9864a3b3', + 'other_repo': base.GIT_REPO, + 'other_ref': 'tag:v0.2.2:137fea89f304a42321d40488091ee2ed419a3686', 'pullrequest_title': 'title', 'pullrequest_desc': 'description', '_session_csrf_secret_token': self.session_csrf_secret_token(), }, status=302) # location is of the form: - # http://localhost/vcs_test_hg/pull-request/54/_/title - m = re.search(r'/pull-request/(\d+)/', response.location) - assert m is not None - pull_request_id = m.group(1) - - # update it - response = self.app.post(base.url(controller='pullrequests', action='post', - repo_name=base.HG_REPO, pull_request_id=pull_request_id), - { - 'updaterev': '4f7e2131323e0749a740c0a56ab68ae9269c562a', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - 'owner': base.TEST_USER_ADMIN_LOGIN, - '_session_csrf_secret_token': self.session_csrf_secret_token(), - 'review_members': [str(invalid_user_id)], - }, - status=400) - response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) - - def test_edit_with_invalid_reviewer(self): - invalid_user_id = 99999 - self.log_user() - # create a valid pull request - response = self.app.post(base.url(controller='pullrequests', action='create', - repo_name=base.HG_REPO), - { - 'org_repo': base.HG_REPO, - 'org_ref': 'branch:stable:4f7e2131323e0749a740c0a56ab68ae9269c562a', - 'other_repo': base.HG_REPO, - 'other_ref': 'branch:default:96507bd11ecc815ebc6270fdf6db110928c09c1e', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - '_session_csrf_secret_token': self.session_csrf_secret_token(), - }, - status=302) - # location is of the form: - # http://localhost/vcs_test_hg/pull-request/54/_/title + # http://localhost/vcs_test_git/pull-request/54/_/title m = re.search(r'/pull-request/(\d+)/', response.location) assert m is not None pull_request_id = m.group(1) # edit it response = self.app.post(base.url(controller='pullrequests', action='post', - repo_name=base.HG_REPO, pull_request_id=pull_request_id), + repo_name=base.GIT_REPO, pull_request_id=pull_request_id), { 'pullrequest_title': 'title', 'pullrequest_desc': 'description', @@ -208,92 +71,12 @@ class TestPullrequestsController(base.Te status=400) response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id) - def test_iteration_refs(self): - # Repo graph excerpt: - # o fb95b340e0d0 webvcs - # /: - # o : 41d2568309a0 default - # : : - # : o 5ec21f21aafe webvcs - # : : - # : o 9e6119747791 webvcs - # : : - # o : 3d1091ee5a53 default - # :/ - # o 948da46b29c1 default - - self.log_user() - - # create initial PR - response = self.app.post( - base.url(controller='pullrequests', action='create', repo_name=base.HG_REPO), - { - 'org_repo': base.HG_REPO, - 'org_ref': 'rev:9e6119747791:9e6119747791ff886a5abe1193a730b6bf874e1c', - 'other_repo': base.HG_REPO, - 'other_ref': 'branch:default:3d1091ee5a533b1f4577ec7d8a226bb315fb1336', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - '_session_csrf_secret_token': self.session_csrf_secret_token(), - }, - status=302) - pr1_id = int(re.search(r'/pull-request/(\d+)/', response.location).group(1)) - pr1 = PullRequest.get(pr1_id) - - assert pr1.org_ref == 'branch:webvcs:9e6119747791ff886a5abe1193a730b6bf874e1c' - assert pr1.other_ref == 'branch:default:948da46b29c125838a717f6a8496eb409717078d' - - Session().rollback() # invalidate loaded PR objects before issuing next request. - - # create PR 2 (new iteration with same ancestor) - response = self.app.post( - base.url(controller='pullrequests', action='post', repo_name=base.HG_REPO, pull_request_id=pr1_id), - { - 'updaterev': '5ec21f21aafe95220f1fc4843a4a57c378498b71', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - 'owner': base.TEST_USER_REGULAR_LOGIN, - '_session_csrf_secret_token': self.session_csrf_secret_token(), - }, - status=302) - pr2_id = int(re.search(r'/pull-request/(\d+)/', response.location).group(1)) - pr1 = PullRequest.get(pr1_id) - pr2 = PullRequest.get(pr2_id) - - assert pr2_id != pr1_id - assert pr1.status == PullRequest.STATUS_CLOSED - assert pr2.org_ref == 'branch:webvcs:5ec21f21aafe95220f1fc4843a4a57c378498b71' - assert pr2.other_ref == pr1.other_ref - - Session().rollback() # invalidate loaded PR objects before issuing next request. - - # create PR 3 (new iteration with new ancestor) - response = self.app.post( - base.url(controller='pullrequests', action='post', repo_name=base.HG_REPO, pull_request_id=pr2_id), - { - 'updaterev': 'fb95b340e0d03fa51f33c56c991c08077c99303e', - 'pullrequest_title': 'title', - 'pullrequest_desc': 'description', - 'owner': base.TEST_USER_REGULAR_LOGIN, - '_session_csrf_secret_token': self.session_csrf_secret_token(), - }, - status=302) - pr3_id = int(re.search(r'/pull-request/(\d+)/', response.location).group(1)) - pr2 = PullRequest.get(pr2_id) - pr3 = PullRequest.get(pr3_id) - - assert pr3_id != pr2_id - assert pr2.status == PullRequest.STATUS_CLOSED - assert pr3.org_ref == 'branch:webvcs:fb95b340e0d03fa51f33c56c991c08077c99303e' - assert pr3.other_ref == 'branch:default:41d2568309a05f422cffb8008e599d385f8af439' - - @pytest.mark.usefixtures("test_context_fixture") # apply fixture for all test methods class TestPullrequestsGetRepoRefs(base.TestController): def setup_method(self, method): self.repo_name = 'main' - repo = fixture.create_repo(self.repo_name, repo_type='hg') + repo = fixture.create_repo(self.repo_name, repo_type='git') self.repo_scm_instance = repo.scm_instance Session().commit() self.c = PullrequestsController() @@ -306,75 +89,74 @@ class TestPullrequestsGetRepoRefs(base.T def test_repo_refs_empty_repo(self): # empty repo with no commits, no branches, no bookmarks, just one tag refs, default = self.c._get_repo_refs(self.repo_scm_instance) - assert default == 'tag:null:0000000000000000000000000000000000000000' + assert default == '' # doesn't make sense, but better than nothing def test_repo_refs_one_commit_no_hints(self): cs0 = fixture.commit_change(self.repo_name, filename='file1', - content='line1\n', message='commit1', vcs_type='hg', + content='line1\n', message='commit1', vcs_type='git', parent=None, newfile=True) refs, default = self.c._get_repo_refs(self.repo_scm_instance) - assert default == 'branch:default:%s' % cs0.raw_id - assert ([('branch:default:%s' % cs0.raw_id, 'default (current tip)')], - 'Branches') in refs + assert default == 'branch:master:%s' % cs0.raw_id + assert ([('branch:master:%s' % cs0.raw_id, 'master')], 'Branches') in refs def test_repo_refs_one_commit_rev_hint(self): cs0 = fixture.commit_change(self.repo_name, filename='file1', - content='line1\n', message='commit1', vcs_type='hg', + content='line1\n', message='commit1', vcs_type='git', parent=None, newfile=True) refs, default = self.c._get_repo_refs(self.repo_scm_instance, rev=cs0.raw_id) - expected = 'branch:default:%s' % cs0.raw_id + expected = 'branch:master:%s' % cs0.raw_id assert default == expected - assert ([(expected, 'default (current tip)')], 'Branches') in refs + assert ([(expected, 'master')], 'Branches') in refs def test_repo_refs_two_commits_no_hints(self): cs0 = fixture.commit_change(self.repo_name, filename='file1', - content='line1\n', message='commit1', vcs_type='hg', + content='line1\n', message='commit1', vcs_type='git', parent=None, newfile=True) cs1 = fixture.commit_change(self.repo_name, filename='file2', - content='line2\n', message='commit2', vcs_type='hg', + content='line2\n', message='commit2', vcs_type='git', parent=None, newfile=True) refs, default = self.c._get_repo_refs(self.repo_scm_instance) - expected = 'branch:default:%s' % cs1.raw_id + expected = 'branch:master:%s' % cs1.raw_id assert default == expected - assert ([(expected, 'default (current tip)')], 'Branches') in refs + assert ([(expected, 'master')], 'Branches') in refs def test_repo_refs_two_commits_rev_hints(self): cs0 = fixture.commit_change(self.repo_name, filename='file1', - content='line1\n', message='commit1', vcs_type='hg', + content='line1\n', message='commit1', vcs_type='git', parent=None, newfile=True) cs1 = fixture.commit_change(self.repo_name, filename='file2', - content='line2\n', message='commit2', vcs_type='hg', + content='line2\n', message='commit2', vcs_type='git', parent=None, newfile=True) refs, default = self.c._get_repo_refs(self.repo_scm_instance, rev=cs0.raw_id) expected = 'rev:%s:%s' % (cs0.raw_id, cs0.raw_id) assert default == expected assert ([(expected, 'Changeset: %s' % cs0.raw_id[0:12])], 'Special') in refs - assert ([('branch:default:%s' % cs1.raw_id, 'default (current tip)')], 'Branches') in refs + assert ([('branch:master:%s' % cs1.raw_id, 'master')], 'Branches') in refs refs, default = self.c._get_repo_refs(self.repo_scm_instance, rev=cs1.raw_id) - expected = 'branch:default:%s' % cs1.raw_id + expected = 'branch:master:%s' % cs1.raw_id assert default == expected - assert ([(expected, 'default (current tip)')], 'Branches') in refs + assert ([(expected, 'master')], 'Branches') in refs def test_repo_refs_two_commits_branch_hint(self): cs0 = fixture.commit_change(self.repo_name, filename='file1', - content='line1\n', message='commit1', vcs_type='hg', + content='line1\n', message='commit1', vcs_type='git', parent=None, newfile=True) cs1 = fixture.commit_change(self.repo_name, filename='file2', - content='line2\n', message='commit2', vcs_type='hg', + content='line2\n', message='commit2', vcs_type='git', parent=None, newfile=True) - refs, default = self.c._get_repo_refs(self.repo_scm_instance, branch='default') - expected = 'branch:default:%s' % cs1.raw_id + refs, default = self.c._get_repo_refs(self.repo_scm_instance, branch='master') + expected = 'branch:master:%s' % cs1.raw_id assert default == expected - assert ([(expected, 'default (current tip)')], 'Branches') in refs + assert ([(expected, 'master')], 'Branches') in refs def test_repo_refs_one_branch_no_hints(self): cs0 = fixture.commit_change(self.repo_name, filename='file1', - content='line1\n', message='commit1', vcs_type='hg', + content='line1\n', message='commit1', vcs_type='git', parent=None, newfile=True) # TODO