From: Julia Varlamova Date: Fri, 21 Feb 2014 06:50:46 +0000 (+0400) Subject: Replace tearDown with addCleanup - Part 4 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=6ee7901df927a1e94d0e63950b46c85e5b8cd732;p=openstack-build%2Fcinder-build.git 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 63bf2b14a..ee36fe22e 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 69911c4cf..0b0f27869 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 f0b04ac58..bcd160a8a 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 0b4fed0ba..5446c2f5f 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 50a76e99e..a6357a4b5 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 750fd77ff..74b25f524 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 6c8ad0fa8..90ab61beb 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 3cb0a5a49..86307296d 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 32a790c5b..7a05590a6 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 7c2b45b30..aeca67646 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 9c35bb0a8..3d2cba7e0 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 20a1767b6..8946e6205 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 97d963eaa..4183a2bdb 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 a527fa6b4..c2c572c3b 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 63b33b33c..470eca5ff 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 618889846..f4693b484 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}