Merge lp:~allenap/maas/argparse-the-annoying--bug-1557434 into lp:~maas-committers/maas/trunk

[Gavin Panella]

For the MAAS CLI, mimic the error behaviour provided by argparse 1.1 on PyPI.

This differs substantially from the argparse 1.1 found in the standard library. This branch also forces the development environment to use argparse as found in the standard library instead of the one pulled in by unittest2.

[Mike Pontillo (mpontillo)]

Just tested this locally. The behavior is *much* better. Many thanks for this.

review: Approve

[Gavin Panella (allenap)]

Thanks!

=== modified file 'buildout.cfg'
--- buildout.cfg	2016-07-07 18:52:38 +0000
+++ buildout.cfg	2016-07-13 11:35:29 +0000
@@ -45,15 +45,22 @@
   testscenarios
   testtools
 initialization =
-  ${common:path-sort}
+  ${common:path-munge}
   ${common:warnings}
   ${common:environment}
-path-sort =
-  from pathlib import Path
-  from sys import path, prefix
-  prefix = Path(prefix)
-  sortkey = lambda path: prefix in Path(path).parents
-  path.sort(key=sortkey)
+path-munge =
+  import pathlib, sys
+  # Eliminate argparse usage outside of the standard library. This is
+  # needed because some deps <cough>unittest2<cough> explicitly require
+  # argparse, which zc.buildout then dutifully installs. Unfortunately
+  # argparse 1.1 from PyPI differs substantially to argparse 1.1 in the
+  # standard library. For consistency we want the latter.
+  p_argparse_egg = lambda path: pathlib.Path(path).match("*/argparse-*.egg")
+  sys.path[:] = [path for path in sys.path if not p_argparse_egg(path)]
+  # Sort system paths towards the end of sys.path so that deps defined
+  # here are used in preference to those installed system-wide.
+  p_sys_prefix = lambda path, p=pathlib.Path: p(sys.prefix) in p(path).parents
+  sys.path.sort(key=p_sys_prefix)
 environment =
   from os import environ
   environ.setdefault("MAAS_ROOT", "${buildout:directory}/run")
@@ -80,7 +87,7 @@
 extra-paths =
   ${common:extra-paths}
 initialization =
-  ${common:path-sort}
+  ${common:path-munge}
 interpreter =
 entry-points = database=postgresfixture.main:main
 scripts = database
@@ -144,7 +151,7 @@
 eggs =
   ${region:eggs}
 initialization =
-  ${common:path-sort}
+  ${common:path-munge}
 entry-points =
   maas=maascli:main
 extra-paths =
@@ -160,7 +167,7 @@
 entry-points =
   test.cli=maastesting.noseplug:main
 initialization =
-  ${common:path-sort}
+  ${common:path-munge}
   ${common:warnings}
   options = [
     "--with-resources",
@@ -196,7 +203,7 @@
 entry-points =
   test.testing=maastesting.noseplug:main
 initialization =
-  ${common:path-sort}
+  ${common:path-munge}
   ${common:warnings}
   options = [
     "--with-resources",
@@ -283,7 +290,7 @@
 scripts =
   test.e2e
 initialization =
-  ${common:path-sort}
+  ${common:path-munge}
   from os import environ
   environ.setdefault("MAAS_RACK_DEVELOP", "TRUE")
   environ.setdefault("MAAS_ROOT", "${buildout:directory}/run-e2e")
@@ -298,7 +305,7 @@
 entry-points =
   flake8=flake8.run:main
 initialization =
-  ${common:path-sort}
+  ${common:path-munge}
   ${common:warnings}
 
 [coverage]
@@ -308,7 +315,7 @@
 entry-points =
   coverage=coverage.cmdline:main
 initialization =
-  ${common:path-sort}
+  ${common:path-munge}
   ${common:warnings}
 scripts =
   coverage
 
=== modified file 'src/maascli/__init__.py'
--- src/maascli/__init__.py	2016-03-28 13:54:47 +0000
+++ src/maascli/__init__.py	2016-07-13 11:35:29 +0000
@@ -25,9 +25,16 @@
 
     try:
         options = parser.parse_args(argv[1:])
-        options.execute(options)
+        if hasattr(options, "execute"):
+            options.execute(options)
+        else:
+            # This mimics the error behaviour provided by argparse 1.1 from
+            # PyPI (which differs from argparse 1.1 in the standard library).
+            parser.error("too few arguments")
     except KeyboardInterrupt:
         raise SystemExit(1)
+    except SystemExit:
+        raise  # Pass-through.
     except Exception as error:
         if options.debug:
             raise
 
=== modified file 'src/maascli/tests/test_cli.py'
--- src/maascli/tests/test_cli.py	2016-06-22 17:03:02 +0000
+++ src/maascli/tests/test_cli.py	2016-07-13 11:35:29 +0000
@@ -26,7 +26,7 @@
 from testtools.matchers import DocTestMatches
 
 
-class TestRegisterommands(MAASTestCase):
+class TestRegisterCommands(MAASTestCase):
     """Tests for registers CLI commands."""
 
     def test_registers_subparsers(self):
 
=== modified file 'src/maascli/tests/test_integration.py'
--- src/maascli/tests/test_integration.py	2015-12-01 18:12:59 +0000
+++ src/maascli/tests/test_integration.py	2016-07-13 11:35:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Integration-test the `maascli` command."""
@@ -6,14 +6,23 @@
 __all__ = []
 
 import os.path
+import random
 from subprocess import (
     CalledProcessError,
     check_output,
     STDOUT,
 )
+from textwrap import dedent
 
+from maascli import main
+from maascli.config import ProfileConfig
+from maascli.testing.config import make_configs
+from maascli.utils import handler_command_name
 from maastesting import root
+from maastesting.fixtures import CaptureStandardIO
+from maastesting.matchers import DocTestMatches
 from maastesting.testcase import MAASTestCase
+from testtools.matchers import Equals
 
 
 def locate_maascli():
@@ -54,3 +63,35 @@
         else:
             # The test is that we get here without error.
             pass
+
+
+class TestMain(MAASTestCase):
+    """Tests of `maascli.main` directly."""
+
+    def fake_profile(self):
+        """Fake a profile."""
+        configs = make_configs()  # Instance of FakeConfig.
+        self.patch(ProfileConfig, 'open').return_value = configs
+        return configs
+
+    def test_complains_about_too_few_arguments(self):
+        configs = self.fake_profile()
+        [profile_name] = configs
+        resources = configs[profile_name]["description"]["resources"]
+        resource_name = random.choice(resources)["name"]
+        command = "maas", profile_name, handler_command_name(resource_name)
+
+        with CaptureStandardIO() as stdio:
+            error = self.assertRaises(SystemExit, main, command)
+
+        self.assertThat(error.code, Equals(2))
+        self.assertThat(
+            stdio.getError(),
+            DocTestMatches(dedent(
+                """\
+                usage: maas [-h] COMMAND ...
+                <BLANKLINE>
+                ...
+                <BLANKLINE>
+                too few arguments
+                """)))