PR#3857: Add a new API endpoint to retrieve the list of files changed in a PR - pagure

		`@@ -84,7 +84,7 @@`
		`)`
		`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 @@`
		`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,13 +111,14 @@`
		`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 "`
		`"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):`

pagure/api/fork.py

file modified

+190

		`@@ -1251,3 +1251,193 @@`

		`jsonout = flask.jsonify(request.to_json(public=True, api=True))`
		`return jsonout`
		`+`
		`+`
		`+ @API.route("/<repo>/pull-request/<int:requestid>/diffstats")`
		`+ @API.route("/<namespace>/<repo>/pull-request/<int:requestid>/diffstats")`
		`+ @API.route("/fork/<username>/<repo>/pull-request/<int:requestid>/diffstats")`
		`+ @API.route(`
		`+ "/fork/<username>/<namespace>/<repo>/pull-request/<int:requestid>/"`
		`+ "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/<repo>/pull-request/<request id>/diffstats`
		`+ GET /api/0/<namespace>/<repo>/pull-request/<request id>/diffstats`
		`+`
		`+ ::`
		`+`
		`+ GET /api/0/fork/<username>/<repo>/pull-request/<request id>/diffstats`
		`+ GET /api/0/fork/<username>/<namespace>/<repo>/pull-request/<request id>/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`

pagure/lib/git.py

file modified

+9 -6

		`@@ -1770,7 +1770,9 @@`
		`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 @@`
		`)`
		`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 @@`
		`)`

		`_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)`

pagure/lib/tasks.py

file modified

+4 -3

		`@@ -282,8 +282,7 @@`
		`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:`
		`@@ -908,7 +907,9 @@`
		`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)`

tests/__init__.py

file modified

+93 -79

		`@@ -59,6 +59,7 @@`
		`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,8 +696,13 @@`
		`session.commit()`


		`- def add_content_git_repo(folder, branch='master'):`
		`- """ 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)`
		`@@ -704,29 +710,52 @@`
		`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)`
		`+`
		`+ if branch_ref:`
		`+ return (repo, newfolder, 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]`

		`+ 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'`
		`+ 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 +766,8 @@`
		`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 +787,7 @@`
		`'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,14 +809,9 @@`
		`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)`
		`- 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, branch)`

		`if readme_name == 'README.rst':`
		`content = """Pagure`
		`@@ -822,15 +839,6 @@`
		`that should never get displayed on the website if there is a README.rst in the repo.`
		`"""`

		`- 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, readme_name), 'w') as stream:`
		`stream.write(content)`
		`@@ -843,8 +851,9 @@`
		`'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 +865,8 @@`

		`# 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)`

		`@@ -867,12 +874,8 @@`
		`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)`
		`+ 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`
		`@@ -884,8 +887,11 @@`
		`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 +902,9 @@`
		`'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,22 +913,18 @@`
		`# 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)`


		`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:`
		`@@ -929,15 +932,6 @@`
		`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(`
		`@@ -967,12 +961,7 @@`

		`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`
		`@@ -981,15 +970,6 @@`
		`\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)`
		`@@ -1023,6 +1003,40 @@`
		`shutil.rmtree(newfolder)`


		`+ def remove_file_git_repo(folder, filename, branch='master'):`
		`+ """ Delete the specified file on the give git repo and branch. """`
		`+ repo, newfolder, parents = _clone_and_top_commits(folder, branch)`
		`+`
		`+ # 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`

tests/test_pagure_flask_api.py

file modified

+5 -5

		`@@ -237,7 +237,7 @@`
		`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 @@`
		`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'`
		`]`
		`)`

tests/test_pagure_flask_api_fork.py

file modified

+170

		`@@ -2703,6 +2703,176 @@`
		`}`
		`)`

		`+ 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)`

tests/test_pagure_flask_internal.py

file modified

		`@@ -2792,6 +2792,7 @@`
		`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`

tests/test_pagure_flask_ui_repo.py

file modified

+9 -6

		`@@ -2557,8 +2557,8 @@`
		`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 @@`
		`'data-line-number="1"></a></td>', output_text)`
		`self.assertIn(`
		`'<td class="cell2"><pre><code> bar</code></pre></td>', 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 @@`
		`self.assertIn(`
		`'<td class="cell2"><pre><code> bar</code></pre></td>', 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')`

pingou commented 5 years ago

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 pingou@pingoured.fr

ngompa commented 5 years ago

Can you rebase this and add tests?

ryanlerch commented 5 years ago

@pingou this is duplicating a lot of logic that we currently have in the templates _repo_renderdiff.html

Would it be a good idea to abstract some of this out so we can use it in both places? I'd love to not have the pygit version checking craziness in that template :)

pingou commented 5 years ago

@ryanlerch that's exactly where I took the logic from :-p

big +1 to try to re-use this somehow. Should we plan for this in 5.2?

pingou commented 5 years ago

Actually, I look up the logic in repo_pull_request.html does that mean we have it in three places?

pingou commented 5 years ago

Can you rebase this and add tests?

That was the idea, but I pinged @fbo on IRC to ask him if he has enough information with this or if he needs more

rebased onto 50caff497ec58a9954ff498c4fb351b72bd5f659

5 years ago

ngompa commented 5 years ago

There are several test failures:

14:10:18 FAILED test: py-test_pagure_flask_ui_old_commit
14:10:18 FAILED test: py-test_style
14:10:18 FAILED test: py-test_pagure_flask_ui_repo
14:10:18 FAILED test: py-test_pagure_flask_internal
14:10:18 FAILED test: py-test_pagure_lib_git
14:10:18 FAILED test: py-test_pagure_lib
14:10:18 FAILED test: py-test_pagure_flask_api

rebased onto 0a6c36891d2f71805bc0796b73abfdea9244bf7b

5 years ago

rebased onto efe89b6daa237f5671b1d69670eced170a68c3e0

5 years ago

rebased onto 89e9387cbc9165964c18af973bc3eca6e5e14579

5 years ago

7 new commits added

Debug things
Flake8 fixes
Adjust logged messages to fit the actual module file/name
Fix quotes in strings (left over from a time we didn't run black)
Add a new API endpoint to retrieve the list of files changed in a PR
Add an utility method to remove a file from a given git repo in the tests
Ensure two utility methods in the tests respect the branch they are given

5 years ago

8 new commits added

Fix the tests
Debug things
Flake8 fixes
Adjust logged messages to fit the actual module file/name
Fix quotes in strings (left over from a time we didn't run black)
Add a new API endpoint to retrieve the list of files changed in a PR
Add an utility method to remove a file from a given git repo in the tests
Ensure two utility methods in the tests respect the branch they are given

5 years ago

9 new commits added

fixup tests api
Fixup a logging call
Debug things
Flake8 fixes
Adjust logged messages to fit the actual module file/name
Fix quotes in strings (left over from a time we didn't run black)
Add a new API endpoint to retrieve the list of files changed in a PR
Add an utility method to remove a file from a given git repo in the tests
Ensure two utility methods in the tests respect the branch they are given

5 years ago

fbo commented 5 years ago

I was traveling today. So yes looking at the response sample that seems perfect. With that new feature Zuul will be able via the API to detect if a file named .zuul.yaml or zuul.d/*.yaml have changed :).

9 new commits added

fixup tests api
Fixup a logging call
Debug things
Flake8 fixes
Adjust logged messages to fit the actual module file/name
Fix quotes in strings (left over from a time we didn't run black)
Add a new API endpoint to retrieve the list of files changed in a PR
Add an utility method to remove a file from a given git repo in the tests
Ensure two utility methods in the tests respect the branch they are given

5 years ago

rebased onto 1dbff6fa229edfd99546a154c358c0062c8931c3

5 years ago

8 new commits added

Fixup a logging call
Debug things
Flake8 fixes
Adjust logged messages to fit the actual module file/name
Fix quotes in strings (left over from a time we didn't run black)
Add a new API endpoint to retrieve the list of files changed in a PR
Add an utility method to remove a file from a given git repo in the tests
Ensure two utility methods in the tests respect the branch they are given

5 years ago

ngompa commented 5 years ago

@pingou Can you clean up the commits for merge? Aside from that, the code looks good to me.

7 new commits added

Fixup a logging call
Flake8 fixes
Adjust logged messages to fit the actual module file/name
Fix quotes in strings (left over from a time we didn't run black)
Add a new API endpoint to retrieve the list of files changed in a PR
Add an utility method to remove a file from a given git repo in the tests
Ensure two utility methods in the tests respect the branch they are given

5 years ago

7 new commits added

Fixup a logging call
Flake8 fixes
Adjust logged messages to fit the actual module file/name
Fix quotes in strings (left over from a time we didn't run black)
Add a new API endpoint to retrieve the list of files changed in a PR
Add an utility method to remove a file from a given git repo in the tests
Ensure two utility methods in the tests respect the branch they are given

5 years ago

ngompa commented 5 years ago

A single failed test:

23:32:13 Failed tests:
23:32:13 FAILED test: py-test_pagure_flask_api_fork

ryanlerch commented 5 years ago

Actually, I look up the logic in repo_pull_request.html does that mean we have it in three places?

Yes :/

Once for the changes summary in the comments tab, and once for the actual diff view as well. This is the way it always has been :/

1 new commit added

debug

5 years ago

1 new commit added

fixup test

5 years ago

7 new commits added

Fixup a logging call
Flake8 fixes
Adjust logged messages to fit the actual module file/name
Fix quotes in strings (left over from a time we didn't run black)
Add a new API endpoint to retrieve the list of files changed in a PR
Add an utility method to remove a file from a given git repo in the tests
Ensure two utility methods in the tests respect the branch they are given

5 years ago

rebased onto 6fdafd0

5 years ago

cverna commented on line 27 of tests/__init__.py 5 years ago

The following block of code is repeated 3 times, maybe it could be refactored to as a function to avoid code duplication.

cverna commented 5 years ago

I agree with @ryanlerch, I think it would be nice to create functions in the lib/git.py maybe that takes care of the low level logic so these function can then just be reused in the templates and the API.

:thumbsup: with a ticket to refactor this so we don't forget

pingou commented 5 years ago

I agree with @ryanlerch, I think it would be nice to create functions in the lib/git.py maybe that takes care of the low level logic so these function can then just be reused in the templates and the API.
👍 with a ticket to refactor this so we don't forget

Thanks for the review, I opened the ticket at : https://pagure.io/pagure/issue/3868

I'm also looking at the refactoring of the redundant code in the tests, got something done and tests are running locally, if they pass I'll push and if jenkins is happy I'll merge :)

To give you an idea, these are the stats:

 tests/__init__.py | 108 ++++++++++++++++++++------------------------------------------------------------------------
 1 file changed, 23 insertions(+), 85 deletions(-)

1 new commit added