From acc2d106abfb4fed0ff5d0d0c246d69f9ea1758b Mon Sep 17 00:00:00 2001 From: Ankur Gupta Date: Fri, 24 Mar 2017 12:39:22 -0500 Subject: [PATCH] Refactor Extension show and list command 1.keep the column display order consist in extension list with and without "--long" option. 2.rework for network extentsion list, openstacksdk return object, so the logic should be same with other service. 3.add some unit test cases, like: extension list --network --long, extension list --network --compute, to cover regular use cases. 4.raise exact exception when network extension don't exist, avoid internal TypeError in "extension show" commands. Change-Id: I2e23ced80d8da8aa1106b22472db850367b351ce Closes-Bug: #1689233 --- openstackclient/common/extension.py | 47 ++++------- .../tests/functional/common/test_extension.py | 31 +++++++- .../tests/unit/common/test_extension.py | 78 +++++++++++++++---- .../notes/bug-1689233-c3f98e159c75374e.yaml | 7 ++ 4 files changed, 111 insertions(+), 52 deletions(-) create mode 100644 releasenotes/notes/bug-1689233-c3f98e159c75374e.yaml diff --git a/openstackclient/common/extension.py b/openstackclient/common/extension.py index d5b722389..139f43abf 100644 --- a/openstackclient/common/extension.py +++ b/openstackclient/common/extension.py @@ -15,7 +15,6 @@ """Extension action implementations""" -import itertools import logging from osc_lib.command import command @@ -66,8 +65,8 @@ class ListExtension(command.Lister): def take_action(self, parsed_args): if parsed_args.long: - columns = ('Name', 'Namespace', 'Description', - 'Alias', 'Updated', 'Links') + columns = ('Name', 'Alias', 'Description', + 'Namespace', 'Updated', 'Links') else: columns = ('Name', 'Alias', 'Description') @@ -104,35 +103,22 @@ class ListExtension(command.Lister): "Block Storage API") LOG.warning(message) - # Resource classes for the above + if parsed_args.network or show_all: + network_client = self.app.client_manager.network + try: + data += network_client.extensions() + except Exception: + message = _("Failed to retrieve extensions list " + "from Network API") + LOG.warning(message) + extension_tuples = ( utils.get_item_properties( s, columns, - formatters={}, ) for s in data ) - # Dictionaries for the below - if parsed_args.network or show_all: - network_client = self.app.client_manager.network - try: - data = network_client.extensions() - dict_tuples = ( - utils.get_item_properties( - s, - columns, - formatters={}, - ) for s in data - ) - extension_tuples = itertools.chain( - extension_tuples, - dict_tuples - ) - except Exception: - message = _("Extensions list not supported by Network API") - LOG.warning(message) - return (columns, extension_tuples) @@ -152,14 +138,7 @@ class ShowExtension(command.ShowOne): def take_action(self, parsed_args): client = self.app.client_manager.network - columns = ('Alias', 'Description', 'Links', 'Name', - 'Namespace', 'Updated') ext = str(parsed_args.extension) - obj = client.find_extension(ext) - dict_tuples = (utils.get_item_properties( - obj, - columns, - formatters={},) - ) + obj = client.find_extension(ext, ignore_missing=False).to_dict() - return columns, dict_tuples + return zip(*sorted(obj.items())) diff --git a/openstackclient/tests/functional/common/test_extension.py b/openstackclient/tests/functional/common/test_extension.py index cc4cb7e16..d7dc398b5 100644 --- a/openstackclient/tests/functional/common/test_extension.py +++ b/openstackclient/tests/functional/common/test_extension.py @@ -15,6 +15,8 @@ import json +from tempest.lib import exceptions as tempest_exc + from openstackclient.tests.functional import base @@ -23,7 +25,6 @@ class ExtensionTests(base.TestCase): @classmethod def setUpClass(cls): - # super(NetworkTests, cls).setUp() cls.haz_network = base.is_service_enabled('network') def test_extension_list_compute(self): @@ -38,6 +39,18 @@ class ExtensionTests(base.TestCase): name_list, ) + def test_extension_list_volume(self): + """Test volume extension list""" + json_output = json.loads(self.openstack( + 'extension list -f json ' + + '--volume' + )) + name_list = [item.get('Name') for item in json_output] + self.assertIn( + 'TypesManage', + name_list, + ) + def test_extension_list_network(self): """Test network extension list""" if not self.haz_network: @@ -79,5 +92,19 @@ class ExtensionTests(base.TestCase): )) self.assertEqual( name, - json_output.get('Alias'), + json_output.get('alias'), ) + + def test_extension_show_not_exist(self): + """Test extension show with not existed name""" + if not self.haz_network: + self.skipTest("No Network service present") + + name = 'not_existed_ext' + try: + self.openstack('extension show ' + name) + except tempest_exc.CommandFailed as e: + self.assertIn('ResourceNotFound', str(e)) + self.assertIn(name, str(e)) + else: + self.fail('CommandFailed should be raised') diff --git a/openstackclient/tests/unit/common/test_extension.py b/openstackclient/tests/unit/common/test_extension.py index 68fdf17d4..eefa814c4 100644 --- a/openstackclient/tests/unit/common/test_extension.py +++ b/openstackclient/tests/unit/common/test_extension.py @@ -67,7 +67,7 @@ class TestExtension(utils.TestCommand): class TestExtensionList(TestExtension): columns = ('Name', 'Alias', 'Description') - long_columns = ('Name', 'Namespace', 'Description', 'Alias', 'Updated', + long_columns = ('Name', 'Alias', 'Description', 'Namespace', 'Updated', 'Links') volume_extension = volume_fakes.FakeExtension.create_one_extension() @@ -145,33 +145,33 @@ class TestExtensionList(TestExtension): datalist = ( ( self.identity_extension.name, - self.identity_extension.namespace, - self.identity_extension.description, self.identity_extension.alias, + self.identity_extension.description, + self.identity_extension.namespace, self.identity_extension.updated, self.identity_extension.links, ), ( self.compute_extension.name, - self.compute_extension.namespace, - self.compute_extension.description, self.compute_extension.alias, + self.compute_extension.description, + self.compute_extension.namespace, self.compute_extension.updated, self.compute_extension.links, ), ( self.volume_extension.name, - self.volume_extension.namespace, - self.volume_extension.description, self.volume_extension.alias, + self.volume_extension.description, + self.volume_extension.namespace, self.volume_extension.updated, self.volume_extension.links, ), ( self.network_extension.name, - self.network_extension.namespace, - self.network_extension.description, self.network_extension.alias, + self.network_extension.description, + self.network_extension.namespace, self.network_extension.updated, self.network_extension.links, ), @@ -214,6 +214,27 @@ class TestExtensionList(TestExtension): self._test_extension_list_helper(arglist, verifylist, datalist) self.network_extensions_mock.assert_called_with() + def test_extension_list_network_with_long(self): + arglist = [ + '--network', + '--long', + ] + verifylist = [ + ('network', True), + ('long', True), + ] + datalist = (( + self.network_extension.name, + self.network_extension.alias, + self.network_extension.description, + self.network_extension.namespace, + self.network_extension.updated, + self.network_extension.links, + ), ) + self._test_extension_list_helper(arglist, verifylist, datalist, + long=True) + self.network_extensions_mock.assert_called_with() + def test_extension_list_compute(self): arglist = [ '--compute', @@ -229,6 +250,31 @@ class TestExtensionList(TestExtension): self._test_extension_list_helper(arglist, verifylist, datalist) self.compute_extensions_mock.show_all.assert_called_with() + def test_extension_list_compute_and_network(self): + arglist = [ + '--compute', + '--network', + ] + verifylist = [ + ('compute', True), + ('network', True), + ] + datalist = ( + ( + self.compute_extension.name, + self.compute_extension.alias, + self.compute_extension.description, + ), + ( + self.network_extension.name, + self.network_extension.alias, + self.network_extension.description, + ), + ) + self._test_extension_list_helper(arglist, verifylist, datalist) + self.compute_extensions_mock.show_all.assert_called_with() + self.network_extensions_mock.assert_called_with() + def test_extension_list_volume(self): arglist = [ '--volume', @@ -251,12 +297,12 @@ class TestExtensionShow(TestExtension): ) columns = ( - 'Alias', - 'Description', - 'Links', - 'Name', - 'Namespace', - 'Updated' + 'alias', + 'description', + 'links', + 'name', + 'namespace', + 'updated', ) data = ( @@ -296,7 +342,7 @@ class TestExtensionShow(TestExtension): columns, data = self.cmd.take_action(parsed_args) self.app.client_manager.network.find_extension.assert_called_with( - self.extension_details.alias) + self.extension_details.alias, ignore_missing=False) self.assertEqual(self.columns, columns) self.assertEqual(self.data, data) diff --git a/releasenotes/notes/bug-1689233-c3f98e159c75374e.yaml b/releasenotes/notes/bug-1689233-c3f98e159c75374e.yaml new file mode 100644 index 000000000..3cecc46e3 --- /dev/null +++ b/releasenotes/notes/bug-1689233-c3f98e159c75374e.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Raise exact exception when extension don't exist in ``extension show`` + command, and keep the column display order consist in ``extension list`` + with and without ``--long`` option. + [Bug `1689233 `_]