From 0ec761cb96c62f3d1f7a8f20f4c5c03e3c76aa1a Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Mon, 27 Feb 2017 14:33:19 -0600 Subject: [PATCH] Update hosts commands to use pluggable formatters After our last change, we can begin to allow users to specify formatters for host command output. Change-Id: I22bc9421bf0b47ef07ae7c149a32ff8b074906d2 --- cratonclient/shell/v1/hosts_shell.py | 11 ++-- .../integration/shell/v1/test_hosts_shell.py | 52 ++++++++++-------- cratonclient/tests/unit/shell/base.py | 8 ++- .../tests/unit/shell/v1/test_hosts_shell.py | 53 +++++++++---------- 4 files changed, 65 insertions(+), 59 deletions(-) diff --git a/cratonclient/shell/v1/hosts_shell.py b/cratonclient/shell/v1/hosts_shell.py index f69541b..9e07f12 100644 --- a/cratonclient/shell/v1/hosts_shell.py +++ b/cratonclient/shell/v1/hosts_shell.py @@ -26,8 +26,7 @@ from cratonclient.v1 import hosts def do_host_show(cc, args): """Show detailed information about a host.""" host = cc.hosts.get(args.id) - data = {f: getattr(host, f, '') for f in hosts.HOST_FIELDS} - cliutils.print_dict(data, wrap=72) + args.formatter.configure(wrap=72).handle(host) @cliutils.arg('-r', '--region', @@ -123,7 +122,7 @@ def do_host_list(cc, args): params['autopaginate'] = args.all host_list = cc.hosts.list(**params) - cliutils.print_list(host_list, list(fields)) + args.formatter.configure(fields=list(fields)).handle(host_list) @cliutils.arg('-n', '--name', @@ -169,8 +168,7 @@ def do_host_create(cc, args): fields = {k: v for (k, v) in vars(args).items() if k in hosts.HOST_FIELDS and (v or v is False)} host = cc.hosts.create(**fields) - data = {f: getattr(host, f, '') for f in hosts.HOST_FIELDS} - cliutils.print_dict(data, wrap=72) + args.formatter.configure(wrap=72).handle(host) @cliutils.arg('id', @@ -213,8 +211,7 @@ def do_host_update(cc, args): item_id = fields.pop('id') host = cc.hosts.update(item_id, **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) + args.formatter.configure(wrap=72).handle(host) @cliutils.arg('id', diff --git a/cratonclient/tests/integration/shell/v1/test_hosts_shell.py b/cratonclient/tests/integration/shell/v1/test_hosts_shell.py index 45498bd..8e1fc86 100644 --- a/cratonclient/tests/integration/shell/v1/test_hosts_shell.py +++ b/cratonclient/tests/integration/shell/v1/test_hosts_shell.py @@ -29,22 +29,30 @@ class TestHostsShell(base.ShellTestCase): re_options = re.DOTALL | re.MULTILINE host_valid_fields = None - host_invalid_field = None + host_invalid_fields = None def setUp(self): """Setup required test fixtures.""" super(TestHostsShell, self).setUp() - self.host_valid_fields = Namespace(project_id=1, - region_id=1, - name='mock_host', - ip_address='127.0.0.1', - active=True) - self.host_invalid_field = Namespace(project_id=1, - region_id=1, - name='mock_host', - ip_address='127.0.0.1', - active=True, - invalid_foo='ignored') + self.host_valid_kwargs = { + 'project_id': 1, + 'region_id': 1, + 'name': 'mock_host', + 'ip_address': '127.0.0.1', + 'active': True, + } + self.host_invalid_kwargs = { + 'project_id': 1, + 'region_id': 1, + 'name': 'mock_host', + 'ip_address': '127.0.0.1', + 'active': True, + 'invalid_foo': 'ignored', + } + self.host_valid_fields = Namespace(**self.host_valid_kwargs) + self.host_valid_fields.formatter = mock.Mock() + self.host_invalid_fields = Namespace(**self.host_invalid_kwargs) + self.host_invalid_fields.formatter = mock.Mock() @mock.patch('cratonclient.v1.hosts.HostManager.list') def test_host_list_success(self, mock_list): @@ -121,8 +129,7 @@ class TestHostsShell(base.ShellTestCase): ) @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): + def test_host_list_fields_success(self, mock_list): """Verify --fields argument successfully passed to Client.""" self.shell('host-list -r 1 --fields id name') mock_list.assert_called_once_with( @@ -131,9 +138,6 @@ class TestHostsShell(base.ShellTestCase): marker=None, autopaginate=False, ) - mock_printlist.assert_called_once_with(mock.ANY, - list({'id': 'ID', - 'name': 'Name'})) @mock.patch('cratonclient.v1.hosts.HostManager.list') def test_host_list_sort_key_field_key_success(self, mock_list): @@ -216,7 +220,7 @@ class TestHostsShell(base.ShellTestCase): region_id=mock.ANY, ) hosts_shell.do_host_create(client, self.host_valid_fields) - mock_create.assert_called_once_with(**vars(self.host_valid_fields)) + mock_create.assert_called_once_with(**self.host_valid_kwargs) @mock.patch('cratonclient.v1.hosts.HostManager.create') def test_do_host_create_ignores_unknown_fields(self, mock_create): @@ -226,8 +230,8 @@ class TestHostsShell(base.ShellTestCase): mock.ANY, 'http://127.0.0.1/', region_id=mock.ANY, ) - hosts_shell.do_host_create(client, self.host_invalid_field) - mock_create.assert_called_once_with(**vars(self.host_valid_fields)) + hosts_shell.do_host_create(client, self.host_invalid_fields) + mock_create.assert_called_once_with(**self.host_valid_kwargs) def test_host_update_missing_required_args(self): """Verify that missing required args results in error message.""" @@ -251,7 +255,8 @@ class TestHostsShell(base.ShellTestCase): ) valid_input = Namespace(region=1, id=1, - name='mock_host') + name='mock_host', + formatter=mock.Mock()) hosts_shell.do_host_update(client, valid_input) mock_update.assert_called_once_with(1, name='mock_host') @@ -266,6 +271,7 @@ class TestHostsShell(base.ShellTestCase): invalid_input = Namespace(region=1, id=1, name='mock_host', + formatter=mock.Mock(), invalid=True) hosts_shell.do_host_update(client, invalid_input) mock_update.assert_called_once_with(1, name='mock_host') @@ -291,8 +297,12 @@ class TestHostsShell(base.ShellTestCase): region_id=mock.ANY, ) test_args = Namespace(id=1, region=1) + formatter = test_args.formatter = mock.Mock() + formatter.configure.return_value = formatter hosts_shell.do_host_show(client, test_args) mock_get.assert_called_once_with(vars(test_args)['id']) + self.assertTrue(formatter.handle.called) + self.assertEqual(1, formatter.handle.call_count) def test_host_delete_missing_required_args(self): """Verify that missing required args results in error message.""" diff --git a/cratonclient/tests/unit/shell/base.py b/cratonclient/tests/unit/shell/base.py index c5f046e..cbec34a 100644 --- a/cratonclient/tests/unit/shell/base.py +++ b/cratonclient/tests/unit/shell/base.py @@ -26,6 +26,8 @@ class TestShellCommand(base.TestCase): def setUp(self): """Initialize test fixtures.""" super(TestShellCommand, self).setUp() + self.formatter = mock.Mock() + self.formatter.configure.return_value = self.formatter self.craton_client = mock.Mock() self.inventory = mock.Mock() self.craton_client.inventory.return_value = self.inventory @@ -39,6 +41,7 @@ class TestShellCommand(base.TestCase): def args_for(self, **kwargs): """Return a Namespace object with the specified kwargs.""" + kwargs.setdefault('formatter', self.formatter) return argparse.Namespace(**kwargs) @@ -87,5 +90,6 @@ class TestShellCommandUsingPrintList(TestShellCommand): 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])) + kwargs = self.formatter.configure.call_args[1] + self.assertListEqual(expected_fields, + sorted(kwargs['fields'])) diff --git a/cratonclient/tests/unit/shell/v1/test_hosts_shell.py b/cratonclient/tests/unit/shell/v1/test_hosts_shell.py index e5af751..a807dfa 100644 --- a/cratonclient/tests/unit/shell/v1/test_hosts_shell.py +++ b/cratonclient/tests/unit/shell/v1/test_hosts_shell.py @@ -17,7 +17,6 @@ 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): @@ -33,10 +32,6 @@ class TestDoHostShow(base.TestShellCommandUsingPrintDict): hosts_shell.do_host_show(self.craton_client, args) self.craton_client.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): @@ -289,9 +284,9 @@ class TestDoHostCreate(base.TestShellCommandUsingPrintDict): active=True, region_id=123, ) - self.print_dict.assert_called_once_with( - {f: mock.ANY for f in hosts.HOST_FIELDS}, - wrap=72, + self.formatter.configure.assert_called_once_with(wrap=72) + self.formatter.handle.assert_called_once_with( + self.craton_client.hosts.create.return_value ) def test_with_a_note(self): @@ -309,9 +304,9 @@ class TestDoHostCreate(base.TestShellCommandUsingPrintDict): 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, + self.formatter.configure.assert_called_once_with(wrap=72) + self.formatter.handle.assert_called_once_with( + self.craton_client.hosts.create.return_value ) def test_with_labels(self): @@ -329,9 +324,9 @@ class TestDoHostCreate(base.TestShellCommandUsingPrintDict): 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, + self.formatter.configure.assert_called_once_with(wrap=72) + self.formatter.handle.assert_called_once_with( + self.craton_client.hosts.create.return_value ) @@ -378,9 +373,9 @@ class TestDoHostUpdate(base.TestShellCommandUsingPrintDict): 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, + self.formatter.configure.assert_called_once_with(wrap=72) + self.formatter.handle.assert_called_once_with( + self.craton_client.hosts.update.return_value ) def test_with_name(self): @@ -397,9 +392,9 @@ class TestDoHostUpdate(base.TestShellCommandUsingPrintDict): 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, + self.formatter.configure.assert_called_once_with(wrap=72) + self.formatter.handle.assert_called_once_with( + self.craton_client.hosts.update.return_value ) def test_with_ip_address(self): @@ -416,9 +411,9 @@ class TestDoHostUpdate(base.TestShellCommandUsingPrintDict): 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, + self.formatter.configure.assert_called_once_with(wrap=72) + self.formatter.handle.assert_called_once_with( + self.craton_client.hosts.update.return_value ) def test_disable_host(self): @@ -434,9 +429,9 @@ class TestDoHostUpdate(base.TestShellCommandUsingPrintDict): 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, + self.formatter.configure.assert_called_once_with(wrap=72) + self.formatter.handle.assert_called_once_with( + self.craton_client.hosts.update.return_value ) def test_optional_parameters(self): @@ -465,9 +460,9 @@ class TestDoHostUpdate(base.TestShellCommandUsingPrintDict): 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, + self.formatter.configure.assert_called_once_with(wrap=72) + self.formatter.handle.assert_called_once_with( + self.craton_client.hosts.update.return_value )