Restructure subcommands parser creation

Restructure creation of subcommands to pass a full subparser object to
the command class to allow more complex parser creation to facilitate
adding argument groups and mutually exclusive groups.

Change-Id: Ic6f7e3834cf8bd29b87d5d6cafdb8a9f041f4cf6

git_upstream/commands/__init__.py

@@ -16,9 +16,9 @@
 # limitations under the License.
 #
 
+import abc
 import argparse
 import os
-import sys
 
 
 class AppendReplaceAction(argparse._AppendAction):
@@ -38,10 +38,37 @@ class AppendReplaceAction(argparse._AppendAction):
                                                   option_string)
 
 
-def get_subcommands(subparsers):
+class GitUpstreamCommand(object):
+    """Base command class
+
+    To create commands simply subclass and implement the necessary abstract
+    methods.
+    """
+
+    __metaclass__ = abc.ABCMeta
+
+    def __init__(self, parser):
+        self.parser = parser
+
+    def validate(self, args):
+        """Verify the arguments passed for this command"""
+        return
+
+    @abc.abstractmethod
+    def run(self, args):
+        """Execute this command"""
+        return
+
+
+def get_subcommands(parser):
+
+    subparsers = parser.add_subparsers(title="commands", metavar='<command>',
+                                       dest='subcommand')
 
     subcommands = _find_actions(subparsers, os.path.dirname(__file__))
 
+    parser.set_defaults(subcommands=subcommands)
+
     return subcommands
 
 
@@ -51,23 +78,18 @@ def _find_actions(subparsers, module_path):
     for mod in (p[:-len('.py')] for p in os.listdir(module_path) if
                 p.endswith('.py')):
         __import__(__name__ + '.' + mod)
-        module = sys.modules[__name__ + '.' + mod]
-        for attr in (a for a in dir(module) if a.startswith('do_')):
-            command = attr[3:].replace('_', '-')
-            func = getattr(module, attr)
-            desc = func.__doc__ or ''
-            help = desc.strip().split('\n')[0]
-            args = getattr(func, 'arguments', [])
 
-            subparser = subparsers.add_parser(
+    for cmd_class in GitUpstreamCommand.__subclasses__():
-                command,
+        command = cmd_class.name
-                help=help,
+        desc = cmd_class.__doc__ or None
-                description=desc)
+        help = desc.strip().split('\n')[0]
-            subparser.register('action', 'append_replace', AppendReplaceAction)
 
-            for (args, kwargs) in args:
+        subparser = subparsers.add_parser(
-                subparser.add_argument(*args, **kwargs)
+            command,
-            subparser.set_defaults(func=func)
+            help=help,
-            subcommands[command] = subparser
+            description=desc)
+        subparser.register('action', 'append_replace', AppendReplaceAction)
+        subparser.set_defaults(cmd=cmd_class(subparser))
+        subcommands[command] = subparser
 
     return subcommands

git_upstream/commands/drop.py

@@ -15,16 +15,14 @@
 # limitations under the License.
 #
 
-import inspect
 import re
 
 from git import BadObject
 
+from git_upstream.commands import GitUpstreamCommand
 from git_upstream.errors import GitUpstreamError
 from git_upstream.lib.utils import GitMixin
-from git_upstream import log
 from git_upstream.log import LogDedentMixin
-from git_upstream import subcommand
 
 try:
     from git import BadName
@@ -50,6 +48,7 @@ class Drop(LogDedentMixin, GitMixin):
     Dropped: Walter White <heisenberg@hp.com>
 
     """
+
     DROP_HEADER = 'Dropped:'
     NOTE_REF = 'refs/notes/upstream-merge'
 
@@ -124,25 +123,31 @@ class Drop(LogDedentMixin, GitMixin):
                 self.commit)
 
 
-@subcommand.arg('commit', metavar='<commit>', nargs=None,
+class DropCommand(LogDedentMixin, GitUpstreamCommand):
-                help='Commit to be marked as dropped')
+    """Mark a commit as dropped.
-@subcommand.arg('-a', '--author', metavar='<author>',
+
-                dest='author',
-                default=None,
-                help='Git author for the mark')
-def do_drop(args):
-    """
-    Mark a commit as dropped.
     Marked commits will be skipped during the upstream rebasing process.
     See also the "git upstream import" command.
     """
+    name = "drop"
 
-    logger = log.get_logger('%s.%s' % (__name__,
+    def __init__(self, *args, **kwargs):
-                                       inspect.stack()[0][0].f_code.co_name))
+        # make sure to correctly initialize inherited objects before performing
+        # any computation
+        super(DropCommand, self).__init__(*args, **kwargs)
 
-    drop = Drop(git_object=args.commit, author=args.author)
+        self.parser.add_argument(
+            'commit', metavar='<commit>', nargs=None,
+            help='Commit to be marked as dropped')
+        self.parser.add_argument(
+            '-a', '--author', metavar='<author>', dest='author', default=None,
+            help='Git author for the mark')
 
-    if drop.mark():
+    def run(self, args):
-        logger.notice("Drop mark created successfully")
+
+        drop = Drop(git_object=args.commit, author=args.author)
+
+        if drop.mark():
+            self.log.notice("Drop mark created successfully")
 
 # vim:sw=4:sts=4:ts=4:et:

git_upstream/commands/help.py

@@ -0,0 +1,40 @@
+#
+# Copyright (c) 2012-2015 Hewlett-Packard Development 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.
+#
+
+from git_upstream.commands import GitUpstreamCommand
+from git_upstream.log import LogDedentMixin
+
+
+class HelpCommand(LogDedentMixin, GitUpstreamCommand):
+    """Display help about this program or one of its commands."""
+    name = "help"
+
+    def __init__(self, *args, **kwargs):
+        super(HelpCommand, self).__init__(*args, **kwargs)
+
+        self.parser.add_argument('command', metavar='<command>', nargs='?',
+                                 help="command to display help about")
+
+    def run(self, args, parent_parser=None):
+        if getattr(args, 'command', None):
+            if args.command in args.subcommands:
+                args.subcommands[args.command].print_help()
+            else:
+                self.parser.error("'%s' is not a valid subcommand" %
+                                  args.command)
+        else:
+            parent_parser.print_help()

git_upstream/commands/import.py

@@ -18,10 +18,10 @@
 from abc import ABCMeta
 from abc import abstractmethod
 from collections import Sequence
-import inspect
 
 from git import GitCommandError
 
+from git_upstream.commands import GitUpstreamCommand
 from git_upstream.errors import GitUpstreamError
 from git_upstream.lib.rebaseeditor import RebaseEditor
 from git_upstream.lib.searchers import DiscardDuplicateGerritChangeId
@@ -31,9 +31,7 @@ from git_upstream.lib.searchers import ReverseCommitFilter
 from git_upstream.lib.searchers import SupersededCommitFilter
 from git_upstream.lib.searchers import UpstreamMergeBaseSearcher
 from git_upstream.lib.utils import GitMixin
-from git_upstream import log
 from git_upstream.log import LogDedentMixin
-from git_upstream import subcommand
 
 
 class ImportUpstreamError(GitUpstreamError):
@@ -519,45 +517,7 @@ class LocateChangesWalk(LocateChangesStrategy):
         return super(LocateChangesWalk, self).filtered_iter()
 
 
-@subcommand.arg('-d', '--dry-run', dest='dry_run', action='store_true',
+class ImportCommand(LogDedentMixin, GitUpstreamCommand):
-                default=False,
-                help='Only print out the list of commits that would be '
-                     'applied.')
-@subcommand.arg('-i', '--interactive', action='store_true', default=False,
-                help='Let the user edit the list of commits before applying.')
-@subcommand.arg('-f', '--force', dest='force', required=False,
-                action='store_true', default=False,
-                help='Force overwrite of existing import branch if it exists.')
-@subcommand.arg('--merge', dest='merge', required=False, action='store_true',
-                default=True,
-                help='Merge the resulting import branch into the target branch'
-                     ' once complete')
-@subcommand.arg('--no-merge', dest='merge', required=False,
-                action='store_false',
-                help="Disable merge of the resulting import branch")
-@subcommand.arg('-s', '--strategy', metavar='<strategy>',
-                choices=ImportStrategiesFactory.list_strategies(),
-                default=LocateChangesWalk.get_strategy_name(),
-                help='Use the given strategy to re-apply locally carried '
-                     'changes to the import branch. (default: %(default)s)')
-@subcommand.arg('--search-refs', action='append_replace', metavar='<pattern>',
-                default=['upstream/*'], dest='search_refs',
-                help='Refs to search for previous import commit. May be '
-                     'specified multiple times.')
-@subcommand.arg('--into', dest='branch', metavar='<branch>', default='HEAD',
-                help='Branch to take changes from, and replace with imported '
-                     'branch.')
-@subcommand.arg('--import-branch', metavar='<import-branch>',
-                help='Name of import branch to use',
-                default='import/{describe}')
-@subcommand.arg('upstream_branch', metavar='<upstream-branch>', nargs='?',
-                default='upstream/master',
-                help='Upstream branch to import. Must be specified if '
-                     'you wish to provide additional branches.')
-@subcommand.arg('branches', metavar='<branches>', nargs='*',
-                help='Branches to additionally merge into the import branch '
-                     'using default git merging behaviour')
-def do_import(args):
     """Import code from specified upstream branch.
 
     Creates an import branch from the specified upstream branch, and optionally
@@ -569,81 +529,136 @@ def do_import(args):
     Once complete it will merge and replace the contents of the target branch
     with those from the import branch, unless --no-merge is specified.
     """
+    name = "import"
 
-    logger = log.get_logger('%s.%s' % (__name__,
+    def __init__(self, *args, **kwargs):
-                                       inspect.stack()[0][0].f_code.co_name))
+        super(ImportCommand, self).__init__(*args, **kwargs)
 
-    import_upstream = ImportUpstream(branch=args.branch,
+        self.parser.add_argument(
-                                     upstream=args.upstream_branch,
+            '-i', '--interactive', action='store_true', default=False,
-                                     import_branch=args.import_branch,
+            help='Let the user edit the list of commits before applying.')
-                                     extra_branches=args.branches)
+        self.parser.add_argument(
+            '-d', '--dry-run', dest='dry_run', action='store_true',
+            default=False,
+            help='Only print out the list of commits that would be applied.')
+        self.parser.add_argument(
+            '-f', '--force', dest='force', required=False,
+            action='store_true', default=False,
+            help='Force overwrite of existing import branch if it exists.')
+        self.parser.add_argument(
+            '--merge', dest='merge', required=False, action='store_true',
+            default=True,
+            help='Merge the resulting import branch into the target branch '
+                 'once complete')
+        self.parser.add_argument(
+            '--no-merge', dest='merge', required=False, action='store_false',
+            help='Disable merge of the resulting import branch')
+        self.parser.add_argument(
+            '--search-refs', action='append_replace', metavar='<pattern>',
+            default=['upstream/*'], dest='search_refs',
+            help='Refs to search for previous import commit. May be '
+                 'specified multiple times.')
+        self.parser.add_argument(
+            '-s', '--strategy', metavar='<strategy>',
+            choices=ImportStrategiesFactory.list_strategies(),
+            default=LocateChangesWalk.get_strategy_name(),
+            help='Use the given strategy to re-apply locally carried '
+                 'changes to the import branch. (default: %(default)s)')
+        self.parser.add_argument(
+            '--into', dest='branch', metavar='<branch>', default='HEAD',
+            help='Branch to take changes from, and replace with imported '
+                 'branch.')
+        self.parser.add_argument(
+            '--import-branch', metavar='<import-branch>',
+            default='import/{describe}', help='Name of import branch to use')
+        self.parser.add_argument(
+            'upstream_branch', metavar='<upstream-branch>', nargs='?',
+            default='upstream/master',
+            help='Upstream branch to import. Must be specified if you wish to '
+                 'provide additional branches.')
+        self.parser.add_argument(
+            'branches', metavar='<branches>', nargs='*',
+            help='Branches to additionally merge into the import branch using '
+                 'default git merging behaviour')
 
-    logger.notice("Searching for previous import")
+    def run(self, args):
-    strategy = ImportStrategiesFactory.create_strategy(
-        args.strategy, branch=args.branch, upstream=args.upstream_branch,
-        search_refs=args.search_refs)
 
-    if len(strategy) == 0:
+        import_upstream = ImportUpstream(
-        raise ImportUpstreamError("Cannot find previous import")
+            branch=args.branch,
+            upstream=args.upstream_branch,
+            import_branch=args.import_branch,
+            extra_branches=args.branches)
 
-    # if last commit in the strategy was a merge, then the additional branches
+        self.log.notice("Searching for previous import")
-    # that were merged in previously can be extracted based on the commits
+        strategy = ImportStrategiesFactory.create_strategy(
-    # merged.
+            args.strategy, branch=args.branch, upstream=args.upstream_branch,
-    prev_import_merge = strategy[-1]
+            search_refs=args.search_refs)
-    if len(prev_import_merge.parents) > 1:
-        idxs = [idx for idx, commit in enumerate(prev_import_merge.parents)
-                if commit.hexsha != strategy.searcher.commit.hexsha]
 
-        if idxs:
+        if len(strategy) == 0:
-            additional_commits = [prev_import_merge.parents[i] for i in idxs]
+            raise ImportUpstreamError("Cannot find previous import")
-            if additional_commits and len(args.branches) == 0:
-                logger.warning("""\
-                    **************** WARNING ****************
-                    Previous import merged additional branches but none have
-                    been specified on the command line for this import.\n""")
 
-    if args.dry_run:
+        # if last commit in the strategy was a merge, then the additional
-        commit_list = [c.hexsha[:6] + " - " + c.summary[:60] +
+        # branches that were merged in previously can be extracted based on
-                       (c.summary[60:] and "...")
+        # the commits merged.
-                       for c in list(strategy.filtered_iter())]
+        prev_import_merge = strategy[-1]
-        logger.notice("""\
+        if len(prev_import_merge.parents) > 1:
-            Requested a dry-run: printing the list of commit that should be
+            idxs = [idx for idx, commit in enumerate(prev_import_merge.parents)
-            rebased
+                    if commit.hexsha != strategy.searcher.commit.hexsha]
 
-                %s
+            if idxs:
-            """, "\n    ".join(commit_list))
+                additional_commits = [prev_import_merge.parents[i]
-        return True
+                                      for i in idxs]
+                if additional_commits and len(args.branches) == 0:
+                    self.log.warning("""\
+                        **************** WARNING ****************
+                        Previous import merged additional branches but none
+                        have been specified on the command line for this
+                        import.\n""")
 
-    logger.notice("Starting import of upstream")
+        if args.dry_run:
-    import_upstream.create_import(force=args.force)
+            commit_list = [c.hexsha[:6] + " - " + c.summary[:60] +
-    logger.notice("Successfully created import branch")
+                           (c.summary[60:] and "...")
+                           for c in list(strategy.filtered_iter())]
+            self.log.notice("""\
+                Requested a dry-run: printing the list of commit that should be
+                rebased
 
-    if not import_upstream.apply(strategy, args.interactive):
+                    %s
-        logger.notice("Import cancelled")
+                """, "\n    ".join(commit_list))
-        return False
+            return True
 
-    if not args.merge:
+        self.log.notice("Starting import of upstream")
-        logger.notice(
+        import_upstream.create_import(force=args.force)
-            """\
+        self.log.notice("Successfully created import branch")
-            Import complete, not merging to target branch '%s' as requested.
-            """, args.branch)
-        return True
 
-    logger.notice("Merging import to requested branch '%s'", args.branch)
+        if not import_upstream.apply(strategy, args.interactive):
-    if import_upstream.finish():
+            self.log.notice("Import cancelled")
-        logger.notice(
+            return False
-            """\
+
-            Successfully finished import:
+        if not args.merge:
-                target branch: '%s'
+            self.log.notice(
-                upstream branch: '%s'
+                """\
-                import branch: '%s'""", args.branch, args.upstream_branch,
+                Import complete, not merging to target branch '%s' as
-            import_upstream.import_branch)
+                requested.
-        if args.branches:
+                """, args.branch)
-            for branch in args.branches:
+            return True
-                logger.notice("    extra branch: '%s'", branch, dedent=False)
+
-        return True
+        self.log.notice("Merging import to requested branch '%s'", args.branch)
-    else:
+        if import_upstream.finish():
-        return False
+            self.log.notice(
+                """\
+                Successfully finished import:
+                    target branch: '%s'
+                    upstream branch: '%s'
+                    import branch: '%s'""", args.branch, args.upstream_branch,
+                import_upstream.import_branch)
+            if args.branches:
+                for branch in args.branches:
+                    self.log.notice("    extra branch: '%s'", branch,
+                                    dedent=False)
+            return True
+        else:
+            return False
 
 
 # vim:sw=4:sts=4:ts=4:et:

git_upstream/commands/supersede.py

@@ -15,19 +15,17 @@
 # limitations under the License.
 #
 
-import inspect
 import re
 
 from git import BadObject
 from git import Head
 
+from git_upstream.commands import GitUpstreamCommand
 from git_upstream.errors import GitUpstreamError
 from git_upstream.lib import note  # noqa
 from git_upstream.lib.searchers import CommitMessageSearcher
 from git_upstream.lib.utils import GitMixin
-from git_upstream import log
 from git_upstream.log import LogDedentMixin
-from git_upstream import subcommand
 
 try:
     from git import BadName
@@ -169,40 +167,51 @@ class Supersede(LogDedentMixin, GitMixin):
             self.log.warning('Note has not been added')
 
 
-@subcommand.arg('commit', metavar='<commit>', nargs=None,
+class SupersedeCommand(LogDedentMixin, GitUpstreamCommand):
-                help='Commit to be marked as superseded')
+    """Mark a commit as superseded by a set of change-ids.
-@subcommand.arg('change_ids', metavar='<change id>', nargs='+',
+
-                help='Change id which makes <commit> obsolete. The change id '
-                     'must be present in <upstream-branch> to drop <commit>. '
-                     'If more than one change id is specified, all must be '
-                     'present in <upstream-branch> to drop <commit>')
-@subcommand.arg('-f', '--force', dest='force', required=False,
-                action='store_true', default=False,
-                help='Apply the commit mark even if one or more change ids '
-                     'could not be found. Use this flag carefully as commits '
-                     'will not be dropped during import command execution as '
-                     'long as all associated change ids are present in the '
-                     'local copy of the upstream branch')
-@subcommand.arg('-u', '--upstream-branch', metavar='<upstream-branch>',
-                dest='upstream_branch', required=False,
-                default='upstream/master',
-                help='Search change ids values in <upstream-branch> branch '
-                     '(default: %(default)s)')
-def do_supersede(args):
-    """
-    Mark a commit as superseded by a set of change-ids.
     Marked commits will be skipped during the upstream rebasing process.
     See also the "git upstream import" command.
     """
+    name = "supersede"
 
-    logger = log.get_logger('%s.%s' % (__name__,
+    def __init__(self, *args, **kwargs):
-                                       inspect.stack()[0][0].f_code.co_name))
 
-    supersede = Supersede(git_object=args.commit, change_ids=args.change_ids,
+        # make sure to correctly initialize inherited objects before performing
-                          upstream_branch=args.upstream_branch,
+        # any computation
-                          force=args.force)
+        super(SupersedeCommand, self).__init__(*args, **kwargs)
 
-    if supersede.mark():
+        self.parser.add_argument(
-        logger.notice("Supersede mark created successfully")
+            'commit', metavar='<commit>', nargs=None,
+            help='Commit to be marked as superseded')
+        self.parser.add_argument(
+            'change_ids', metavar='<change id>', nargs='+',
+            help='Change id which makes <commit> obsolete. The change id must '
+                 'be present in <upstream-branch> to drop <commit>. If more '
+                 'than one change id is specified, all must be present in '
+                 '<upstream-branch> to drop <commit>')
+        self.parser.add_argument(
+            '-f', '--force', dest='force', required=False, action='store_true',
+            default=False,
+            help='Apply the commit mark even if one or more change ids could '
+                 'not be found. Use this flag carefully as commits will not '
+                 'be dropped during import command execution as long as all '
+                 'associated change ids are present in the local copy of the '
+                 'upstream branch')
+        self.parser.add_argument(
+            '-u', '--upstream-branch', metavar='<upstream-branch>',
+            dest='upstream_branch', required=False, default='upstream/master',
+            help='Search change ids values in <upstream-branch> branch '
+                 '(default: %(default)s)')
+
+    def run(self, args):
+
+        supersede = Supersede(git_object=args.commit,
+                              change_ids=args.change_ids,
+                              upstream_branch=args.upstream_branch,
+                              force=args.force)
+
+        if supersede.mark():
+            self.logger.notice("Supersede mark created successfully")
 
 # vim:sw=4:sts=4:ts=4:et:

git_upstream/main.py

@@ -30,7 +30,6 @@ from git_upstream import __version__
 from git_upstream import commands
 from git_upstream.errors import GitUpstreamError
 from git_upstream import log
-from git_upstream import subcommand
 
 try:
     import argcomplete
@@ -39,7 +38,7 @@ except ImportError:
     argparse_loaded = False
 
 
-def get_parser():
+def build_parsers():
     parser = argparse.ArgumentParser(
         description=__doc__.strip(),
         epilog='See "%(prog)s help COMMAND" for help on a specific command.',
@@ -62,39 +61,11 @@ def get_parser():
                         help=argparse.SUPPRESS)
     parser.add_argument('--log-file', dest='log_file', help=argparse.SUPPRESS)
 
-    subparsers = parser.add_subparsers(title="commands", metavar='<command>',
+    subcommand_parsers = commands.get_subcommands(parser)
-                                       dest='subcommand')
-
-    # it would be nicer if we could hide this help command
-    desc = help.__doc__ or ''
-    subparser = subparsers.add_parser(
-        'help',
-        help=desc.strip().split('\n')[0],
-        description=desc,
-    )
-    for (args, kwargs) in getattr(help, 'arguments', []):
-        subparser.add_argument(*args, **kwargs)
-    subparser.set_defaults(func=help)
-
-    subcommand_parsers = commands.get_subcommands(subparsers)
 
     return subcommand_parsers, parser
 
 
-@subcommand.arg('command', metavar='<command>', nargs='?',
-                help='Display help for <command>')
-def help(parser, args, commands=None):
-    """Display help about this program or one of its commands."""
-    if getattr(args, 'command', None):
-        if args.command in commands:
-            commands[args.command].print_help()
-        else:
-            parser.error("'%s' is not a valid subcommand" %
-                         args.command)
-    else:
-        parser.print_help()
-
-
 def setup_console_logging(options):
 
     options.log_level = getattr(logging, options.log_level.upper(),
@@ -147,17 +118,18 @@ def main(argv=None):
     if not argv:
         argv = sys.argv[1:]
 
-    (cmds, parser) = get_parser()
+    (cmds, parser) = build_parsers()
 
-    if not sys.argv:
+    if not argv:
-        help(parser, argv)
+        parser.print_help()
         return 0
 
     if argparse_loaded:
         argcomplete.autocomplete(parser)
     args = parser.parse_args(argv)
-    if args.func == help:
+
-        help(parser, args, cmds)
+    if args.cmd.name == "help":
+        args.cmd.run(args, parser)
         return 0
 
     logger = setup_console_logging(args)
@@ -167,7 +139,9 @@ def main(argv=None):
         sys.exit(1)
 
     try:
-        args.func(args)
+        cmd = args.cmd
+        cmd.validate(args)
+        cmd.run(args)
     except GitUpstreamError as e:
         logger.fatal("%s", e[0])
         logger.debug("Git-Upstream: %s", e[0], exc_info=e)

git_upstream/subcommand.py

@@ -1,26 +0,0 @@
-#
-# Copyright (c) 2012 OpenStack LLC.
-#
-# 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.
-
-
-# following function taken from python-keystoneclient - keystoneclient/utils.py
-# Decorator for cli-args
-def arg(*args, **kwargs):
-    def _decorator(func):
-        # Because of the sematics of decorator composition if we just append
-        # to the options list positional options will appear to be backwards.
-        func.__dict__.setdefault('arguments', []).insert(0, (args, kwargs))
-        return func
-    return _decorator

git_upstream/tests/test_commands.py

@@ -24,13 +24,12 @@ from git_upstream import commands as c
 class TestGetSubcommands(testtools.TestCase):
     """Test case for get_subcommands function"""
 
-    _available_subcommands = ('import', 'supersede', 'drop')
+    _available_subcommands = ('help', 'import', 'supersede', 'drop')
 
     def test_available_subcommands(self):
         """Test available subcommands"""
         parser = ArgumentParser()
-        subparsers = parser.add_subparsers()
+        subcommands = c.get_subcommands(parser)
-        subcommands = c.get_subcommands(subparsers)
         self.assertEqual(len(TestGetSubcommands._available_subcommands),
                          len(subcommands.keys()))
         for command in subcommands.keys():

git_upstream/tests/test_import_cmd.py

@@ -44,7 +44,7 @@ class SubstringMatcher(object):
 
 class TestImportCommand(BaseTestCase):
 
-    commands, parser = main.get_parser()
+    commands, parser = main.build_parsers()
 
     def test_basic(self):
         """Test that default behaviour and options work
@@ -74,7 +74,7 @@ class TestImportCommand(BaseTestCase):
         self._build_git_tree(tree, branches.values())
         self.git.tag(inspect.currentframe().f_code.co_name, 'upstream/master')
         args = self.parser.parse_args(['-q', 'import', 'upstream/master'])
-        self.assertThat(args.func(args), Equals(True),
+        self.assertThat(args.cmd.run(args), Equals(True),
                         "import command failed to complete succesfully")
 
     def test_basic_additional(self):
@@ -111,7 +111,7 @@ class TestImportCommand(BaseTestCase):
         self.git.tag(inspect.currentframe().f_code.co_name, 'upstream/master')
         args = self.parser.parse_args(['-q', 'import', 'upstream/master',
                                        'packaging/master'])
-        self.assertThat(args.func(args), Equals(True),
+        self.assertThat(args.cmd.run(args), Equals(True),
                         "import command failed to complete succesfully")
 
     def test_basic_additional_missed(self):
@@ -149,15 +149,13 @@ class TestImportCommand(BaseTestCase):
         self.git.tag(inspect.currentframe().f_code.co_name, 'upstream/master')
         args = self.parser.parse_args(['import', 'upstream/master'])
 
-        mock_logger = mock.MagicMock()
+        with mock.patch.object(args.cmd.log, 'warning') as mock_logger:
-        with mock.patch('git_upstream.log.get_logger',
+            self.assertThat(args.cmd.run(args), Equals(True),
-                        return_value=mock_logger):
-            self.assertThat(args.func(args), Equals(True),
                             "import command failed to complete succesfully")
 
-        mock_logger.warning.assert_called_with(
+            mock_logger.assert_called_with(
-            SubstringMatcher(
+                SubstringMatcher(
-                containing="Previous import merged additional"))
+                    containing="Previous import merged additional"))
 
     def test_import_switch_branches_search(self):
         """Test that the import sub-command can correctly switch branches when
@@ -206,7 +204,7 @@ class TestImportCommand(BaseTestCase):
         self._build_git_tree(tree, branches.values())
         self.git.tag(inspect.currentframe().f_code.co_name, 'upstream/master')
         args = self.parser.parse_args(['-q', 'import'])
-        self.assertThat(args.func(args), Equals(True),
+        self.assertThat(args.cmd.run(args), Equals(True),
                         "import command failed to complete succesfully")
         changes = list(Commit.iter_items(
             self.repo, 'upstream/master..master^2'))
@@ -274,7 +272,7 @@ class TestImportCommand(BaseTestCase):
         self.git.tag(inspect.currentframe().f_code.co_name, 'custom/master')
         args = self.parser.parse_args(['-q', 'import',
                                        '--into=master', 'custom/master'])
-        self.assertThat(args.func(args), Equals(True),
+        self.assertThat(args.cmd.run(args), Equals(True),
                         "import command failed to complete succesfully")
         changes = list(Commit.iter_items(
             self.repo, 'custom/master..master^2'))
@@ -343,7 +341,7 @@ class TestImportCommand(BaseTestCase):
                                        '--search-refs=custom/*',
                                        '--search-refs=custom-d/*',
                                        '--into=master', 'custom/master'])
-        self.assertThat(args.func(args), Equals(True),
+        self.assertThat(args.cmd.run(args), Equals(True),
                         "import command failed to complete succesfully")
         changes = list(Commit.iter_items(
             self.repo, 'custom/master..master^2'))