Refactor shell unittests and add hosts shell tests

This refactors our shell unittest helpers into three base classes for
further unit test classes to subclass and extend. This also corrects
some problems with the hosts shell parameters that either didn't
previous work or were wrong and no longer belong. Finally, it removes
project_id from places where it no longer belongs.

Change-Id: Ibd6a36714135d14355c3b4f48fced83b852581a9

cratonclient/shell/v1/hosts_shell.py

@@ -12,9 +12,11 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 """Hosts resource and resource shell wrapper."""
+from __future__ import print_function
+
 from cratonclient.common import cliutils
 from cratonclient import exceptions as exc
-from cratonclient.v1.hosts import HOST_FIELDS as h_fields
+from cratonclient.v1 import hosts
 
 
 @cliutils.arg('region',
@@ -28,7 +30,7 @@ from cratonclient.v1.hosts import HOST_FIELDS as h_fields
 def do_host_show(cc, args):
     """Show detailed information about a host."""
     host = cc.inventory(args.region).hosts.get(args.id)
-    data = {f: getattr(host, f, '') for f in h_fields}
+    data = {f: getattr(host, f, '') for f in hosts.HOST_FIELDS}
     cliutils.print_dict(data, wrap=72)
 
 
@@ -56,6 +58,7 @@ def do_host_show(cc, args):
 @cliutils.arg('--sort-dir',
               metavar='<direction>',
               default='asc',
+              choices=('asc', 'desc'),
               help='Sort direction: "asc" (default) or "desc".')
 @cliutils.arg('--fields',
               nargs='+',
@@ -76,33 +79,34 @@ def do_host_list(cc, args):
                                    'non-negative limit, got {0}'
                                    .format(args.limit))
         params['limit'] = args.limit
+
+    if args.fields and args.detail:
+        raise exc.CommandError('Cannot specify both --fields and --detail.')
+
     if args.detail:
-        fields = h_fields
+        fields = hosts.HOST_FIELDS
         params['detail'] = args.detail
     elif args.fields:
-        fields = {x: h_fields[x] for x in args.fields}
-    else:
-        fields = {x: h_fields[x] for x in default_fields}
-    if args.sort_key is not None:
-        fields_map = dict(zip(fields.keys(), fields.keys()))
-        # TODO(cmspence): Do we want to allow sorting by field heading value?
         try:
-            sort_key = fields_map[args.sort_key]
+            fields = {x: hosts.HOST_FIELDS[x] for x in args.fields}
-        except KeyError:
+        except KeyError as keyerr:
+            raise exc.CommandError('Invalid field "{}"'.format(keyerr.args[0]))
+    else:
+        fields = {x: hosts.HOST_FIELDS[x] for x in default_fields}
+    sort_key = args.sort_key and args.sort_key.lower()
+    if sort_key is not None:
+        if sort_key not in hosts.HOST_FIELDS:
             raise exc.CommandError(
                 '{0} is an invalid key for sorting,  valid values for '
-                '--sort-key are: {1}'.format(args.sort_key, h_fields.keys())
+                '--sort-key are: {1}'.format(
+                    args.sort_key, hosts.HOST_FIELDS.keys()
+                )
             )
         params['sort_key'] = sort_key
-        if args.sort_dir is not None:
-            if args.sort_dir not in ('asc', 'desc'):
-                raise exc.CommandError('Invalid sort direction specified. The '
-                                       'expected valid values for --sort-dir '
-                                       'are: "asc", "desc".')
     params['sort_dir'] = args.sort_dir
 
-    hosts = cc.inventory(args.region).hosts.list(**params)
+    host_list = cc.inventory(args.region).hosts.list(**params)
-    cliutils.print_list(hosts, list(fields))
+    cliutils.print_list(host_list, list(fields))
 
 
 @cliutils.arg('-n', '--name',
@@ -113,12 +117,6 @@ def do_host_list(cc, args):
               metavar='<ipaddress>',
               required=True,
               help='IP Address of the host.')
-@cliutils.arg('-p', '--project',
-              dest='project_id',
-              metavar='<project>',
-              type=int,
-              required=True,
-              help='ID of the project that the host belongs to.')
 @cliutils.arg('-r', '--region',
               dest='region_id',
               metavar='<region>',
@@ -151,9 +149,9 @@ def do_host_list(cc, args):
 def do_host_create(cc, args):
     """Register a new host with the Craton service."""
     fields = {k: v for (k, v) in vars(args).items()
-              if k in h_fields and not (v is None)}
+              if k in hosts.HOST_FIELDS and (v or v is False)}
     host = cc.inventory(args.region_id).hosts.create(**fields)
-    data = {f: getattr(host, f, '') for f in h_fields}
+    data = {f: getattr(host, f, '') for f in hosts.HOST_FIELDS}
     cliutils.print_dict(data, wrap=72)
 
 
@@ -171,11 +169,6 @@ def do_host_create(cc, args):
 @cliutils.arg('-i', '--ip_address',
               metavar='<ipaddress>',
               help='IP Address of the host.')
-@cliutils.arg('-p', '--project',
-              dest='project_id',
-              metavar='<project>',
-              type=int,
-              help='Desired ID of the project that the host should change to.')
 @cliutils.arg('-r', '--region',
               dest='region_id',
               metavar='<region>',
@@ -189,8 +182,6 @@ def do_host_create(cc, args):
 @cliutils.arg('-a', '--active',
               default=True,
               help='Status of the host.  Active or inactive.')
-@cliutils.arg('-t', '--type',
-              help='Type of the host.')
 @cliutils.arg('--note',
               help='Note about the host.')
 @cliutils.arg('--access_secret',
@@ -204,10 +195,11 @@ def do_host_create(cc, args):
 def do_host_update(cc, args):
     """Update a host that is registered with the Craton service."""
     fields = {k: v for (k, v) in vars(args).items()
-              if k in h_fields and not (v is None)}
+              if k in hosts.HOST_FIELDS and (v or v is False)}
-    host = cc.inventory(args.region).hosts.update(**fields)
+    item_id = fields.pop('id')
-    print("Host {0} has been successfully update.".format(host.id))
+    host = cc.inventory(args.region).hosts.update(item_id, **fields)
-    data = {f: getattr(host, f, '') for f in h_fields}
+    print("Host {0} has been successfully updated.".format(host.id))
+    data = {f: getattr(host, f, '') for f in hosts.HOST_FIELDS}
     cliutils.print_dict(data, wrap=72)
 
 
@@ -221,6 +213,14 @@ def do_host_update(cc, args):
               help='ID of the host.')
 def do_host_delete(cc, args):
     """Delete a host that is registered with the Craton service."""
+    try:
         response = cc.inventory(args.region).hosts.delete(args.id)
-    print("Host {0} was {1}successfully deleted.".
+    except exc.ClientException as client_exc:
-          format(args.id, '' if response else 'un'))
+        raise exc.CommandError(
+            'Failed to delete cell {} due to "{}:{}"'.format(
+                args.id, client_exc.__class__, str(client_exc),
+            )
+        )
+    else:
+        print("Host {0} was {1} deleted.".
+              format(args.id, 'successfully' if response else 'not'))

cratonclient/tests/integration/test_hosts_shell.py

@@ -62,7 +62,7 @@ class TestHostsShell(base.ShellTestCase):
     def test_host_list_limit_0_success(self, mock_list):
         """Verify that --limit 0 prints out all project hosts."""
         self.shell('host-list -r 1 --limit 0')
-        mock_list.assert_called_once_with(limit=0)
+        mock_list.assert_called_once_with(limit=0, sort_dir='asc')
 
     @mock.patch('cratonclient.v1.hosts.HostManager.list')
     def test_host_list_limit_positive_num_success(self, mock_list):
@@ -71,7 +71,7 @@ class TestHostsShell(base.ShellTestCase):
         The command will print out X number of project hosts.
         """
         self.shell('host-list -r 1 --limit 1')
-        mock_list.assert_called_once_with(limit=1)
+        mock_list.assert_called_once_with(limit=1, sort_dir='asc')
 
     def test_host_list_limit_negative_num_failure(self):
         """Verify --limit X, where X is a negative integer, fails.
@@ -87,31 +87,25 @@ class TestHostsShell(base.ShellTestCase):
         """Verify --cell arguments successfully pass cell to Client."""
         for cell_arg in ['-c', '--cell']:
             self.shell('host-list -r 1 {0} 1'.format(cell_arg))
-            mock_list.assert_called_once_with(cell_id=1)
+            mock_list.assert_called_once_with(cell_id=1, sort_dir='asc')
             mock_list.reset_mock()
 
     @mock.patch('cratonclient.v1.hosts.HostManager.list')
     def test_host_list_detail_success(self, mock_list):
         """Verify --detail argument successfully pass detail to Client."""
         self.shell('host-list -r 1 --detail')
-        mock_list.assert_called_once_with(detail=True)
+        mock_list.assert_called_once_with(detail=True, sort_dir='asc')
 
     @mock.patch('cratonclient.v1.hosts.HostManager.list')
     @mock.patch('cratonclient.common.cliutils.print_list')
     def test_host_list_fields_success(self, mock_printlist, mock_list):
         """Verify --fields argument successfully passed to Client."""
         self.shell('host-list -r 1 --fields id name')
-        mock_list.assert_called_once_with()
+        mock_list.assert_called_once_with(sort_dir='asc')
         mock_printlist.assert_called_once_with(mock.ANY,
                                                list({'id': 'ID',
                                                      'name': 'Name'}))
 
-    @mock.patch('cratonclient.v1.hosts.HostManager.list')
-    def test_host_list_detail_and_fields_specified(self, mock_list):
-        """Verify --fields ignored when --detail argument passed in."""
-        self.shell('host-list -r 1 --fields id name --detail')
-        mock_list.assert_called_once_with(detail=True)
-
     @mock.patch('cratonclient.v1.hosts.HostManager.list')
     def test_host_list_sort_key_field_key_success(self, mock_list):
         """Verify --sort-key arguments successfully passed to Client."""
@@ -129,7 +123,7 @@ class TestHostsShell(base.ShellTestCase):
     def test_host_list_sort_dir_not_passed_without_sort_key(self, mock_list):
         """Verify --sort-dir arg ignored without --sort-key."""
         self.shell('host-list -r 1 --sort-dir desc')
-        mock_list.assert_called_once_with()
+        mock_list.assert_called_once_with(sort_dir='desc')
 
     @mock.patch('cratonclient.v1.hosts.HostManager.list')
     def test_host_list_sort_dir_asc_success(self, mock_list):
@@ -147,9 +141,10 @@ class TestHostsShell(base.ShellTestCase):
 
     def test_host_list_sort_dir_invalid_value(self):
         """Verify --sort-dir with invalid args, fails with Command Error."""
-        self.assertRaises(exc.CommandError,
+        (_, error) = self.shell(
-                          self.shell,
+            'host-list -r 1 --sort-key name --sort-dir invalid'
-                          'host-list -r 1 --sort-key name --sort-dir invalid')
+        )
+        self.assertIn("invalid choice: 'invalid'", error)
 
     def test_host_create_missing_required_args(self):
         """Verify that missing required args results in error message."""
@@ -215,8 +210,7 @@ class TestHostsShell(base.ShellTestCase):
                                 id=1,
                                 name='mock_host')
         hosts_shell.do_host_update(client, valid_input)
-        vars(valid_input).pop('region')
+        mock_update.assert_called_once_with(1, name='mock_host')
-        mock_update.assert_called_once_with(**vars(valid_input))
 
     @mock.patch('cratonclient.v1.hosts.HostManager.update')
     def test_do_host_update_ignores_unknown_fields(self, mock_update):
@@ -233,9 +227,7 @@ class TestHostsShell(base.ShellTestCase):
                                   name='mock_host',
                                   invalid=True)
         hosts_shell.do_host_update(client, invalid_input)
-        vars(invalid_input).pop('region')
+        mock_update.assert_called_once_with(1, name='mock_host')
-        vars(invalid_input).pop('invalid')
-        mock_update.assert_called_once_with(**vars(invalid_input))
 
     def test_host_show_missing_required_args(self):
         """Verify that missing required args results in error message."""

cratonclient/tests/unit/shell/base.py

@@ -0,0 +1,91 @@
+# -*- coding: utf-8 -*-
+
+# 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.
+"""Base class for shell unit tests."""
+import argparse
+
+import mock
+
+from cratonclient import exceptions
+from cratonclient.tests import base
+
+
+class TestShellCommand(base.TestCase):
+    """Base class for shell command unit tests."""
+
+    def setUp(self):
+        """Initialize test fixtures."""
+        super(TestShellCommand, self).setUp()
+        self.craton_client = mock.Mock()
+        self.inventory = mock.Mock()
+        self.craton_client.inventory.return_value = self.inventory
+
+    def assertRaisesCommandErrorWith(self, func, args):
+        """Assert the shell command raises CommandError."""
+        self.assertRaises(
+            exceptions.CommandError,
+            func, self.craton_client, args,
+        )
+
+    def args_for(self, **kwargs):
+        """Return a Namespace object with the specified kwargs."""
+        return argparse.Namespace(**kwargs)
+
+
+class TestShellCommandUsingPrintDict(TestShellCommand):
+    """Base class for shell commands using print_dict."""
+
+    def setUp(self):
+        """Initialize test fixtures."""
+        super(TestShellCommandUsingPrintDict, self).setUp()
+        self.print_dict_patch = mock.patch(
+            'cratonclient.common.cliutils.print_dict'
+        )
+        self.print_dict = self.print_dict_patch.start()
+
+    def tearDown(self):
+        """Clean-up test fixtures."""
+        super(TestShellCommandUsingPrintDict, self).tearDown()
+        self.print_dict_patch.stop()
+
+    def assertNothingWasCalled(self):
+        """Assert inventory, list, and print_dict were not called."""
+        self.assertFalse(self.craton_client.inventory.called)
+        self.assertFalse(self.print_dict.called)
+
+
+class TestShellCommandUsingPrintList(TestShellCommand):
+    """Base class for shell commands using print_list."""
+
+    def setUp(self):
+        """Initialize test fixtures."""
+        super(TestShellCommandUsingPrintList, self).setUp()
+        self.print_list_patch = mock.patch(
+            'cratonclient.common.cliutils.print_list'
+        )
+        self.print_list = self.print_list_patch.start()
+
+    def tearDown(self):
+        """Clean-up test fixtures."""
+        super(TestShellCommandUsingPrintList, self).tearDown()
+        self.print_list_patch.stop()
+
+    def assertNothingWasCalled(self):
+        """Assert inventory, list, and print_dict were not called."""
+        self.assertFalse(self.craton_client.inventory.called)
+        self.assertFalse(self.print_list.called)
+
+    def assertSortedPrintListFieldsEqualTo(self, expected_fields):
+        """Assert the sorted fields parameter is equal expected_fields."""
+        self.assertEqual(expected_fields,
+                         sorted(self.print_list.call_args[0][-1]))

cratonclient/tests/unit/shell/v1/test_cells_shell.py

@@ -12,63 +12,20 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 """Tests for the shell functions for the cells resource."""
-import argparse
-
 import mock
 
 from cratonclient import exceptions
 from cratonclient.shell.v1 import cells_shell
-from cratonclient.tests import base
+from cratonclient.tests.unit.shell import base
 from cratonclient.v1 import cells
 
 
-class TestCells(base.TestCase):
+class TestDoShellShow(base.TestShellCommandUsingPrintDict):
-    """Base class for cells_shell commands."""
-
-    def setUp(self):
-        """Initialize test fixtures."""
-        super(TestCells, self).setUp()
-        self.craton_client = mock.Mock()
-        self.inventory = mock.Mock()
-        self.craton_client.inventory.return_value = self.inventory
-
-    def assertRaisesCommandErrorWith(self, func, args):
-        """Assert do_cell_create raises CommandError."""
-        self.assertRaises(
-            exceptions.CommandError,
-            func, self.craton_client, args,
-        )
-
-
-class TestCellsPrintDict(TestCells):
-    """Base class for commands using print_dict."""
-
-    def setUp(self):
-        """Initialize test fixtures."""
-        super(TestCellsPrintDict, self).setUp()
-        self.print_dict_patch = mock.patch(
-            'cratonclient.common.cliutils.print_dict'
-        )
-        self.print_dict = self.print_dict_patch.start()
-
-    def tearDown(self):
-        """Clean-up test fixtures."""
-        super(TestCellsPrintDict, self).tearDown()
-        self.print_dict_patch.stop()
-
-    def assertNothingWasCalled(self):
-        """Assert inventory, list, and print_dict were not called."""
-        self.assertFalse(self.craton_client.inventory.called)
-        self.assertFalse(self.inventory.cells.list.called)
-        self.assertFalse(self.print_dict.called)
-
-
-class TestDoShellShow(TestCellsPrintDict):
     """Unit tests for the cell show command."""
 
     def test_simple_usage(self):
         """Verify the behaviour of do_cell_show."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             id=456,
         )
@@ -83,31 +40,17 @@ class TestDoShellShow(TestCellsPrintDict):
         )
 
 
-class TestDoCellList(TestCells):
+class TestDoCellList(base.TestShellCommandUsingPrintList):
     """Unit tests for the cell list command."""
 
-    def setUp(self):
-        """Initialize test fixtures."""
-        super(TestDoCellList, self).setUp()
-        self.print_list_patch = mock.patch(
-            'cratonclient.common.cliutils.print_list'
-        )
-        self.print_list = self.print_list_patch.start()
-
-    def tearDown(self):
-        """Clean-up test fixtures."""
-        super(TestDoCellList, self).tearDown()
-        self.print_list_patch.stop()
-
     def assertNothingWasCalled(self):
         """Assert inventory, list, and print_list were not called."""
-        self.assertFalse(self.craton_client.inventory.called)
+        super(TestDoCellList, self).assertNothingWasCalled()
-        self.assertFalse(self.inventory.cells.list.called)
         self.assertFalse(self.print_list.called)
 
     def test_with_defaults(self):
         """Verify the behaviour of do_cell_list with mostly default values."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=False,
             limit=None,
@@ -126,7 +69,7 @@ class TestDoCellList(TestCells):
 
     def test_negative_limit(self):
         """Ensure we raise an exception for negative limits."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=False,
             limit=-1,
@@ -140,7 +83,7 @@ class TestDoCellList(TestCells):
 
     def test_positive_limit(self):
         """Verify that we pass positive limits to the call to list."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=False,
             limit=5,
@@ -162,7 +105,7 @@ class TestDoCellList(TestCells):
 
     def test_valid_sort_key(self):
         """Verify that we pass on our sort key."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=False,
             limit=None,
@@ -184,7 +127,7 @@ class TestDoCellList(TestCells):
 
     def test_invalid_sort_key(self):
         """Verify that do not we pass on our sort key."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=False,
             limit=None,
@@ -198,7 +141,7 @@ class TestDoCellList(TestCells):
 
     def test_detail(self):
         """Verify the behaviour of specifying --detail."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=True,
             limit=None,
@@ -219,7 +162,7 @@ class TestDoCellList(TestCells):
 
     def test_raises_exception_with_detail_and_fields(self):
         """Verify that we fail when users specify --detail and --fields."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=True,
             limit=None,
@@ -233,7 +176,7 @@ class TestDoCellList(TestCells):
 
     def test_fields(self):
         """Verify that we print out specific fields."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=False,
             limit=None,
@@ -253,7 +196,7 @@ class TestDoCellList(TestCells):
 
     def test_invalid_fields(self):
         """Verify that we error out with invalid fields."""
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             detail=False,
             limit=None,
@@ -266,12 +209,12 @@ class TestDoCellList(TestCells):
         self.assertNothingWasCalled()
 
 
-class TestDoCellCreate(TestCellsPrintDict):
+class TestDoCellCreate(base.TestShellCommandUsingPrintDict):
     """Unit tests for the cell create command."""
 
     def test_create_without_note(self):
         """Verify our parameters to cells.create."""
-        args = argparse.Namespace(
+        args = self.args_for(
             name='New Cell',
             region_id=123,
             note=None,
@@ -288,7 +231,7 @@ class TestDoCellCreate(TestCellsPrintDict):
 
     def test_create_with_note(self):
         """Verify that we include the note argument when present."""
-        args = argparse.Namespace(
+        args = self.args_for(
             name='New Cell',
             region_id=123,
             note='This is a note',
@@ -305,12 +248,12 @@ class TestDoCellCreate(TestCellsPrintDict):
         self.print_dict.assert_called_once_with(mock.ANY, wrap=72)
 
 
-class TestDoCellUpdate(TestCellsPrintDict):
+class TestDoCellUpdate(base.TestShellCommandUsingPrintDict):
     """Unit tests for the cell update command."""
 
     def test_update_without_name_region_or_note_fails(self):
         """Verify we raise a command error when there's nothing to update."""
-        args = argparse.Namespace(
+        args = self.args_for(
             id=123,
             region=345,
             name=None,
@@ -323,7 +266,7 @@ class TestDoCellUpdate(TestCellsPrintDict):
 
     def test_update_with_name(self):
         """Verify we update with only the new name."""
-        args = argparse.Namespace(
+        args = self.args_for(
             id=123,
             region=345,
             name='New name',
@@ -342,7 +285,7 @@ class TestDoCellUpdate(TestCellsPrintDict):
 
     def test_update_with_new_region(self):
         """Verify we update with only the new region id."""
-        args = argparse.Namespace(
+        args = self.args_for(
             id=123,
             region=345,
             name=None,
@@ -361,7 +304,7 @@ class TestDoCellUpdate(TestCellsPrintDict):
 
     def test_update_with_new_note(self):
         """Verify we update with only the new note text."""
-        args = argparse.Namespace(
+        args = self.args_for(
             id=123,
             region=345,
             name=None,
@@ -380,7 +323,7 @@ class TestDoCellUpdate(TestCellsPrintDict):
 
     def test_update_with_everything(self):
         """Verify we update with everything."""
-        args = argparse.Namespace(
+        args = self.args_for(
             id=123,
             region=345,
             name='A new name for a new region',
@@ -400,7 +343,7 @@ class TestDoCellUpdate(TestCellsPrintDict):
         self.print_dict.assert_called_once_with(mock.ANY, wrap=72)
 
 
-class TestDoCellDelete(TestCells):
+class TestDoCellDelete(base.TestShellCommand):
     """Tests for the do_cell_delete command."""
 
     def setUp(self):
@@ -419,7 +362,7 @@ class TestDoCellDelete(TestCells):
     def test_successful(self):
         """Verify the message we print when successful."""
         self.inventory.cells.delete.return_value = True
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             id=456,
         )
@@ -435,7 +378,7 @@ class TestDoCellDelete(TestCells):
     def test_failed(self):
         """Verify the message we print when deletion fails."""
         self.inventory.cells.delete.return_value = False
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             id=456,
         )
@@ -451,7 +394,7 @@ class TestDoCellDelete(TestCells):
     def test_failed_with_exception(self):
         """Verify the message we print when deletion fails."""
         self.inventory.cells.delete.side_effect = exceptions.NotFound
-        args = argparse.Namespace(
+        args = self.args_for(
             region=123,
             id=456,
         )

cratonclient/tests/unit/shell/v1/test_hosts_shell.py

@@ -0,0 +1,490 @@
+# -*- coding: utf-8 -*-
+
+# 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 shell functions for the hosts resource."""
+import mock
+
+from cratonclient import exceptions
+from cratonclient.shell.v1 import hosts_shell
+from cratonclient.tests.unit.shell import base
+from cratonclient.v1 import hosts
+
+
+class TestDoHostShow(base.TestShellCommandUsingPrintDict):
+    """Unit tests for the host show command."""
+
+    def test_print_host_data(self):
+        """Verify we print info for the specified host."""
+        args = self.args_for(
+            region=135,
+            id=246,
+        )
+
+        hosts_shell.do_host_show(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(135)
+        self.inventory.hosts.get.assert_called_once_with(246)
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+
+class TestDoHostList(base.TestShellCommandUsingPrintList):
+    """Unit tests for the host list command."""
+
+    def args_for(self, **kwargs):
+        """Generate a Namespace for do_host_list."""
+        kwargs.setdefault('region', 246)
+        kwargs.setdefault('cell', None)
+        kwargs.setdefault('detail', False)
+        kwargs.setdefault('limit', None)
+        kwargs.setdefault('sort_key', None)
+        kwargs.setdefault('sort_dir', 'asc')
+        kwargs.setdefault('fields', [])
+        return super(TestDoHostList, self).args_for(**kwargs)
+
+    def test_only_required_parameters(self):
+        """Verify the behaviour with the minimum number of params."""
+        args = self.args_for()
+
+        hosts_shell.do_host_list(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(246)
+        self.inventory.hosts.list.assert_called_once_with(sort_dir='asc')
+        self.assertSortedPrintListFieldsEqualTo([
+            'active', 'cell_id', 'device_type', 'id', 'name'
+        ])
+
+    def test_with_cell_id(self):
+        """Verify that we include the cell_id in the params."""
+        args = self.args_for(cell=789)
+
+        hosts_shell.do_host_list(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(246)
+        self.inventory.hosts.list.assert_called_once_with(
+            cell_id=789,
+            sort_dir='asc',
+        )
+        self.assertSortedPrintListFieldsEqualTo([
+            'active', 'cell_id', 'device_type', 'id', 'name',
+        ])
+
+    def test_with_detail(self):
+        """Verify the behaviour of specifying --detail."""
+        args = self.args_for(detail=True)
+
+        hosts_shell.do_host_list(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(246)
+        self.inventory.hosts.list.assert_called_once_with(
+            detail=True,
+            sort_dir='asc',
+        )
+        self.assertSortedPrintListFieldsEqualTo([
+            'access_secret_id',
+            'active',
+            'cell_id',
+            'created_at',
+            'device_type',
+            'id',
+            'ip_address',
+            'labels',
+            'name',
+            'note',
+            'project_id',
+            'region_id',
+            'update_at',
+        ])
+
+    def test_with_limit(self):
+        """Verify the behaviour with --limit specified."""
+        args = self.args_for(limit=20)
+
+        hosts_shell.do_host_list(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(246)
+        self.inventory.hosts.list.assert_called_once_with(
+            limit=20,
+            sort_dir='asc',
+        )
+        self.assertSortedPrintListFieldsEqualTo([
+            'active', 'cell_id', 'device_type', 'id', 'name'
+        ])
+
+    def test_negative_limit_raises_command_error(self):
+        """Verify that we forbid negative limit values."""
+        args = self.args_for(limit=-10)
+
+        self.assertRaisesCommandErrorWith(hosts_shell.do_host_list, args)
+        self.assertNothingWasCalled()
+
+    def test_fields(self):
+        """Verify that we can specify custom fields."""
+        args = self.args_for(fields=['id', 'name', 'cell_id'])
+
+        hosts_shell.do_host_list(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(246)
+        self.inventory.hosts.list.assert_called_once_with(sort_dir='asc')
+        self.assertSortedPrintListFieldsEqualTo([
+            'cell_id', 'id', 'name',
+        ])
+
+    def test_invalid_sort_key(self):
+        """Verify that we disallow invalid sort keys."""
+        args = self.args_for(sort_key='my-fake-sort-key')
+
+        self.assertRaisesCommandErrorWith(
+            hosts_shell.do_host_list, args
+        )
+        self.assertNothingWasCalled()
+
+    def test_sort_key(self):
+        """Verify we pass sort_key to our list call."""
+        args = self.args_for(sort_key='ip_address')
+
+        hosts_shell.do_host_list(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(246)
+        self.inventory.hosts.list.assert_called_once_with(
+            sort_key='ip_address',
+            sort_dir='asc',
+        )
+
+    def test_fields_and_detail_raise_command_error(self):
+        """Verify combining fields and detail cause an error."""
+        args = self.args_for(detail=True, fields=['id', 'name', 'ip_address'])
+
+        self.assertRaisesCommandErrorWith(
+            hosts_shell.do_host_list, args,
+        )
+        self.assertNothingWasCalled()
+
+    def test_invalid_fields_raise_command_error(self):
+        """Verify sending an invalid field raises a CommandError."""
+        args = self.args_for(fields=['fake-field', 'id'])
+
+        self.assertRaisesCommandErrorWith(
+            hosts_shell.do_host_list, args,
+        )
+        self.assertNothingWasCalled()
+
+
+class TestDoHostCreate(base.TestShellCommandUsingPrintDict):
+    """Tests for the do_host_create shell command."""
+
+    def args_for(self, **kwargs):
+        """Generate the Namespace object needed for host create."""
+        kwargs.setdefault('region', 123)
+        kwargs.setdefault('name', 'test-hostname')
+        kwargs.setdefault('ip_address', '10.0.1.10')
+        kwargs.setdefault('region_id', 123)
+        kwargs.setdefault('cell_id', 246)
+        kwargs.setdefault('device_type', 'host')
+        kwargs.setdefault('active', True)
+        kwargs.setdefault('note', None)
+        kwargs.setdefault('access_secret_id', None)
+        kwargs.setdefault('labels', [])
+        return super(TestDoHostCreate, self).args_for(**kwargs)
+
+    def test_only_the_required_arguments(self):
+        """Verify that the required arguments are passed appropriately."""
+        args = self.args_for()
+
+        hosts_shell.do_host_create(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.create.assert_called_once_with(
+            name='test-hostname',
+            ip_address='10.0.1.10',
+            cell_id=246,
+            device_type='host',
+            active=True,
+            region_id=123,
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+    def test_with_a_note(self):
+        """Verify that we pass along the note."""
+        args = self.args_for(note='This is a note.')
+
+        hosts_shell.do_host_create(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.create.assert_called_once_with(
+            name='test-hostname',
+            ip_address='10.0.1.10',
+            cell_id=246,
+            device_type='host',
+            active=True,
+            region_id=123,
+            note='This is a note.',
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+    def test_with_access_secret(self):
+        """Verify that we pass along an access secret."""
+        args = self.args_for(access_secret_id=789)
+
+        hosts_shell.do_host_create(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.create.assert_called_once_with(
+            name='test-hostname',
+            ip_address='10.0.1.10',
+            cell_id=246,
+            device_type='host',
+            active=True,
+            region_id=123,
+            access_secret_id=789,
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+    def test_with_labels(self):
+        """Verify that we pass along our labels."""
+        args = self.args_for(labels=['label-0', 'label-1'])
+
+        hosts_shell.do_host_create(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.create.assert_called_once_with(
+            name='test-hostname',
+            ip_address='10.0.1.10',
+            cell_id=246,
+            device_type='host',
+            active=True,
+            region_id=123,
+            labels=['label-0', 'label-1'],
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+
+class TestDoHostUpdate(base.TestShellCommandUsingPrintDict):
+    """Tests host-update shell command."""
+
+    def setUp(self):
+        """Also patch out the print function."""
+        super(TestDoHostUpdate, self).setUp()
+        self.print_mocker = mock.patch(
+            'cratonclient.shell.v1.hosts_shell.print'
+        )
+        self.print_mock = self.print_mocker.start()
+        self.inventory.hosts.update.return_value = mock.Mock(id=246)
+
+    def tearDown(self):
+        """Stop mocking print."""
+        super(TestDoHostUpdate, self).tearDown()
+        self.print_mocker.stop()
+
+    def args_for(self, **kwargs):
+        """Generate arguments for host-update command."""
+        kwargs.setdefault('region', 123)
+        kwargs.setdefault('id', 246)
+        kwargs.setdefault('name', None)
+        kwargs.setdefault('ip_address', None)
+        kwargs.setdefault('region_id', None)
+        kwargs.setdefault('cell_id', None)
+        kwargs.setdefault('active', True)
+        kwargs.setdefault('note', None)
+        kwargs.setdefault('access_secret_id', None)
+        kwargs.setdefault('labels', [])
+        return super(TestDoHostUpdate, self).args_for(**kwargs)
+
+    def test_with_basic_required_parameters(self):
+        """Verify the basic update call works."""
+        args = self.args_for()
+
+        hosts_shell.do_host_update(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.update.assert_called_once_with(
+            246,
+            active=True,
+        )
+        self.print_mock.assert_called_once_with(
+            'Host 246 has been successfully updated.'
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+    def test_with_name(self):
+        """Verify the new name is passed along."""
+        args = self.args_for(name='New name')
+
+        hosts_shell.do_host_update(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.update.assert_called_once_with(
+            246,
+            name='New name',
+            active=True,
+        )
+        self.print_mock.assert_called_once_with(
+            'Host 246 has been successfully updated.'
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+    def test_with_ip_address(self):
+        """Verify the new IP Address is passed along."""
+        args = self.args_for(ip_address='10.1.0.10')
+
+        hosts_shell.do_host_update(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.update.assert_called_once_with(
+            246,
+            ip_address='10.1.0.10',
+            active=True,
+        )
+        self.print_mock.assert_called_once_with(
+            'Host 246 has been successfully updated.'
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+    def test_disable_host(self):
+        """Verify active is passed even when False."""
+        args = self.args_for(active=False)
+
+        hosts_shell.do_host_update(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.update.assert_called_once_with(
+            246,
+            active=False,
+        )
+        self.print_mock.assert_called_once_with(
+            'Host 246 has been successfully updated.'
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+    def test_optional_parameters(self):
+        """Verify all optional parameters are passed along when specified."""
+        args = self.args_for(
+            name='New name',
+            ip_address='10.1.1.1',
+            region_id=789,
+            cell_id=101,
+            note='A note about a host',
+            access_secret_id=1001,
+            labels=['label1', 'label2'],
+        )
+
+        hosts_shell.do_host_update(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.update.assert_called_once_with(
+            246,
+            active=True,
+            name='New name',
+            ip_address='10.1.1.1',
+            region_id=789,
+            cell_id=101,
+            note='A note about a host',
+            access_secret_id=1001,
+            labels=['label1', 'label2'],
+        )
+        self.print_mock.assert_called_once_with(
+            'Host 246 has been successfully updated.'
+        )
+        self.print_dict.assert_called_once_with(
+            {f: mock.ANY for f in hosts.HOST_FIELDS},
+            wrap=72,
+        )
+
+
+class TestDoHostDelete(base.TestShellCommand):
+    """Tests for the host-delete shell command."""
+
+    def setUp(self):
+        """Set-up a print function mock."""
+        super(TestDoHostDelete, self).setUp()
+        self.print_mocker = mock.patch(
+            'cratonclient.shell.v1.hosts_shell.print'
+        )
+        self.print_mock = self.print_mocker.start()
+
+    def tearDown(self):
+        """Clean up the print function mock."""
+        super(TestDoHostDelete, self).tearDown()
+        self.print_mocker.stop()
+
+    def test_successful(self):
+        """Verify we print our successful message."""
+        self.inventory.hosts.delete.return_value = True
+        args = self.args_for(
+            region=123,
+            id=246,
+        )
+
+        hosts_shell.do_host_delete(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.delete.assert_called_once_with(246)
+        self.print_mock.assert_called_once_with(
+            'Host 246 was successfully deleted.'
+        )
+
+    def test_failed(self):
+        """Verify the message we print when deletion fails."""
+        self.inventory.hosts.delete.return_value = False
+        args = self.args_for(
+            region=123,
+            id=246,
+        )
+
+        hosts_shell.do_host_delete(self.craton_client, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.delete.assert_called_once_with(246)
+        self.print_mock.assert_called_once_with(
+            'Host 246 was not deleted.'
+        )
+
+    def test_failed_with_exception(self):
+        """Verify we raise a CommandError on client exceptions."""
+        self.inventory.hosts.delete.side_effect = exceptions.NotFound
+        args = self.args_for(
+            region=123,
+            id=246,
+        )
+
+        self.assertRaisesCommandErrorWith(hosts_shell.do_host_delete, args)
+
+        self.craton_client.inventory.assert_called_once_with(123)
+        self.inventory.hosts.delete.assert_called_once_with(246)
+        self.assertFalse(self.print_mock.called)

cratonclient/v1/hosts.py

@@ -41,5 +41,6 @@ HOST_FIELDS = {
     'note': 'Note',
     'access_secret_id': "Access Secret ID",
     'created_at': 'Created At',
-    'update_at': 'Updated At'
+    'update_at': 'Updated At',
+    'labels': 'Labels',
 }