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

@@ -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)))

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())

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)

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 <https://bugs.launchpad.net/python-openstackclient/+bug/1661814>`_]