From 6db02a3089e8b23f9cd1b59742328d7f88f129cb Mon Sep 17 00:00:00 2001 From: Lubomír Sedlář Date: Jun 09 2017 11:17:14 +0000 Subject: [PATCH 1/3] Dim output of subprocess If colorama package is available, dim the output of invoked commands. --- diff --git a/pag/utils.py b/pag/utils.py index 7b379ac..5ee4978 100644 --- a/pag/utils.py +++ b/pag/utils.py @@ -7,6 +7,13 @@ import click import requests import yaml +try: + from colorama import Style + DIM = Style.DIM + RESET = Style.RESET_ALL +except ImportError: + DIM = RESET = '' + CONF_FILE = os.path.expanduser('~/.config/pag') @@ -17,7 +24,7 @@ def run(cmd, echo=True, graceful=True): output, _ = proc.communicate() output = output.decode('utf-8') if echo: - click.echo(output) + click.echo(DIM + output + RESET) if not graceful and proc.returncode != 0: sys.exit(1) return proc.returncode, output From 61c5fe8ec752c6d70637d45bfee4973a14d04fe4 Mon Sep 17 00:00:00 2001 From: Lubomír Sedlář Date: Jun 13 2017 13:55:04 +0000 Subject: [PATCH 2/3] Add review command It allows listing opened pull requests on a project, checking them out locally and opening the web page from CLI. --- diff --git a/pag/app.py b/pag/app.py index 529bc60..4e5f9b1 100644 --- a/pag/app.py +++ b/pag/app.py @@ -38,6 +38,7 @@ from .commands import fork from .commands import remote from .commands import pullrequest from .commands import gitaliasing +from .commands import review if __name__ == '__main__': app() diff --git a/pag/commands/review.py b/pag/commands/review.py new file mode 100644 index 0000000..8ff6e15 --- /dev/null +++ b/pag/commands/review.py @@ -0,0 +1,153 @@ +import click +import requests +import textwrap + +from pag.app import app +from pag.utils import ( + assert_local_repo, + die, + eager_command, + in_git_repo, + repo_url, + run +) + +try: + # If colorama is available, let's use it to print prettier output. If not, + # fall back to boring monochrome. + from colorama import Fore, Style + GREEN = Fore.GREEN + DIM = Style.DIM + RESET = Style.RESET_ALL +except ImportError: + GREEN = DIM = RESET = '' + + +def list_pull_requests(name): + """Get a list of opened pull requests for a project. + + For an exact description of the return value see Pagure API documentation. + + :param str name: name of the project + :returns: a list of dicts describing opened pull requests + """ + url = 'https://pagure.io/api/0/{name}/pull-requests'.format(name=name) + response = requests.get(url) + return response.json()['requests'] + + +def get_repo(pr_info): + """Find url for repo that the response belongs to. + + :param dict pr_info: Information about a pull request as returned by Pagure + API + :returns: git repo url + :rtype: str + """ + name = pr_info['repo_from']['fullname'].replace('forks/', '', 1) + return repo_url(name, git=True) + + +def get_local_branch(pr): + """Get a branch name for the new review. + + Finds latest branch for this particular pull request and bumps the + revision. The branch name is in the format of ``review/PR_ID/REV`` where + ``REV`` is an automatically incremented number starting at 1. + + :param int pr: identifier of a pull request + :returns: name of a new branch + :rtype: str + """ + _, out = run(['git', 'branch', '--list', 'review/{}/*'.format(pr)], + echo=False, graceful=False) + existing = [int(x.rsplit('/', 1)[-1]) + for x in out.strip().split('\n') + if x] + num = max(existing + [0]) + 1 + return 'review/{}/{}'.format(pr, num) + + +@eager_command +def list_pulls(ctx): + """Print information about opened pull requests. + + This function never returns due to the decorator exiting the whole program. + + :param ctx: Click context. Passed automatically by the decorator + """ + repo = in_git_repo() + pulls = list_pull_requests(repo) + if pulls: + longest_name = max((pr['user']['name'] for pr in pulls), key=len) + for pr in pulls: + prefix = '{green}{id: >5}{rst} {dim}{user[name]:{width}}{rst}'.format( + width=len(longest_name), green=GREEN, dim=DIM, rst=RESET, **pr) + width = 79 - len(prefix) + len(GREEN) + len(DIM) + 2 * len(RESET) + for line in textwrap.wrap(pr['title'], width=width): + click.echo('{} {}'.format(prefix, line)) + prefix = ' ' * (79 - width) + + +@eager_command +def open_current(ctx): + """Open pull request page in web browser. + + If not called when review branch is checked out, an error will be reported. + + This function never returns due to the decorator exiting the whole program. + + :param ctx: Click context. Passed automatically by the decorator + """ + _, branch = run(['git', 'rev-parse', '--abbrev-ref', 'HEAD'], echo=False) + branch = branch.strip() + if not branch.startswith('review/'): + click.echo('Not on a review branch, aborting...', err=True) + ctx.exit(1) + + url = '{}/pull-request/{}'.format(repo_url(in_git_repo()), + branch.split('/')[1]) + + run(['xdg-open', url]) + + +@app.command() +@click.argument('pr', metavar='PR_ID') +@click.option('-l', '--list', is_flag=True, callback=list_pulls, + expose_value=False, is_eager=True, + help='List opened pull requests on this repo') +@click.option('-o', '--open', is_flag=True, callback=open_current, + expose_value=False, is_eager=True, + help='Open currently reviewed PR in browser') +@assert_local_repo +def review(pr): + """Check out a pull request locally.""" + name = in_git_repo() + url = 'https://pagure.io/api/0/{name}/pull-request/{id}'.format( + name=name, id=pr) + response = requests.get(url).json() + try: + branch = response['branch_from'] + repo = response.get('remote_git') or get_repo(response) + except KeyError: + die('Bad pull request id') + + # Find review branches that include the last commit in the pull request. + ret, branches = run(['git', 'branch', + '--contains', response['commit_stop'], + 'review/{}/*'.format(pr)], echo=False) + + if ret == 0 and branches: + # There is a branch with that commit, find the latest one and check it + # out. There really should be only one. + latest_branch = branches.strip().split('\n')[-1].split(' ')[-1] + click.echo('Pull request {} already checked out as {}'.format( + pr, latest_branch)) + run(['git', 'checkout', latest_branch], graceful=False) + else: + # Download the commits from the branch. + run(['git', 'fetch', repo, branch], graceful=False) + # Find a suitable name for local branch and create it. + local_branch = get_local_branch(pr) + run(['git', 'checkout', '-b', local_branch, 'FETCH_HEAD'], + graceful=False) diff --git a/pag/utils.py b/pag/utils.py index 5ee4978..6caa2cd 100644 --- a/pag/utils.py +++ b/pag/utils.py @@ -51,6 +51,20 @@ def assert_local_repo(func): return inner +def eager_command(func): + """Decorator for an option callback that should abort man command. + + Useful when an option completely changes the execution flow. + """ + @functools.wraps(func) + def inner(ctx, param, value): + if not value or ctx.resilient_parsing: + return + func(ctx) + ctx.exit() + return inner + + def get_default_upstream_branch(name): url = 'https://pagure.io/api/0/projects' response = requests.get(url, params=dict(pattern=name, fork=False)) From 705cc28a01b9a116d4270b8eafad8328e428227e Mon Sep 17 00:00:00 2001 From: Lubomír Sedlář Date: Jun 13 2017 14:15:27 +0000 Subject: [PATCH 3/3] Add basic tests for review subcommand --- diff --git a/tests/test_review.py b/tests/test_review.py new file mode 100644 index 0000000..fca81b8 --- /dev/null +++ b/tests/test_review.py @@ -0,0 +1,154 @@ +import unittest +import mock + +from click.testing import CliRunner + +from pag.commands.review import review + + +# This is an abbreviated response from list pull request API that contains +# minimal amount of data needed. +LIST_RESPONSE = { + "requests": [ + { + "id": 2344, + "title": "Fix the API docs", + "user": {"name": "mprahl"} + }, + { + "id": 2343, + "title": ("Add ways to customize the gitolite configuration file " + "with snippets, and some extra details"), + "user": {"name": "pingou"} + } + ], + "total_requests": 2 +} + +# This is expected output when listing pull requests returns the data above. +LIST_OUTPUT = """ + 2344 mprahl Fix the API docs + 2343 pingou Add ways to customize the gitolite configuration file with + snippets, and some extra details +""".lstrip('\n') + +# Abbreviated response for listing details about a pull request. +GET_RESPONSE = { + "branch_from": "fix-api-docs", + "commit_stop": "a08f507b99afeda8b9d1f5cf2024eb723726924b", + "id": 2344, + "remote_git": None, + "repo_from": { + "fullname": "forks/mprahl/pagure", + }, +} + + +class ReviewTest(unittest.TestCase): + def setUp(self): + self.runner = CliRunner() + self.maxDiff = None + + @mock.patch('pag.commands.review.run') + @mock.patch('pag.commands.review.in_git_repo', new=lambda: 'my-project') + def test_open_web(self, run): + run.side_effect = [ + (0, 'review/12/3\n'), + (0, ''), + ] + + with self.runner.isolated_filesystem(): + result = self.runner.invoke(review, ['--open']) + + self.assertEqual(result.exit_code, 0) + self.assertEqual( + run.call_args_list, + [mock.call( + ['git', 'rev-parse', '--abbrev-ref', 'HEAD'], echo=False), + mock.call( + ['xdg-open', 'https://pagure.io/my-project/pull-request/12'])] + ) + + @mock.patch('pag.commands.review.run') + def test_open_web_on_bad_branch(self, run): + run.return_value = (0, 'master\n') + + with self.runner.isolated_filesystem(): + result = self.runner.invoke(review, ['--open']) + + self.assertEqual(result.exit_code, 1) + self.assertIn('Not on a review branch', result.output) + + @mock.patch('pag.commands.review.list_pull_requests') + @mock.patch('pag.commands.review.in_git_repo', new=lambda: 'pagure') + def test_list(self, list_pull_requests): + list_pull_requests.return_value = LIST_RESPONSE['requests'] + + result = self.runner.invoke(review, ['--list']) + + self.assertEqual( + list_pull_requests.call_args_list, + [mock.call('pagure')]) + self.assertEqual(result.exit_code, 0) + self.assertEqual(result.output, LIST_OUTPUT) + + @mock.patch('requests.get') + @mock.patch('pag.commands.review.run') + @mock.patch('pag.commands.review.in_git_repo', new=lambda: 'pagure') + def test_checkout_new(self, run, get): + get.return_value = mock.Mock( + json=mock.Mock(return_value=GET_RESPONSE) + ) + run.side_effect = [ + (1, ''), + (0, ''), + (0, 'review/2344/1\n'), + (0, ''), + ] + + result = self.runner.invoke(review, ['2344']) + + self.assertEqual( + get.call_args_list, + [mock.call('https://pagure.io/api/0/pagure/pull-request/2344')]) + self.assertEqual( + run.call_args_list, + [mock.call(['git', 'branch', '--contains', + 'a08f507b99afeda8b9d1f5cf2024eb723726924b', + 'review/2344/*'], echo=False), + mock.call( + ['git', 'fetch', 'https://pagure.io/forks/mprahl/pagure.git', + 'fix-api-docs'], graceful=False), + mock.call(['git', 'branch', '--list', 'review/2344/*'], + echo=False, graceful=False), + mock.call(['git', 'checkout', '-b', 'review/2344/2', + 'FETCH_HEAD'], graceful=False), + ]) + self.assertEqual(result.exit_code, 0) + + @mock.patch('requests.get') + @mock.patch('pag.commands.review.run') + @mock.patch('pag.commands.review.in_git_repo', new=lambda: 'pagure') + def test_checkout_existing(self, run, get): + get.return_value = mock.Mock( + json=mock.Mock(return_value=GET_RESPONSE) + ) + run.side_effect = [ + (0, 'review/2344/1\n'), + (0, ''), + ] + + result = self.runner.invoke(review, ['2344']) + + self.assertEqual( + get.call_args_list, + [mock.call('https://pagure.io/api/0/pagure/pull-request/2344')]) + self.assertEqual( + run.call_args_list, + [ + mock.call(['git', 'branch', '--contains', + 'a08f507b99afeda8b9d1f5cf2024eb723726924b', + 'review/2344/*'], echo=False), + mock.call(['git', 'checkout', 'review/2344/1'], graceful=False), + ]) + self.assertEqual(result.exit_code, 0)