From 6ee7901df927a1e94d0e63950b46c85e5b8cd732 Mon Sep 17 00:00:00 2001 From: Julia Varlamova Date: Fri, 21 Feb 2014 10:50:46 +0400 Subject: [PATCH] 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 --- cinder/tests/api/v1/test_limits.py | 10 +++++----- cinder/tests/api/v1/test_volumes.py | 6 +----- cinder/tests/db/test_finish_migration.py | 6 ------ cinder/tests/db/test_name_id.py | 3 --- cinder/tests/db/test_qos_specs.py | 3 --- cinder/tests/db/test_transfers.py | 3 --- cinder/tests/test_backup.py | 3 --- cinder/tests/test_backup_ceph.py | 3 --- cinder/tests/test_backup_driver_base.py | 6 ------ cinder/tests/test_backup_tsm.py | 3 --- cinder/tests/test_drivers_compatibility.py | 3 --- cinder/tests/test_emc_smis.py | 12 ++---------- cinder/tests/test_huawei_t_dorado.py | 8 ++------ cinder/tests/test_rbd.py | 6 ------ cinder/tests/test_volume_configuration.py | 5 ----- cinder/tests/test_zadara.py | 3 --- 16 files changed, 10 insertions(+), 73 deletions(-) diff --git a/cinder/tests/api/v1/test_limits.py b/cinder/tests/api/v1/test_limits.py index 63bf2b14a16..ee36fe22eb8 100644 --- 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 69911c4cfdd..0b0f278699b 100644 --- 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 f0b04ac5892..bcd160a8acc 100644 --- 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 0b4fed0ba57..5446c2f5f4a 100644 --- 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 50a76e99ef5..a6357a4b5da 100644 --- 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 750fd77ff4d..74b25f52475 100644 --- 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 6c8ad0fa80e..90ab61beb68 100644 --- 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 3cb0a5a495e..86307296d89 100644 --- 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 32a790c5b39..7a05590a698 100644 --- 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 7c2b45b3012..aeca6764663 100644 --- 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 9c35bb0a8ce..3d2cba7e033 100644 --- 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 20a1767b69d..8946e6205a8 100644 --- 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 97d963eaa4e..4183a2bdb19 100644 --- 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 a527fa6b4db..c2c572c3b5b 100644 --- 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 63b33b33c94..470eca5ffaf 100644 --- 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 61888984684..f4693b4842e 100644 --- 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}