소스 검색

Extract common tests for object creation

Nuwan Goonasekera 8 년 전
부모
커밋
9a1e5c9e2e

+ 65 - 0
test/helpers/standard_interface_tests.py

@@ -5,10 +5,12 @@ This includes:
    2. Checking for object equality and repr
    3. Checking standard behaviour for list, iter, find, get, delete
 """
+import test.helpers as helpers
 import uuid
 
 from cloudbridge.cloud.interfaces.exceptions \
     import InvalidNameException
+from cloudbridge.cloud.interfaces.resources import ObjectLifeCycleMixin
 from cloudbridge.cloud.interfaces.resources import ResultList
 
 
@@ -181,3 +183,66 @@ def check_standard_behaviour(test, service, obj):
         " are not as expected: {4}" .format(objs_list[0].id, objs_iter[0].id,
                                             objs_find[0].id, obj_get.id,
                                             obj.id))
+
+
+def check_crud(test, service, iface, name_prefix,
+               create_func, cleanup_func, extra_test_func=None,
+               custom_check_delete=None):
+    """
+    Checks crud behaviour of a given cloudbridge service. The create_func will
+    be used as a factory function to create a service object and the
+    cleanup_func will be used to destroy the object. Once an object is created
+    using the create_func, all other standard behavioural tests can be run
+    against that object.
+
+    :type  test: ``TestCase``
+    :param test: The TestCase object to use
+
+    :type  service: ``CloudService``
+    :param service: The CloudService object under test. For example,
+                    a VolumeService object.
+
+    :type  iface: ``type``
+    :param iface: The type to test behaviour against. This type must be a
+                  subclass of ``CloudResource``.
+
+    :type  name_prefix: ``str``
+    :param name_prefix: The name to prefix all created objects with. This
+                        function will generated a new name with the
+                        specified name_prefix for each test object created
+                        and pass that name into the create_func
+
+    :type  create_func: ``func``
+    :param create_func: The create_func must accept the name of the object to
+                        create as a parameter and return the constructed
+                        object.
+
+    :type  cleanup_func: ``func``
+    :param cleanup_func: The cleanup_func must accept the created object
+                         and perform all cleanup tasks required to delete the
+                         object.
+
+    :type  extra_test_func: ``func``
+    :param extra_test_func: This function will be called to perform additional
+                            tests after object construction and initialization,
+                            but before object cleanup. It will receive the
+                            created object as a parameter.
+
+    :type  custom_check_delete: ``func``
+    :param custom_check_delete: If provided, this function will be called
+                                instead of the standard check_delete function
+                                to make sure that the object has been deleted.
+    """
+    name = "{0}-{1}".format(name_prefix, helpers.get_uuid())
+    obj = None
+    with helpers.cleanup_action(lambda: cleanup_func(obj)):
+        obj = create_func(name)
+        if issubclass(iface, ObjectLifeCycleMixin):
+            obj.wait_till_ready()
+        check_standard_behaviour(test, service, obj)
+        if extra_test_func:
+            extra_test_func(obj)
+    if custom_check_delete:
+        custom_check_delete(obj)
+    else:
+        check_delete(test, service, obj)

+ 42 - 45
test/test_block_store_service.py

@@ -5,9 +5,13 @@ from test import helpers
 from test.helpers import ProviderTestBase
 from test.helpers import standard_interface_tests as sit
 
+from cloudbridge.cloud.factory import ProviderList
 from cloudbridge.cloud.interfaces import SnapshotState
 from cloudbridge.cloud.interfaces import VolumeState
+from cloudbridge.cloud.interfaces.provider import TestMockHelperMixin
 from cloudbridge.cloud.interfaces.resources import AttachmentInfo
+from cloudbridge.cloud.interfaces.resources import Snapshot
+from cloudbridge.cloud.interfaces.resources import Volume
 
 import six
 
@@ -20,23 +24,19 @@ class CloudBlockStoreServiceTestCase(ProviderTestBase):
         Create a new volume, check whether the expected values are set,
         and delete it
         """
-        name = "cb_createvol-{0}".format(helpers.get_uuid())
-        test_vol = self.provider.block_store.volumes.create(
-            name,
-            1,
-            helpers.get_provider_test_data(self.provider, "placement"))
+        def create_vol(name):
+            return self.provider.block_store.volumes.create(
+                name,
+                1,
+                helpers.get_provider_test_data(self.provider, "placement"))
 
         def cleanup_vol(vol):
             vol.delete()
             vol.wait_for([VolumeState.DELETED, VolumeState.UNKNOWN],
                          terminal_states=[VolumeState.ERROR])
 
-        with helpers.cleanup_action(lambda: cleanup_vol(test_vol)):
-            test_vol.wait_till_ready()
-            sit.check_standard_behaviour(
-                self, self.provider.block_store.volumes, test_vol)
-
-        sit.check_delete(self, self.provider.block_store.volumes, test_vol)
+        sit.check_crud(self, self.provider.block_store.volumes, Volume,
+                       "cb_createvol", create_vol, cleanup_vol)
 
     @helpers.skipIfNoService(['block_store.volumes'])
     def test_attach_detach_volume(self):
@@ -142,9 +142,10 @@ class CloudBlockStoreServiceTestCase(ProviderTestBase):
             helpers.get_provider_test_data(self.provider, "placement"))
         with helpers.cleanup_action(lambda: test_vol.delete()):
             test_vol.wait_till_ready()
-            snap_name = "cb_snap-{0}".format(name)
-            test_snap = test_vol.create_snapshot(name=snap_name,
-                                                 description=snap_name)
+
+            def create_snap(name):
+                return test_vol.create_snapshot(name=name,
+                                                description=name)
 
             def cleanup_snap(snap):
                 snap.delete()
@@ -152,39 +153,19 @@ class CloudBlockStoreServiceTestCase(ProviderTestBase):
                     [SnapshotState.UNKNOWN],
                     terminal_states=[SnapshotState.ERROR])
 
-            with helpers.cleanup_action(lambda: cleanup_snap(test_snap)):
-                test_snap.wait_till_ready()
-                sit.check_standard_behaviour(
-                    self, self.provider.block_store.snapshots, test_snap)
-
-                # Test volume creation from a snapshot (via VolumeService)
-                sv_name = "cb_snapvol_{0}".format(name)
-                snap_vol = self.provider.block_store.volumes.create(
-                    sv_name,
-                    1,
-                    helpers.get_provider_test_data(self.provider, "placement"),
-                    snapshot=test_snap)
-                with helpers.cleanup_action(lambda: snap_vol.delete()):
-                    snap_vol.wait_till_ready()
-
-                # Test volume creation from a snapshot (via Snapshot)
-                snap_vol2 = test_snap.create_volume(
-                    helpers.get_provider_test_data(self.provider, "placement"))
-                with helpers.cleanup_action(lambda: snap_vol2.delete()):
-                    snap_vol2.wait_till_ready()
-
-            sit.check_delete(
-                self, self.provider.block_store.snapshots, test_snap)
+            sit.check_crud(self, self.provider.block_store.snapshots, Snapshot,
+                           "cb_snap", create_snap, cleanup_snap)
 
             # Test creation of a snap via SnapshotService
-            snap_two_name = "cb_snaptwo-{0}".format(name)
-            time.sleep(15)  # Or get SnapshotCreationPerVolumeRateExceeded
-            test_snap_two = self.provider.block_store.snapshots.create(
-                name=snap_two_name, volume=test_vol, description=snap_two_name)
-            with helpers.cleanup_action(lambda: cleanup_snap(test_snap_two)):
-                test_snap_two.wait_till_ready()
-                sit.check_standard_behaviour(
-                    self, self.provider.block_store.snapshots, test_snap_two)
+            def create_snap2(name):
+                return self.provider.block_store.snapshots.create(
+                    name=name, volume=test_vol, description=name)
+
+            if not isinstance(self.provider, TestMockHelperMixin) and \
+               self.provider.PROVIDER_ID == ProviderList.AWS:
+                time.sleep(15)  # Or get SnapshotCreationPerVolumeRateExceeded
+            sit.check_crud(self, self.provider.block_store.snapshots, Snapshot,
+                           "cb_snaptwo", create_snap2, cleanup_snap)
 
     @helpers.skipIfNoService(['block_store.snapshots'])
     def test_snapshot_properties(self):
@@ -227,3 +208,19 @@ class CloudBlockStoreServiceTestCase(ProviderTestBase):
                 test_snap.refresh()
                 self.assertEqual(test_snap.name, 'snapnewname1')
                 self.assertEqual(test_snap.description, 'snapnewdescription1')
+
+                # Test volume creation from a snapshot (via VolumeService)
+                sv_name = "cb_snapvol_{0}".format(test_snap.name)
+                snap_vol = self.provider.block_store.volumes.create(
+                    sv_name,
+                    1,
+                    helpers.get_provider_test_data(self.provider, "placement"),
+                    snapshot=test_snap)
+                with helpers.cleanup_action(lambda: snap_vol.delete()):
+                    snap_vol.wait_till_ready()
+
+                # Test volume creation from a snapshot (via Snapshot)
+                snap_vol2 = test_snap.create_volume(
+                    helpers.get_provider_test_data(self.provider, "placement"))
+                with helpers.cleanup_action(lambda: snap_vol2.delete()):
+                    snap_vol2.wait_till_ready()

+ 25 - 14
test/test_compute_service.py

@@ -9,6 +9,7 @@ from cloudbridge.cloud.interfaces import InstanceState
 from cloudbridge.cloud.interfaces import InvalidConfigurationException
 from cloudbridge.cloud.interfaces import TestMockHelperMixin
 from cloudbridge.cloud.interfaces.exceptions import WaitStateException
+from cloudbridge.cloud.interfaces.resources import Instance
 from cloudbridge.cloud.interfaces.resources import InstanceType
 from cloudbridge.cloud.interfaces.resources import SnapshotState
 
@@ -22,24 +23,34 @@ class CloudComputeServiceTestCase(ProviderTestBase):
         name = "cb_instcrud-{0}".format(helpers.get_uuid())
         # Declare these variables and late binding will allow
         # the cleanup method access to the most current values
-        inst = None
         net = None
+        subnet = None
+
+        def create_inst(name):
+            return helpers.get_test_instance(self.provider, name,
+                                             subnet=subnet)
+
+        def cleanup_inst(inst):
+            inst.terminate()
+            inst.wait_for([InstanceState.TERMINATED, InstanceState.UNKNOWN])
+
+        def check_deleted(inst):
+            deleted_inst = self.provider.compute.instances.get(
+                inst.id)
+            self.assertTrue(
+                deleted_inst is None or deleted_inst.state in (
+                    InstanceState.TERMINATED,
+                    InstanceState.UNKNOWN),
+                "Instance %s should have been deleted but still exists." %
+                name)
+
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
-                inst, net)):
+                                               network=net)):
             net, subnet = helpers.create_test_network(self.provider, name)
-            inst = helpers.get_test_instance(self.provider, name,
-                                             subnet=subnet)
 
-            sit.check_standard_behaviour(
-                self, self.provider.compute.instances, inst)
-        deleted_inst = self.provider.compute.instances.get(
-            inst.id)
-        self.assertTrue(
-            deleted_inst is None or deleted_inst.state in (
-                InstanceState.TERMINATED,
-                InstanceState.UNKNOWN),
-            "Instance %s should have been deleted but still exists." %
-            name)
+            sit.check_crud(self, self.provider.compute.instances, Instance,
+                           "cb_instcrud", create_inst, cleanup_inst,
+                           custom_check_delete=check_deleted)
 
     def _is_valid_ip(self, address):
         try:

+ 21 - 22
test/test_image_service.py

@@ -4,6 +4,7 @@ from test.helpers import standard_interface_tests as sit
 
 from cloudbridge.cloud.interfaces import MachineImageState
 from cloudbridge.cloud.interfaces import TestMockHelperMixin
+from cloudbridge.cloud.interfaces.resources import MachineImage
 
 
 class CloudImageServiceTestCase(ProviderTestBase):
@@ -22,6 +23,23 @@ class CloudImageServiceTestCase(ProviderTestBase):
         # the cleanup method access to the most current values
         test_instance = None
         net = None
+
+        def create_img(name):
+            return test_instance.create_image(name)
+
+        def cleanup_img(img):
+            img.delete()
+            img.wait_for(
+                [MachineImageState.UNKNOWN, MachineImageState.ERROR])
+
+        def extra_tests(img):
+            # TODO: Fix moto so that the BDM is populated correctly
+            if not isinstance(self.provider, TestMockHelperMixin):
+                # check image size
+                img.refresh()
+                self.assertGreater(img.min_disk, 0, "Minimum disk"
+                                   " size required by image is invalid")
+
         with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
                 test_instance, net)):
             net, subnet = helpers.create_test_network(
@@ -29,25 +47,6 @@ class CloudImageServiceTestCase(ProviderTestBase):
             test_instance = helpers.get_test_instance(
                 self.provider, instance_name, subnet=subnet)
 
-            name = "cb_listimg-{0}".format(helpers.get_uuid())
-            test_image = test_instance.create_image(name)
-
-            def cleanup_img(img):
-                img.delete()
-                img.wait_for(
-                    [MachineImageState.UNKNOWN, MachineImageState.ERROR])
-
-            with helpers.cleanup_action(lambda: cleanup_img(test_image)):
-                test_image.wait_till_ready()
-                sit.check_standard_behaviour(
-                    self, self.provider.compute.images, test_image)
-
-                # TODO: Fix moto so that the BDM is populated correctly
-                if not isinstance(self.provider, TestMockHelperMixin):
-                    # check image size
-                    test_image.refresh()
-                    self.assertGreater(test_image.min_disk, 0, "Minimum disk"
-                                       " size required by image is invalid")
-            # TODO: Images take a long time to deregister on EC2. Needs
-            # investigation
-            sit.check_delete(self, self.provider.compute.images, test_image)
+            sit.check_crud(self, self.provider.compute.images, MachineImage,
+                           "cb_listimg", create_img, cleanup_img,
+                           extra_test_func=extra_tests)

+ 66 - 56
test/test_network_service.py

@@ -3,6 +3,7 @@ import test.helpers as helpers
 from test.helpers import ProviderTestBase
 from test.helpers import standard_interface_tests as sit
 
+from cloudbridge.cloud.interfaces.resources import Network
 from cloudbridge.cloud.interfaces.resources import RouterState
 from cloudbridge.cloud.interfaces.resources import Subnet
 
@@ -10,63 +11,17 @@ from cloudbridge.cloud.interfaces.resources import Subnet
 class CloudNetworkServiceTestCase(ProviderTestBase):
 
     @helpers.skipIfNoService(['networking.networks'])
-    def test_crud_network_service(self):
-        name = 'cb_crudnetwork-{0}'.format(helpers.get_uuid())
-        subnet_name = 'cb_crudsubnet-{0}'.format(helpers.get_uuid())
-        net = self.provider.networking.networks.create(
-            name=name, cidr_block='10.0.0.0/16')
-        with helpers.cleanup_action(
-            lambda:
-                self.provider.networking.networks.delete(network_id=net.id)
-        ):
-            sit.check_standard_behaviour(
-                self, self.provider.networking.networks, net)
-
-            # check subnet
-            subnet = self.provider.networking.subnets.create(
-                network=net, cidr_block="10.0.0.1/24", name=subnet_name)
-            with helpers.cleanup_action(
-                lambda:
-                    self.provider.networking.subnets.delete(subnet=subnet)
-            ):
-                sit.check_standard_behaviour(
-                    self, self.provider.networking.subnets, subnet)
-
-            sit.check_delete(self, self.provider.networking.subnets, subnet)
-
-            # Check floating IP address
-            ip = self.provider.networking.networks.create_floating_ip()
-            ip_id = ip.id
-            with helpers.cleanup_action(lambda: ip.delete()):
-                ipl = self.provider.networking.networks.floating_ips
-                self.assertTrue(
-                    ip in ipl,
-                    "Floating IP address {0} should exist in the list {1}"
-                    .format(ip.id, ipl))
-                # 2016-08: address filtering not implemented in moto
-                # empty_ipl = self.provider.network.floating_ips('dummy-net')
-                # self.assertFalse(
-                #     empty_ipl,
-                #     "Bogus network should not have any floating IPs: {0}"
-                #     .format(empty_ipl))
-                self.assertIn(
-                    ip.public_ip, repr(ip),
-                    "repr(obj) should contain the address public IP value.")
-                self.assertFalse(
-                    ip.private_ip,
-                    "Floating IP should not have a private IP value ({0})."
-                    .format(ip.private_ip))
-                self.assertFalse(
-                    ip.in_use(),
-                    "Newly created floating IP address should not be in use.")
-            ipl = self.provider.networking.networks.floating_ips
-            found_ip = [a for a in ipl if a.id == ip_id]
-            self.assertTrue(
-                len(found_ip) == 0,
-                "Floating IP {0} should have been deleted but still exists."
-                .format(ip_id))
+    def test_crud_network(self):
+
+        def create_net(name):
+            return self.provider.networking.networks.create(
+                name=name, cidr_block='10.0.0.0/16')
+
+        def cleanup_net(net):
+            self.provider.networking.networks.delete(network_id=net.id)
 
-        sit.check_delete(self, self.provider.networking.networks, net)
+        sit.check_crud(self, self.provider.networking.networks, Network,
+                       "cb_crudnetwork", create_net, cleanup_net)
 
     @helpers.skipIfNoService(['networking.networks'])
     def test_network_properties(self):
@@ -109,6 +64,61 @@ class CloudNetworkServiceTestCase(ProviderTestBase):
                     "Subnet's CIDR %s should match the specified one %s." % (
                         sn.cidr_block, cidr))
 
+    def test_crud_subnet(self):
+        # Late binding will make sure that create_subnet gets the
+        # correct value
+        net = None
+
+        def create_subnet(name):
+            return self.provider.networking.subnets.create(
+                network=net, cidr_block="10.0.0.1/24", name=name)
+
+        def cleanup_subnet(subnet):
+            self.provider.networking.subnets.delete(subnet=subnet)
+
+        net_name = 'cb_crudsubnet-{0}'.format(helpers.get_uuid())
+        net = self.provider.networking.networks.create(
+            name=net_name, cidr_block='10.0.0.0/16')
+        with helpers.cleanup_action(
+            lambda:
+                self.provider.networking.networks.delete(network_id=net.id)
+        ):
+            sit.check_crud(self, self.provider.networking.subnets, Subnet,
+                           "cb_crudsubnet", create_subnet, cleanup_subnet)
+
+    def test_floating_ip_properties(self):
+        # Check floating IP address
+        ip = self.provider.networking.networks.create_floating_ip()
+        ip_id = ip.id
+        with helpers.cleanup_action(lambda: ip.delete()):
+            ipl = self.provider.networking.networks.floating_ips
+            self.assertTrue(
+                ip in ipl,
+                "Floating IP address {0} should exist in the list {1}"
+                .format(ip.id, ipl))
+            # 2016-08: address filtering not implemented in moto
+            # empty_ipl = self.provider.network.floating_ips('dummy-net')
+            # self.assertFalse(
+            #     empty_ipl,
+            #     "Bogus network should not have any floating IPs: {0}"
+            #     .format(empty_ipl))
+            self.assertIn(
+                ip.public_ip, repr(ip),
+                "repr(obj) should contain the address public IP value.")
+            self.assertFalse(
+                ip.private_ip,
+                "Floating IP should not have a private IP value ({0})."
+                .format(ip.private_ip))
+            self.assertFalse(
+                ip.in_use(),
+                "Newly created floating IP address should not be in use.")
+        ipl = self.provider.networking.networks.floating_ips
+        found_ip = [a for a in ipl if a.id == ip_id]
+        self.assertTrue(
+            len(found_ip) == 0,
+            "Floating IP {0} should have been deleted but still exists."
+            .format(ip_id))
+
     @helpers.skipIfNoService(['networking.networks.routers'])
     def test_crud_router(self):
 

+ 32 - 9
test/test_object_store_service.py

@@ -10,6 +10,7 @@ from test.helpers import ProviderTestBase
 from test.helpers import standard_interface_tests as sit
 from unittest import skip
 
+from cloudbridge.cloud.interfaces.resources import Bucket
 from cloudbridge.cloud.interfaces.resources import BucketObject
 
 import requests
@@ -23,16 +24,40 @@ class CloudObjectStoreServiceTestCase(ProviderTestBase):
         Create a new bucket, check whether the expected values are set,
         and delete it.
         """
-        name = "cbtestcreatebucket-{0}".format(uuid.uuid4())
-        test_bucket = self.provider.object_store.create(name)
-        with helpers.cleanup_action(lambda: test_bucket.delete()):
-            sit.check_standard_behaviour(
-                self, self.provider.object_store, test_bucket)
+        def create_bucket(name):
+            return self.provider.object_store.create(name)
+
+        def cleanup_bucket(bucket):
+            bucket.delete()
+
+        sit.check_crud(self, self.provider.object_store, Bucket,
+                       "cb_crudbucket", create_bucket, cleanup_bucket)
+
+    @helpers.skipIfNoService(['object_store'])
+    def test_crud_bucket_object(self):
+        test_bucket = None
+
+        def create_bucket_obj(name):
+            obj = test_bucket.create_object(name)
+            # TODO: This is wrong. We shouldn't have to have a separate
+            # call to upload some content before being able to delete
+            # the content. Maybe the create_object method should accept
+            # the file content as a parameter.
+            obj.upload("dummy content")
+            return obj
 
-        sit.check_delete(self, self.provider.object_store, test_bucket)
+        def cleanup_bucket_obj(bucket_obj):
+            bucket_obj.delete()
+
+        with helpers.cleanup_action(lambda: test_bucket.delete()):
+            name = "cb_crudbucketobj-{0}".format(uuid.uuid4())
+            test_bucket = self.provider.object_store.create(name)
+            sit.check_crud(self, test_bucket, BucketObject,
+                           "cb_bucketobj", create_bucket_obj,
+                           cleanup_bucket_obj)
 
     @helpers.skipIfNoService(['object_store'])
-    def test_crud_bucket_objects(self):
+    def test_crud_bucket_object_properties(self):
         """
         Create a new bucket, upload some contents into the bucket, and
         check whether list properly detects the new content.
@@ -72,8 +97,6 @@ class CloudObjectStoreServiceTestCase(ProviderTestBase):
                 iter_objs = list(test_bucket)
                 self.assertListEqual(iter_objs, objs)
 
-                sit.check_standard_behaviour(self, test_bucket, obj)
-
                 obj_too = test_bucket.get(obj_name)
                 self.assertTrue(
                     isinstance(obj_too, BucketObject),

+ 28 - 38
test/test_security_service.py

@@ -6,26 +6,29 @@ from test.helpers import ProviderTestBase
 from test.helpers import standard_interface_tests as sit
 
 from cloudbridge.cloud.interfaces import TestMockHelperMixin
+from cloudbridge.cloud.interfaces.resources import KeyPair
+from cloudbridge.cloud.interfaces.resources import SecurityGroup
 
 
 class CloudSecurityServiceTestCase(ProviderTestBase):
 
     @helpers.skipIfNoService(['security.key_pairs'])
     def test_crud_key_pair_service(self):
-        name = 'cb_crudkp-{0}'.format(helpers.get_uuid())
-        kp = self.provider.security.key_pairs.create(name=name)
-        with helpers.cleanup_action(
-            lambda:
-                self.provider.security.key_pairs.delete(key_pair_id=kp.id)
-        ):
-            sit.check_standard_behaviour(
-                self, self.provider.security.key_pairs, kp)
 
+        def create_kp(name):
+            return self.provider.security.key_pairs.create(name=name)
+
+        def cleanup_kp(kp):
+            self.provider.security.key_pairs.delete(key_pair_id=kp.id)
+
+        def extra_tests(kp):
             # Recreating existing keypair should raise an exception
             with self.assertRaises(Exception):
-                self.provider.security.key_pairs.create(name=name)
+                self.provider.security.key_pairs.create(name=kp.name)
 
-        sit.check_delete(self, self.provider.security.key_pairs, kp)
+        sit.check_crud(self, self.provider.security.key_pairs, KeyPair,
+                       "cb_crudkp", create_kp, cleanup_kp,
+                       extra_test_func=extra_tests)
 
     @helpers.skipIfNoService(['security.key_pairs'])
     def test_key_pair_properties(self):
@@ -36,35 +39,31 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
                 kp.material,
                 "KeyPair material is empty but it should not be.")
 
-    def cleanup_sg(self, sg, net):
-        with helpers.cleanup_action(
-                lambda: self.provider.network.delete(network_id=net.id)):
-            self.provider.security.security_groups.delete(group_id=sg.id)
-
     @helpers.skipIfNoService(['security.security_groups'])
-    def test_crud_security_group_service(self):
+    def test_crud_security_group(self):
         name = 'cb_crudsg-{0}'.format(helpers.get_uuid())
 
         # Declare these variables and late binding will allow
         # the cleanup method access to the most current values
         net = None
-        sg = None
-        with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
-                network=net, security_group=sg)):
-            net, _ = helpers.create_test_network(self.provider, name)
-            sg = self.provider.security.security_groups.create(
+
+        def create_sg(name):
+            return self.provider.security.security_groups.create(
                 name=name, description=name, network_id=net.id)
 
-            self.assertEqual(name, sg.description)
+        def cleanup_sg(sg):
+            sg.delete()
 
-            sit.check_standard_behaviour(
-                self, self.provider.security.security_groups, sg)
+        with helpers.cleanup_action(lambda: helpers.cleanup_test_resources(
+                network=net)):
+            net, _ = helpers.create_test_network(self.provider, name)
 
-        sit.check_delete(self, self.provider.security.security_groups, sg)
+            sit.check_crud(self, self.provider.security.security_groups,
+                           SecurityGroup, "cb_crudsg", create_sg, cleanup_sg)
 
     @helpers.skipIfNoService(['security.security_groups'])
-    def test_security_group(self):
-        """Test for proper creation of a security group."""
+    def test_security_group_properties(self):
+        """Test properties of a security group."""
         name = 'cb_propsg-{0}'.format(helpers.get_uuid())
 
         # Declare these variables and late binding will allow
@@ -77,6 +76,8 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
             sg = self.provider.security.security_groups.create(
                 name=name, description=name, network_id=net.id)
 
+            self.assertEqual(name, sg.description)
+
             rule = sg.add_rule(ip_protocol='tcp', from_port=1111, to_port=1111,
                                cidr_ip='0.0.0.0/0')
             found_rule = sg.get_rule(ip_protocol='tcp', from_port=1111,
@@ -101,17 +102,6 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
             self.assertFalse(
                 sg != sg,
                 "The same security groups should still be equal?")
-#             json_repr = json.dumps(
-#                 {"description": name, "name": name, "id": sg.id,
-#                  "rules":
-#                     [{"from_port": 1111, "group": "", "cidr_ip": "0.0.0.0/0",
-#                       "parent": sg.id, "to_port": 1111, "ip_protocol": "tcp",
-#                       "id": sg.rules[0].id}]},
-#                 sort_keys=True)
-#             self.assertTrue(
-#                 sg.to_json() == json_repr,
-#                 "JSON SG representation {0} does not match expected {1}"
-#                 .format(sg.to_json(), json_repr))
 
         sit.check_delete(self, self.provider.security.security_groups, sg)
         sgl = self.provider.security.security_groups.list()