Replace tearDown with addCleanup - Part 4

Infra team has indicated that tearDown should not be used and should
be replaced with addCleanup in all places.

All addCleanup methods will be executed even if one of them fails, while
a failure in tearDown method can leave the rest of the tearDown un-executed,
which can leave stale state laying around.

Moreover, tearDown methods won't run if an exception raises in setUp method,
while addCleanup will run in such case.

This patch replaces tearDown with addCleanup or removes redundant tearDown
methods in cinder unit tests.

Implements blueprint replace-teardown-with-addcleanup

Change-Id: I6947eafa419ed7dda53582484090cd5210274f73

diff --git a/cinder/tests/api/v1/test_limits.py b/cinder/tests/api/v1/test_limits.py
index 63bf2b14a160346417b4aa71049d1d30f9efc65e..ee36fe22eb83818cb59a9c9bbc323aff9fe04a28 100644 (file)
--- a/cinder/tests/api/v1/test_limits.py
+++ b/cinder/tests/api/v1/test_limits.py
@@ -730,6 +730,11 @@ class WsgiLimiterProxyTest(BaseLimitTestSuite):
         self.oldHTTPConnection = (
             wire_HTTPConnection_to_WSGI("169.254.0.1:80", self.app))
         self.proxy = limits.WsgiLimiterProxy("169.254.0.1:80")
+        self.addCleanup(self._restore, self.oldHTTPConnection)
+
+    def _restore(self, oldHTTPConnection):
+        # restore original HTTPConnection object
+        httplib.HTTPConnection = oldHTTPConnection
 
     def test_200(self):
         """Successful request test."""
@@ -749,11 +754,6 @@ class WsgiLimiterProxyTest(BaseLimitTestSuite):
 
         self.assertEqual((delay, error), expected)
 
-    def tearDown(self):
-        # restore original HTTPConnection object
-        httplib.HTTPConnection = self.oldHTTPConnection
-        super(WsgiLimiterProxyTest, self).tearDown()
-
 
 class LimitsViewBuilderTest(test.TestCase):
     def setUp(self):

diff --git a/cinder/tests/api/v1/test_volumes.py b/cinder/tests/api/v1/test_volumes.py
index 69911c4cfdd23a90e146065d089d24ed27b22f19..0b0f278699bfc7f2d2fe8b7988f4670bb4838051 100644 (file)
--- a/cinder/tests/api/v1/test_volumes.py
+++ b/cinder/tests/api/v1/test_volumes.py
@@ -55,6 +55,7 @@ def stub_snapshot_get(self, context, snapshot_id):
 class VolumeApiTest(test.TestCase):
     def setUp(self):
         super(VolumeApiTest, self).setUp()
+        self.addCleanup(fake_notifier.reset)
         self.ext_mgr = extensions.ExtensionManager()
         self.ext_mgr.extensions = {}
         fake_image.stub_out_image_service(self.stubs)
@@ -62,16 +63,11 @@ class VolumeApiTest(test.TestCase):
 
         self.flags(host='fake',
                    notification_driver=[fake_notifier.__name__])
-
         self.stubs.Set(db, 'volume_get_all', stubs.stub_volume_get_all)
         self.stubs.Set(db, 'service_get_all_by_topic',
                        stubs.stub_service_get_all_by_topic)
         self.stubs.Set(volume_api.API, 'delete', stubs.stub_volume_delete)
 
-    def tearDown(self):
-        super(VolumeApiTest, self).tearDown()
-        fake_notifier.reset()
-
     def test_volume_create(self):
         self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)

diff --git a/cinder/tests/db/test_finish_migration.py b/cinder/tests/db/test_finish_migration.py
index f0b04ac5892452a4eb81e875a637ad732267d1ef..bcd160a8accfe395fc87f44153d6557e6f18ec3a 100644 (file)
--- a/cinder/tests/db/test_finish_migration.py
+++ b/cinder/tests/db/test_finish_migration.py
@@ -24,12 +24,6 @@ from cinder.tests import utils as testutils
 class FinishVolumeMigrationTestCase(test.TestCase):
     """Test cases for finish_volume_migration."""
 
-    def setUp(self):
-        super(FinishVolumeMigrationTestCase, self).setUp()
-
-    def tearDown(self):
-        super(FinishVolumeMigrationTestCase, self).tearDown()
-
     def test_finish_volume_migration(self):
         ctxt = context.RequestContext(user_id='user_id',
                                       project_id='project_id',

diff --git a/cinder/tests/db/test_name_id.py b/cinder/tests/db/test_name_id.py
index 0b4fed0ba57928b679df2ce39466b0dcff54335f..5446c2f5f4a807550bcc91ac1d7faedf24ac7c53 100644 (file)
--- a/cinder/tests/db/test_name_id.py
+++ b/cinder/tests/db/test_name_id.py
@@ -33,9 +33,6 @@ class NameIDsTestCase(test.TestCase):
         self.ctxt = context.RequestContext(user_id='user_id',
                                            project_id='project_id')
 
-    def tearDown(self):
-        super(NameIDsTestCase, self).tearDown()
-
     def test_name_id_same(self):
         """New volume should have same 'id' and 'name_id'."""
         vol_ref = testutils.create_volume(self.ctxt, size=1)

diff --git a/cinder/tests/db/test_qos_specs.py b/cinder/tests/db/test_qos_specs.py
index 50a76e99ef51a1f5dc2299ed3849df24037fd24e..a6357a4b5dafeded9d44d29059651674aa4833ef 100644 (file)
--- a/cinder/tests/db/test_qos_specs.py
+++ b/cinder/tests/db/test_qos_specs.py
@@ -43,9 +43,6 @@ class QualityOfServiceSpecsTableTestCase(test.TestCase):
                                            project_id='project_id',
                                            is_admin=True)
 
-    def tearDown(self):
-        super(QualityOfServiceSpecsTableTestCase, self).tearDown()
-
     def _create_qos_specs(self, name, values=None):
         """Create a transfer object."""
         if values:

diff --git a/cinder/tests/db/test_transfers.py b/cinder/tests/db/test_transfers.py
index 750fd77ff4d8aa180e5ac95b0962a5cec95c68cf..74b25f5247550d27a47b1f6bfdf2fffe63b1099d 100644 (file)
--- a/cinder/tests/db/test_transfers.py
+++ b/cinder/tests/db/test_transfers.py
@@ -34,9 +34,6 @@ class TransfersTableTestCase(test.TestCase):
         self.ctxt = context.RequestContext(user_id='user_id',
                                            project_id='project_id')
 
-    def tearDown(self):
-        super(TransfersTableTestCase, self).tearDown()
-
     def _create_transfer(self, volume_id=None):
         """Create a transfer object."""
         transfer = {'display_name': 'display_name',

diff --git a/cinder/tests/test_backup.py b/cinder/tests/test_backup.py
index 6c8ad0fa80e1a612ea7735b83bbc8e3f8fd85863..90ab61beb681c283c39764094840985ef603fb8b 100644 (file)
--- a/cinder/tests/test_backup.py
+++ b/cinder/tests/test_backup.py
@@ -53,9 +53,6 @@ class BackupTestCase(test.TestCase):
         self.ctxt = context.get_admin_context()
         self.backup_mgr.driver.set_initialized()
 
-    def tearDown(self):
-        super(BackupTestCase, self).tearDown()
-
     def _create_backup_db_entry(self, volume_id=1, display_name='test_backup',
                                 display_description='this is a test backup',
                                 container='volumebackups',

diff --git a/cinder/tests/test_backup_ceph.py b/cinder/tests/test_backup_ceph.py
index 3cb0a5a495ece72af2f3cdea32dd02d06fdaed45..86307296d8924c6327f9d2c8d94b484bc0e6789f 100644 (file)
--- a/cinder/tests/test_backup_ceph.py
+++ b/cinder/tests/test_backup_ceph.py
@@ -922,9 +922,6 @@ class VolumeMetadataBackupTestCase(test.TestCase):
         self.backup_id = str(uuid.uuid4())
         self.mb = ceph.VolumeMetadataBackup(mock.Mock(), self.backup_id)
 
-    def tearDown(self):
-        super(VolumeMetadataBackupTestCase, self).tearDown()
-
     @common_meta_backup_mocks
     def test_name(self):
         self.assertEqual(self.mb.name, 'backup.%s.meta' % (self.backup_id))

diff --git a/cinder/tests/test_backup_driver_base.py b/cinder/tests/test_backup_driver_base.py
index 32a790c5b3935e4809807f4bf60518c9f7065a7e..7a05590a6984d7a3438a7b3b95e5aeb4c35e73cd 100644 (file)
--- a/cinder/tests/test_backup_driver_base.py
+++ b/cinder/tests/test_backup_driver_base.py
@@ -101,9 +101,6 @@ class BackupBaseDriverTestCase(test.TestCase):
         self.assertRaises(NotImplementedError,
                           self.driver.verify, self.backup)
 
-    def tearDown(self):
-        super(BackupBaseDriverTestCase, self).tearDown()
-
 
 class BackupMetadataAPITestCase(test.TestCase):
 
@@ -247,6 +244,3 @@ class BackupMetadataAPITestCase(test.TestCase):
             mock_dumps.side_effect = TypeError
             self.assertFalse(self.bak_meta_api._is_serializable(data))
             mock_dumps.assert_called_once()
-
-    def tearDown(self):
-        super(BackupMetadataAPITestCase, self).tearDown()

diff --git a/cinder/tests/test_backup_tsm.py b/cinder/tests/test_backup_tsm.py
index 7c2b45b30124a466c6643bc7c59ffedcb6de7a19..aeca67646630e5a342d0eccfb73120ca6299ae61 100644 (file)
--- a/cinder/tests/test_backup_tsm.py
+++ b/cinder/tests/test_backup_tsm.py
@@ -242,9 +242,6 @@ class BackupTSMTestCase(test.TestCase):
         self.stubs.Set(utils, 'execute', fake_exec)
         self.stubs.Set(os, 'stat', fake_stat_image)
 
-    def tearDown(self):
-        super(BackupTSMTestCase, self).tearDown()
-
     def _create_volume_db_entry(self, volume_id):
         vol = {'id': volume_id,
                'size': 1,

diff --git a/cinder/tests/test_drivers_compatibility.py b/cinder/tests/test_drivers_compatibility.py
index 9c35bb0a8ce5317cdae4eec6363432e649254602..3d2cba7e033a25ba4cfef077b9e132c8ce404562 100644 (file)
--- a/cinder/tests/test_drivers_compatibility.py
+++ b/cinder/tests/test_drivers_compatibility.py
@@ -45,9 +45,6 @@ class VolumeDriverCompatibility(test.TestCase):
         self.manager = importutils.import_object(CONF.volume_manager)
         self.context = context.get_admin_context()
 
-    def tearDown(self):
-        super(VolumeDriverCompatibility, self).tearDown()
-
     def _load_driver(self, driver):
         if 'SolidFire' in driver:
             # SolidFire driver does update_cluster stat on init

diff --git a/cinder/tests/test_emc_smis.py b/cinder/tests/test_emc_smis.py
index 20a1767b69d751623b801f835699b687a53d0fa5..8946e6205a81c2d5d4dd433049c92708057f7259 100644 (file)
--- a/cinder/tests/test_emc_smis.py
+++ b/cinder/tests/test_emc_smis.py
@@ -1107,9 +1107,11 @@ class EMCSMISFCDriverTestCase(test.TestCase):
         self.data = EMCSMISCommonData()
 
         self.tempdir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.tempdir)
         super(EMCSMISFCDriverTestCase, self).setUp()
         self.config_file_path = None
         self.create_fake_config_file()
+        self.addCleanup(os.remove, self.config_file_path)
 
         configuration = mock.Mock()
         configuration.cinder_emc_config_file = self.config_file_path
@@ -1352,13 +1354,3 @@ class EMCSMISFCDriverTestCase(test.TestCase):
         volume_with_vt = self.data.test_volume
         volume_with_vt['volume_type_id'] = 1
         self.driver.create_volume(volume_with_vt)
-
-    def _cleanup(self):
-        bExists = os.path.exists(self.config_file_path)
-        if bExists:
-            os.remove(self.config_file_path)
-        shutil.rmtree(self.tempdir)
-
-    def tearDown(self):
-        self._cleanup()
-        super(EMCSMISFCDriverTestCase, self).tearDown()

diff --git a/cinder/tests/test_huawei_t_dorado.py b/cinder/tests/test_huawei_t_dorado.py
index 97d963eaa4e4c3ef9571fe4dbaab94dae89c00ff..4183a2bdb1953c280300c80c85c509395aaa7555 100644 (file)
--- a/cinder/tests/test_huawei_t_dorado.py
+++ b/cinder/tests/test_huawei_t_dorado.py
@@ -1705,15 +1705,11 @@ class HuaweiUtilsTestCase(test.TestCase):
         super(HuaweiUtilsTestCase, self).setUp()
 
         self.tmp_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.tmp_dir)
         self.fake_conf_file = self.tmp_dir + '/cinder_huawei_conf.xml'
+        self.addCleanup(os.remove, self.fake_conf_file)
         create_fake_conf_file(self.fake_conf_file)
 
-    def tearDown(self):
-        if os.path.exists(self.fake_conf_file):
-            os.remove(self.fake_conf_file)
-        shutil.rmtree(self.tmp_dir)
-        super(HuaweiUtilsTestCase, self).tearDown()
-
     def test_parse_xml_file_ioerror(self):
         tmp_fonf_file = '/xxx/cinder_huawei_conf.xml'
         self.assertRaises(IOError, huawei_utils.parse_xml_file, tmp_fonf_file)

diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py
index a527fa6b4dbde79f4272bdd2dd2d6815d4348e54..c2c572c3b5be9346b4d51218afc42b32df31b959 100644 (file)
--- a/cinder/tests/test_rbd.py
+++ b/cinder/tests/test_rbd.py
@@ -159,9 +159,6 @@ class RBDTestCase(test.TestCase):
         self.snapshot = dict(volume_name=self.volume_name,
                              name=self.snapshot_name)
 
-    def tearDown(self):
-        super(RBDTestCase, self).tearDown()
-
     @common_mocks
     def test_create_volume(self):
         client = self.mock_client.return_value
@@ -739,9 +736,6 @@ class RBDImageIOWrapperTestCase(test.TestCase):
         self.data_length = 1024
         self.full_data = 'abcd' * 256
 
-    def tearDown(self):
-        super(RBDImageIOWrapperTestCase, self).tearDown()
-
     def test_init(self):
         self.assertEqual(self.mock_rbd_wrapper._rbd_meta, self.meta)
         self.assertEqual(self.mock_rbd_wrapper._offset, 0)

diff --git a/cinder/tests/test_volume_configuration.py b/cinder/tests/test_volume_configuration.py
index 63b33b33c940b6bc1ade2445d86b5de1862fb8fe..470eca5ffaffeb716cd114264762d5000dd05399 100644 (file)
--- a/cinder/tests/test_volume_configuration.py
+++ b/cinder/tests/test_volume_configuration.py
@@ -40,11 +40,6 @@ CONF.register_opts(more_volume_opts)
 
 
 class VolumeConfigurationTest(test.TestCase):
-    def setUp(self):
-        super(VolumeConfigurationTest, self).setUp()
-
-    def tearDown(self):
-        super(VolumeConfigurationTest, self).tearDown()
 
     def test_group_grafts_opts(self):
         c = configuration.Configuration(volume_opts, config_group='foo')

diff --git a/cinder/tests/test_zadara.py b/cinder/tests/test_zadara.py
index 618889846849df4c7b682935c83295cf9cf24458..f4693b4842ef52ae59fe12a9252eac588133d9f5 100644 (file)
--- a/cinder/tests/test_zadara.py
+++ b/cinder/tests/test_zadara.py
@@ -493,9 +493,6 @@ class ZadaraVPSADriverTestCase(test.TestCase):
         self.stubs.Set(httplib, 'HTTPSConnection', FakeHTTPSConnection)
         self.driver.do_setup(None)
 
-    def tearDown(self):
-        super(ZadaraVPSADriverTestCase, self).tearDown()
-
     def test_create_destroy(self):
         """Create/Delete volume."""
         volume = {'name': 'test_volume_01', 'size': 1}