From 7dd634a3710f9666a2cf966a3c5ce11c99ee64c8 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Mon, 17 Oct 2016 14:16:27 +0100 Subject: [PATCH] Fix help subcommand Ensure help subcommand behaves as expected when given options and a command as an argument to lookup. Refactor to provide the parent parser as a default argument (using weakref to avoid a reference cycle) so that the execute method can be made consistent across all subcommands while still providing access to the parent parser for the help subcommand. Co-Authored-By: Paul Bourke Change-Id: I9f7e10bbc2268d111dca7c3dbe55a87e3f057d87 --- git_upstream/commands/__init__.py | 2 +- git_upstream/commands/help.py | 12 +-- git_upstream/main.py | 7 +- git_upstream/tests/commands/help/__init__.py | 0 .../tests/commands/help/test_execute.py | 75 +++++++++++++++++++ 5 files changed, 87 insertions(+), 9 deletions(-) create mode 100644 git_upstream/tests/commands/help/__init__.py create mode 100644 git_upstream/tests/commands/help/test_execute.py diff --git a/git_upstream/commands/__init__.py b/git_upstream/commands/__init__.py index 0d26fe0..434cca3 100644 --- a/git_upstream/commands/__init__.py +++ b/git_upstream/commands/__init__.py @@ -60,7 +60,7 @@ class GitUpstreamCommand(object): """Additional updating of the args to set values""" @abc.abstractmethod - def execute(self, args): + def execute(self): """Execute this command""" raise NotImplementedError diff --git a/git_upstream/commands/help.py b/git_upstream/commands/help.py index 6c559ea..669ea7d 100644 --- a/git_upstream/commands/help.py +++ b/git_upstream/commands/help.py @@ -29,12 +29,12 @@ class HelpCommand(LogDedentMixin, GitUpstreamCommand): self.parser.add_argument('command', metavar='', nargs='?', help="command to display help about") - def execute(self, args, parent_parser=None): - if getattr(args, 'command', None): - if args.command in args.subcommands: - args.subcommands[args.command].print_help() + def execute(self): + if getattr(self.args, 'command', None): + if self.args.command in self.args.subcommands: + self.args.subcommands[self.args.command].print_help() else: self.parser.error("'%s' is not a valid subcommand" % - args.command) + self.args.command) else: - parent_parser.print_help() + self.args.parent_parser.print_help() diff --git a/git_upstream/main.py b/git_upstream/main.py index a7ff341..15dc487 100644 --- a/git_upstream/main.py +++ b/git_upstream/main.py @@ -24,6 +24,7 @@ import argparse import logging import os import sys +import weakref import git @@ -71,7 +72,8 @@ def build_parsers(): else: script_cmdline = [program] - parser.set_defaults(script_cmdline=script_cmdline) + parser.set_defaults(script_cmdline=script_cmdline, + parent_parser=weakref.proxy(parser)) return subcommand_parsers, parser @@ -140,8 +142,9 @@ def main(argv=None): logger = setup_console_logging(args) + # allow help subcommand to be called before checking git version if args.cmd.name == "help": - args.cmd.run(args, parser) + args.cmd.run(args) return 0 if git.Git().version_info < (1, 7, 5): diff --git a/git_upstream/tests/commands/help/__init__.py b/git_upstream/tests/commands/help/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/git_upstream/tests/commands/help/test_execute.py b/git_upstream/tests/commands/help/test_execute.py new file mode 100644 index 0000000..79c73ba --- /dev/null +++ b/git_upstream/tests/commands/help/test_execute.py @@ -0,0 +1,75 @@ +# Copyright (c) 2016 Hewlett Packard Enterprise Company, L.P. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for the 'help' command""" + +import os + +import mock +import testscenarios +import testtools +from testtools import matchers + +from git_upstream import main + + +class TestHelpCommandExecute(testscenarios.TestWithScenarios, + testtools.TestCase): + + scenarios = [ + ('s1', dict(args=['help'], + failure="Cannot display basic help")), + ('s2', dict(args=['help', 'help'], + failure="Cannot display help of help")), + # argparse calls SystemExit with 0 when handling '--help' opts + # instead of returning to the previous program + ('s3', dict(args=['help', '--help'], + exception=SystemExit, exc_attr='code', exc_value=0, + failure="Cannot display help of help")), + ('s4', dict(args=['help', 'import'], + failure="Cannot display help of import")), + # test exit with error code of 2, indicating user input incorrect + ('s5', dict(args=['help', 'invalid'], + exception=SystemExit, exc_attr='code', exc_value=2, + failure="Fail to detect invalid subcommand")) + ] + + def setUp(self): + super(TestHelpCommandExecute, self).setUp() + self.commands, self.parser = main.build_parsers() + + script_cmdline = self.parser.get_default('script_cmdline') + script_cmdline[-1] = os.path.join(os.getcwd(), main.__file__) + self.parser.set_defaults(script_cmdline=script_cmdline) + + # must mock out setup_console_logging for any testing of main as it + # will interfere with fixtures capturing logging in other test threads + conlog_patcher = mock.patch('git_upstream.main.setup_console_logging') + conlog_patcher.start() + self.addCleanup(conlog_patcher.stop) + + def test_command(self): + """Test that help command runs successfully""" + + with mock.patch.multiple('sys', stdout=mock.DEFAULT, + stderr=mock.DEFAULT): + if getattr(self, 'exception', None): + e = self.assertRaises(self.exception, main.main, self.args) + self.assertThat(getattr(e, self.exc_attr), + matchers.Equals(self.exc_value), + message=self.failure) + else: + self.assertThat(main.main(self.args), matchers.Equals(0), + message=self.failure)