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

2 files changed, 74 insertions, 9 deletions

diff --git a/openstackclient/tests/functional/common/test_module.py b/openstackclient/tests/functional/common/test_module.py
new file mode 100644
index 00000000..f56c1627
--- /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 eb54dbe0..4b586d3b 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)