tests: Move json decoding to base test class

We do this everywhere. Add a simple knob to simplify the pattern. Only
one use is migrated initially. The rest will be done separately.

Change-Id: Ic3b8958bd4fb1459a8ac3adaff216c2a26628491
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>

2 files changed, 48 insertions, 30 deletions

diff --git a/openstackclient/tests/functional/base.py b/openstackclient/tests/functional/base.py
index b6867f81..0c430267 100644
--- a/openstackclient/tests/functional/base.py
+++ b/openstackclient/tests/functional/base.py
@@ -10,6 +10,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import json
 import logging
 import os
 import shlex
@@ -48,33 +49,52 @@ def execute(cmd, fail_ok=False, merge_stderr=False):
 class TestCase(testtools.TestCase):
 
     @classmethod
-    def openstack(cls, cmd, cloud=ADMIN_CLOUD, fail_ok=False):
+    def openstack(
+        cls,
+        cmd,
+        *,
+        cloud=ADMIN_CLOUD,
+        fail_ok=False,
+        parse_output=False,
+    ):
         """Executes openstackclient command for the given action
 
-        NOTE(dtroyer): There is a subtle distinction between passing
-        cloud=None and cloud='': for compatibility reasons passing
-        cloud=None continues to include the option '--os-auth-type none'
-        in the command while passing cloud='' omits the '--os-auth-type'
-         option completely to let the default handlers be invoked.
+        :param cmd: A string representation of the command to execute.
+        :param cloud: The cloud to execute against. This can be a string, empty
+            string, or None. A string results in '--os-auth-type $cloud', an
+            empty string results in the '--os-auth-type' option being
+            omitted, and None resuts in '--os-auth-type none' for legacy
+            reasons.
+        :param fail_ok: If failure is permitted. If False (default), a command
+            failure will result in `~tempest.lib.exceptions.CommandFailed`
+            being raised.
+        :param parse_output: If true, pass the '-f json' parameter and decode
+            the output.
+        :returns: The output from the command.
+        :raises: `~tempest.lib.exceptions.CommandFailed` if the command failed
+            and ``fail_ok`` was ``False``.
         """
+        auth_args = []
         if cloud is None:
             # Execute command with no auth
-            return execute(
-                'openstack --os-auth-type none ' + cmd,
-                fail_ok=fail_ok
-            )
-        elif cloud == '':
-            # Execute command with no auth options at all
-            return execute(
-                'openstack ' + cmd,
-                fail_ok=fail_ok
-            )
-        else:
+            auth_args.append('--os-auth-type none')
+        elif cloud != '':
             # Execute command with an explicit cloud specified
-            return execute(
-                'openstack --os-cloud=' + cloud + ' ' + cmd,
-                fail_ok=fail_ok
-            )
+            auth_args.append(f'--os-cloud {cloud}')
+
+        format_args = []
+        if parse_output:
+            format_args.append('-f json')
+
+        output = execute(
+            ' '.join(['openstack'] + auth_args + [cmd] + format_args),
+            fail_ok=fail_ok,
+        )
+
+        if parse_output:
+            return json.loads(output)
+        else:
+            return output
 
     @classmethod
     def is_service_enabled(cls, service, version=None):
diff --git a/openstackclient/tests/functional/common/test_module.py b/openstackclient/tests/functional/common/test_module.py
index 41aabb7f..967d3b49 100644
--- a/openstackclient/tests/functional/common/test_module.py
+++ b/openstackclient/tests/functional/common/test_module.py
@@ -11,9 +11,6 @@
 #   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
 
@@ -31,14 +28,14 @@ class ModuleTest(base.TestCase):
 
     def test_module_list(self):
         # Test module list
-        cmd_output = json.loads(self.openstack('module list -f json'))
+        cmd_output = self.openstack('module list', parse_output=True)
         for one_module in self.CLIENTS:
             self.assertIn(one_module, cmd_output.keys())
         for one_module in self.LIBS:
             self.assertNotIn(one_module, cmd_output.keys())
 
         # Test module list --all
-        cmd_output = json.loads(self.openstack('module list --all -f json'))
+        cmd_output = self.openstack('module list --all', parse_output=True)
         for one_module in self.CLIENTS + self.LIBS:
             self.assertIn(one_module, cmd_output.keys())
 
@@ -56,7 +53,7 @@ class CommandTest(base.TestCase):
     ]
 
     def test_command_list_no_option(self):
-        cmd_output = json.loads(self.openstack('command list -f json'))
+        cmd_output = self.openstack('command list', parse_output=True)
         group_names = [each.get('Command Group') for each in cmd_output]
         for one_group in self.GROUPS:
             self.assertIn(one_group, group_names)
@@ -70,9 +67,10 @@ class CommandTest(base.TestCase):
             'compute.v2'
         ]
         for each_input in input_groups:
-            cmd_output = json.loads(self.openstack(
-                'command list --group %s -f json' % each_input
-            ))
+            cmd_output = self.openstack(
+                'command list --group %s' % each_input,
+                parse_output=True,
+            )
             group_names = [each.get('Command Group') for each in cmd_output]
             for each_name in group_names:
                 self.assertIn(each_input, each_name)