Refactor command dispatch to improve unit testing and support better error messages.

Signed-off-by: Daniel Nephin <[email protected]>

compose/cli/docopt_command.py

@@ -1,7 +1,6 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
-import sys
 from inspect import getdoc
 
 from docopt import docopt
@@ -15,24 +14,21 @@ def docopt_full_help(docstring, *args, **kwargs):
         raise SystemExit(docstring)
 
 
-class DocoptCommand(object):
-    def docopt_options(self):
-        return {'options_first': True}
+class DocoptDispatcher(object):
 
-    def sys_dispatch(self):
-        self.dispatch(sys.argv[1:])
-
-    def dispatch(self, argv):
-        self.perform_command(*self.parse(argv))
+    def __init__(self, command_class, options):
+        self.command_class = command_class
+        self.options = options
 
     def parse(self, argv):
-        options = docopt_full_help(getdoc(self), argv, **self.docopt_options())
+        command_help = getdoc(self.command_class)
+        options = docopt_full_help(command_help, argv, **self.options)
         command = options['COMMAND']
 
         if command is None:
-            raise SystemExit(getdoc(self))
+            raise SystemExit(command_help)
 
-        handler = self.get_handler(command)
+        handler = get_handler(self.command_class, command)
         docstring = getdoc(handler)
 
         if docstring is None:
@@ -41,17 +37,18 @@ class DocoptCommand(object):
         command_options = docopt_full_help(docstring, options['ARGS'], options_first=True)
         return options, handler, command_options
 
-    def get_handler(self, command):
-        command = command.replace('-', '_')
-        # we certainly want to have "exec" command, since that's what docker client has
-        # but in python exec is a keyword
-        if command == "exec":
-            command = "exec_command"
 
-        if not hasattr(self, command):
-            raise NoSuchCommand(command, self)
+def get_handler(command_class, command):
+    command = command.replace('-', '_')
+    # we certainly want to have "exec" command, since that's what docker client has
+    # but in python exec is a keyword
+    if command == "exec":
+        command = "exec_command"
+
+    if not hasattr(command_class, command):
+        raise NoSuchCommand(command, command_class)
 
-        return getattr(self, command)
+    return getattr(command_class, command)
 
 
 class NoSuchCommand(Exception):

compose/cli/main.py

@@ -3,6 +3,7 @@ from __future__ import print_function
 from __future__ import unicode_literals
 
 import contextlib
+import functools
 import json
 import logging
 import re
@@ -33,7 +34,8 @@ from ..service import NeedsBuildError
 from .command import friendly_error_message
 from .command import get_config_path_from_options
 from .command import project_from_options
-from .docopt_command import DocoptCommand
+from .docopt_command import DocoptDispatcher
+from .docopt_command import get_handler
 from .docopt_command import NoSuchCommand
 from .errors import UserError
 from .formatter import ConsoleWarningFormatter
@@ -52,19 +54,16 @@ console_handler = logging.StreamHandler(sys.stderr)
 
 def main():
     setup_logging()
+    command = dispatch()
+
     try:
-        command = TopLevelCommand()
-        command.sys_dispatch()
+        command()
     except (KeyboardInterrupt, signals.ShutdownException):
         log.error("Aborting.")
         sys.exit(1)
     except (UserError, NoSuchService, ConfigurationError) as e:
         log.error(e.msg)
         sys.exit(1)
-    except NoSuchCommand as e:
-        commands = "\n".join(parse_doc_section("commands:", getdoc(e.supercommand)))
-        log.error("No such command: %s\n\n%s", e.command, commands)
-        sys.exit(1)
     except APIError as e:
         log_api_error(e)
         sys.exit(1)
@@ -88,6 +87,40 @@ def main():
         sys.exit(1)
 
 
+def dispatch():
+    dispatcher = DocoptDispatcher(
+        TopLevelCommand,
+        {'options_first': True, 'version': get_version_info('compose')})
+
+    try:
+        options, handler, command_options = dispatcher.parse(sys.argv[1:])
+    except NoSuchCommand as e:
+        commands = "\n".join(parse_doc_section("commands:", getdoc(e.supercommand)))
+        log.error("No such command: %s\n\n%s", e.command, commands)
+        sys.exit(1)
+
+    setup_console_handler(console_handler, options.get('--verbose'))
+    return functools.partial(perform_command, options, handler, command_options)
+
+
+def perform_command(options, handler, command_options):
+    if options['COMMAND'] in ('help', 'version'):
+        # Skip looking up the compose file.
+        handler(command_options)
+        return
+
+    if options['COMMAND'] == 'config':
+        command = TopLevelCommand(None)
+        handler(command, options, command_options)
+        return
+
+    project = project_from_options('.', options)
+    command = TopLevelCommand(project)
+    with friendly_error_message():
+        # TODO: use self.project
+        handler(command, project, command_options)
+
+
 def log_api_error(e):
     if 'client is newer than server' in e.explanation:
         # we need JSON formatted errors. In the meantime...
@@ -134,7 +167,7 @@ def parse_doc_section(name, source):
     return [s.strip() for s in pattern.findall(source)]
 
 
-class TopLevelCommand(DocoptCommand):
+class TopLevelCommand(object):
     """Define and run multi-container applications with Docker.
 
     Usage:
@@ -173,26 +206,8 @@ class TopLevelCommand(DocoptCommand):
     """
     base_dir = '.'
 
-    def docopt_options(self):
-        options = super(TopLevelCommand, self).docopt_options()
-        options['version'] = get_version_info('compose')
-        return options
-
-    def perform_command(self, options, handler, command_options):
-        setup_console_handler(console_handler, options.get('--verbose'))
-
-        if options['COMMAND'] in ('help', 'version'):
-            # Skip looking up the compose file.
-            handler(None, command_options)
-            return
-
-        if options['COMMAND'] == 'config':
-            handler(options, command_options)
-            return
-
-        project = project_from_options(self.base_dir, options)
-        with friendly_error_message():
-            handler(project, command_options)
+    def __init__(self, project):
+        self.project = project
 
     def build(self, project, options):
         """
@@ -352,13 +367,14 @@ class TopLevelCommand(DocoptCommand):
         exit_code = project.client.exec_inspect(exec_id).get("ExitCode")
         sys.exit(exit_code)
 
-    def help(self, project, options):
+    @classmethod
+    def help(cls, options):
         """
         Get help on a command.
 
         Usage: help COMMAND
         """
-        handler = self.get_handler(options['COMMAND'])
+        handler = get_handler(cls, options['COMMAND'])
         raise SystemExit(getdoc(handler))
 
     def kill(self, project, options):
@@ -739,7 +755,8 @@ class TopLevelCommand(DocoptCommand):
                 print("Aborting on container exit...")
                 project.stop(service_names=service_names, timeout=timeout)
 
-    def version(self, project, options):
+    @classmethod
+    def version(cls, options):
         """
         Show version informations
 

tests/unit/cli_test.py

@@ -64,26 +64,20 @@ class CLITestCase(unittest.TestCase):
         self.assertTrue(project.client)
         self.assertTrue(project.services)
 
-    def test_help(self):
-        command = TopLevelCommand()
-        with self.assertRaises(SystemExit):
-            command.dispatch(['-h'])
-
     def test_command_help(self):
-        with self.assertRaises(SystemExit) as ctx:
-            TopLevelCommand().dispatch(['help', 'up'])
+        with pytest.raises(SystemExit) as exc:
+            TopLevelCommand.help({'COMMAND': 'up'})
 
-        self.assertIn('Usage: up', str(ctx.exception))
+        assert 'Usage: up' in exc.exconly()
 
     def test_command_help_nonexistent(self):
-        with self.assertRaises(NoSuchCommand):
-            TopLevelCommand().dispatch(['help', 'nonexistent'])
+        with pytest.raises(NoSuchCommand):
+            TopLevelCommand.help({'COMMAND': 'nonexistent'})
 
     @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason="requires dockerpty")
     @mock.patch('compose.cli.main.RunOperation', autospec=True)
     @mock.patch('compose.cli.main.PseudoTerminal', autospec=True)
     def test_run_interactive_passes_logs_false(self, mock_pseudo_terminal, mock_run_operation):
-        command = TopLevelCommand()
         mock_client = mock.create_autospec(docker.Client)
         project = Project.from_config(
             name='composetest',
@@ -92,6 +86,7 @@ class CLITestCase(unittest.TestCase):
                 'service': {'image': 'busybox'}
             }),
         )
+        command = TopLevelCommand(project)
 
         with pytest.raises(SystemExit):
             command.run(project, {
@@ -126,7 +121,7 @@ class CLITestCase(unittest.TestCase):
             }),
         )
 
-        command = TopLevelCommand()
+        command = TopLevelCommand(project)
         command.run(project, {
             'SERVICE': 'service',
             'COMMAND': None,
@@ -147,7 +142,7 @@ class CLITestCase(unittest.TestCase):
             'always'
         )
 
-        command = TopLevelCommand()
+        command = TopLevelCommand(project)
         command.run(project, {
             'SERVICE': 'service',
             'COMMAND': None,
@@ -168,7 +163,6 @@ class CLITestCase(unittest.TestCase):
         )
 
     def test_command_manula_and_service_ports_together(self):
-        command = TopLevelCommand()
         project = Project.from_config(
             name='composetest',
             client=None,
@@ -176,6 +170,7 @@ class CLITestCase(unittest.TestCase):
                 'service': {'image': 'busybox'},
             }),
         )
+        command = TopLevelCommand(project)
 
         with self.assertRaises(UserError):
             command.run(project, {