From 6fdafd0b79a6883cc61b3078bd5c57cbe2de4d4b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 07:15:47 +0000 Subject: [PATCH 1/8] Ensure two utility methods in the tests respect the branch they are given Otherwise they were not really committing in the branch asked for. Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/__init__.py b/tests/__init__.py index 61601fe..f3e9723 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -59,6 +59,7 @@ import pagure.api from pagure.api.ci import jenkins import pagure.flask_app import pagure.lib +import pagure.lib.git import pagure.lib.model import pagure.lib.tasks_mirror import pagure.perfrepo as perfrepo @@ -695,7 +696,7 @@ def create_tokens_acl(session, token_id='aaabbbcccddd', acl_name=None): session.commit() -def add_content_git_repo(folder, branch='master'): +def add_content_git_repo(folder, branch='master', append=None): """ Create some content for the specified git repo. """ if not os.path.exists(folder): os.makedirs(folder) @@ -704,29 +705,42 @@ def add_content_git_repo(folder, branch='master'): newfolder = tempfile.mkdtemp(prefix='pagure-tests') repo = pygit2.clone_repository(folder, newfolder) - # Create a file in that git repo - with open(os.path.join(newfolder, 'sources'), 'w') as stream: - stream.write('foo\n bar') - repo.index.add('sources') - repo.index.write() + branch_ref_obj = None + if "origin/%s" % branch in repo.listall_branches(pygit2.GIT_BRANCH_ALL): + branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) + repo.checkout(branch_ref_obj) parents = [] commit = None try: - commit = repo.revparse_single( - 'HEAD' if branch == 'master' else branch) + if branch_ref_obj: + commit = repo[branch_ref_obj.get_object().hex] + else: + commit = repo.revparse_single('HEAD') except KeyError: pass if commit: parents = [commit.oid.hex] + # Create a file in that git repo + filename = os.path.join(newfolder, 'sources') + content = 'foo\n bar' + if os.path.exists(filename): + content = 'foo\n bar\nbaz' + if append: + content += append + with open(filename, 'w') as stream: + stream.write(content) + repo.index.add('sources') + repo.index.write() + # Commits the files added tree = repo.index.write_tree() author = pygit2.Signature( 'Alice Author', 'alice@authors.tld') committer = pygit2.Signature( 'Cecil Committer', 'cecil@committers.tld') - repo.create_commit( + commit = repo.create_commit( 'refs/heads/%s' % branch, # the name of the reference to update author, committer, @@ -737,15 +751,8 @@ def add_content_git_repo(folder, branch='master'): parents, ) - parents = [] - commit = None - try: - commit = repo.revparse_single( - 'HEAD' if branch == 'master' else branch) - except KeyError: - pass if commit: - parents = [commit.oid.hex] + parents = [commit.hex] subfolder = os.path.join('folder1', 'folder2') if not os.path.exists(os.path.join(newfolder, subfolder)): @@ -765,7 +772,7 @@ def add_content_git_repo(folder, branch='master'): 'Alice Author', 'alice@authors.tld') committer = pygit2.Signature( 'Cecil Committer', 'cecil@committers.tld') - repo.create_commit( + commit =repo.create_commit( 'refs/heads/%s' % branch, # the name of the reference to update author, committer, @@ -787,7 +794,7 @@ def add_content_git_repo(folder, branch='master'): shutil.rmtree(newfolder) -def add_readme_git_repo(folder, readme_name='README.rst'): +def add_readme_git_repo(folder, readme_name='README.rst', branch='master'): """ Create a README file for the specified git repo. """ if not os.path.exists(folder): os.makedirs(folder) @@ -796,6 +803,11 @@ def add_readme_git_repo(folder, readme_name='README.rst'): newfolder = tempfile.mkdtemp(prefix='pagure-tests') repo = pygit2.clone_repository(folder, newfolder) + branch_ref_obj = None + if "origin/%s" % branch in repo.listall_branches(pygit2.GIT_BRANCH_ALL): + branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) + repo.checkout(branch_ref_obj) + if readme_name == 'README.rst': content = """Pagure ====== @@ -825,8 +837,11 @@ that should never get displayed on the website if there is a README.rst in the r parents = [] commit = None try: - commit = repo.revparse_single('HEAD') - except KeyError: + if branch_ref_obj: + commit = repo[branch_ref_obj.get_object().hex] + else: + commit = repo.revparse_single('HEAD') + except (KeyError, AttributeError): pass if commit: parents = [commit.oid.hex] @@ -843,8 +858,9 @@ that should never get displayed on the website if there is a README.rst in the r 'Alice Author', 'alice@authors.tld') committer = pygit2.Signature( 'Cecil Committer', 'cecil@committers.tld') + branch_ref = "refs/heads/%s" % branch repo.create_commit( - 'refs/heads/master', # the name of the reference to update + branch_ref, # the name of the reference to update author, committer, 'Add a README file', @@ -856,10 +872,8 @@ that should never get displayed on the website if there is a README.rst in the r # Push to origin ori_remote = repo.remotes[0] - master_ref = repo.lookup_reference('HEAD').resolve() - refname = '%s:%s' % (master_ref.name, master_ref.name) - PagureRepo.push(ori_remote, refname) + PagureRepo.push(ori_remote, '%s:%s' % (branch_ref, branch_ref)) shutil.rmtree(newfolder) @@ -874,6 +888,11 @@ def add_commit_git_repo(folder, ncommits=10, filename='sources', newfolder = tempfile.mkdtemp(prefix='pagure-tests') repo = pygit2.clone_repository(folder, newfolder) + branch_ref_obj = None + if "origin/%s" % branch in repo.listall_branches(pygit2.GIT_BRANCH_ALL): + branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) + repo.checkout(branch_ref_obj) + for index in range(ncommits): # Create a file in that git repo with open(os.path.join(newfolder, filename), 'a') as stream: @@ -884,8 +903,11 @@ def add_commit_git_repo(folder, ncommits=10, filename='sources', parents = [] commit = None try: - commit = repo.revparse_single('HEAD') - except KeyError: + if branch_ref_obj: + commit = repo[branch_ref_obj.get_object().hex] + else: + commit = repo.revparse_single('HEAD') + except (KeyError, AttributeError): pass if commit: parents = [commit.oid.hex] @@ -896,8 +918,9 @@ def add_commit_git_repo(folder, ncommits=10, filename='sources', 'Alice Author', 'alice@authors.tld') committer = pygit2.Signature( 'Cecil Committer', 'cecil@committers.tld') + branch_ref = "refs/heads/%s" % branch repo.create_commit( - 'refs/heads/master', + branch_ref, author, committer, 'Add row %s to %s file' % (index, filename), @@ -906,10 +929,11 @@ def add_commit_git_repo(folder, ncommits=10, filename='sources', # list of binary strings representing parents of the new commit parents, ) + branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) # Push to origin ori_remote = repo.remotes[0] - PagureRepo.push(ori_remote, 'HEAD:refs/heads/%s' % branch) + PagureRepo.push(ori_remote, '%s:%s' % (branch_ref, branch_ref)) shutil.rmtree(newfolder) diff --git a/tests/test_pagure_flask_internal.py b/tests/test_pagure_flask_internal.py index 9346bb4..7e008bb 100644 --- a/tests/test_pagure_flask_internal.py +++ b/tests/test_pagure_flask_internal.py @@ -2792,6 +2792,7 @@ class PagureFlaskInternaltests(tests.Modeltests): os.path.join(self.path, 'repos', 'forks', 'foo'), bare=True) tests.add_content_git_repo( os.path.join(self.path, 'repos', 'forks', 'foo', 'test.git'), + append="testing from foo's fork", branch='feature') # Create foo's fork of the test project diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 4f1916e..ae79058 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -2557,8 +2557,8 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertEqual(output.status_code, 404) # Add some content to the git repo - tests.add_content_git_repo(os.path.join(self.path, 'repos', - 'test.git')) + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'test.git')) tests.add_content_git_repo( os.path.join(self.path, 'repos', 'test.git'), branch='feature') @@ -2600,8 +2600,9 @@ class PagureFlaskRepotests(tests.Modeltests): 'data-line-number="1">', output_text) self.assertIn( '
 bar
', output_text) - data = regex.findall(output_text) - self.assertEqual(len(data), 2) + data1 = regex.findall(output_text) + self.assertEqual(len(data1), 2) + self.assertEqual(data, data1) # View in feature branch output = self.app.get('/test/blame/sources?identifier=feature') @@ -2614,8 +2615,10 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertIn( '
 bar
', output_text) data2 = regex.findall(output_text) - self.assertEqual(len(data2), 2) - self.assertNotEqual(data, data2) + self.assertEqual(len(data2), 3) + self.assertEqual(data[0], data2[0]) + self.assertNotEqual(data2[0], data2[1]) + self.assertEqual(data2[1], data2[2]) # View what's supposed to be an image output = self.app.get('/test/blame/test.jpg') From 3470e11726693a9aa15a7ec47f90ac7c351dbc06 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 07:15:47 +0000 Subject: [PATCH 2/8] Add an utility method to remove a file from a given git repo in the tests Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/__init__.py b/tests/__init__.py index f3e9723..208a4f4 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1047,6 +1047,58 @@ def add_binary_git_repo(folder, filename): shutil.rmtree(newfolder) +def remove_file_git_repo(folder, filename, branch='master'): + """ Delete the specified file on the give git repo and branch. """ + newfolder = tempfile.mkdtemp(prefix='pagure-tests') + repo = pygit2.clone_repository(folder, newfolder) + + branch_ref_obj = None + if "origin/%s" % branch in repo.listall_branches(pygit2.GIT_BRANCH_ALL): + branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) + repo.checkout(branch_ref_obj) + + parents = [] + commit = None + try: + if branch_ref_obj: + commit = repo[branch_ref_obj.get_object().hex] + else: + commit = repo.revparse_single('HEAD') + except (KeyError, AttributeError): + pass + if commit: + parents = [commit.oid.hex] + + # Remove file + repo.index.remove(filename) + + # Write the change and commit it + tree = repo.index.write_tree() + + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + branch_ref = "refs/heads/%s" % branch + repo.create_commit( + branch_ref, # the name of the reference to update + author, + committer, + 'Remove file %s' % filename, + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + parents + ) + + # Push to origin + ori_remote = repo.remotes[0] + + PagureRepo.push(ori_remote, '%s:%s' % (branch_ref, branch_ref)) + + shutil.rmtree(newfolder) + + @contextmanager def capture_output(merge_stderr=True): oldout, olderr = sys.stdout, sys.stderr From c67f41a56e8225a09196eb3092200000535ef1ac Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 07:15:47 +0000 Subject: [PATCH 3/8] Add a new API endpoint to retrieve the list of files changed in a PR This new API endpoint allows to retrieve the list of files changed as well as the number of lines added/removed and whether the file was added removed, modified or renamed (cf the status field). Fixes https://pagure.io/pagure/issue/3686 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index b54ac71..f565ea8 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -118,6 +118,7 @@ class APIERROR(enum.Enum): "by their author." ) ETRACKERREADONLY = "The issue tracker of this project is read-only" + ENOPRSTATS = "No statistics could be computed for this PR" def get_authorized_api_project(session, repo, user=None, namespace=None): diff --git a/pagure/api/fork.py b/pagure/api/fork.py index f8d97bc..9c2ab9e 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -1251,3 +1251,193 @@ def api_pull_request_create(repo, username=None, namespace=None): jsonout = flask.jsonify(request.to_json(public=True, api=True)) return jsonout + + +@API.route("//pull-request//diffstats") +@API.route("///pull-request//diffstats") +@API.route("/fork///pull-request//diffstats") +@API.route( + "/fork////pull-request//" + "diffstats" +) +@api_method +def api_pull_request_diffstats(repo, requestid, username=None, namespace=None): + """ + Pull-request diff statistics + ---------------------------- + Retrieve the statistics about the diff of a specific pull request. + + :: + + GET /api/0//pull-request//diffstats + GET /api/0///pull-request//diffstats + + :: + + GET /api/0/fork///pull-request//diffstats + GET /api/0/fork////pull-request//diffstats + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "README.rst": { + "lines_added": 1, + "lines_removed": 1, + "old_path": "README.rst", + "status": "M" + }, + "blame_file.txt": { + "lines_added": 0, + "lines_removed": 0, + "old_path": "blame_file", + "status": "R" + }, + "test": { + "lines_added": 0, + "lines_removed": 8, + "old_path": "test", + "status": "D" + }, + "test3": { + "lines_added": 3, + "lines_removed": 0, + "old_path": "test3", + "status": "A" + } + } + + + + """ # noqa + + repo = get_authorized_api_project( + flask.g.session, repo, user=username, namespace=namespace + ) + + if repo is None: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + + if not repo.settings.get("pull_requests", True): + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.EPULLREQUESTSDISABLED + ) + + request = pagure.lib.search_pull_requests( + flask.g.session, project_id=repo.id, requestid=requestid + ) + + if not request: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOREQ) + + if request.remote: + repopath = pagure.utils.get_remote_repo_path( + request.remote_git, request.branch_from + ) + parentpath = pagure.utils.get_repo_path(request.project) + else: + repo_from = request.project_from + parentpath = pagure.utils.get_repo_path(request.project) + repopath = parentpath + if repo_from: + repopath = pagure.utils.get_repo_path(repo_from) + + repo_obj = pygit2.Repository(repopath) + orig_repo = pygit2.Repository(parentpath) + + diff_commits = [] + diff = None + # Closed pull-request + if request.status != "Open": + commitid = request.commit_stop + try: + for commit in repo_obj.walk(commitid, pygit2.GIT_SORT_NONE): + diff_commits.append(commit) + if commit.oid.hex == request.commit_start: + break + except KeyError: + # This happens when repo.walk() cannot find commitid + pass + + if diff_commits: + # Ensure the first commit in the PR as a parent, otherwise + # point to it + start = diff_commits[-1].oid.hex + if diff_commits[-1].parents: + start = diff_commits[-1].parents[0].oid.hex + + # If the start and the end commits are the same, it means we are, + # dealing with one commit that has no parent, so just diff that + # one commit + if start == diff_commits[0].oid.hex: + diff = diff_commits[0].tree.diff_to_tree(swap=True) + else: + diff = repo_obj.diff( + repo_obj.revparse_single(start), + repo_obj.revparse_single(diff_commits[0].oid.hex), + ) + else: + try: + diff_commits, diff = pagure.lib.git.diff_pull_request( + flask.g.session, request, repo_obj, orig_repo + ) + except pagure.exceptions.PagureException as err: + flask.flash("%s" % err, "error") + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.exception(err) + flask.flash( + "Could not update this pull-request in the database", "error" + ) + + if diff: + diff.find_similar() + + output = {} + if diff: + for patch in diff: + linesadded = patch.line_stats[1] + linesremoved = patch.line_stats[2] + if hasattr(patch, "new_file_path"): + # Older pygit2 + status = patch.status + if patch.new_file_path != patch.old_file_path: + status = "R" + output[patch.new_file_path] = { + "status": patch.status, + "old_path": patch.old_file_path, + "lines_added": linesadded, + "lines_removed": linesremoved, + } + elif hasattr(patch, "delta"): + # Newer pygit2 + if ( + patch.delta.new_file.mode == 0 + and patch.delta.old_file.mode in [33188, 33261] + ): + status = "D" + elif ( + patch.delta.new_file.mode in [33188, 33261] + and patch.delta.old_file.mode == 0 + ): + status = "A" + elif patch.delta.new_file.mode in [ + 33188, + 33261, + ] and patch.delta.old_file.mode in [33188, 33261]: + status = "M" + if patch.delta.new_file.path != patch.delta.old_file.path: + status = "R" + output[patch.delta.new_file.path] = { + "status": status, + "old_path": patch.delta.old_file.path, + "lines_added": linesadded, + "lines_removed": linesremoved, + } + else: + raise pagure.exceptions.APIError(400, error_code=APIERROR.ENOPRSTATS) + + jsonout = flask.jsonify(output) + return jsonout diff --git a/tests/test_pagure_flask_api.py b/tests/test_pagure_flask_api.py index b8afdc3..45149dd 100644 --- a/tests/test_pagure_flask_api.py +++ b/tests/test_pagure_flask_api.py @@ -237,7 +237,7 @@ class PagureFlaskApitests(tests.SimplePagureTest): output = self.app.get('/api/0/-/error_codes') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(len(data), 32) + self.assertEqual(len(data), 33) self.assertEqual( sorted(data.keys()), [ @@ -248,10 +248,10 @@ class PagureFlaskApitests(tests.SimplePagureTest): u'EMODIFYPROJECTNOTALLOWED', u'ENEWPROJECTDISABLED', u'ENOCODE', u'ENOCOMMENT', u'ENOCOMMIT', u'ENOGROUP', u'ENOISSUE', u'ENOPRCLOSE', u'ENOPROJECT', u'ENOPROJECTS', - u'ENOREQ', u'ENOSIGNEDOFF', u'ENOTASSIGNED', u'ENOTASSIGNEE', - u'ENOTHIGHENOUGH', u'ENOTMAINADMIN', u'ENOUSER', u'EPRSCORE', - u'EPULLREQUESTSDISABLED', u'ETIMESTAMP', u'ETRACKERDISABLED', - u'ETRACKERREADONLY' + u'ENOPRSTATS', u'ENOREQ', u'ENOSIGNEDOFF', u'ENOTASSIGNED', + u'ENOTASSIGNEE', u'ENOTHIGHENOUGH', u'ENOTMAINADMIN', + u'ENOUSER', u'EPRSCORE', u'EPULLREQUESTSDISABLED', + u'ETIMESTAMP', u'ETRACKERDISABLED', u'ETRACKERREADONLY' ] ) diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index f17a9bd..a8f7c47 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -2703,6 +2703,176 @@ class PagureFlaskApiForktests(tests.Modeltests): } ) +class PagureFlaskApiForkPRDiffStatstests(tests.Modeltests): + """ Tests for the flask API of pagure for the diff stats endpoint of PRs + """ + + maxDiff = None + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskApiForkPRDiffStatstests, self).setUp() + + pagure.config.config['REQUESTS_FOLDER'] = None + + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + tests.create_projects_git(os.path.join(self.path, 'requests'), + bare=True) + tests.add_readme_git_repo(os.path.join(self.path, 'repos', 'test.git')) + tests.add_commit_git_repo( + os.path.join(self.path, 'repos', 'test.git'), ncommits=5) + tests.add_commit_git_repo( + os.path.join(self.path, 'repos', 'test.git'), branch='test') + + # Create the pull-request to close + repo = pagure.lib.get_authorized_project(self.session, 'test') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=repo, + branch_from='test', + repo_to=repo, + branch_to='master', + title='test pull-request', + user='pingou', + ) + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'test pull-request') + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_no_repo(self): + """ Test the api_pull_request_merge method of the flask api. """ + output = self.app.get('/api/0/invalid/pull-request/404/diffstats') + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + {'error': 'Project not found', 'error_code': 'ENOPROJECT'} + ) + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_no_pr(self): + """ Test the api_pull_request_merge method of the flask api. """ + output = self.app.get('/api/0/test/pull-request/404/diffstats') + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + {'error': 'Pull-Request not found', 'error_code': 'ENOREQ'} + ) + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_file_modified(self): + """ Test the api_pull_request_merge method of the flask api. """ + output = self.app.get('/api/0/test/pull-request/1/diffstats') + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + 'sources': { + 'lines_added': 10, + 'lines_removed': 0, + 'old_path': 'sources', + 'status': 'M' + } + } + ) + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_file_added_mofidied(self): + """ Test the api_pull_request_merge method of the flask api. """ + tests.add_commit_git_repo( + os.path.join(self.path, 'repos', 'test.git'), ncommits=5) + tests.add_readme_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + readme_name='README.md', branch='test') + + repo = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(repo.requests), 1) + + output = self.app.get('/api/0/test/pull-request/1/diffstats') + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertTrue( + data in + [ + { + "README.md": { + "lines_added": 5, + "lines_removed": 0, + "old_path": "README.md", + "status": "A" + }, + "sources": { + "lines_added": 5, + "lines_removed": 0, + "old_path": "sources", + "status": "M" + } + }, + { + "README.md": { + "lines_added": 5, + "lines_removed": 0, + "old_path": "README.md", + "status": "A" + }, + "sources": { + "lines_added": 10, + "lines_removed": 0, + "old_path": "sources", + "status": "M" + } + } + ] + ) + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_file_modified_deleted(self): + """ Test the api_pull_request_merge method of the flask api. """ + repo = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(repo.requests), 1) + pagure.lib.tasks.update_pull_request(repo.requests[0].uid) + + tests.add_readme_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + readme_name='README.md', branch='test') + tests.remove_file_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + filename='sources', branch='test') + + repo = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(repo.requests), 1) + pagure.lib.tasks.update_pull_request(repo.requests[0].uid) + + output = self.app.get('/api/0/test/pull-request/1/diffstats') + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + "README.md": { + "lines_added": 5, + "lines_removed": 0, + "old_path": "README.md", + "status": "A" + }, + "sources": { + "lines_added": 0, + "lines_removed": 5, + "old_path": "sources", + "status": "D" + } + } + ) + if __name__ == '__main__': unittest.main(verbosity=2) From 87b46dac4fb70c8486458f5e016e846e460e6b7b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 07:15:47 +0000 Subject: [PATCH 4/8] Fix quotes in strings (left over from a time we didn't run black) Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index f565ea8..1258e2c 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -84,7 +84,7 @@ class APIERROR(enum.Enum): ) ENOREQ = "Pull-Request not found" ENOPRCLOSE = ( - "You are not allowed to merge/close pull-request for " "this project" + "You are not allowed to merge/close pull-request for this project" ) EPRSCORE = ( "This request does not have the minimum review score " @@ -95,13 +95,13 @@ class APIERROR(enum.Enum): ENOUSER = "No such user found" ENOCOMMENT = "Comment not found" ENEWPROJECTDISABLED = ( - "Creating project have been disabled for this " "instance" + "Creating project have been disabled for this instance" ) ETIMESTAMP = "Invalid timestamp format" EDATETIME = "Invalid datetime format" EINVALIDISSUEFIELD = "Invalid custom field submitted" EINVALIDISSUEFIELD_LINK = ( - "Invalid custom field submitted, the value " "is not a link" + "Invalid custom field submitted, the value is not a link" ) EINVALIDPRIORITY = "Invalid priority submitted" ENOGROUP = "Group not found" @@ -111,7 +111,7 @@ class APIERROR(enum.Enum): EGITERROR = "An error occurred during a git operation" ENOCOMMIT = "No such commit found in this repository" ENOTHIGHENOUGH = ( - "You do not have sufficient permissions to perform " "this action" + "You do not have sufficient permissions to perform this action" ) ENOSIGNEDOFF = ( "This repo enforces that all commits are signed off " From 6bd26beb1a5f139f6f5f084ef78c447c9ff4c0ee Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 07:15:47 +0000 Subject: [PATCH 5/8] Adjust logged messages to fit the actual module file/name Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index f8de106..a7819b6 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1770,7 +1770,9 @@ def get_diff_info(repo_obj, orig_repo, branch_from, branch_to, prid=None): repo_obj = orig_repo if not repo_obj.is_empty and not orig_repo.is_empty: - _log.info("pagure.lib.get_diff_info: Pulling into a non-empty repo") + _log.info( + "pagure.lib.git.get_diff_info: Pulling into a non-empty repo" + ) if branch: orig_commit = orig_repo[branch.get_object().hex] main_walker = orig_repo.walk( @@ -1828,13 +1830,13 @@ def get_diff_info(repo_obj, orig_repo, branch_from, branch_to, prid=None): ) elif first_commit.oid.hex == diff_commits[0].oid.hex: _log.info( - "pagure.lib.get_diff_info: First commit is also the last " - "commit" + "pagure.lib.git.get_diff_info: First commit is also the " + "last commit" ) diff = diff_commits[0].tree.diff_to_tree(swap=True) elif orig_repo.is_empty and not repo_obj.is_empty: - _log.info("pagure.lib.get_diff_info: Pulling into an empty repo") + _log.info("pagure.lib.git.get_diff_info: Pulling into an empty repo") if "master" in repo_obj.listall_branches(): repo_commit = repo_obj[repo_obj.head.target] else: @@ -1853,9 +1855,10 @@ def get_diff_info(repo_obj, orig_repo, branch_from, branch_to, prid=None): ) _log.info( - "pagure.lib.get_diff_info: diff_commits length: %s", len(diff_commits) + "pagure.lib.git.get_diff_info: diff_commits length: %s", + len(diff_commits), ) - _log.info("pagure.lib.get_diff_info: original commit: %s", orig_commit) + _log.info("pagure.lib.git.get_diff_info: original commit: %s", orig_commit) return (diff, diff_commits, orig_commit) From 114311f6a32f189d9ec3430021f856f3e2456337 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 07:15:47 +0000 Subject: [PATCH 6/8] Flake8 fixes Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index bff47d3..1059032 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -282,8 +282,7 @@ def create_project( if templ: if not os.path.exists(templ): _log.warning( - "Invalid git template configured: %s, not found on disk", - templ, + "Invalid git template configured: %s, not found on disk", templ ) templ = None else: From b8cc62c7a96abe758ebb2e65c636b2f51f581dff Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 07:15:47 +0000 Subject: [PATCH 7/8] Fixup a logging call Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 1059032..89913b2 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -907,7 +907,9 @@ def update_pull_request(self, session, pr_uid): repopath = parentpath if repo_from: repopath = pagure.utils.get_repo_path(repo_from) - _log.debug(" working on the repo in: %s and", repopath, parentpath) + _log.debug( + " working on the repo in: %s and %s", repopath, parentpath + ) repo_obj = pygit2.Repository(repopath) orig_repo = pygit2.Repository(parentpath) From 8a76fa5dd4bd365c5d98a136ea85839ca7ab7454 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 08:58:33 +0000 Subject: [PATCH 8/8] Refactor redundant code in the tests Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/__init__.py b/tests/__init__.py index 208a4f4..0908eb4 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -696,8 +696,13 @@ def create_tokens_acl(session, token_id='aaabbbcccddd', acl_name=None): session.commit() -def add_content_git_repo(folder, branch='master', append=None): - """ Create some content for the specified git repo. """ +def _clone_and_top_commits(folder, branch, branch_ref=False): + """ Clone the repository, checkout the specified branch and return + the top commit of that branch if there is one. + Returns the repo, the path to the clone and the top commit(s) in a tuple + or the repo, the path to the clone and the reference to the branch + object if branch_ref is True. + """ if not os.path.exists(folder): os.makedirs(folder) brepo = pygit2.init_repository(folder, bare=True) @@ -710,6 +715,9 @@ def add_content_git_repo(folder, branch='master', append=None): branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) repo.checkout(branch_ref_obj) + if branch_ref: + return (repo, newfolder, branch_ref_obj) + parents = [] commit = None try: @@ -722,6 +730,13 @@ def add_content_git_repo(folder, branch='master', append=None): if commit: parents = [commit.oid.hex] + return (repo, newfolder, parents) + + +def add_content_git_repo(folder, branch='master', append=None): + """ Create some content for the specified git repo. """ + repo, newfolder, parents = _clone_and_top_commits(folder, branch) + # Create a file in that git repo filename = os.path.join(newfolder, 'sources') content = 'foo\n bar' @@ -796,17 +811,7 @@ def add_content_git_repo(folder, branch='master', append=None): def add_readme_git_repo(folder, readme_name='README.rst', branch='master'): """ Create a README file for the specified git repo. """ - if not os.path.exists(folder): - os.makedirs(folder) - brepo = pygit2.init_repository(folder, bare=True) - - newfolder = tempfile.mkdtemp(prefix='pagure-tests') - repo = pygit2.clone_repository(folder, newfolder) - - branch_ref_obj = None - if "origin/%s" % branch in repo.listall_branches(pygit2.GIT_BRANCH_ALL): - branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) - repo.checkout(branch_ref_obj) + repo, newfolder, parents = _clone_and_top_commits(folder, branch) if readme_name == 'README.rst': content = """Pagure @@ -834,18 +839,6 @@ This is a placeholder """ + readme_name + """ that should never get displayed on the website if there is a README.rst in the repo. """ - parents = [] - commit = None - try: - if branch_ref_obj: - commit = repo[branch_ref_obj.get_object().hex] - else: - commit = repo.revparse_single('HEAD') - except (KeyError, AttributeError): - pass - if commit: - parents = [commit.oid.hex] - # Create a file in that git repo with open(os.path.join(newfolder, readme_name), 'w') as stream: stream.write(content) @@ -881,17 +874,8 @@ that should never get displayed on the website if there is a README.rst in the r def add_commit_git_repo(folder, ncommits=10, filename='sources', branch='master'): """ Create some more commits for the specified git repo. """ - if not os.path.exists(folder): - os.makedirs(folder) - pygit2.init_repository(folder, bare=True) - - newfolder = tempfile.mkdtemp(prefix='pagure-tests') - repo = pygit2.clone_repository(folder, newfolder) - - branch_ref_obj = None - if "origin/%s" % branch in repo.listall_branches(pygit2.GIT_BRANCH_ALL): - branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) - repo.checkout(branch_ref_obj) + repo, newfolder, branch_ref_obj = _clone_and_top_commits( + folder, branch, branch_ref=True) for index in range(ncommits): # Create a file in that git repo @@ -940,12 +924,7 @@ def add_commit_git_repo(folder, ncommits=10, filename='sources', def add_content_to_git(folder, filename='sources', content='foo'): """ Create some more commits for the specified git repo. """ - if not os.path.exists(folder): - os.makedirs(folder) - brepo = pygit2.init_repository(folder, bare=True) - - newfolder = tempfile.mkdtemp(prefix='pagure-tests') - repo = pygit2.clone_repository(folder, newfolder) + repo, newfolder, parents = _clone_and_top_commits(folder, 'master') # Create a file in that git repo with open(os.path.join(newfolder, filename), 'a', encoding="utf-8") as stream: @@ -953,15 +932,6 @@ def add_content_to_git(folder, filename='sources', content='foo'): repo.index.add(filename) repo.index.write() - parents = [] - commit = None - try: - commit = repo.revparse_single('HEAD') - except KeyError: - pass - if commit: - parents = [commit.oid.hex] - # Commits the files added tree = repo.index.write_tree() author = pygit2.Signature( @@ -991,12 +961,7 @@ def add_content_to_git(folder, filename='sources', content='foo'): def add_binary_git_repo(folder, filename): """ Create a fake image file for the specified git repo. """ - if not os.path.exists(folder): - os.makedirs(folder) - brepo = pygit2.init_repository(folder, bare=True) - - newfolder = tempfile.mkdtemp(prefix='pagure-tests') - repo = pygit2.clone_repository(folder, newfolder) + repo, newfolder, parents = _clone_and_top_commits(folder, 'master') content = b"""\x00\x00\x01\x00\x01\x00\x18\x18\x00\x00\x01\x00 \x00\x88 \t\x00\x00\x16\x00\x00\x00(\x00\x00\x00\x18\x00x00\x00\x01\x00 \x00\x00\x00 @@ -1005,15 +970,6 @@ def add_binary_git_repo(folder, filename): \xa4fF\x04\xa2dE\x95\xa2cD8\xa1a """ - parents = [] - commit = None - try: - commit = repo.revparse_single('HEAD') - except KeyError: - pass - if commit: - parents = [commit.oid.hex] - # Create a file in that git repo with open(os.path.join(newfolder, filename), 'wb') as stream: stream.write(content) @@ -1049,25 +1005,7 @@ def add_binary_git_repo(folder, filename): def remove_file_git_repo(folder, filename, branch='master'): """ Delete the specified file on the give git repo and branch. """ - newfolder = tempfile.mkdtemp(prefix='pagure-tests') - repo = pygit2.clone_repository(folder, newfolder) - - branch_ref_obj = None - if "origin/%s" % branch in repo.listall_branches(pygit2.GIT_BRANCH_ALL): - branch_ref_obj = pagure.lib.git.get_branch_ref(repo, branch) - repo.checkout(branch_ref_obj) - - parents = [] - commit = None - try: - if branch_ref_obj: - commit = repo[branch_ref_obj.get_object().hex] - else: - commit = repo.revparse_single('HEAD') - except (KeyError, AttributeError): - pass - if commit: - parents = [commit.oid.hex] + repo, newfolder, parents = _clone_and_top_commits(folder, branch) # Remove file repo.index.remove(filename)