From 8630b6c6f5bed9185101b3a1d2ffc5f7847f3196 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 27 2017 11:45:55 +0000 Subject: [PATCH 1/4] If the walker found no commit, do not add None to the list of commits --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 66e67c2..0a3bca2 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -314,7 +314,9 @@ def get_pull_request_ready_branch(): tmp = set(branch_commits) if main_commits.intersection(tmp): break - branch_commits.append(branch_commit.hex) + + if branch_commit: + branch_commits.append(branch_commit.hex) # If master is ahead of branch, we need to remove the commits # that are already in master diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 702119a..e177772 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1395,7 +1395,8 @@ def diff_pull_request( if main_commits.intersection(branch_commits): break - diff_commits.append(branch_commit) + if branch_commit: + diff_commits.append(branch_commit) # If master is ahead of branch, we need to remove the commits # that are already in master diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index e8923fc..95b516e 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -114,7 +114,8 @@ def _get_pr_info(repo_obj, orig_repo, branch_from, branch_to): if main_commits.intersection(branch_commits): break - diff_commits.append(branch_commit) + if branch_commit: + diff_commits.append(branch_commit) # If master is ahead of branch, we need to remove the commits # that are already in master From 7e7bfcd3486faf40b3a1e5e8d1ec9ed346083b38 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 27 2017 11:52:29 +0000 Subject: [PATCH 2/4] Fix typo about which repository is walked by the main walker --- diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 95b516e..8338e93 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -86,7 +86,7 @@ def _get_pr_info(repo_obj, orig_repo, branch_from, branch_to): orig_repo.lookup_branch(branch_to).get_object().hex] repo_commit = repo_obj[commitid] - main_walker = repo_obj.walk( + main_walker = orig_repo.walk( orig_commit.oid.hex, pygit2.GIT_SORT_TIME) branch_walker = repo_obj.walk( repo_commit.oid.hex, pygit2.GIT_SORT_TIME) From 0add6c1256bbd0f9acfe671962eda74ba1c3b272 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 28 2017 10:52:49 +0000 Subject: [PATCH 3/4] Add unit-tests testing for when the main branch is ahead of the feature one This for the internal API endpoint used to gather the list of branches having commits not in the main branch. --- diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 5335b11..fdcdad1 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -1157,7 +1157,7 @@ index 0000000..2a552bb '/test/pull-request/cancel/1', data=data, follow_redirects=True) self.assertEqual(output.status_code, 404) - # Project w/o pull-request + # Project w/ pull-request repo = pagure.lib.get_project(self.session, 'test') settings = repo.settings settings['pull_requests'] = True @@ -1914,6 +1914,136 @@ index 0000000..2a552bb output.data) @patch('pagure.lib.notify.send_email') + def test_internal_endpoint_main_ahead(self, send_email): + """ Test the new_request_pull endpoint when the main repo is ahead + of the fork. + """ + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + + self.set_up_git_repo( + new_project=None, + branch_from='feature') + + gitrepo = os.path.join(self.path, 'repos', 'test.git') + repo = pygit2.init_repository(gitrepo, bare=True) + + # Make the main repo be ahead of the fork + + # First commit + newpath = tempfile.mkdtemp(prefix='pagure-test') + repopath = os.path.join(newpath, 'test') + clone_repo = pygit2.clone_repository(gitrepo, repopath) + + # Create a file in that git repo + with open(os.path.join(repopath, 'testfile'), 'w') as stream: + stream.write('foo\n bar') + clone_repo.index.add('testfile') + clone_repo.index.write() + + # Commits the files added + last_commit = clone_repo.revparse_single('HEAD') + tree = clone_repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + clone_repo.create_commit( + 'refs/heads/master', # the name of the reference to update + author, + committer, + 'Add testfile file for testing', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [last_commit.oid.hex] + ) + + # Second commit + with open(os.path.join(repopath, 'testfile'), 'a') as stream: + stream.write('\nfoo2\n bar2') + clone_repo.index.add('testfile') + clone_repo.index.write() + + # Commits the files added + last_commit = clone_repo.revparse_single('HEAD') + tree = clone_repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + clone_repo.create_commit( + 'refs/heads/master', # the name of the reference to update + author, + committer, + 'Add a second commit to testfile for testing', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [last_commit.oid.hex] + ) + + # Third commit + with open(os.path.join(repopath, 'testfile'), 'a') as stream: + stream.write('\nfoo3\n bar3') + clone_repo.index.add('testfile') + clone_repo.index.write() + + # Commits the files added + last_commit = clone_repo.revparse_single('HEAD') + tree = clone_repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + clone_repo.create_commit( + 'refs/heads/master', # the name of the reference to update + author, + committer, + 'Add a third commit to testfile for testing', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [last_commit.oid.hex] + ) + + refname = 'refs/heads/master:refs/heads/master' + ori_remote = clone_repo.remotes[0] + PagureRepo.push(ori_remote, refname) + + shutil.rmtree(newpath) + + user = tests.FakeUser() + user.username = 'foo' + with tests.user_set(pagure.APP, user): + + output = self.app.get('/new') + csrf_token = output.data.split( + 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + + output = self.app.post( + '/pv/pull-request/ready', + data={'repo': 'test', 'csrf_token': csrf_token} + ) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertEqual( + data, + { + "code": "OK", + "message": { + "branch_w_pr": { + "feature": 1 + }, + "new_branch": {} + } + } + ) + + @patch('pagure.lib.notify.send_email') def test_fork_edit_file(self, send_email): """ Test the fork_edit file endpoint. """ From bea002e0b53e6d0bc07d60bf04df0286c4f33de9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 28 2017 10:53:41 +0000 Subject: [PATCH 4/4] Small style changes --- diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index fdcdad1..bab9140 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -1130,17 +1130,20 @@ index 0000000..2a552bb # Invalid project output = self.app.post( - '/foo/pull-request/cancel/1', data=data, follow_redirects=True) + '/foo/pull-request/cancel/1', data=data, + follow_redirects=True) self.assertEqual(output.status_code, 404) # Invalid PR id output = self.app.post( - '/test/pull-request/cancel/100', data=data, follow_redirects=True) + '/test/pull-request/cancel/100', data=data, + follow_redirects=True) self.assertEqual(output.status_code, 404) # Invalid user for this project output = self.app.post( - '/test/pull-request/cancel/1', data=data, follow_redirects=True) + '/test/pull-request/cancel/1', data=data, + follow_redirects=True) self.assertEqual(output.status_code, 403) user.username = 'pingou' @@ -1154,7 +1157,8 @@ index 0000000..2a552bb self.session.commit() output = self.app.post( - '/test/pull-request/cancel/1', data=data, follow_redirects=True) + '/test/pull-request/cancel/1', data=data, + follow_redirects=True) self.assertEqual(output.status_code, 404) # Project w/ pull-request @@ -1166,7 +1170,8 @@ index 0000000..2a552bb self.session.commit() output = self.app.post( - '/test/pull-request/cancel/1', data=data, follow_redirects=True) + '/test/pull-request/cancel/1', data=data, + follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( 'Overview - test - Pagure', output.data)