launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1058282-ditch-api into lp:maas.

Commit message:
Ditch the "api" module identifier on maascli command lines.  It's the only module we have right now, and in a future where we have others, it's still the sensible default.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1058282 in MAAS: "maas-cli is awkward to use"
  https://bugs.launchpad.net/maas/+bug/1058282

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1058282-ditch-api/+merge/127959

Pre-imped with Gavin.  This shortens all maascli command lines by one word: not enough to make it properly user-friendly, but it's one step in the right direction.

You'll note that there was no integration test for this change to break.  So I added one.  Not quite detailed enough that it would have noticed the change, but enough that it would have noticed if I broke the command entirely.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1058282-ditch-api/+merge/127959
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-1058282-ditch-api into lp:maas.

=== modified file 'src/maascli/__init__.py'
--- src/maascli/__init__.py	2012-09-18 16:45:00 +0000
+++ src/maascli/__init__.py	2012-10-04 08:39:29 +0000
@@ -31,11 +31,6 @@
     )
 
 
-modules = {
-    "api": "maascli.api",
-    }
-
-
 class ArgumentParser(argparse.ArgumentParser):
     """Specialisation of argparse's parser with better support for subparsers.
 
@@ -64,19 +59,12 @@
     if argv is None:
         argv = sys.argv[:1] + osutils.get_unicode_argv()
 
-    # Create the base argument parser.
+    module = __import__('maascli.api', fromlist=True)
+    help_title, help_body = parse_docstring(module)
     parser = ArgumentParser(
-        description="Control MAAS from the command-line.",
-        prog=argv[0], epilog="http://maas.ubuntu.com/";)
-
-    # Register declared modules.
-    for name, module in sorted(modules.items()):
-        if isinstance(module, basestring):
-            module = __import__(module, fromlist=True)
-        help_title, help_body = parse_docstring(module)
-        module_parser = parser.subparsers.add_parser(
-            name, help=help_title, description=help_body)
-        register(module, module_parser)
+        description=help_body, prog=argv[0],
+        epilog="http://maas.ubuntu.com/";)
+    register(module, parser)
 
     # Run, doing polite things with exceptions.
     try:

=== added file 'src/maascli/tests/test_integration.py'
--- src/maascli/tests/test_integration.py	1970-01-01 00:00:00 +0000
+++ src/maascli/tests/test_integration.py	2012-10-04 08:39:29 +0000
@@ -0,0 +1,53 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Integration-test the `maascli` command."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+import os.path
+from subprocess import (
+    CalledProcessError,
+    check_call,
+    )
+
+from maastesting.testcase import TestCase
+
+
+def locate_dev_root():
+    """Root of development tree that this test is in."""
+    return os.path.join(
+        os.path.dirname(__file__), os.pardir, os.pardir, os.pardir)
+
+
+def locate_maascli():
+    return os.path.join(locate_dev_root(), 'bin', 'maascli')
+
+
+class TestMAASCli(TestCase):
+
+    def run_command(self, *args):
+        with open('/dev/null', 'ab') as dev_null:
+            check_call(
+                [locate_maascli()] + list(args),
+                stdout=dev_null, stderr=dev_null)
+
+    def test_run_without_args_fails(self):
+        self.assertRaises(CalledProcessError, self.run_command)
+
+    def test_help_option_succeeds(self):
+        self.run_command('-h')
+        # The test is that we get here without error.
+        pass
+
+    def test_list_command_succeeds(self):
+        self.run_command('list')
+        # The test is that we get here without error.
+        pass