Improve help text for base arguments

Add more unit tests for the cratonclient.shell.main module and refactor
some of the logic in that module to be more future-proof.

Change-Id: Iddfc5a170d15de77e1e400c2fa54b4a91cd94b7a

cratonclient/shell/main.py

@@ -14,23 +14,23 @@
 from __future__ import print_function
 
 import argparse
-import six
 import sys
 
 from oslo_utils import encodeutils
+from oslo_utils import importutils
 
 from cratonclient import __version__
 from cratonclient import session as craton
 
 from cratonclient.common import cliutils
-from cratonclient.shell.v1 import shell
 from cratonclient.v1 import client
 
 
 class CratonShell(object):
     """Class used to handle shell definition and parsing."""
 
-    def get_base_parser(self):
+    @staticmethod
+    def get_base_parser():
         """Configure base craton arguments and parsing."""
         parser = argparse.ArgumentParser(
             prog='craton',
@@ -51,32 +51,47 @@ class CratonShell(object):
                             )
         parser.add_argument('--craton-url',
                             default=cliutils.env('CRATON_URL'),
-                            help='Defaults to env[CRATON_URL]',
+                            help='The base URL of the running Craton service.'
+                                 ' Defaults to env[CRATON_URL].',
                             )
-        parser.add_argument('--craton-project-id',
+        parser.add_argument('--craton-version',
                             type=int,
-                            default=1,
+                            default=cliutils.env('CRATON_VERSION',
-                            help='Defaults to 1',
+                                                 default=1),
+                            help='The version of the Craton API to use. '
+                                 'Defaults to env[CRATON_VERSION].'
+                            )
+        parser.add_argument('--os-project-id',
+                            default=cliutils.env('OS_PROJECT_ID'),
+                            help='The project ID used to authenticate to '
+                                 'Craton. Defaults to env[OS_PROJECT_ID].',
                             )
         parser.add_argument('--os-username',
                             default=cliutils.env('OS_USERNAME'),
-                            help='Defaults to env[OS_USERNAME]',
+                            help='The username used to authenticate to '
+                                 'Craton. Defaults to env[OS_USERNAME].',
                             )
         parser.add_argument('--os-password',
                             default=cliutils.env('OS_PASSWORD'),
-                            help='Defaults to env[OS_PASSWORD]',
+                            help='The password used to authenticate to '
+                                 'Craton. Defaults to env[OS_PASSWORD].',
                             )
         return parser
 
     # NOTE(cmspence): Credit for this get_subcommand_parser function
     # goes to the magnumclient developers and contributors.
-    def get_subcommand_parser(self):
+    def get_subcommand_parser(self, api_version):
         """Get subcommands by parsing COMMAND_MODULES."""
         parser = self.get_base_parser()
 
         self.subcommands = {}
         subparsers = parser.add_subparsers(metavar='<subcommand>',
                                            dest='subparser_name')
+        shell = importutils.import_versioned_module(
+            'cratonclient.shell',
+            api_version,
+            'shell',
+        )
         command_modules = shell.COMMAND_MODULES
         for command_module in command_modules:
             self._find_subparsers(subparsers, command_module)
@@ -113,7 +128,7 @@ class CratonShell(object):
         parser = self.get_base_parser()
         (options, args) = parser.parse_known_args(argv)
         subcommand_parser = (
-            self.get_subcommand_parser()
+            self.get_subcommand_parser(options.craton_version)
         )
         self.parser = subcommand_parser
 
@@ -125,7 +140,7 @@ class CratonShell(object):
         session = craton.Session(
             username=args.os_username,
             token=args.os_password,
-            project_id=args.craton_project_id,
+            project_id=args.os_project_id,
         )
         self.cc = client.Client(session, args.craton_url)
         args.func(self.cc, args)
@@ -136,10 +151,9 @@ def main():
     try:
         CratonShell().main([encodeutils.safe_decode(a) for a in sys.argv[1:]])
     except Exception as e:
-        print("ERROR: %s" % encodeutils.safe_encode(six.text_type(e)),
+        print("ERROR: {}".format(encodeutils.exception_to_unicode(e)),
               file=sys.stderr)
         sys.exit(1)
-
     return 0
 
 

cratonclient/tests/integration/test_main_shell.py

@@ -67,11 +67,11 @@ class TestMainShell(base.ShellTestCase):
     @mock.patch('cratonclient.session.Session')
     @mock.patch('cratonclient.v1.client.Client')
     def test_main_craton_project_id(self, mock_client, mock_session):
-        """Verify --craton-project-id command is used for client connection."""
+        """Verify --os-project-id command is used for client connection."""
-        self.shell('--craton-project-id 99 host-list -r 1')
+        self.shell('--os-project-id 99 host-list -r 1')
         mock_session.assert_called_with(username=mock.ANY,
                                         token=mock.ANY,
-                                        project_id=99)
+                                        project_id='99')
         mock_client.assert_called_with(mock.ANY, mock.ANY)
 
     @mock.patch('cratonclient.session.Session')
@@ -108,7 +108,7 @@ class TestMainShell(base.ShellTestCase):
         url = '--craton-url test_url'
         username = '--os-username test_name'
         pw = '--os-password test_pw'
-        proj_id = '--craton-project-id 1'
+        proj_id = '--os-project-id 1'
         self.shell('{} {} {} {} host-create'.format(url,
                                                     username,
                                                     pw,

cratonclient/tests/unit/shell/test_main.py

@@ -0,0 +1,167 @@
+# 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 cratonclient.shell.main module."""
+import argparse
+import sys
+
+import mock
+import six
+
+import cratonclient
+from cratonclient.shell import main
+from cratonclient.tests import base
+
+
+class TestEntryPoint(base.TestCase):
+    """Tests for the craton shell entry-point."""
+
+    def setUp(self):
+        """Patch out the CratonShell class."""
+        super(TestEntryPoint, self).setUp()
+        self.class_mock = mock.patch('cratonclient.shell.main.CratonShell')
+        self.craton_shell = self.class_mock.start()
+        self.addCleanup(self.class_mock.stop)
+        self.print_mock = mock.patch('cratonclient.shell.main.print')
+        self.print_func = self.print_mock.start()
+        self.addCleanup(self.print_mock.stop)
+        self.sys_exit_mock = mock.patch('sys.exit')
+        self.sys_exit = self.sys_exit_mock.start()
+        self.addCleanup(self.sys_exit_mock.stop)
+
+    def test_entry_point_creates_a_shell_instance(self):
+        """Verify that our main entry-point uses CratonShell."""
+        CratonShell = self.craton_shell
+
+        main.main()
+
+        CratonShell.assert_called_once_with()
+
+    def test_entry_point_calls_shell_main_method(self):
+        """Verify we call the main method on our CratonShell instance."""
+        shell_instance = mock.Mock()
+        self.craton_shell.return_value = shell_instance
+
+        main.main()
+
+        self.assertTrue(shell_instance.main.called)
+
+    def test_entry_point_converts_args_to_text(self):
+        """Verify we call the main method with a list of text objects."""
+        shell_instance = mock.Mock()
+        self.craton_shell.return_value = shell_instance
+
+        main.main()
+
+        # NOTE(sigmavirus24): call_args is a tuple of positional arguments and
+        # keyword arguments, so since we pass a list positionally, we want the
+        # first of the positional arguments.
+        arglist = shell_instance.main.call_args[0][0]
+        self.assertTrue(
+            all(isinstance(arg, six.text_type) for arg in arglist)
+        )
+
+    def test_entry_point_handles_all_exceptions(self):
+        """Verify that we handle unexpected exceptions and print a message."""
+        shell_instance = mock.Mock()
+        shell_instance.main.side_effect = ValueError
+        self.craton_shell.return_value = shell_instance
+
+        main.main()
+
+        self.print_func.assert_called_once_with(
+            "ERROR: ",
+            file=sys.stderr,
+        )
+
+
+class TestCratonShell(base.TestCase):
+    """Tests for the CratonShell class."""
+
+    def setUp(self):
+        """Create an instance of CratonShell for each test."""
+        super(TestCratonShell, self).setUp()
+        self.shell = main.CratonShell()
+
+    def test_get_base_parser(self):
+        """Verify how we construct our basic Argument Parser."""
+        with mock.patch('argparse.ArgumentParser') as ArgumentParser:
+            parser = self.shell.get_base_parser()
+
+        self.assertEqual(ArgumentParser.return_value, parser)
+        ArgumentParser.assert_called_once_with(
+            prog='craton',
+            description=('Main shell for parsing arguments directed toward '
+                         'Craton.'),
+            epilog='See "craton help COMMAND" for help on a specific command.',
+            add_help=False,
+            formatter_class=argparse.HelpFormatter,
+        )
+
+    def test_get_base_parser_sets_default_options(self):
+        """Verify how we construct our basic Argument Parser."""
+        with mock.patch('cratonclient.common.cliutils.env') as env:
+            env.return_value = ''
+            with mock.patch('argparse.ArgumentParser'):
+                parser = self.shell.get_base_parser()
+
+        self.assertEqual([
+            mock.call(
+                '-h', '--help', action='store_true', help=argparse.SUPPRESS,
+            ),
+            mock.call(
+                '--version', action='version',
+                version=cratonclient.__version__,
+            ),
+            mock.call(
+                '--craton-url', default='',
+                help='The base URL of the running Craton service. '
+                     'Defaults to env[CRATON_URL].',
+            ),
+            mock.call(
+                '--craton-version', default='',
+                type=int,
+                help='The version of the Craton API to use. '
+                     'Defaults to env[CRATON_VERSION].'
+            ),
+            mock.call(
+                '--os-project-id', default='',
+                help='The project ID used to authenticate to Craton. '
+                     'Defaults to env[OS_PROJECT_ID].',
+            ),
+            mock.call(
+                '--os-username', default='',
+                help='The username used to authenticate to Craton. '
+                     'Defaults to env[OS_USERNAME].',
+            ),
+            mock.call(
+                '--os-password', default='',
+                help='The password used to authenticate to Craton. '
+                     'Defaults to env[OS_PASSWORD].',
+            ),
+            ],
+            parser.add_argument.call_args_list,
+        )
+
+    def test_get_base_parser_retrieves_environment_values(self):
+        """Verify the environment variables that are requested."""
+        with mock.patch('cratonclient.common.cliutils.env') as env:
+            self.shell.get_base_parser()
+
+        self.assertEqual([
+            mock.call('CRATON_URL'),
+            mock.call('CRATON_VERSION', default=1),
+            mock.call('OS_PROJECT_ID'),
+            mock.call('OS_USERNAME'),
+            mock.call('OS_PASSWORD'),
+            ],
+            env.call_args_list,
+        )