Merge "Use cliff formattable columns in image commands"

9 files changed, 69 insertions, 66 deletions

diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py
index caf3d54f..c2dab3ee 100644
--- a/openstackclient/image/v1/image.py
+++ b/openstackclient/image/v1/image.py
@@ -21,8 +21,10 @@ import logging
 import os
 import sys
 
+from cliff import columns as cliff_columns
 from glanceclient.common import utils as gc_utils
 from osc_lib.api import utils as api_utils
+from osc_lib.cli import format_columns
 from osc_lib.cli import parseractions
 from osc_lib.command import command
 from osc_lib import utils
@@ -46,19 +48,18 @@ DISK_CHOICES = ["ami", "ari", "aki", "vhd", "vmdk", "raw", "qcow2", "vhdx",
 LOG = logging.getLogger(__name__)
 
 
-def _format_visibility(data):
-    """Return a formatted visibility string
+class VisibilityColumn(cliff_columns.FormattableColumn):
+    def human_readable(self):
+        """Return a formatted visibility string
 
-    :param data:
-        The server's visibility (is_public) status value: True, False
-    :rtype:
-        A string formatted to public/private
-    """
+        :rtype:
+            A string formatted to public/private
+        """
 
-    if data:
-        return 'public'
-    else:
-        return 'private'
+        if self._value:
+            return 'public'
+        else:
+            return 'private'
 
 
 class CreateImage(command.ShowOne):
@@ -268,7 +269,8 @@ class CreateImage(command.ShowOne):
                     kwargs['data'].close()
 
             info.update(image._info)
-            info['properties'] = utils.format_dict(info.get('properties', {}))
+            info['properties'] = format_columns.DictColumn(
+                info.get('properties', {}))
         return zip(*sorted(six.iteritems(info)))
 
 
@@ -429,8 +431,8 @@ class ListImage(command.Lister):
                 s,
                 columns,
                 formatters={
-                    'is_public': _format_visibility,
-                    'properties': utils.format_dict,
+                    'is_public': VisibilityColumn,
+                    'properties': format_columns.DictColumn,
                 },
             ) for s in data)
         )
@@ -714,5 +716,6 @@ class ShowImage(command.ShowOne):
         if parsed_args.human_readable:
             if 'size' in info:
                 info['size'] = utils.format_size(info['size'])
-        info['properties'] = utils.format_dict(info.get('properties', {}))
+        info['properties'] = format_columns.DictColumn(
+            info.get('properties', {}))
         return zip(*sorted(six.iteritems(info)))
diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py
index 97169a92..2b10c3ad 100644
--- a/openstackclient/image/v2/image.py
+++ b/openstackclient/image/v2/image.py
@@ -22,6 +22,7 @@ import logging
 from glanceclient.common import utils as gc_utils
 from openstack.image import image_signer
 from osc_lib.api import utils as api_utils
+from osc_lib.cli import format_columns
 from osc_lib.cli import parseractions
 from osc_lib.command import command
 from osc_lib import exceptions
@@ -65,11 +66,11 @@ def _format_image(image):
             properties[key] = image.get(key)
 
     # format the tags if they are there
-    info['tags'] = utils.format_list(image.get('tags'))
+    info['tags'] = format_columns.ListColumn(image.get('tags'))
 
     # add properties back into the dictionary as a top-level key
     if properties:
-        info['properties'] = utils.format_dict(properties)
+        info['properties'] = format_columns.DictColumn(properties)
 
     return info
 
@@ -656,7 +657,7 @@ class ListImage(command.Lister):
                 s,
                 columns,
                 formatters={
-                    'tags': utils.format_list,
+                    'tags': format_columns.ListColumn,
                 },
             ) for s in data)
         )
diff --git a/openstackclient/tests/functional/image/v1/test_image.py b/openstackclient/tests/functional/image/v1/test_image.py
index 30490bf5..b9774ab5 100644
--- a/openstackclient/tests/functional/image/v1/test_image.py
+++ b/openstackclient/tests/functional/image/v1/test_image.py
@@ -104,6 +104,6 @@ class ImageTests(base.BaseImageTests):
             self.name
         ))
         self.assertEqual(
-            "a='b', c='d'",
+            {'a': 'b', 'c': 'd'},
             json_output["properties"],
         )
diff --git a/openstackclient/tests/functional/image/v2/test_image.py b/openstackclient/tests/functional/image/v2/test_image.py
index 3185c3bd..264ba519 100644
--- a/openstackclient/tests/functional/image/v2/test_image.py
+++ b/openstackclient/tests/functional/image/v2/test_image.py
@@ -14,7 +14,6 @@ import json
 import uuid
 
 import fixtures
-# from glanceclient import exc as image_exceptions
 
 from openstackclient.tests.functional.image import base
 
@@ -81,7 +80,7 @@ class ImageTests(base.BaseImageTests):
         json_output = json.loads(self.openstack(
             'image list --tag ' + self.image_tag + ' --long -f json'
         ))
-        for taglist in [img['Tags'].split(', ') for img in json_output]:
+        for taglist in [img['Tags'] for img in json_output]:
             self.assertIn(
                 self.image_tag,
                 taglist
@@ -127,10 +126,8 @@ class ImageTests(base.BaseImageTests):
             'image show -f json ' +
             self.name
         ))
-        # NOTE(dtroyer): Don't do a full-string compare so we are tolerant of
-        #                new artributes in the returned data
-        self.assertIn("a='b'", json_output["properties"])
-        self.assertIn("c='d'", json_output["properties"])
+        self.assertIn("a", json_output["properties"])
+        self.assertIn("c", json_output["properties"])
 
         self.openstack(
             'image unset ' +
@@ -142,15 +139,13 @@ class ImageTests(base.BaseImageTests):
             'image show -f json ' +
             self.name
         ))
-        # NOTE(dtroyer): Don't do a full-string compare so we are tolerant of
-        #                new artributes in the returned data
-        self.assertNotIn("a='b'", json_output["properties"])
-        self.assertNotIn("c='d'", json_output["properties"])
+        self.assertNotIn("a", json_output["properties"])
+        self.assertNotIn("c", json_output["properties"])
 
         # Test tags
         self.assertNotIn(
             '01',
-            json_output["tags"].split(', ')
+            json_output["tags"]
         )
         self.openstack(
             'image set ' +
@@ -163,7 +158,7 @@ class ImageTests(base.BaseImageTests):
         ))
         self.assertIn(
             '01',
-            json_output["tags"].split(', ')
+            json_output["tags"]
         )
 
         self.openstack(
@@ -177,7 +172,7 @@ class ImageTests(base.BaseImageTests):
         ))
         self.assertNotIn(
             '01',
-            json_output["tags"].split(', ')
+            json_output["tags"]
         )
 
     def test_image_set_rename(self):
diff --git a/openstackclient/tests/unit/compute/v2/test_server_backup.py b/openstackclient/tests/unit/compute/v2/test_server_backup.py
index 9a481e0a..24a94531 100644
--- a/openstackclient/tests/unit/compute/v2/test_server_backup.py
+++ b/openstackclient/tests/unit/compute/v2/test_server_backup.py
@@ -13,6 +13,7 @@
 
 import mock
 
+from osc_lib.cli import format_columns
 from osc_lib import exceptions
 from osc_lib import utils as common_utils
 
@@ -69,7 +70,7 @@ class TestServerBackupCreate(TestServerBackup):
             image['owner'],
             image['protected'],
             'active',
-            common_utils.format_list(image.get('tags')),
+            format_columns.ListColumn(image.get('tags')),
             image['visibility'],
         )
         return datalist
@@ -134,7 +135,7 @@ class TestServerBackupCreate(TestServerBackup):
         )
 
         self.assertEqual(self.image_columns(images[0]), columns)
-        self.assertEqual(self.image_data(images[0]), data)
+        self.assertItemEqual(self.image_data(images[0]), data)
 
     def test_server_backup_create_options(self):
         servers = self.setup_servers_mock(count=1)
@@ -168,7 +169,7 @@ class TestServerBackupCreate(TestServerBackup):
         )
 
         self.assertEqual(self.image_columns(images[0]), columns)
-        self.assertEqual(self.image_data(images[0]), data)
+        self.assertItemEqual(self.image_data(images[0]), data)
 
     @mock.patch.object(common_utils, 'wait_for_status', return_value=False)
     def test_server_backup_wait_fail(self, mock_wait_for_status):
@@ -268,4 +269,4 @@ class TestServerBackupCreate(TestServerBackup):
         )
 
         self.assertEqual(self.image_columns(images[0]), columns)
-        self.assertEqual(self.image_data(images[0]), data)
+        self.assertItemEqual(self.image_data(images[0]), data)
diff --git a/openstackclient/tests/unit/compute/v2/test_server_image.py b/openstackclient/tests/unit/compute/v2/test_server_image.py
index 47bd682f..02e43129 100644
--- a/openstackclient/tests/unit/compute/v2/test_server_image.py
+++ b/openstackclient/tests/unit/compute/v2/test_server_image.py
@@ -12,6 +12,7 @@
 #
 import mock
 
+from osc_lib.cli import format_columns
 from osc_lib import exceptions
 from osc_lib import utils as common_utils
 
@@ -67,7 +68,7 @@ class TestServerImageCreate(TestServerImage):
             image['owner'],
             image['protected'],
             'active',
-            common_utils.format_list(image.get('tags')),
+            format_columns.ListColumn(image.get('tags')),
             image['visibility'],
         )
         return datalist
@@ -129,7 +130,7 @@ class TestServerImageCreate(TestServerImage):
         )
 
         self.assertEqual(self.image_columns(images[0]), columns)
-        self.assertEqual(self.image_data(images[0]), data)
+        self.assertItemEqual(self.image_data(images[0]), data)
 
     def test_server_image_create_options(self):
         servers = self.setup_servers_mock(count=1)
@@ -157,7 +158,7 @@ class TestServerImageCreate(TestServerImage):
         )
 
         self.assertEqual(self.image_columns(images[0]), columns)
-        self.assertEqual(self.image_data(images[0]), data)
+        self.assertItemEqual(self.image_data(images[0]), data)
 
     @mock.patch.object(common_utils, 'wait_for_status', return_value=False)
     def test_server_create_image_wait_fail(self, mock_wait_for_status):
@@ -225,4 +226,4 @@ class TestServerImageCreate(TestServerImage):
         )
 
         self.assertEqual(self.image_columns(images[0]), columns)
-        self.assertEqual(self.image_data(images[0]), data)
+        self.assertItemEqual(self.image_data(images[0]), data)
diff --git a/openstackclient/tests/unit/image/v1/test_image.py b/openstackclient/tests/unit/image/v1/test_image.py
index 6babd4ff..0997d765 100644
--- a/openstackclient/tests/unit/image/v1/test_image.py
+++ b/openstackclient/tests/unit/image/v1/test_image.py
@@ -16,8 +16,9 @@
 import copy
 
 import mock
+
+from osc_lib.cli import format_columns
 from osc_lib import exceptions
-from osc_lib import utils
 
 from openstackclient.image.v1 import image
 from openstackclient.tests.unit import fakes
@@ -58,7 +59,7 @@ class TestImageCreate(TestImage):
         new_image.min_ram,
         new_image.name,
         new_image.owner,
-        utils.format_dict(new_image.properties),
+        format_columns.DictColumn(new_image.properties),
         new_image.protected,
     )
 
@@ -106,7 +107,7 @@ class TestImageCreate(TestImage):
         self.assertEqual(self.images_mock.update.call_args_list, [])
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_image_reserve_options(self):
         mock_exception = {
@@ -160,7 +161,7 @@ class TestImageCreate(TestImage):
         self.assertEqual(self.images_mock.update.call_args_list, [])
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     @mock.patch('openstackclient.image.v1.image.io.open', name='Open')
     def test_image_create_file(self, mock_open):
@@ -224,7 +225,7 @@ class TestImageCreate(TestImage):
         self.assertEqual(self.images_mock.update.call_args_list, [])
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
 
 class TestImageDelete(TestImage):
@@ -410,12 +411,13 @@ class TestImageList(TestImage):
             '',
             '',
             '',
-            'public',
+            image.VisibilityColumn(True),
             False,
             self._image.owner,
-            "Alpha='a', Beta='b', Gamma='g'",
+            format_columns.DictColumn(
+                {'Alpha': 'a', 'Beta': 'b', 'Gamma': 'g'}),
         ), )
-        self.assertEqual(datalist, tuple(data))
+        self.assertListItemEqual(datalist, tuple(data))
 
     @mock.patch('osc_lib.api.utils.simple_filter')
     def test_image_list_property_option(self, sf_mock):
@@ -742,7 +744,7 @@ class TestImageShow(TestImage):
         _image.min_ram,
         _image.name,
         _image.owner,
-        utils.format_dict(_image.properties),
+        format_columns.DictColumn(_image.properties),
         _image.protected,
         _image.size,
     )
@@ -773,7 +775,7 @@ class TestImageShow(TestImage):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_image_show_human_readable(self):
         arglist = [
diff --git a/openstackclient/tests/unit/image/v2/fakes.py b/openstackclient/tests/unit/image/v2/fakes.py
index d82d8114..f69a2bc3 100644
--- a/openstackclient/tests/unit/image/v2/fakes.py
+++ b/openstackclient/tests/unit/image/v2/fakes.py
@@ -19,7 +19,7 @@ import uuid
 
 from glanceclient.v2 import schemas
 import mock
-from osc_lib import utils as common_utils
+from osc_lib.cli import format_columns
 import warlock
 
 from openstackclient.tests.unit import fakes
@@ -48,7 +48,7 @@ IMAGE_columns = tuple(sorted(IMAGE))
 IMAGE_data = tuple((IMAGE[x] for x in sorted(IMAGE)))
 
 IMAGE_SHOW = copy.copy(IMAGE)
-IMAGE_SHOW['tags'] = ''
+IMAGE_SHOW['tags'] = format_columns.ListColumn(IMAGE_SHOW['tags'])
 IMAGE_SHOW_data = tuple((IMAGE_SHOW[x] for x in sorted(IMAGE_SHOW)))
 
 # Just enough v2 schema to do some testing
@@ -281,7 +281,7 @@ class FakeImage(object):
                 if x == 'tags':
                     # The 'tags' should be format_list
                     data_list.append(
-                        common_utils.format_list(getattr(image, x)))
+                        format_columns.ListColumn(getattr(image, x)))
                 else:
                     data_list.append(getattr(image, x))
         return tuple(data_list)
diff --git a/openstackclient/tests/unit/image/v2/test_image.py b/openstackclient/tests/unit/image/v2/test_image.py
index 45b5a0a8..748a61aa 100644
--- a/openstackclient/tests/unit/image/v2/test_image.py
+++ b/openstackclient/tests/unit/image/v2/test_image.py
@@ -18,8 +18,8 @@ import copy
 from glanceclient.common import utils as glanceclient_utils
 from glanceclient.v2 import schemas
 import mock
+from osc_lib.cli import format_columns
 from osc_lib import exceptions
-from osc_lib import utils as common_utils
 import warlock
 
 from openstackclient.image.v2 import image
@@ -116,7 +116,7 @@ class TestImageCreate(TestImage):
         self.assertEqual(
             image_fakes.FakeImage.get_image_columns(self.new_image),
             columns)
-        self.assertEqual(
+        self.assertItemEqual(
             image_fakes.FakeImage.get_image_data(self.new_image),
             data)
 
@@ -184,7 +184,7 @@ class TestImageCreate(TestImage):
         self.assertEqual(
             image_fakes.FakeImage.get_image_columns(self.new_image),
             columns)
-        self.assertEqual(
+        self.assertItemEqual(
             image_fakes.FakeImage.get_image_data(self.new_image),
             data)
 
@@ -284,7 +284,7 @@ class TestImageCreate(TestImage):
         self.assertEqual(
             image_fakes.FakeImage.get_image_columns(self.new_image),
             columns)
-        self.assertEqual(
+        self.assertItemEqual(
             image_fakes.FakeImage.get_image_data(self.new_image),
             data)
 
@@ -508,7 +508,7 @@ class TestImageList(TestImage):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        self.assertListItemEqual(self.datalist, tuple(data))
 
     def test_image_list_public_option(self):
         arglist = [
@@ -533,7 +533,7 @@ class TestImageList(TestImage):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        self.assertListItemEqual(self.datalist, tuple(data))
 
     def test_image_list_private_option(self):
         arglist = [
@@ -558,7 +558,7 @@ class TestImageList(TestImage):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        self.assertListItemEqual(self.datalist, tuple(data))
 
     def test_image_list_community_option(self):
         arglist = [
@@ -608,7 +608,7 @@ class TestImageList(TestImage):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        self.assertListItemEqual(self.datalist, tuple(data))
 
     def test_image_list_shared_member_status_option(self):
         arglist = [
@@ -696,9 +696,9 @@ class TestImageList(TestImage):
             self._image.visibility,
             self._image.protected,
             self._image.owner,
-            common_utils.format_list(self._image.tags),
+            format_columns.ListColumn(self._image.tags),
         ), )
-        self.assertEqual(datalist, tuple(data))
+        self.assertListItemEqual(datalist, tuple(data))
 
     @mock.patch('osc_lib.api.utils.simple_filter')
     def test_image_list_property_option(self, sf_mock):
@@ -727,7 +727,7 @@ class TestImageList(TestImage):
         )
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        self.assertListItemEqual(self.datalist, tuple(data))
 
     @mock.patch('osc_lib.utils.sort_items')
     def test_image_list_sort_option(self, si_mock):
@@ -750,7 +750,7 @@ class TestImageList(TestImage):
             str,
         )
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, tuple(data))
+        self.assertListItemEqual(self.datalist, tuple(data))
 
     def test_image_list_limit_option(self):
         ret_limit = 1
@@ -1460,7 +1460,7 @@ class TestImageShow(TestImage):
         )
 
         self.assertEqual(image_fakes.IMAGE_columns, columns)
-        self.assertEqual(image_fakes.IMAGE_SHOW_data, data)
+        self.assertItemEqual(image_fakes.IMAGE_SHOW_data, data)
 
     def test_image_show_human_readable(self):
         self.images_mock.get.return_value = self.new_image