From 3afd2b7ff25af7e7998e9c8f4adac8a58a079675 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Sat, 4 Feb 2017 17:01:21 +0800 Subject: [PATCH] Fix "module list --all" failed KeyError cause the command "module list --all" failed, fix it, and do refactor to filter private modules and reduce the loop times, add related unit tests and functional tests. Change-Id: Icd77739502e05b5f763a04a92547497bf82d5d63 Closes-Bug: #1661814 --- openstackclient/common/module.py | 26 ++++++----- .../tests/functional/common/test_module.py | 40 +++++++++++++++++ .../tests/unit/common/test_module.py | 43 +++++++++++++++---- .../notes/bug-1661814-1692e68a1d2d9770.yaml | 6 +++ 4 files changed, 96 insertions(+), 19 deletions(-) create mode 100644 openstackclient/tests/functional/common/test_module.py create mode 100644 releasenotes/notes/bug-1661814-1692e68a1d2d9770.yaml diff --git a/openstackclient/common/module.py b/openstackclient/common/module.py index 15719a30e8..f471b2aa02 100644 --- a/openstackclient/common/module.py +++ b/openstackclient/common/module.py @@ -74,15 +74,21 @@ class ListModule(command.ShowOne): mods = sys.modules for k in mods.keys(): k = k.split('.')[0] - # TODO(dtroyer): Need a better way to decide which modules to - # show for the default (not --all) invocation. - # It should be just the things we actually care - # about like client and plugin modules... - if (parsed_args.all or 'client' in k): - try: - data[k] = mods[k].__version__ - except AttributeError: - # aw, just skip it - pass + # Skip private modules and the modules that had been added, + # like: keystoneclient, keystoneclient.exceptions and + # keystoneclient.auth + if not k.startswith('_') and k not in data: + # TODO(dtroyer): Need a better way to decide which modules to + # show for the default (not --all) invocation. + # It should be just the things we actually care + # about like client and plugin modules... + if (parsed_args.all or + # Handle xxxclient and openstacksdk + (k.endswith('client') or k == 'openstack')): + try: + data[k] = mods[k].__version__ + except Exception: + # Catch all exceptions, just skip it + pass return zip(*sorted(six.iteritems(data))) diff --git a/openstackclient/tests/functional/common/test_module.py b/openstackclient/tests/functional/common/test_module.py new file mode 100644 index 0000000000..f56c1627bb --- /dev/null +++ b/openstackclient/tests/functional/common/test_module.py @@ -0,0 +1,40 @@ +# Copyright 2017 Huawei, Inc. All rights reserved. +# +# 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. +# + +import json + +from openstackclient.tests.functional import base + + +class ModuleTest(base.TestCase): + """Functional tests for openstackclient module list output.""" + + CLIENTS = ['openstackclient', + 'keystoneclient', + 'novaclient'] + + LIBS = ['osc_lib', + 'os_client_config', + 'keystoneauth1'] + + def test_module_list_no_options(self): + json_output = json.loads(self.openstack('module list -f json')) + for one_module in self.CLIENTS: + self.assertIn(one_module, json_output.keys()) + + def test_module_list_with_all_option(self): + json_output = json.loads(self.openstack('module list --all -f json')) + for one_module in (self.CLIENTS + self.LIBS): + self.assertIn(one_module, json_output.keys()) diff --git a/openstackclient/tests/unit/common/test_module.py b/openstackclient/tests/unit/common/test_module.py index eb54dbe04b..4b586d3b2d 100644 --- a/openstackclient/tests/unit/common/test_module.py +++ b/openstackclient/tests/unit/common/test_module.py @@ -26,19 +26,28 @@ from openstackclient.tests.unit import utils # currently == '*client*' module_name_1 = 'fakeclient' module_version_1 = '0.1.2' -MODULE_1 = { - '__version__': module_version_1, -} module_name_2 = 'zlib' module_version_2 = '1.1' -MODULE_2 = { - '__version__': module_version_2, -} + +# module_3 match openstacksdk +module_name_3 = 'openstack' +module_version_3 = '0.9.13' + +# module_4 match sub module of fakeclient +module_name_4 = 'fakeclient.submodule' +module_version_4 = '0.2.2' + +# module_5 match private module +module_name_5 = '_private_module.lib' +module_version_5 = '0.0.1' MODULES = { module_name_1: fakes.FakeModule(module_name_1, module_version_1), module_name_2: fakes.FakeModule(module_name_2, module_version_2), + module_name_3: fakes.FakeModule(module_name_3, module_version_3), + module_name_4: fakes.FakeModule(module_name_4, module_version_4), + module_name_5: fakes.FakeModule(module_name_5, module_version_5), } @@ -105,9 +114,18 @@ class TestModuleList(utils.TestCommand): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - # Additional modules may be present, just check our additions + # Output xxxclient and openstacksdk, but not regular module, like: zlib self.assertIn(module_name_1, columns) self.assertIn(module_version_1, data) + self.assertNotIn(module_name_2, columns) + self.assertNotIn(module_version_2, data) + self.assertIn(module_name_3, columns) + self.assertIn(module_version_3, data) + # Filter sub and private modules + self.assertNotIn(module_name_4, columns) + self.assertNotIn(module_version_4, data) + self.assertNotIn(module_name_5, columns) + self.assertNotIn(module_version_5, data) def test_module_list_all(self): arglist = [ @@ -123,8 +141,15 @@ class TestModuleList(utils.TestCommand): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - # Additional modules may be present, just check our additions + # Output xxxclient, openstacksdk and regular module, like: zlib self.assertIn(module_name_1, columns) - self.assertIn(module_name_2, columns) self.assertIn(module_version_1, data) + self.assertIn(module_name_2, columns) self.assertIn(module_version_2, data) + self.assertIn(module_name_3, columns) + self.assertIn(module_version_3, data) + # Filter sub and private modules + self.assertNotIn(module_name_4, columns) + self.assertNotIn(module_version_4, data) + self.assertNotIn(module_name_5, columns) + self.assertNotIn(module_version_5, data) diff --git a/releasenotes/notes/bug-1661814-1692e68a1d2d9770.yaml b/releasenotes/notes/bug-1661814-1692e68a1d2d9770.yaml new file mode 100644 index 0000000000..b321138744 --- /dev/null +++ b/releasenotes/notes/bug-1661814-1692e68a1d2d9770.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fix ``module list --all`` command failed, and enhance the related unit + tests and funcational tests. + [Bug `1661814 `_]