From: Tom Barron <tpb@dyncloud.net>
Date: Wed, 2 Mar 2016 02:43:17 +0000 (-0500)
Subject: Don't run test_volume.VolumeTestCase twice
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4efeeb1e4532943dd4257033667ffd7e3fea2eee;p=openstack-build%2Fcinder-build.git

Don't run test_volume.VolumeTestCase twice

Currently the test_volume.VolumeMigrationTestCase class inherits
from test_volume.VolumeTestCase and therefore runs all the unit tests
in the former class in addition to the 29 tests defined in its
own class.  This provides no gain in coverage and is costly since
the tests in VolumeTestCase are among the most expensive we have.

This commit refactors cinder/tests/unit/test_volume.py such that
VolumeMigrationTestCase inherits directly from BaseVolumeTestCase,
thereby reducing the number of tests run from test_volume from
557 to 412, and total execution time (on my system) from 335s to
202s.  Only duplicate tests are removed.

Depends-On: Ia70e51719abb9a6ed357802446847ffa81d7427e

Change-Id: Ief4706f50838e1f906119992302a39a6014126b3
---

diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py
index 6cf65569b..3d301b354 100644
--- a/cinder/tests/unit/test_volume.py
+++ b/cinder/tests/unit/test_volume.py
@@ -86,6 +86,24 @@ fake_opt = [
 ]
 
 
+def create_snapshot(volume_id, size=1, metadata=None, ctxt=None,
+                    **kwargs):
+    """Create a snapshot object."""
+    metadata = metadata or {}
+    snap = objects.Snapshot(ctxt or context.get_admin_context())
+    snap.volume_size = size
+    snap.user_id = 'fake'
+    snap.project_id = 'fake'
+    snap.volume_id = volume_id
+    snap.status = "creating"
+    if metadata is not None:
+        snap.metadata = metadata
+    snap.update(kwargs)
+
+    snap.create()
+    return snap
+
+
 class FakeImageService(object):
     def __init__(self, db_driver=None, image_service=None):
         pass
@@ -1120,8 +1138,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume_src = tests_utils.create_volume(self.context,
                                                **self.volume_params)
         self.volume.create_volume(self.context, volume_src['id'])
-        snapshot_id = self._create_snapshot(volume_src['id'],
-                                            size=volume_src['size'])['id']
+        snapshot_id = create_snapshot(volume_src['id'],
+                                      size=volume_src['size'])['id']
         snapshot_obj = objects.Snapshot.get_by_id(self.context, snapshot_id)
         self.volume.create_snapshot(self.context, volume_src['id'],
                                     snapshot_obj)
@@ -1374,8 +1392,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume_src = tests_utils.create_volume(self.context,
                                                **self.volume_params)
         self.volume.create_volume(self.context, volume_src['id'])
-        snapshot_id = self._create_snapshot(volume_src['id'],
-                                            size=volume_src['size'])['id']
+        snapshot_id = create_snapshot(volume_src['id'],
+                                      size=volume_src['size'])['id']
         snapshot_obj = objects.Snapshot.get_by_id(self.context, snapshot_id)
 
         self.volume.driver._initialized = False
@@ -1427,8 +1445,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         # no lock
         self.volume.create_volume(self.context, src_vol_id)
 
-        snap_id = self._create_snapshot(src_vol_id,
-                                        size=src_vol['size'])['id']
+        snap_id = create_snapshot(src_vol_id,
+                                  size=src_vol['size'])['id']
         snapshot_obj = objects.Snapshot.get_by_id(self.context, snap_id)
         # no lock
         self.volume.create_snapshot(self.context, src_vol_id, snapshot_obj)
@@ -1657,7 +1675,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         db.volume_update(self.context, src_vol['id'], {'bootable': True})
 
         # create volume from snapshot
-        snapshot_id = self._create_snapshot(src_vol['id'])['id']
+        snapshot_id = create_snapshot(src_vol['id'])['id']
         snapshot_obj = objects.Snapshot.get_by_id(self.context, snapshot_id)
         self.volume.create_snapshot(self.context, src_vol['id'], snapshot_obj)
 
@@ -1716,7 +1734,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume = db.volume_get(self.context, src_vol_id)
 
         # create snapshot of volume
-        snapshot_id = self._create_snapshot(volume['id'])['id']
+        snapshot_id = create_snapshot(volume['id'])['id']
         snapshot_obj = objects.Snapshot.get_by_id(self.context, snapshot_id)
         self.volume.create_snapshot(self.context, volume['id'], snapshot_obj)
 
@@ -1783,8 +1801,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.volume.create_volume(self.context, src_vol_id)
 
         # create snapshot
-        snap_id = self._create_snapshot(src_vol_id,
-                                        size=src_vol['size'])['id']
+        snap_id = create_snapshot(src_vol_id,
+                                  size=src_vol['size'])['id']
         snapshot_obj = objects.Snapshot.get_by_id(self.context, snap_id)
         # no lock
         self.volume.create_snapshot(self.context, src_vol_id, snapshot_obj)
@@ -1963,7 +1981,7 @@ class VolumeTestCase(BaseVolumeTestCase):
                                                availability_zone='az2',
                                                **self.volume_params)
         self.volume.create_volume(self.context, volume_src['id'])
-        snapshot = self._create_snapshot(volume_src['id'])
+        snapshot = create_snapshot(volume_src['id'])
 
         self.volume.create_snapshot(self.context, volume_src['id'],
                                     snapshot)
@@ -2937,24 +2955,6 @@ class VolumeTestCase(BaseVolumeTestCase):
         # This will allow us to test cross-node interactions
         pass
 
-    @staticmethod
-    def _create_snapshot(volume_id, size=1, metadata=None, ctxt=None,
-                         **kwargs):
-        """Create a snapshot object."""
-        metadata = metadata or {}
-        snap = objects.Snapshot(ctxt or context.get_admin_context())
-        snap.volume_size = size
-        snap.user_id = 'fake'
-        snap.project_id = 'fake'
-        snap.volume_id = volume_id
-        snap.status = "creating"
-        if metadata is not None:
-            snap.metadata = metadata
-        snap.update(kwargs)
-
-        snap.create()
-        return snap
-
     def test_create_delete_snapshot(self):
         """Test snapshot can be created and deleted."""
         volume = tests_utils.create_volume(
@@ -2980,7 +2980,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.assertEqual(2, len(self.notifier.notifications),
                          self.notifier.notifications)
 
-        snapshot = self._create_snapshot(volume['id'], size=volume['size'])
+        snapshot = create_snapshot(volume['id'], size=volume['size'])
         snapshot_id = snapshot.id
         self.volume.create_snapshot(self.context, volume['id'], snapshot)
         self.assertEqual(
@@ -3047,8 +3047,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         """Test snapshot can be created with metadata and deleted."""
         test_meta = {'fake_key': 'fake_value'}
         volume = tests_utils.create_volume(self.context, **self.volume_params)
-        snapshot = self._create_snapshot(volume['id'], size=volume['size'],
-                                         metadata=test_meta)
+        snapshot = create_snapshot(volume['id'], size=volume['size'],
+                                   metadata=test_meta)
         snapshot_id = snapshot.id
 
         result_dict = snapshot.metadata
@@ -3176,7 +3176,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         """Test volume can't be deleted with dependent snapshots."""
         volume = tests_utils.create_volume(self.context, **self.volume_params)
         self.volume.create_volume(self.context, volume['id'])
-        snapshot = self._create_snapshot(volume['id'], size=volume['size'])
+        snapshot = create_snapshot(volume['id'], size=volume['size'])
         self.volume.create_snapshot(self.context, volume['id'], snapshot)
         self.assertEqual(
             snapshot.id, objects.Snapshot.get_by_id(self.context,
@@ -3197,8 +3197,8 @@ class VolumeTestCase(BaseVolumeTestCase):
     def test_can_delete_errored_snapshot(self):
         """Test snapshot can be created and deleted."""
         volume = tests_utils.create_volume(self.context, CONF.host)
-        snapshot = self._create_snapshot(volume.id, size=volume['size'],
-                                         ctxt=self.context, status='bad')
+        snapshot = create_snapshot(volume.id, size=volume['size'],
+                                   ctxt=self.context, status='bad')
         self.assertRaises(exception.InvalidSnapshot,
                           self.volume_api.delete_snapshot,
                           self.context,
@@ -3275,7 +3275,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.assertTrue(vol_glance_meta)
 
         # create snapshot from bootable volume
-        snap = self._create_snapshot(volume_id)
+        snap = create_snapshot(volume_id)
         self.volume.create_snapshot(ctxt, volume_id, snap)
 
         # get snapshot's volume_glance_metadata
@@ -3313,7 +3313,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         ctxt = context.get_admin_context()
         vol_glance_meta = db.volume_glance_metadata_get(ctxt, volume_id)
         self.assertTrue(vol_glance_meta)
-        snap = self._create_snapshot(volume_id)
+        snap = create_snapshot(volume_id)
         snap_stat = snap.status
         self.assertTrue(snap.id)
         self.assertTrue(snap_stat)
@@ -3351,7 +3351,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         # set bootable flag of volume to True
         db.volume_update(self.context, volume_id, {'bootable': True})
 
-        snapshot = self._create_snapshot(volume['id'])
+        snapshot = create_snapshot(volume['id'])
         self.volume.create_snapshot(self.context, volume['id'], snapshot)
         self.assertRaises(exception.GlanceMetadataNotFound,
                           db.volume_snapshot_glance_metadata_get,
@@ -3375,7 +3375,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume = tests_utils.create_volume(self.context, **self.volume_params)
         volume_id = volume['id']
         self.volume.create_volume(self.context, volume_id)
-        snapshot = self._create_snapshot(volume_id, size=volume['size'])
+        snapshot = create_snapshot(volume_id, size=volume['size'])
         self.volume.create_snapshot(self.context, volume_id, snapshot)
 
         self.mox.StubOutWithMock(self.volume.driver, 'delete_snapshot')
@@ -3402,7 +3402,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume = tests_utils.create_volume(self.context, **self.volume_params)
         volume_id = volume['id']
         self.volume.create_volume(self.context, volume_id)
-        snapshot = self._create_snapshot(volume_id)
+        snapshot = create_snapshot(volume_id)
         snapshot_id = snapshot.id
         self.volume.create_snapshot(self.context, volume_id, snapshot)
 
@@ -3824,7 +3824,7 @@ class VolumeTestCase(BaseVolumeTestCase):
     def test_volume_api_update_snapshot(self):
         # create raw snapshot
         volume = tests_utils.create_volume(self.context, **self.volume_params)
-        snapshot = self._create_snapshot(volume['id'])
+        snapshot = create_snapshot(volume['id'])
         snapshot_id = snapshot.id
         self.assertIsNone(snapshot.display_name)
         # use volume.api to update name
@@ -4302,7 +4302,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         """Test volume deletion with dependent snapshots."""
         volume = tests_utils.create_volume(self.context, **self.volume_params)
         self.volume.create_volume(self.context, volume['id'])
-        snapshot = self._create_snapshot(volume['id'], size=volume['size'])
+        snapshot = create_snapshot(volume['id'], size=volume['size'])
         self.volume.create_snapshot(self.context, volume['id'], snapshot)
         self.assertEqual(
             snapshot.id, objects.Snapshot.get_by_id(self.context,
@@ -4321,7 +4321,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         """Test volume deletion with dependent snapshots."""
         volume = tests_utils.create_volume(self.context, **self.volume_params)
         self.volume.create_volume(self.context, volume['id'])
-        snapshot = self._create_snapshot(volume['id'], size=volume['size'])
+        snapshot = create_snapshot(volume['id'], size=volume['size'])
         self.volume.create_snapshot(self.context, volume['id'], snapshot)
         self.assertEqual(
             snapshot.id, objects.Snapshot.get_by_id(self.context,
@@ -4343,7 +4343,19 @@ class VolumeTestCase(BaseVolumeTestCase):
 
 
 @ddt.ddt
-class VolumeMigrationTestCase(VolumeTestCase):
+class VolumeMigrationTestCase(BaseVolumeTestCase):
+
+    def setUp(self):
+        super(VolumeMigrationTestCase, self).setUp()
+        self._clear_patch = mock.patch('cinder.volume.utils.clear_volume',
+                                       autospec=True)
+        self._clear_patch.start()
+        self.expected_status = 'available'
+
+    def tearDown(self):
+        super(VolumeMigrationTestCase, self).tearDown()
+        self._clear_patch.stop()
+
     def test_migrate_volume_driver(self):
         """Test volume migration done by driver."""
         # stub out driver and rpc functions
@@ -4860,7 +4872,7 @@ class VolumeMigrationTestCase(VolumeTestCase):
         volume.previous_status = 'available'
         volume.save()
         if snap:
-            self._create_snapshot(volume.id, size=volume.size)
+            create_snapshot(volume.id, size=volume.size)
         if driver or diff_equal:
             host_obj = {'host': CONF.host, 'capabilities': {}}
         else: