From 875a0888b497a27ed14a8a451dd52604aaf621a4 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Apr 13 2017 10:23:27 +0000 Subject: [PATCH 1/3] Remove duplicated code We used to have three places doing the diff between multiple branches of the same or different repo, this is now down to a single one. This should make it easier to test and fix as needed. --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 5420b1d..17ac510 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -23,6 +23,7 @@ from sqlalchemy.exc import SQLAlchemyError PV = flask.Blueprint('internal_ns', __name__, url_prefix='/pv') import pagure +import pagure.exceptions import pagure.forms import pagure.lib import pagure.lib.git @@ -268,62 +269,26 @@ def get_pull_request_ready_branch(): branches = {} if not repo_obj.is_empty and repo_obj.listall_branches() > 1: if not repo_obj.head_is_unborn: - compare_branch = repo_obj.lookup_branch( - repo_obj.head.shorthand) + compare_branch = repo_obj.head.shorthand else: compare_branch = None for branchname in repo_obj.listall_branches(): - branch = repo_obj.lookup_branch(branchname) # Do not compare a branch to itself if compare_branch \ - and compare_branch.branch_name == branch.branch_name: + and compare_branch == branchname: continue - repo_commit = repo_obj[branch.get_object().hex] - - if compare_branch: - main_walker = repo_obj.walk( - compare_branch.get_object().hex, - pygit2.GIT_SORT_TIME) - branch_walker = repo_obj.walk( - repo_commit.oid.hex, - pygit2.GIT_SORT_TIME) - main_commits = set() - branch_commits = list() - while 1: - com = None - if compare_branch: - try: - com = main_walker.next() - main_commits.add(com.oid.hex) - except StopIteration: - com = None - try: - branch_commit = branch_walker.next() - except StopIteration: - branch_commit = None - - # We sure never end up here but better safe than sorry - if com is None and branch_commit is None: - break - - if branch_commit: - branch_commits.append(branch_commit.hex) - if main_commits.intersection(set(branch_commits)): - break - - # If master is ahead of branch, we need to remove the commits - # that are after the first one found in master - i = 0 - for i in range(len(branch_commits)): - if branch_commits[i] in main_commits: - break - branch_commits = branch_commits[:i] + diff_commits = None + try: + _, diff_commits, _ = pagure.lib.git.get_diff_info( + repo_obj, repo_obj, branchname, compare_branch) + except pagure.exceptions.PagureException: + pass - if branch_commits: - branches[branchname] = branch_commits + if diff_commits: + branches[branchname] = [c.oid.hex for c in diff_commits] prs = pagure.lib.search_pull_requests( pagure.SESSION, diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 887672c..020dad3 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1358,46 +1358,60 @@ def merge_pull_request( return 'Changes merged!' -def diff_pull_request( - session, request, repo_obj, orig_repo, requestfolder, - with_diff=True): - """ Returns the diff and the list of commits between the two git repos - mentionned in the given pull-request. - """ +def get_diff_info(repo_obj, orig_repo, branch_from, branch_to): + ''' Return the info needed to see a diff or make a Pull-Request between + the two specified repo. - commitid = None - diff = None - diff_commits = [] - branch = repo_obj.lookup_branch(request.branch_from) - if branch: - commitid = branch.get_object().hex + :arg repo_obj: The pygit2.Repository object of the first git repo + :arg orig_repo: The pygit2.Repository object of the second git repo + :arg branch_from: the name of the branch having the changes, in the + first git repo + : arg branch_to: the name of the branch in which we want to merge the + changes in the second git repo - if repo_obj.is_empty: - raise pagure.exceptions.PagureException( - 'Fork is empty, there are no commits to create a pull request with' + ''' + frombranch = repo_obj.lookup_branch(branch_from) + if not frombranch and not repo_obj.is_empty: + raise pagure.exceptions.BranchNotFoundException( + 'Branch %s does not exist' % branch_from ) - if not orig_repo.is_empty \ - and request.branch not in orig_repo.listall_branches(): - raise pagure.exceptions.PagureException( - 'The branch into which this pull-request was to be merged: %s ' - 'seems to no longer be present in this repo' % request.branch) + branch = None + if branch_to: + branch = orig_repo.lookup_branch(branch_to) + if not branch and not orig_repo.is_empty: + raise pagure.exceptions.BranchNotFoundException( + 'Branch %s could not be found in the target repo' % branch_to + ) + commitid = None + if frombranch: + commitid = frombranch.get_object().hex + + diff_commits = [] + diff = None + orig_commit = None if not repo_obj.is_empty and not orig_repo.is_empty: + if branch: + orig_commit = orig_repo[branch.get_object().hex] + main_walker = orig_repo.walk( + orig_commit.oid.hex, pygit2.GIT_SORT_TIME) + + repo_commit = repo_obj[commitid] + branch_walker = repo_obj.walk( + repo_commit.oid.hex, pygit2.GIT_SORT_TIME) - main_walker = orig_repo.walk( - orig_repo.lookup_branch(request.branch).get_object().hex, - pygit2.GIT_SORT_TIME) - branch_walker = repo_obj.walk(commitid, pygit2.GIT_SORT_TIME) main_commits = set() branch_commits = set() while 1: - try: - com = main_walker.next() - main_commits.add(com.oid.hex) - except StopIteration: - com = None + com = None + if branch: + try: + com = main_walker.next() + main_commits.add(com.oid.hex) + except StopIteration: + com = None try: branch_commit = branch_walker.next() @@ -1417,82 +1431,111 @@ def diff_pull_request( # If master is ahead of branch, we need to remove the commits # that are after the first one found in master i = 0 - for i in range(len(diff_commits)): - if diff_commits[i].oid.hex in main_commits: - break - diff_commits = diff_commits[:i] + if diff_commits and main_commits: + for i in range(len(diff_commits)): + if diff_commits[i].oid.hex in main_commits: + break + diff_commits = diff_commits[:i] - if request.status and diff_commits: + if diff_commits: first_commit = repo_obj[diff_commits[-1].oid.hex] - # Check if we can still rely on the merge_status - commenttext = None - if request.commit_start != first_commit.oid.hex or\ - request.commit_stop != diff_commits[0].oid.hex: - request.merge_status = None - if request.commit_start: - new_commits_count = 0 - commenttext = "" - for i in diff_commits: - if i.oid.hex == request.commit_stop: - break - new_commits_count = new_commits_count + 1 - commenttext = '%s * %s\n' % ( - commenttext, i.message.strip().split('\n')[0]) - if new_commits_count == 1: - commenttext = "**%d new commit added**\n\n%s" % ( - new_commits_count, commenttext) - else: - commenttext = "**%d new commits added**\n\n%s" % ( - new_commits_count, commenttext) - if request.commit_start and \ - request.commit_start != first_commit.oid.hex: - commenttext = 'rebased' - request.commit_start = first_commit.oid.hex - request.commit_stop = diff_commits[0].oid.hex - session.add(request) - session.commit() - if commenttext: - pagure.lib.add_pull_request_comment( - session, request, - commit=None, tree_id=None, filename=None, row=None, - comment='%s' % commenttext, - user=request.user.username, - requestfolder=requestfolder, - notify=False, notification=True + if len(first_commit.parents) > 0: + diff = repo_obj.diff( + repo_obj.revparse_single(first_commit.parents[0].oid.hex), + repo_obj.revparse_single(diff_commits[0].oid.hex) ) - session.commit() - pagure.lib.git.update_git( - request, repo=request.project, - repofolder=requestfolder) - - if diff_commits and with_diff and len(diff_commits[-1].parents) > 0: - diff = repo_obj.diff( - repo_obj.revparse_single(diff_commits[-1].parents[0].oid.hex), - repo_obj.revparse_single(diff_commits[0].oid.hex) - ) - elif orig_repo.is_empty and not repo_obj.is_empty: - for commit in repo_obj.walk(commitid, pygit2.GIT_SORT_TIME): + if 'master' in repo_obj.listall_branches(): + repo_commit = repo_obj[repo_obj.head.target] + else: + branch = repo_obj.lookup_branch(branch_from) + repo_commit = branch.get_object() + + for commit in repo_obj.walk( + repo_commit.oid.hex, pygit2.GIT_SORT_TIME): diff_commits.append(commit) - if request.status and diff_commits: - first_commit = repo_obj[diff_commits[-1].oid.hex] - # Check if we can still rely on the merge_status - if request.commit_start != first_commit.oid.hex or\ - request.commit_stop != diff_commits[0].oid.hex: - request.merge_status = None - request.commit_start = first_commit.oid.hex - request.commit_stop = diff_commits[0].oid.hex - session.add(request) - session.commit() - pagure.lib.git.update_git( - request, repo=request.project, - repofolder=requestfolder) - repo_commit = repo_obj[request.commit_stop] - if with_diff: - diff = repo_commit.tree.diff_to_tree(swap=True) + diff = repo_commit.tree.diff_to_tree(swap=True) + else: + raise pagure.exceptions.PagureException( + 'Fork is empty, there are no commits to create a pull ' + 'request with' + ) + + return(diff, diff_commits, orig_commit) + + +def diff_pull_request( + session, request, repo_obj, orig_repo, requestfolder, + with_diff=True): + """ Returns the diff and the list of commits between the two git repos + mentionned in the given pull-request. - return (diff_commits, diff) + :arg session: The sqlalchemy session to connect to the database + :arg request: The pagure.lib.model.PullRequest object of the pull-request + to look into + :arg repo_obj: The pygit2.Repository object of the first git repo + :arg orig_repo: The pygit2.Repository object of the second git repo + :arg requestfolder: The folder in which are stored the git repositories + containing the metadata of the different pull-requests + :arg with_diff: A boolean on whether to return the diff with the list + of commits (or just the list of commits) + + """ + + commitid = None + diff = None + diff_commits = [] + diff, diff_commits, _ = get_diff_info( + repo_obj, orig_repo, request.branch_from, request.branch) + + if request.status and diff_commits: + first_commit = repo_obj[diff_commits[-1].oid.hex] + # Check if we can still rely on the merge_status + commenttext = None + if request.commit_start != first_commit.oid.hex or\ + request.commit_stop != diff_commits[0].oid.hex: + request.merge_status = None + if request.commit_start: + new_commits_count = 0 + commenttext = "" + for i in diff_commits: + if i.oid.hex == request.commit_stop: + break + new_commits_count = new_commits_count + 1 + commenttext = '%s * %s\n' % ( + commenttext, i.message.strip().split('\n')[0]) + if new_commits_count == 1: + commenttext = "**%d new commit added**\n\n%s" % ( + new_commits_count, commenttext) + else: + commenttext = "**%d new commits added**\n\n%s" % ( + new_commits_count, commenttext) + if request.commit_start and \ + request.commit_start != first_commit.oid.hex: + commenttext = 'rebased' + request.commit_start = first_commit.oid.hex + request.commit_stop = diff_commits[0].oid.hex + session.add(request) + session.commit() + if commenttext: + pagure.lib.add_pull_request_comment( + session, request, + commit=None, tree_id=None, filename=None, row=None, + comment='%s' % commenttext, + user=request.user.username, + requestfolder=requestfolder, + notify=False, notification=True + ) + session.commit() + pagure.lib.git.update_git( + request, repo=request.project, + repofolder=requestfolder) + + if with_diff: + return (diff_commits, diff) + else: + diff_commits def get_git_tags(project): diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 75d4818..df2b502 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -58,100 +58,6 @@ def _get_parent_request_repo_path(repo): return parentpath -def _get_pr_info(repo_obj, orig_repo, branch_from, branch_to): - ''' Return the info needed to see a diff or make a Pull-Request between - the two specified repo. - ''' - frombranch = repo_obj.lookup_branch(branch_from) - if not frombranch and not repo_obj.is_empty: - flask.abort( - 400, - 'Branch %s does not exist' % branch_from) - - branch = orig_repo.lookup_branch(branch_to) - if not branch and not orig_repo.is_empty: - flask.abort( - 400, - 'Branch %s could not be found in the target repo' % branch_to) - - branch = repo_obj.lookup_branch(branch_from) - commitid = None - if branch: - commitid = branch.get_object().hex - - diff_commits = [] - diff = None - if not repo_obj.is_empty and not orig_repo.is_empty: - orig_commit = orig_repo[ - orig_repo.lookup_branch(branch_to).get_object().hex] - repo_commit = repo_obj[commitid] - - 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) - main_commits = set() - branch_commits = set() - - while 1: - try: - com = main_walker.next() - main_commits.add(com.oid.hex) - except StopIteration: - com = None - - try: - branch_commit = branch_walker.next() - except StopIteration: - branch_commit = None - - # We sure never end up here but better safe than sorry - if com is None and branch_commit is None: - break - - if branch_commit: - branch_commits.add(branch_commit.oid.hex) - diff_commits.append(branch_commit) - if main_commits.intersection(branch_commits): - break - - # If master is ahead of branch, we need to remove the commits - # that are after the first one found in master - i = 0 - for i in range(len(diff_commits)): - if diff_commits[i].oid.hex in main_commits: - break - diff_commits = diff_commits[:i] - - if diff_commits: - first_commit = repo_obj[diff_commits[-1].oid.hex] - if len(first_commit.parents) > 0: - diff = repo_obj.diff( - repo_obj.revparse_single(first_commit.parents[0].oid.hex), - repo_obj.revparse_single(diff_commits[0].oid.hex) - ) - - elif orig_repo.is_empty and not repo_obj.is_empty: - orig_commit = None - if 'master' in repo_obj.listall_branches(): - repo_commit = repo_obj[repo_obj.head.target] - else: - branch = repo_obj.lookup_branch(branch_from) - repo_commit = branch.get_object() - - for commit in repo_obj.walk( - repo_commit.oid.hex, pygit2.GIT_SORT_TIME): - diff_commits.append(commit) - - diff = repo_commit.tree.diff_to_tree(swap=True) - else: - raise pagure.exceptions.PagureException( - 'Fork is empty, there are no commits to create a pull request with' - ) - - return(diff, diff_commits, orig_commit) - - @APP.route('//pull-requests/') @APP.route('//pull-requests') @APP.route('///pull-requests/') @@ -1059,13 +965,10 @@ def new_request_pull( orig_repo = pygit2.Repository(parentpath) try: - diff, diff_commits, orig_commit = _get_pr_info( + diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( repo_obj, orig_repo, branch_from, branch_to) except pagure.exceptions.PagureException as err: - flask.flash(err.message, 'error') - return flask.redirect(flask.url_for( - 'view_repo', username=username, repo=repo.name, - namespace=namespace)) + flask.abort(400, str(err)) repo_committer = flask.g.repo_committer @@ -1216,7 +1119,7 @@ def new_remote_request_pull(repo, username=None, namespace=None): repo_obj = pygit2.Repository(repopath) try: - diff, diff_commits, orig_commit = _get_pr_info( + diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( repo_obj, orig_repo, branch_from, branch_to) except pagure.exceptions.PagureException as err: flask.flash(err.message, 'error') diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index bab9140..fbe0b33 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -1536,12 +1536,10 @@ index 0000000..2a552bb output = self.app.get( '/fork/foo/test/diff/master..feature', follow_redirects=True) - self.assertEqual(output.status_code, 200) - self.assertIn( - 'Overview - test - Pagure', output.data) + self.assertEqual(output.status_code, 400) self.assertIn( - '\n Fork is empty, there are ' - 'no commits to create a pull request with', output.data) + '

Fork is empty, there are no commits to create a ' + 'pull request with

', output.data) output = self.app.get('/test/new_issue') csrf_token = output.data.split( @@ -1554,12 +1552,10 @@ index 0000000..2a552bb output = self.app.post( '/test/diff/master..feature', data=data, follow_redirects=True) - self.assertEqual(output.status_code, 200) - self.assertIn( - 'Overview - test - Pagure', output.data) + self.assertEqual(output.status_code, 400) self.assertIn( - '\n Fork is empty, there are ' - 'no commits to create a pull request with', output.data) + '

Fork is empty, there are no commits to create a ' + 'pull request with

', output.data) shutil.rmtree(newpath) @@ -1591,11 +1587,10 @@ index 0000000..2a552bb with tests.user_set(pagure.APP, user): output = self.app.get( '/fork/foo/test/diff/master..master', follow_redirects=True) + self.assertEqual(output.status_code, 400) self.assertIn( - 'Overview - test - Pagure', output.data) - self.assertIn( - '\n Fork is empty, there are ' - 'no commits to create a pull request with', output.data) + '

Fork is empty, there are no commits to create a ' + 'pull request with

', output.data) shutil.rmtree(newpath) diff --git a/tests/test_pagure_flask_ui_fork_pr.py b/tests/test_pagure_flask_ui_fork_pr.py index cc30493..d02ea1e 100644 --- a/tests/test_pagure_flask_ui_fork_pr.py +++ b/tests/test_pagure_flask_ui_fork_pr.py @@ -160,6 +160,8 @@ class PagureFlaskForkPrtests(tests.Modeltests): refname = 'refs/heads/feature_foo:refs/heads/feature_foo' PagureRepo.push(ori_remote, refname) + shutil.rmtree(newpath) + def test_get_pr_info(self): """ Test pagure.ui.fork._get_pr_info """ @@ -167,7 +169,7 @@ class PagureFlaskForkPrtests(tests.Modeltests): gitrepo2 = os.path.join( self.path, 'repos', 'forks', 'pingou', 'test.git') - diff, diff_commits, orig_commit = pagure.ui.fork._get_pr_info( + diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( repo_obj=PagureRepo(gitrepo2), orig_repo=PagureRepo(gitrepo), branch_from='feature_foo', From 4a085b60ee7fd7849dc17851d5a2c1a66f101e08 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Apr 13 2017 10:23:27 +0000 Subject: [PATCH 2/3] Rename the test file to reflect more what it tests --- diff --git a/tests/test_pagure_flask_ui_fork_pr.py b/tests/test_pagure_flask_ui_fork_pr.py deleted file mode 100644 index d02ea1e..0000000 --- a/tests/test_pagure_flask_ui_fork_pr.py +++ /dev/null @@ -1,190 +0,0 @@ -# -*- coding: utf-8 -*- - -""" - (c) 2017 - Copyright Red Hat Inc - - Authors: - Pierre-Yves Chibon - -""" - -__requires__ = ['SQLAlchemy >= 0.8'] -import pkg_resources # noqa - -import json # noqa -import unittest # noqa -import shutil # noqa -import sys # noqa -import tempfile # noqa -import os # noqa - -import pygit2 # noqa -from mock import patch # noqa - -sys.path.insert(0, os.path.join(os.path.dirname( - os.path.abspath(__file__)), '..')) - -import pagure.lib # noqa -import tests # noqa -from pagure.lib.repo import PagureRepo # noqa - - -class PagureFlaskForkPrtests(tests.Modeltests): - """ Tests for flask fork controller of pagure regarding diffing PRs """ - - def setUp(self): - """ Set up the environnment, ran before every tests. """ - super(PagureFlaskForkPrtests, self).setUp() - - pagure.APP.config['GIT_FOLDER'] = os.path.join(self.path, 'repos') - pagure.APP.config['TICKETS_FOLDER'] = os.path.join( - self.path, 'tickets') - pagure.APP.config['DOCS_FOLDER'] = os.path.join( - self.path, 'docs') - pagure.APP.config['REQUESTS_FOLDER'] = os.path.join( - self.path, 'requests') - - # Create two git repos, one has 6 commits, the other 4 of which only - # 1 isn't present in the first repo - gitrepo = os.path.join(self.path, 'repos', 'test.git') - pygit2.init_repository(gitrepo, bare=True) - - gitrepo2 = os.path.join( - self.path, 'repos', 'forks', 'pingou', 'test.git') - pygit2.init_repository(gitrepo2, bare=True) - - newpath = tempfile.mkdtemp(prefix='pagure-fork-test') - repopath = os.path.join(newpath, 'test') - clone_repo = pygit2.clone_repository(gitrepo, repopath) - - # Do 3 commits to the main repo - for i in range(3): - with open(os.path.join(repopath, 'sources'), 'w') as stream: - stream.write('foo%s\n bar%s\n' % (i, i)) - clone_repo.index.add('sources') - clone_repo.index.write() - - parents = [] - try: - last_commit = clone_repo.revparse_single('HEAD') - parents = [last_commit.oid.hex] - except KeyError: - pass - - # Commits the files added - 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, - 'Editing the file sources for testing #%s' % i, - # binary string representing the tree object ID - tree, - # list of binary strings representing parents of the new commit - parents - ) - - # Push to the main repo - refname = 'refs/heads/master:refs/heads/master' - ori_remote = clone_repo.remotes[0] - PagureRepo.push(ori_remote, refname) - - # Push to the fork repo - remote = clone_repo.create_remote('pingou_fork', gitrepo2) - PagureRepo.push(remote, refname) - - # Do another 3 commits to the main repo - for i in range(3, 6): - with open(os.path.join(repopath, 'sources'), 'w') as stream: - stream.write('foo%s\n bar%s\n' % (i, i)) - clone_repo.index.add('sources') - clone_repo.index.write() - - last_commit = clone_repo.revparse_single('HEAD') - - # Commits the files added - 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, - 'Editing the file sources for testing #%s' % i, - # binary string representing the tree object ID - tree, - # list of binary strings representing parents of the new commit - [last_commit.oid.hex] - ) - # Push to the main repo - refname = 'refs/heads/master:refs/heads/master' - ori_remote = clone_repo.remotes[0] - PagureRepo.push(ori_remote, refname) - - # Add one commit to the fork repo - repopath = os.path.join(newpath, 'pingou_test') - clone_repo = pygit2.clone_repository(gitrepo2, repopath) - - with open(os.path.join(repopath, 'sources'), 'w') as stream: - stream.write('foo\n bar\n') - clone_repo.index.add('sources') - clone_repo.index.write() - - last_commit = clone_repo.revparse_single('HEAD') - - # Commits the files added - 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/feature_foo', # the name of the reference to update - author, - committer, - 'New edition on side branch of the file sources for testing', - # binary string representing the tree object ID - tree, - # list of binary strings representing parents of the new commit - [last_commit.oid.hex] - ) - - # Push to the fork repo - ori_remote = clone_repo.remotes[0] - refname = 'refs/heads/feature_foo:refs/heads/feature_foo' - PagureRepo.push(ori_remote, refname) - - shutil.rmtree(newpath) - - def test_get_pr_info(self): - """ Test pagure.ui.fork._get_pr_info """ - - gitrepo = os.path.join(self.path, 'repos', 'test.git') - gitrepo2 = os.path.join( - self.path, 'repos', 'forks', 'pingou', 'test.git') - - diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( - repo_obj=PagureRepo(gitrepo2), - orig_repo=PagureRepo(gitrepo), - branch_from='feature_foo', - branch_to='master' - ) - self.assertEqual(len(diff_commits), 1) - self.assertEqual( - diff_commits[0].message, - 'New edition on side branch of the file sources for testing' - ) - self.assertEqual( - orig_commit.message, - 'Editing the file sources for testing #5' - ) - - -if __name__ == '__main__': - unittest.main(verbosity=2) diff --git a/tests/test_pagure_lib_git_diff_pr.py b/tests/test_pagure_lib_git_diff_pr.py new file mode 100644 index 0000000..d02ea1e --- /dev/null +++ b/tests/test_pagure_lib_git_diff_pr.py @@ -0,0 +1,190 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources # noqa + +import json # noqa +import unittest # noqa +import shutil # noqa +import sys # noqa +import tempfile # noqa +import os # noqa + +import pygit2 # noqa +from mock import patch # noqa + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure.lib # noqa +import tests # noqa +from pagure.lib.repo import PagureRepo # noqa + + +class PagureFlaskForkPrtests(tests.Modeltests): + """ Tests for flask fork controller of pagure regarding diffing PRs """ + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskForkPrtests, self).setUp() + + pagure.APP.config['GIT_FOLDER'] = os.path.join(self.path, 'repos') + pagure.APP.config['TICKETS_FOLDER'] = os.path.join( + self.path, 'tickets') + pagure.APP.config['DOCS_FOLDER'] = os.path.join( + self.path, 'docs') + pagure.APP.config['REQUESTS_FOLDER'] = os.path.join( + self.path, 'requests') + + # Create two git repos, one has 6 commits, the other 4 of which only + # 1 isn't present in the first repo + gitrepo = os.path.join(self.path, 'repos', 'test.git') + pygit2.init_repository(gitrepo, bare=True) + + gitrepo2 = os.path.join( + self.path, 'repos', 'forks', 'pingou', 'test.git') + pygit2.init_repository(gitrepo2, bare=True) + + newpath = tempfile.mkdtemp(prefix='pagure-fork-test') + repopath = os.path.join(newpath, 'test') + clone_repo = pygit2.clone_repository(gitrepo, repopath) + + # Do 3 commits to the main repo + for i in range(3): + with open(os.path.join(repopath, 'sources'), 'w') as stream: + stream.write('foo%s\n bar%s\n' % (i, i)) + clone_repo.index.add('sources') + clone_repo.index.write() + + parents = [] + try: + last_commit = clone_repo.revparse_single('HEAD') + parents = [last_commit.oid.hex] + except KeyError: + pass + + # Commits the files added + 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, + 'Editing the file sources for testing #%s' % i, + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + parents + ) + + # Push to the main repo + refname = 'refs/heads/master:refs/heads/master' + ori_remote = clone_repo.remotes[0] + PagureRepo.push(ori_remote, refname) + + # Push to the fork repo + remote = clone_repo.create_remote('pingou_fork', gitrepo2) + PagureRepo.push(remote, refname) + + # Do another 3 commits to the main repo + for i in range(3, 6): + with open(os.path.join(repopath, 'sources'), 'w') as stream: + stream.write('foo%s\n bar%s\n' % (i, i)) + clone_repo.index.add('sources') + clone_repo.index.write() + + last_commit = clone_repo.revparse_single('HEAD') + + # Commits the files added + 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, + 'Editing the file sources for testing #%s' % i, + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [last_commit.oid.hex] + ) + # Push to the main repo + refname = 'refs/heads/master:refs/heads/master' + ori_remote = clone_repo.remotes[0] + PagureRepo.push(ori_remote, refname) + + # Add one commit to the fork repo + repopath = os.path.join(newpath, 'pingou_test') + clone_repo = pygit2.clone_repository(gitrepo2, repopath) + + with open(os.path.join(repopath, 'sources'), 'w') as stream: + stream.write('foo\n bar\n') + clone_repo.index.add('sources') + clone_repo.index.write() + + last_commit = clone_repo.revparse_single('HEAD') + + # Commits the files added + 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/feature_foo', # the name of the reference to update + author, + committer, + 'New edition on side branch of the file sources for testing', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [last_commit.oid.hex] + ) + + # Push to the fork repo + ori_remote = clone_repo.remotes[0] + refname = 'refs/heads/feature_foo:refs/heads/feature_foo' + PagureRepo.push(ori_remote, refname) + + shutil.rmtree(newpath) + + def test_get_pr_info(self): + """ Test pagure.ui.fork._get_pr_info """ + + gitrepo = os.path.join(self.path, 'repos', 'test.git') + gitrepo2 = os.path.join( + self.path, 'repos', 'forks', 'pingou', 'test.git') + + diff, diff_commits, orig_commit = pagure.lib.git.get_diff_info( + repo_obj=PagureRepo(gitrepo2), + orig_repo=PagureRepo(gitrepo), + branch_from='feature_foo', + branch_to='master' + ) + self.assertEqual(len(diff_commits), 1) + self.assertEqual( + diff_commits[0].message, + 'New edition on side branch of the file sources for testing' + ) + self.assertEqual( + orig_commit.message, + 'Editing the file sources for testing #5' + ) + + +if __name__ == '__main__': + unittest.main(verbosity=2) From c2560f015e0c5c98d1423ad41fab26bef484de30 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Apr 13 2017 10:23:27 +0000 Subject: [PATCH 3/3] Increase the unit-tests to test pagure.lib.git.diff_pull_request --- diff --git a/tests/test_pagure_lib_git_diff_pr.py b/tests/test_pagure_lib_git_diff_pr.py index d02ea1e..f6e588b 100644 --- a/tests/test_pagure_lib_git_diff_pr.py +++ b/tests/test_pagure_lib_git_diff_pr.py @@ -19,7 +19,7 @@ import tempfile # noqa import os # noqa import pygit2 # noqa -from mock import patch # noqa +from mock import patch, MagicMock # noqa sys.path.insert(0, os.path.join(os.path.dirname( os.path.abspath(__file__)), '..')) @@ -32,6 +32,7 @@ from pagure.lib.repo import PagureRepo # noqa class PagureFlaskForkPrtests(tests.Modeltests): """ Tests for flask fork controller of pagure regarding diffing PRs """ + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) def setUp(self): """ Set up the environnment, ran before every tests. """ super(PagureFlaskForkPrtests, self).setUp() @@ -44,6 +45,32 @@ class PagureFlaskForkPrtests(tests.Modeltests): pagure.APP.config['REQUESTS_FOLDER'] = os.path.join( self.path, 'requests') + # Create the main project in the DB + item = pagure.lib.model.Project( + user_id=1, # pingou + name='test', + description='test project #1', + hook_token='aaabbbccc', + ) + item.close_status = [ + 'Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] + self.session.add(item) + self.session.commit() + + # Create the fork + item = pagure.lib.model.Project( + user_id=1, # pingou + name='test', + description='test project #1', + hook_token='aaabbbcccdd', + parent_id=1, + is_fork=True, + ) + item.close_status = [ + 'Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] + self.session.add(item) + self.session.commit() + # Create two git repos, one has 6 commits, the other 4 of which only # 1 isn't present in the first repo gitrepo = os.path.join(self.path, 'repos', 'test.git') @@ -162,6 +189,24 @@ class PagureFlaskForkPrtests(tests.Modeltests): shutil.rmtree(newpath) + # Create the PR between the two repos + repo = pagure.lib.get_project(self.session, 'test') + forked_repo = pagure.lib.get_project( + self.session, 'test', user='pingou') + + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=forked_repo, + branch_from='feature_foo', + repo_to=repo, + branch_to='master', + title='test pull-request', + user='pingou', + requestfolder=None, + ) + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'test pull-request') + def test_get_pr_info(self): """ Test pagure.ui.fork._get_pr_info """ @@ -185,6 +230,54 @@ class PagureFlaskForkPrtests(tests.Modeltests): 'Editing the file sources for testing #5' ) + def test_get_pr_info_raises(self): + """ Test pagure.ui.fork._get_pr_info """ + + gitrepo = os.path.join(self.path, 'repos', 'test.git') + gitrepo2 = os.path.join( + self.path, 'repos', 'forks', 'pingou', 'test.git') + + self.assertRaises( + pagure.exceptions.BranchNotFoundException, + pagure.lib.git.get_diff_info, + repo_obj=PagureRepo(gitrepo2), + orig_repo=PagureRepo(gitrepo), + branch_from='feature', + branch_to='master' + ) + + self.assertRaises( + pagure.exceptions.BranchNotFoundException, + pagure.lib.git.get_diff_info, + repo_obj=PagureRepo(gitrepo2), + orig_repo=PagureRepo(gitrepo), + branch_from='feature_foo', + branch_to='bar' + ) + + def test_diff_pull_request(self): + """ Test pagure.lib.git.diff_pull_request """ + gitrepo = os.path.join(self.path, 'repos', 'test.git') + gitrepo2 = os.path.join( + self.path, 'repos', 'forks', 'pingou', 'test.git') + request = pagure.lib.search_pull_requests( + self.session, requestid=1, project_id=1) + + diff_commits, diff = pagure.lib.git.diff_pull_request( + self.session, + request=request, + repo_obj=PagureRepo(gitrepo2), + orig_repo=PagureRepo(gitrepo), + requestfolder=None, + with_diff=True + ) + + self.assertEqual(len(diff_commits), 1) + self.assertEqual( + diff_commits[0].message, + 'New edition on side branch of the file sources for testing' + ) + if __name__ == '__main__': unittest.main(verbosity=2)