Skip to content

Commit 3afd2b7

Browse files
kiwikkiwik
committed
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
1 parent 083b115 commit 3afd2b7

4 files changed

Lines changed: 96 additions & 19 deletions

File tree

‎openstackclient/common/module.py‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,15 +74,21 @@ def take_action(self, parsed_args):
7474
mods = sys.modules
7575
for k in mods.keys():
7676
k = k.split('.')[0]
77-
# TODO(dtroyer): Need a better way to decide which modules to
78-
# show for the default (not --all) invocation.
79-
# It should be just the things we actually care
80-
# about like client and plugin modules...
81-
if (parsed_args.all or 'client' in k):
82-
try:
83-
data[k] = mods[k].__version__
84-
except AttributeError:
85-
# aw, just skip it
86-
pass
77+
# Skip private modules and the modules that had been added,
78+
# like: keystoneclient, keystoneclient.exceptions and
79+
# keystoneclient.auth
80+
if not k.startswith('_') and k not in data:
81+
# TODO(dtroyer): Need a better way to decide which modules to
82+
# show for the default (not --all) invocation.
83+
# It should be just the things we actually care
84+
# about like client and plugin modules...
85+
if (parsed_args.all or
86+
# Handle xxxclient and openstacksdk
87+
(k.endswith('client') or k == 'openstack')):
88+
try:
89+
data[k] = mods[k].__version__
90+
except Exception:
91+
# Catch all exceptions, just skip it
92+
pass
8793

8894
return zip(*sorted(six.iteritems(data)))

‎openstackclient/tests/functional/common/test_module.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Copyright 2017 Huawei, Inc. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License"); you may
4+
# not use this file except in compliance with the License. You may obtain
5+
# a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
11+
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
12+
# License for the specific language governing permissions and limitations
13+
# under the License.
14+
#
15+
16+
import json
17+
18+
from openstackclient.tests.functional import base
19+
20+
21+
class ModuleTest(base.TestCase):
22+
"""Functional tests for openstackclient module list output."""
23+
24+
CLIENTS = ['openstackclient',
25+
'keystoneclient',
26+
'novaclient']
27+
28+
LIBS = ['osc_lib',
29+
'os_client_config',
30+
'keystoneauth1']
31+
32+
def test_module_list_no_options(self):
33+
json_output = json.loads(self.openstack('module list -f json'))
34+
for one_module in self.CLIENTS:
35+
self.assertIn(one_module, json_output.keys())
36+
37+
def test_module_list_with_all_option(self):
38+
json_output = json.loads(self.openstack('module list --all -f json'))
39+
for one_module in (self.CLIENTS + self.LIBS):
40+
self.assertIn(one_module, json_output.keys())

‎openstackclient/tests/unit/common/test_module.py‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,28 @@
2626
# currently == '*client*'
2727
module_name_1 = 'fakeclient'
2828
module_version_1 = '0.1.2'
29-
MODULE_1 = {
30-
'__version__': module_version_1,
31-
}
3229

3330
module_name_2 = 'zlib'
3431
module_version_2 = '1.1'
35-
MODULE_2 = {
36-
'__version__': module_version_2,
37-
}
32+
33+
# module_3 match openstacksdk
34+
module_name_3 = 'openstack'
35+
module_version_3 = '0.9.13'
36+
37+
# module_4 match sub module of fakeclient
38+
module_name_4 = 'fakeclient.submodule'
39+
module_version_4 = '0.2.2'
40+
41+
# module_5 match private module
42+
module_name_5 = '_private_module.lib'
43+
module_version_5 = '0.0.1'
3844

3945
MODULES = {
4046
module_name_1: fakes.FakeModule(module_name_1, module_version_1),
4147
module_name_2: fakes.FakeModule(module_name_2, module_version_2),
48+
module_name_3: fakes.FakeModule(module_name_3, module_version_3),
49+
module_name_4: fakes.FakeModule(module_name_4, module_version_4),
50+
module_name_5: fakes.FakeModule(module_name_5, module_version_5),
4251
}
4352

4453

@@ -105,9 +114,18 @@ def test_module_list_no_options(self):
105114
# containing the data to be listed.
106115
columns, data = self.cmd.take_action(parsed_args)
107116

108-
# Additional modules may be present, just check our additions
117+
# Output xxxclient and openstacksdk, but not regular module, like: zlib
109118
self.assertIn(module_name_1, columns)
110119
self.assertIn(module_version_1, data)
120+
self.assertNotIn(module_name_2, columns)
121+
self.assertNotIn(module_version_2, data)
122+
self.assertIn(module_name_3, columns)
123+
self.assertIn(module_version_3, data)
124+
# Filter sub and private modules
125+
self.assertNotIn(module_name_4, columns)
126+
self.assertNotIn(module_version_4, data)
127+
self.assertNotIn(module_name_5, columns)
128+
self.assertNotIn(module_version_5, data)
111129

112130
def test_module_list_all(self):
113131
arglist = [
@@ -123,8 +141,15 @@ def test_module_list_all(self):
123141
# containing the data to be listed.
124142
columns, data = self.cmd.take_action(parsed_args)
125143

126-
# Additional modules may be present, just check our additions
144+
# Output xxxclient, openstacksdk and regular module, like: zlib
127145
self.assertIn(module_name_1, columns)
128-
self.assertIn(module_name_2, columns)
129146
self.assertIn(module_version_1, data)
147+
self.assertIn(module_name_2, columns)
130148
self.assertIn(module_version_2, data)
149+
self.assertIn(module_name_3, columns)
150+
self.assertIn(module_version_3, data)
151+
# Filter sub and private modules
152+
self.assertNotIn(module_name_4, columns)
153+
self.assertNotIn(module_version_4, data)
154+
self.assertNotIn(module_name_5, columns)
155+
self.assertNotIn(module_version_5, data)

‎releasenotes/notes/bug-1661814-1692e68a1d2d9770.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
Fix ``module list --all`` command failed, and enhance the related unit
5+
tests and funcational tests.
6+
[Bug `1661814 <https://bugs.launchpad.net/python-openstackclient/+bug/1661814>`_]

0 commit comments

Comments
 (0)