Modify test_hpe3par to support random hash

On Python 3, or on Python 2 with -R command line option, the hash
function is now randomized for security. Changes:

* Rename CommentMatcher to Comment and simplify it.
* Generalize the usage of Comment.

Change-Id: I0233bab571aee3c034f067e6c28e6b38dc017db9

diff --git a/cinder/tests/unit/test_hpe3par.py b/cinder/tests/unit/test_hpe3par.py
index df6d19421aec3b8bdee37231e99be704e0b73e8c..c201450afa17618c6a3b28d96e2f6b90956eb105 100644 (file)
--- a/cinder/tests/unit/test_hpe3par.py
+++ b/cinder/tests/unit/test_hpe3par.py
@@ -71,17 +71,15 @@ QUEUE_LENGTH = 'queue_length'
 AVG_BUSY_PERC = 'avg_busy_perc'
 
 
-class HPE3PARBaseDriver(object):
+class Comment(object):
+    def __init__(self, expected):
+        self.expected = expected
+
+    def __eq__(self, actual):
+        return (dict(ast.literal_eval(actual)) == self.expected)
 
-    class CommentMatcher(object):
-        def __init__(self, f, expect):
-            self.assertEqual = f
-            self.expect = expect
 
-        def __eq__(self, actual):
-            actual_as_dict = dict(ast.literal_eval(actual))
-            self.assertEqual(self.expect, actual_as_dict)
-            return True
+class HPE3PARBaseDriver(object):
 
     VOLUME_ID = 'd03338a9-9115-48a3-8dfc-35cdfcdc15a7'
     CLONE_ID = 'd03338a9-9115-48a3-8dfc-000000000000'
@@ -660,10 +658,11 @@ class HPE3PARBaseDriver(object):
                                '_create_client') as mock_create_client:
             mock_create_client.return_value = mock_client
             self.driver.create_volume(self.volume)
-            comment = (
-                '{"display_name": "Foo Volume", "type": "OpenStack",'
-                ' "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7",'
-                ' "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"}')
+            comment = Comment({
+                "display_name": "Foo Volume",
+                "type": "OpenStack",
+                "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"})
             expected = [
                 mock.call.createVolume(
                     self.VOLUME_3PAR_NAME,
@@ -689,10 +688,11 @@ class HPE3PARBaseDriver(object):
             mock_create_client.return_value = mock_client
 
             return_model = self.driver.create_volume(self.volume_pool)
-            comment = (
-                '{"display_name": "Foo Volume", "type": "OpenStack",'
-                ' "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7",'
-                ' "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"}')
+            comment = Comment({
+                "display_name": "Foo Volume",
+                "type": "OpenStack",
+                "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"})
             expected = [
                 mock.call.createVolume(
                     self.VOLUME_3PAR_NAME,
@@ -842,11 +842,14 @@ class HPE3PARBaseDriver(object):
             mock_create_client.return_value = mock_client
 
             return_model = self.driver.create_volume(self.volume_qos)
-            comment = (
-                '{"volume_type_name": "gold", "display_name": "Foo Volume"'
-                ', "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7'
-                '", "volume_type_id": "gold", "volume_id": "d03338a9-91'
-                '15-48a3-8dfc-35cdfcdc15a7", "qos": {}, "type": "OpenStack"}')
+            comment = Comment({
+                "volume_type_name": "gold",
+                "display_name": "Foo Volume",
+                "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+                "volume_type_id": "gold",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+                "qos": {},
+                "type": "OpenStack"})
 
             expected = [
                 mock.call.getCPG(HPE3PAR_CPG),
@@ -886,12 +889,14 @@ class HPE3PARBaseDriver(object):
             mock_create_client.return_value = mock_client
 
             return_model = self.driver.create_volume(self.volume_dedup)
-            comment = (
-                '{"volume_type_name": "dedup", "display_name": "Foo Volume"'
-                ', "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7'
-                '", "volume_type_id": "d03338a9-9115-48a3-8dfc-11111111111"'
-                ', "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"'
-                ', "qos": {}, "type": "OpenStack"}')
+            comment = Comment({
+                "volume_type_name": "dedup",
+                "display_name": "Foo Volume",
+                "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+                "volume_type_id": "d03338a9-9115-48a3-8dfc-11111111111",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+                "qos": {},
+                "type": "OpenStack"})
 
             expected = [
                 mock.call.getCPG(HPE3PAR_CPG),
@@ -935,13 +940,13 @@ class HPE3PARBaseDriver(object):
             mock_client.FLASH_CACHE_DISABLED = FLASH_CACHE_DISABLED
 
             return_model = self.driver.create_volume(self.volume_flash_cache)
-            comment = (
-                '{"volume_type_name": "flash-cache-on", '
-                '"display_name": "Foo Volume", '
-                '"name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7", '
-                '"volume_type_id": "d03338a9-9115-48a3-8dfc-22222222222", '
-                '"volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7", '
-                '"qos": {}, "type": "OpenStack"}')
+            comment = Comment({
+                "volume_type_name": "flash-cache-on",
+                "display_name": "Foo Volume",
+                "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+                "volume_type_id": "d03338a9-9115-48a3-8dfc-22222222222",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+                "qos": {}, "type": "OpenStack"})
 
             expected = [
                 mock.call.getCPG(HPE3PAR_CPG),
@@ -1468,10 +1473,12 @@ class HPE3PARBaseDriver(object):
 
             osv_matcher = 'osv-' + volume_name_3par
 
+            comment = Comment({"qos": {}, "display_name": "Foo Volume"})
+
             expected = [
                 mock.call.modifyVolume(
                     osv_matcher,
-                    {'comment': '{"qos": {}, "display_name": "Foo Volume"}',
+                    {'comment': comment,
                      'snapCPG': HPE3PAR_CPG_SNAP}),
                 mock.call.modifyVolume(osv_matcher,
                                        {'action': 6,
@@ -1533,17 +1540,17 @@ class HPE3PARBaseDriver(object):
 
             osv_matcher = 'osv-' + volume_name_3par
 
-            expected_comment = {
+            expected_comment = Comment({
                 "display_name": display_name,
                 "volume_type_id": self.RETYPE_VOLUME_TYPE_2['id'],
                 "volume_type_name": self.RETYPE_VOLUME_TYPE_2['name'],
                 "vvs": self.RETYPE_VOLUME_TYPE_2['extra_specs']['vvs']
-            }
+            })
+
             expected = [
                 mock.call.modifyVolume(
                     osv_matcher,
-                    {'comment': self.CommentMatcher(self.assertEqual,
-                                                    expected_comment),
+                    {'comment': expected_comment,
                      'snapCPG': self.RETYPE_VOLUME_TYPE_2
                      ['extra_specs']['snap_cpg']}),
                 mock.call.modifyVolume(
@@ -1635,20 +1642,22 @@ class HPE3PARBaseDriver(object):
 
             osv_matcher = 'osv-' + volume_name_3par
 
-            expected = [
-                mock.call.modifyVolume(
-                    osv_matcher,
-                    {'comment': '{"qos": {}, "display_name": "Foo Volume"}',
-                     'snapCPG': HPE3PAR_CPG_SNAP}),
-                mock.call.modifyVolume(osv_matcher,
-                                       {'action': 6,
-                                        'userCPG': 'CPG-FC1',
-                                        'conversionOperation': 1,
-                                        'tuneOperation': 1}),
-                mock.call.getTask(mock.ANY),
-            ]
+        comment = Comment({"qos": {}, "display_name": "Foo Volume"})
 
-            mock_client.assert_has_calls(expected + self.standard_logout)
+        expected = [
+            mock.call.modifyVolume(
+                osv_matcher,
+                {'comment': comment,
+                 'snapCPG': HPE3PAR_CPG_SNAP}),
+            mock.call.modifyVolume(osv_matcher,
+                                   {'action': 6,
+                                    'userCPG': 'CPG-FC1',
+                                    'conversionOperation': 1,
+                                    'tuneOperation': 1}),
+            mock.call.getTask(mock.ANY),
+        ]
+
+        mock_client.assert_has_calls(expected + self.standard_logout)
 
     @mock.patch.object(volume_types, 'get_volume_type')
     def test_migrate_volume_attached(self, _mock_volume_types):
@@ -1685,12 +1694,13 @@ class HPE3PARBaseDriver(object):
             result = self.driver.migrate_volume(context.get_admin_context(),
                                                 volume, host)
 
-            new_comment = {"qos": {},
-                           "retype_test": "test comment"}
+            new_comment = Comment({
+                "qos": {},
+                "retype_test": "test comment",
+            })
             expected = [
                 mock.call.modifyVolume(osv_matcher,
-                                       {'comment': self.CommentMatcher(
-                                           self.assertEqual, new_comment),
+                                       {'comment': new_comment,
                                         'snapCPG': 'OpenStackCPGSnap'}),
                 mock.call.modifyVolume(osv_matcher,
                                        {'action': 6,
@@ -1838,13 +1848,13 @@ class HPE3PARBaseDriver(object):
             mock_create_client.return_value = mock_client
             self.driver.create_snapshot(self.snapshot)
 
-            comment = (
-                '{"volume_id": "761fc5e5-5191-4ec7-aeba-33e36de44156",'
-                ' "display_name": "fakesnap",'
-                ' "description": "test description name",'
-                ' "volume_name":'
-                ' "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7"}')
-
+            comment = Comment({
+                "volume_id": "761fc5e5-5191-4ec7-aeba-33e36de44156",
+                "display_name": "fakesnap",
+                "description": "test description name",
+                "volume_name":
+                "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+            })
             expected = [
                 mock.call.createSnapshot(
                     'oss-L4I73ONuTci9Fd4ceij-MQ',
@@ -1924,11 +1934,11 @@ class HPE3PARBaseDriver(object):
                 self.snapshot)
             self.assertIsNone(model_update)
 
-            comment = (
-                '{"snapshot_id": "2f823bdc-e36e-4dc8-bd15-de1c7a28ff31",'
-                ' "display_name": "Foo Volume",'
-                ' "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"}')
-
+            comment = Comment({
+                "snapshot_id": "2f823bdc-e36e-4dc8-bd15-de1c7a28ff31",
+                "display_name": "Foo Volume",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+            })
             expected = [
                 mock.call.createSnapshot(
                     self.VOLUME_3PAR_NAME,
@@ -1971,11 +1981,11 @@ class HPE3PARBaseDriver(object):
                 self.snapshot)
             self.assertEqual(None, model_update)
 
-            comment = (
-                '{"snapshot_id": "2f823bdc-e36e-4dc8-bd15-de1c7a28ff31",'
-                ' "display_name": "Foo Volume",'
-                ' "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"}')
-
+            comment = Comment({
+                "snapshot_id": "2f823bdc-e36e-4dc8-bd15-de1c7a28ff31",
+                "display_name": "Foo Volume",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+            })
             volume_name_3par = common._encode_name(volume['id'])
             osv_matcher = 'osv-' + volume_name_3par
             omv_matcher = 'omv-' + volume_name_3par
@@ -2036,10 +2046,11 @@ class HPE3PARBaseDriver(object):
                 self.snapshot)
             self.assertEqual(None, model_update)
 
-            comment = (
-                '{"snapshot_id": "2f823bdc-e36e-4dc8-bd15-de1c7a28ff31",'
-                ' "display_name": "Foo Volume",'
-                ' "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"}')
+            comment = Comment({
+                "snapshot_id": "2f823bdc-e36e-4dc8-bd15-de1c7a28ff31",
+                "display_name": "Foo Volume",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+            })
 
             volume_name_3par = common._encode_name(volume['id'])
             osv_matcher = 'osv-' + volume_name_3par
@@ -2111,10 +2122,11 @@ class HPE3PARBaseDriver(object):
                 self.volume_qos,
                 self.snapshot)
 
-            comment = (
-                '{"snapshot_id": "2f823bdc-e36e-4dc8-bd15-de1c7a28ff31",'
-                ' "display_name": "Foo Volume",'
-                ' "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"}')
+            comment = Comment({
+                "snapshot_id": "2f823bdc-e36e-4dc8-bd15-de1c7a28ff31",
+                "display_name": "Foo Volume",
+                "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7",
+            })
 
             expected = [
                 mock.call.createSnapshot(
@@ -2472,11 +2484,12 @@ class HPE3PARBaseDriver(object):
         _mock_volume_types.return_value = self.volume_type
         mock_client = self.setup_driver()
 
-        new_comment = {"display_name": "Foo Volume",
-                       "name": "volume-007dbfce-7579-40bc-8f90-a20b3902283e",
-                       "volume_id": "007dbfce-7579-40bc-8f90-a20b3902283e",
-                       "type": "OpenStack"}
-
+        new_comment = Comment({
+            "display_name": "Foo Volume",
+            "name": "volume-007dbfce-7579-40bc-8f90-a20b3902283e",
+            "volume_id": "007dbfce-7579-40bc-8f90-a20b3902283e",
+            "type": "OpenStack",
+        })
         volume = {'display_name': None,
                   'host': self.FAKE_CINDER_HOST,
                   'volume_type': 'gold',
@@ -2505,11 +2518,10 @@ class HPE3PARBaseDriver(object):
                 mock.call.getVolume(existing_ref['source-name']),
                 mock.call.modifyVolume(existing_ref['source-name'],
                                        {'newName': osv_matcher,
-                                        'comment': self.CommentMatcher(
-                                            self.assertEqual, new_comment)}),
+                                        'comment': new_comment}),
             ]
 
-            retype_comment_qos = {
+            retype_comment_qos = Comment({
                 "display_name": "Foo Volume",
                 "volume_type_name": self.volume_type['name'],
                 "volume_type_id": self.volume_type['id'],
@@ -2521,14 +2533,12 @@ class HPE3PARBaseDriver(object):
                     'latency': '25',
                     'priority': 'low'
                 }
-            }
+            })
 
             expected_snap_cpg = HPE3PAR_CPG_SNAP
             expected_retype_modify = [
                 mock.call.modifyVolume(osv_matcher,
-                                       {'comment': self.CommentMatcher(
-                                           self.assertEqual,
-                                           retype_comment_qos),
+                                       {'comment': retype_comment_qos,
                                         'snapCPG': expected_snap_cpg}),
                 mock.call.deleteVolumeSet(vvs_matcher),
             ]
@@ -2561,10 +2571,12 @@ class HPE3PARBaseDriver(object):
         _mock_volume_types.return_value = self.volume_type
         mock_client = self.setup_driver()
 
-        new_comment = {"display_name": "Foo Volume",
-                       "name": "volume-007dbfce-7579-40bc-8f90-a20b3902283e",
-                       "volume_id": "007dbfce-7579-40bc-8f90-a20b3902283e",
-                       "type": "OpenStack"}
+        new_comment = Comment({
+            "display_name": "Foo Volume",
+            "name": "volume-007dbfce-7579-40bc-8f90-a20b3902283e",
+            "volume_id": "007dbfce-7579-40bc-8f90-a20b3902283e",
+            "type": "OpenStack",
+        })
 
         volume = {'display_name': None,
                   'host': 'my-stack1@3parxxx#CPGNOTUSED',
@@ -2594,8 +2606,7 @@ class HPE3PARBaseDriver(object):
                 mock.call.modifyVolume(
                     existing_ref['source-name'],
                     {'newName': osv_matcher,
-                     'comment': self.CommentMatcher(self.assertEqual,
-                                                    new_comment),
+                     'comment': new_comment,
                      # manage_existing() should be setting
                      # blank snapCPG to the userCPG
                      'snapCPG': 'testUserCpg0'})
@@ -2616,11 +2627,12 @@ class HPE3PARBaseDriver(object):
         mock_client.getTask.return_value = self.STATUS_DONE
 
         id = '007abcde-7579-40bc-8f90-a20b3902283e'
-        new_comment = {"display_name": "Test Volume",
-                       "name": ("volume-%s" % id),
-                       "volume_id": id,
-                       "type": "OpenStack"}
-
+        new_comment = Comment({
+            "display_name": "Test Volume",
+            "name": ("volume-%s" % id),
+            "volume_id": id,
+            "type": "OpenStack",
+        })
         volume = {'display_name': 'Test Volume',
                   'host': 'my-stack1@3parxxx#CPGNOTUSED',
                   'volume_type': 'gold',
@@ -2645,22 +2657,19 @@ class HPE3PARBaseDriver(object):
                 mock.call.getVolume(existing_ref['source-name']),
                 mock.call.modifyVolume(existing_ref['source-name'],
                                        {'newName': osv_matcher,
-                                        'comment': self.CommentMatcher(
-                                            self.assertEqual, new_comment)})
+                                        'comment': new_comment})
             ]
 
-            retype_comment_vvs = {
+            retype_comment_vvs = Comment({
                 "display_name": "Foo Volume",
                 "volume_type_name": test_volume_type['name'],
                 "volume_type_id": test_volume_type['id'],
                 "vvs": vvs
-            }
+            })
 
             expected_retype = [
                 mock.call.modifyVolume(osv_matcher,
-                                       {'comment': self.CommentMatcher(
-                                           self.assertEqual,
-                                           retype_comment_vvs),
+                                       {'comment': retype_comment_vvs,
                                         'snapCPG': 'OpenStackCPGSnap'}),
                 mock.call.deleteVolumeSet(vvs_matcher),
                 mock.call.addVolumeToVolumeSet(vvs, osv_matcher),
@@ -2681,13 +2690,13 @@ class HPE3PARBaseDriver(object):
     def test_manage_existing_no_volume_type(self):
         mock_client = self.setup_driver()
 
-        comment = (
-            '{"display_name": "Foo Volume"}')
-        new_comment = (
-            '{"type": "OpenStack",'
-            ' "display_name": "Foo Volume",'
-            ' "name": "volume-007dbfce-7579-40bc-8f90-a20b3902283e",'
-            ' "volume_id": "007dbfce-7579-40bc-8f90-a20b3902283e"}')
+        comment = repr({"display_name": "Foo Volume"})
+        new_comment = Comment({
+            "type": "OpenStack",
+            "display_name": "Foo Volume",
+            "name": "volume-007dbfce-7579-40bc-8f90-a20b3902283e",
+            "volume_id": "007dbfce-7579-40bc-8f90-a20b3902283e",
+        })
         volume = {'display_name': None,
                   'volume_type': None,
                   'volume_type_id': None,
@@ -2798,8 +2807,7 @@ class HPE3PARBaseDriver(object):
     def test_manage_existing_volume_type_exception(self):
         mock_client = self.setup_driver()
 
-        comment = (
-            '{"display_name": "Foo Volume"}')
+        comment = repr({"display_name": "Foo Volume"})
         volume = {'display_name': None,
                   'volume_type': 'gold',
                   'volume_type_id': 'bcfa9fa4-54a0-4340-a3d8-bfcf19aea65e',
@@ -3006,11 +3014,11 @@ class HPE3PARBaseDriver(object):
 
         mock_client = self.setup_driver()
 
-        comment = (
-            "{'display_name': 'cg_name',"
-            " 'consistency_group_id':"
-            " '" + self.CONSIS_GROUP_ID + "',"
-            " 'description': 'consistency group'}")
+        comment = Comment({
+            'display_name': 'cg_name',
+            'consistency_group_id': self.CONSIS_GROUP_ID,
+            'description': 'consistency group',
+        })
 
         with mock.patch.object(hpecommon.HPE3PARCommon,
                                '_create_client') as mock_create_client:
@@ -3046,18 +3054,20 @@ class HPE3PARBaseDriver(object):
         mock_client = self.setup_driver()
         volume = self.volume
 
+        cgsnap_comment = Comment({
+            "consistency_group_id": "6044fedf-c889-4752-900f-2039d247a5df",
+            "description": "cgsnapshot",
+            "cgsnapshot_id": "e91c5ed5-daee-4e84-8724-1c9e31e7a1f2",
+        })
+
         cgsnap_optional = (
-            {'comment': '{"consistency_group_id":'
-             ' "6044fedf-c889-4752-900f-2039d247a5df",'
-             ' "description": "cgsnapshot",'
-             ' "cgsnapshot_id": "e91c5ed5-daee-4e84-8724-1c9e31e7a1f2"}',
+            {'comment': cgsnap_comment,
              'readOnly': False})
 
-        cg_comment = (
-            "{'display_name': 'cg_name',"
-            " 'consistency_group_id':"
-            " '" + self.CONSIS_GROUP_ID + "',"
-            " 'description': 'consistency group'}")
+        cg_comment = Comment({
+            'display_name': 'cg_name',
+            'consistency_group_id': self.CONSIS_GROUP_ID,
+            'description': 'consistency group'})
 
         with mock.patch.object(hpecommon.HPE3PARCommon,
                                '_create_client') as mock_create_client:
@@ -3134,11 +3144,10 @@ class HPE3PARBaseDriver(object):
 
         mock_client = self.setup_driver()
 
-        comment = (
-            "{'display_name': 'cg_name',"
-            " 'consistency_group_id':"
-            " '" + self.CONSIS_GROUP_ID + "',"
-            " 'description': 'consistency group'}")
+        comment = Comment({
+            'display_name': 'cg_name',
+            'consistency_group_id': self.CONSIS_GROUP_ID,
+            'description': 'consistency group'})
 
         with mock.patch.object(hpecommon.HPE3PARCommon,
                                '_create_client') as mock_create_client:
@@ -3191,11 +3200,10 @@ class HPE3PARBaseDriver(object):
         mock_client = self.setup_driver()
         volume = self.volume
 
-        comment = (
-            "{'display_name': 'cg_name',"
-            " 'consistency_group_id':"
-            " '" + self.CONSIS_GROUP_ID + "',"
-            " 'description': 'consistency group'}")
+        comment = Comment({
+            'display_name': 'cg_name',
+            'consistency_group_id': self.CONSIS_GROUP_ID,
+            'description': 'consistency group'})
 
         with mock.patch.object(hpecommon.HPE3PARCommon,
                                '_create_client') as mock_create_client:
@@ -3250,11 +3258,10 @@ class HPE3PARBaseDriver(object):
         mock_client = self.setup_driver()
         volume = self.volume
 
-        comment = (
-            "{'display_name': 'cg_name',"
-            " 'consistency_group_id':"
-            " '" + self.CONSIS_GROUP_ID + "',"
-            " 'description': 'consistency group'}")
+        comment = Comment({
+            'display_name': 'cg_name',
+            'consistency_group_id': self.CONSIS_GROUP_ID,
+            'description': 'consistency group'})
 
         with mock.patch.object(hpecommon.HPE3PARCommon,
                                '_create_client') as mock_create_client:
@@ -3327,17 +3334,18 @@ class HPE3PARBaseDriver(object):
         mock_client = self.setup_driver()
         volume = self.volume
 
-        cg_comment = (
-            "{'display_name': 'cg_name',"
-            " 'consistency_group_id':"
-            " '" + self.CONSIS_GROUP_ID + "',"
-            " 'description': 'consistency group'}")
+        cg_comment = Comment({
+            'display_name': 'cg_name',
+            'consistency_group_id': self.CONSIS_GROUP_ID,
+            'description': 'consistency group'})
+
+        cgsnap_comment = Comment({
+            "consistency_group_id": "6044fedf-c889-4752-900f-2039d247a5df",
+            "description": "cgsnapshot",
+            "cgsnapshot_id": "e91c5ed5-daee-4e84-8724-1c9e31e7a1f2"})
 
         cgsnap_optional = (
-            {'comment': '{"consistency_group_id":'
-             ' "6044fedf-c889-4752-900f-2039d247a5df",'
-             ' "description": "cgsnapshot",'
-             ' "cgsnapshot_id": "e91c5ed5-daee-4e84-8724-1c9e31e7a1f2"}',
+            {'comment': cgsnap_comment,
              'readOnly': False})
 
         with mock.patch.object(hpecommon.HPE3PARCommon,
@@ -3411,18 +3419,18 @@ class HPE3PARBaseDriver(object):
         volume = self.volume
         cgsnapshot = self.cgsnapshot
 
-        cg_comment = (
-            "{'display_name': 'cg_name',"
-            " 'consistency_group_id':"
-            " '" + self.CONSIS_GROUP_ID + "',"
-            " 'description': 'consistency group'}")
+        cg_comment = Comment({
+            'display_name': 'cg_name',
+            'consistency_group_id': self.CONSIS_GROUP_ID,
+            'description': 'consistency group'})
 
-        cgsnap_optional = (
-            {'comment': '{"consistency_group_id":'
-             ' "6044fedf-c889-4752-900f-2039d247a5df",'
-             ' "description": "cgsnapshot",'
-             ' "cgsnapshot_id": "e91c5ed5-daee-4e84-8724-1c9e31e7a1f2"}',
-             'readOnly': False})
+        cgsnap_comment = Comment({
+            "consistency_group_id": "6044fedf-c889-4752-900f-2039d247a5df",
+            "description": "cgsnapshot",
+            "cgsnapshot_id": "e91c5ed5-daee-4e84-8724-1c9e31e7a1f2"})
+
+        cgsnap_optional = {'comment': cgsnap_comment,
+                           'readOnly': False}
 
         with mock.patch.object(hpecommon.HPE3PARCommon,
                                '_create_client') as mock_create_client: