From 322126212e44358446c0fcabfa451a2f2b76f146 Mon Sep 17 00:00:00 2001 From: git-harry Date: Sat, 22 Nov 2014 10:16:22 +0000 Subject: [PATCH] Fix calls to assert_called_once in unit tests Mock has a method called assert_called_once_with to check that a mock was called and the arguments it took were as expected. Mock does not have a method called assert_called_once and calling it just creates a mock bound to that name. This means that not only is nothing tested when assert_called_once is used, the tests also don't warn about this. This commit attempts to address this in two ways: - all occurrences of assert_called_once are replaced with a real assertion. - the hacking check that nova uses to guard against this has been copied to cinder's local hacking checks. Fixing the assert_called_once issues also highlighted other mistakes in certain tests which were addressed to make the tests pass. Due to the nature of mock, this issue is also possible if a method is misspelt or just mistakenly used and so the hacking check is only addressing one very specific case. That said, it does appear to be a common mistake and so is worth singling out. Change-Id: Iedcc3f48d91f7ebd8878ccc3bca3d023503774bd Closes-Bug: #1394544 --- HACKING.rst | 1 + cinder/hacking/checks.py | 12 ++ cinder/tests/keymgr/test_barbican.py | 4 +- cinder/tests/test_backup_driver_base.py | 2 +- cinder/tests/test_glusterfs.py | 6 +- cinder/tests/test_hp_msa.py | 2 +- cinder/tests/test_rbd.py | 186 +++++++++--------- cinder/tests/test_smbfs.py | 2 +- cinder/tests/test_utils.py | 8 +- cinder/tests/windows/test_vhdutils.py | 2 +- .../test_brcd_fc_zone_client_cli.py | 41 ++-- .../tests/zonemanager/test_fc_zone_manager.py | 2 +- 12 files changed, 143 insertions(+), 125 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 1d1440974f6..93e0df93528 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -12,6 +12,7 @@ Cinder Specific Commandments - [N322] Ensure default arguments are not mutable. - [N323] Add check for explicit import of _() to ensure proper translation. - [N324] Enforce no use of LOG.audit messages. LOG.info should be used instead. +- [N327] assert_called_once is not a valid Mock method. General diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py index d05ad7efb3d..8256c0f1058 100644 --- a/cinder/hacking/checks.py +++ b/cinder/hacking/checks.py @@ -113,9 +113,21 @@ def check_no_log_audit(logical_line): yield(0, "N324: Found LOG.audit. Use LOG.info instead.") +def check_assert_called_once(logical_line, filename): + msg = ("N327: assert_called_once is a no-op. please use assert_called_" + "once_with to test with explicit parameters or an assertEqual with" + " call_count.") + + if 'cinder/tests/' in filename: + pos = logical_line.find('.assert_called_once(') + if pos != -1: + yield (pos, msg) + + def factory(register): register(no_vi_headers) register(no_translate_debug_logs) register(no_mutable_default_args) register(check_explicit_underscore_import) register(check_no_log_audit) + register(check_assert_called_once) diff --git a/cinder/tests/keymgr/test_barbican.py b/cinder/tests/keymgr/test_barbican.py index ea726c129cf..bc81b1c5e64 100644 --- a/cinder/tests/keymgr/test_barbican.py +++ b/cinder/tests/keymgr/test_barbican.py @@ -124,7 +124,7 @@ class BarbicanKeyManagerTestCase(test_key_mgr.KeyManagerTestCase): original_secret_metadata.bit_length, original_secret_metadata.mode, original_secret_metadata.expiration) - self.store.assert_called() + self.create.return_value.store.assert_called_once_with() def test_copy_null_context(self): self.key_mgr._barbican_client = None @@ -223,7 +223,7 @@ class BarbicanKeyManagerTestCase(test_key_mgr.KeyManagerTestCase): None, 'AES', 256, 'CBC', None) - self.store.assert_called_once() + self.create.return_value.store.assert_called_once_with() def test_store_null_context(self): self.key_mgr._barbican_client = None diff --git a/cinder/tests/test_backup_driver_base.py b/cinder/tests/test_backup_driver_base.py index 67a2d8f0dbb..74ef1aeaeac 100644 --- a/cinder/tests/test_backup_driver_base.py +++ b/cinder/tests/test_backup_driver_base.py @@ -236,4 +236,4 @@ class BackupMetadataAPITestCase(test.TestCase): with mock.patch.object(jsonutils, 'dumps') as mock_dumps: mock_dumps.side_effect = TypeError self.assertFalse(self.bak_meta_api._is_serializable(data)) - mock_dumps.assert_called_once() + mock_dumps.assert_called_once_with(data) diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py index c433f22761e..d06cf69e880 100644 --- a/cinder/tests/test_glusterfs.py +++ b/cinder/tests/test_glusterfs.py @@ -1858,7 +1858,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_qemu_img_info.assert_called_once_with(volume_path) mock_upload_volume.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, upload_path) - mock_create_temporary_file.assert_once_called_with() + self.assertEqual(1, mock_create_temporary_file.call_count) def test_copy_volume_to_image_qcow2_image(self): """Upload a qcow2 image file which has to be converted to raw first.""" @@ -1903,7 +1903,7 @@ class GlusterFsDriverTestCase(test.TestCase): volume_path, upload_path, 'raw') mock_upload_volume.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, upload_path) - mock_create_temporary_file.assert_once_called_with() + self.assertEqual(1, mock_create_temporary_file.call_count) def test_copy_volume_to_image_snapshot_exists(self): """Upload an active snapshot which has to be converted to raw first.""" @@ -1950,4 +1950,4 @@ class GlusterFsDriverTestCase(test.TestCase): volume_path, upload_path, 'raw') mock_upload_volume.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, upload_path) - mock_create_temporary_file.assert_once_called_with() + self.assertEqual(1, mock_create_temporary_file.call_count) diff --git a/cinder/tests/test_hp_msa.py b/cinder/tests/test_hp_msa.py index 205689bdb1d..94510ac29b3 100644 --- a/cinder/tests/test_hp_msa.py +++ b/cinder/tests/test_hp_msa.py @@ -496,7 +496,7 @@ class TestHPMSAFC(test.TestCase): 'data': {'target_wwn': ['id1'], 'target_lun': 1, 'target_discovered': True}}) - mock_ports.assert_called_once() + mock_ports.assert_called_once_with() @mock.patch.object(hp_msa_common.HPMSACommon, 'client_logout') @mock.patch.object(hp_msa_common.HPMSACommon, 'unmap_volume') diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py index 9480c6fbeae..8c721b24f75 100644 --- a/cinder/tests/test_rbd.py +++ b/cinder/tests/test_rbd.py @@ -173,9 +173,9 @@ class RBDTestCase(test.TestCase): 'features': self.mock_rbd.RBD_FEATURE_LAYERING} self.mock_rbd.RBD.return_value.create.assert_called_once_with( *args, **kwargs) - client.__enter__.assert_called_once() - client.__exit__.assert_called_once() - mock_supports_layering.assert_called_once() + client.__enter__.assert_called_once_with() + client.__exit__.assert_called_once_with(None, None, None) + mock_supports_layering.assert_called_once_with() @common_mocks def test_manage_existing_get_size(self): @@ -261,9 +261,9 @@ class RBDTestCase(test.TestCase): 'features': 0} self.mock_rbd.RBD.return_value.create.assert_called_once_with( *args, **kwargs) - client.__enter__.assert_called_once() - client.__exit__.assert_called_once() - mock_supports_layering.assert_called_once() + client.__enter__.assert_called_once_with() + client.__exit__.assert_called_once_with(None, None, None) + mock_supports_layering.assert_called_once_with() @common_mocks def test_delete_backup_snaps(self): @@ -291,21 +291,26 @@ class RBDTestCase(test.TestCase): self.driver.delete_volume(self.volume) - mock_get_clone_info.assert_called_once() + mock_get_clone_info.assert_called_once_with( + self.mock_rbd.Image.return_value, + self.volume_name, + None) (self.driver.rbd.Image.return_value - .list_snaps.assert_called_once()) - client.__enter__.assert_called_once() - client.__exit__.assert_called_once() - mock_delete_backup_snaps.assert_called_once() + .list_snaps.assert_called_once_with()) + client.__enter__.assert_called_once_with() + client.__exit__.assert_called_once_with(None, None, None) + mock_delete_backup_snaps.assert_called_once_with( + self.mock_rbd.Image.return_value) self.assertFalse( self.driver.rbd.Image.return_value.unprotect_snap.called) - self.driver.rbd.RBD.return_value.remove.assert_called_once() + self.assertEqual( + 1, self.driver.rbd.RBD.return_value.remove.call_count) @common_mocks def delete_volume_not_found(self): self.mock_rbd.Image.side_effect = self.mock_rbd.ImageNotFound self.assertIsNone(self.driver.delete_volume(self.volume)) - self.mock_rbd.Image.assert_called_once() + self.mock_rbd.Image.assert_called_once_with() # Make sure the exception was raised self.assertEqual(RAISED_EXCEPTIONS, [self.mock_rbd.ImageNotFound]) @@ -326,14 +331,19 @@ class RBDTestCase(test.TestCase): self.assertRaises(exception.VolumeIsBusy, self.driver.delete_volume, self.volume) - mock_get_clone_info.assert_called_once() + mock_get_clone_info.assert_called_once_with( + self.mock_rbd.Image.return_value, + self.volume_name, + None) (self.mock_rbd.Image.return_value.list_snaps - .assert_called_once()) - mock_rados_client.assert_called_once() - mock_delete_backup_snaps.assert_called_once() + .assert_called_once_with()) + mock_rados_client.assert_called_once_with(self.driver) + mock_delete_backup_snaps.assert_called_once_with( + self.mock_rbd.Image.return_value) self.assertFalse( self.mock_rbd.Image.return_value.unprotect_snap.called) - self.mock_rbd.RBD.return_value.remove.assert_called_once() + self.assertEqual( + 1, self.mock_rbd.RBD.return_value.remove.call_count) # Make sure the exception was raised self.assertEqual(RAISED_EXCEPTIONS, [self.mock_rbd.ImageBusy]) @@ -373,7 +383,7 @@ class RBDTestCase(test.TestCase): self.assertEqual(info, parent_info) self.assertFalse(volume.set_snap.called) - volume.parent_info.assert_called_once() + volume.parent_info.assert_called_once_with() @common_mocks def test_get_clone_info_w_snap(self): @@ -390,9 +400,8 @@ class RBDTestCase(test.TestCase): self.assertEqual(info, parent_info) - volume.set_snap.assert_called_once() self.assertEqual(volume.set_snap.call_count, 2) - volume.parent_info.assert_called_once() + volume.parent_info.assert_called_once_with() @common_mocks def test_get_clone_info_w_exception(self): @@ -408,9 +417,8 @@ class RBDTestCase(test.TestCase): self.assertEqual(info, (None, None, None)) - volume.set_snap.assert_called_once() self.assertEqual(volume.set_snap.call_count, 2) - volume.parent_info.assert_called_once() + volume.parent_info.assert_called_once_with() # Make sure the exception was raised self.assertEqual(RAISED_EXCEPTIONS, [self.mock_rbd.ImageNotFound]) @@ -428,7 +436,7 @@ class RBDTestCase(test.TestCase): self.assertEqual(info, parent_info) self.assertFalse(volume.set_snap.called) - volume.parent_info.assert_called_once() + volume.parent_info.assert_called_once_with() @common_mocks def test_create_cloned_volume(self): @@ -436,24 +444,22 @@ class RBDTestCase(test.TestCase): dst_name = u'volume-00000002' self.cfg.rbd_max_clone_depth = 2 - self.mock_rbd.RBD.clone = mock.Mock() with mock.patch.object(self.driver, '_get_clone_depth') as \ mock_get_clone_depth: # Try with no flatten required mock_get_clone_depth.return_value = 1 - self.mock_rbd.Image.create_snap = mock.Mock() - self.mock_rbd.Image.protect_snap = mock.Mock() - self.mock_rbd.Image.close = mock.Mock() + self.driver.create_cloned_volume({'name': dst_name}, + {'name': src_name}) - self.driver.create_cloned_volume(dict(name=dst_name), - dict(name=src_name)) - - self.mock_rbd.Image.create_snap.assert_called_once() - self.mock_rbd.Image.protect_snap.assert_called_once() - self.mock_rbd.RBD.clone.assert_called_once() - self.mock_rbd.Image.close.assert_called_once() + (self.mock_rbd.Image.return_value.create_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + (self.mock_rbd.Image.return_value.protect_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + self.assertEqual( + 1, self.mock_rbd.RBD.return_value.clone.call_count) + self.mock_rbd.Image.return_value.close.assert_called_once_with() self.assertTrue(mock_get_clone_depth.called) @common_mocks @@ -474,13 +480,17 @@ class RBDTestCase(test.TestCase): self.driver.create_cloned_volume, dict(name=dst_name), dict(name=src_name)) - self.mock_rbd.Image.return_value.create_snap.assert_called_once() - self.mock_rbd.Image.return_value.protect_snap.assert_called_once() - self.mock_rbd.RBD.return_value.clone.assert_called_once() - (self.mock_rbd.Image.return_value - .unprotect_snap.assert_called_once()) - self.mock_rbd.Image.return_value.remove_snap.assert_called_once() - self.mock_rbd.Image.return_value.close.assert_called_once() + (self.mock_rbd.Image.return_value.create_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + (self.mock_rbd.Image.return_value.protect_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + self.assertEqual( + 1, self.mock_rbd.RBD.return_value.clone.call_count) + (self.mock_rbd.Image.return_value.unprotect_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + (self.mock_rbd.Image.return_value.remove_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + self.mock_rbd.Image.return_value.close.assert_called_once_with() self.assertTrue(mock_get_clone_depth.called) @common_mocks @@ -498,14 +508,19 @@ class RBDTestCase(test.TestCase): self.assertRaises(self.mock_rbd.RBD.Error, self.driver.create_cloned_volume, - dict(name=dst_name), dict(name=src_name)) + {'name': dst_name}, {'name': src_name}) - self.mock_rbd.Image.create_snap.assert_called_once() - self.mock_rbd.Image.protect_snap.assert_called_once() - self.mock_rbd.RBD.clone.assert_called_once() - self.mock_rbd.Image.unprotect_snap.assert_called_once() - self.mock_rbd.Image.remove_snap.assert_called_once() - self.mock_rbd.Image.close.assert_called_once() + (self.mock_rbd.Image.return_value.create_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + (self.mock_rbd.Image.return_value.protect_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + self.assertEqual( + 1, self.mock_rbd.RBD.return_value.clone.call_count) + (self.mock_rbd.Image.return_value.unprotect_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + (self.mock_rbd.Image.return_value.remove_snap + .assert_called_once_with('.'.join((dst_name, 'clone_snap')))) + self.mock_rbd.Image.return_value.close.assert_called_once_with() @common_mocks def test_good_locations(self): @@ -558,7 +573,7 @@ class RBDTestCase(test.TestCase): args = [location, {'disk_format': 'raw'}] self.assertFalse(self.driver._is_cloneable(*args)) - self.mock_proxy.assert_called_once() + self.assertEqual(1, self.mock_proxy.call_count) self.assertTrue(mock_get_fsid.called) @common_mocks @@ -617,7 +632,7 @@ class RBDTestCase(test.TestCase): reserved_percentage=0) actual = self.driver.get_volume_stats(True) - client.cluster.get_cluster_stats.assert_called_once() + client.cluster.get_cluster_stats.assert_called_once_with() self.assertDictMatch(expected, actual) @common_mocks @@ -641,7 +656,7 @@ class RBDTestCase(test.TestCase): reserved_percentage=0) actual = self.driver.get_volume_stats(True) - client.cluster.get_cluster_stats.assert_called_once() + client.cluster.get_cluster_stats.assert_called_once_with() self.assertDictMatch(expected, actual) @common_mocks @@ -721,35 +736,24 @@ class RBDTestCase(test.TestCase): self.mox.VerifyAll() - @common_mocks def test_rbd_volume_proxy_init(self): + mock_driver = mock.Mock(name='driver') + mock_driver._connect_to_rados.return_value = (None, None) + with driver.RBDVolumeProxy(mock_driver, self.volume_name): + self.assertEqual(1, mock_driver._connect_to_rados.call_count) + self.assertFalse(mock_driver._disconnect_from_rados.called) + + self.assertEqual(1, mock_driver._disconnect_from_rados.call_count) + + mock_driver.reset_mock() + snap = u'snapshot-name' + with driver.RBDVolumeProxy(mock_driver, self.volume_name, + snapshot=snap): + self.assertEqual(1, mock_driver._connect_to_rados.call_count) + self.assertFalse(mock_driver._disconnect_from_rados.called) - client = self.mock_client.return_value - client.__enter__.return_value = client - - with mock.patch.object(self.driver, '_connect_to_rados') as \ - mock_connect_from_rados: - with mock.patch.object(self.driver, '_disconnect_from_rados') as \ - mock_disconnect_from_rados: - mock_connect_from_rados.return_value = (None, None) - mock_disconnect_from_rados.return_value = (None, None) - - with driver.RBDVolumeProxy(self.driver, self.volume_name): - mock_connect_from_rados.assert_called_once() - self.assertFalse(mock_disconnect_from_rados.called) - - mock_disconnect_from_rados.assert_called_once() - - mock_connect_from_rados.reset_mock() - mock_disconnect_from_rados.reset_mock() - - with driver.RBDVolumeProxy(self.driver, self.volume_name, - snapshot=snap): - mock_connect_from_rados.assert_called_once() - self.assertFalse(mock_disconnect_from_rados.called) - - mock_disconnect_from_rados.assert_called_once() + self.assertEqual(1, mock_driver._disconnect_from_rados.call_count) @common_mocks def test_connect_to_rados(self): @@ -791,8 +795,8 @@ class RBDTestCase(test.TestCase): self.mock_rados.Error) self.assertRaises(exception.VolumeBackendAPIException, self.driver._connect_to_rados) - self.mock_rados.Rados.return_value.open_ioctx.assert_called_once() - self.mock_rados.Rados.return_value.shutdown.assert_called_once() + self.assertTrue(self.mock_rados.Rados.return_value.open_ioctx.called) + self.mock_rados.Rados.return_value.shutdown.assert_called_once_with() class RBDImageIOWrapperTestCase(test.TestCase): @@ -896,12 +900,12 @@ class RBDImageIOWrapperTestCase(test.TestCase): with mock.patch.object(driver, 'LOG') as mock_logger: self.meta.image.flush = mock.Mock() self.mock_rbd_wrapper.flush() - self.meta.image.flush.assert_called_once() + self.meta.image.flush.assert_called_once_with() self.meta.image.flush.reset_mock() # this should be caught and logged silently. self.meta.image.flush.side_effect = AttributeError self.mock_rbd_wrapper.flush() - self.meta.image.flush.assert_called_once() + self.meta.image.flush.assert_called_once_with() msg = _("flush() not supported in this version of librbd") mock_logger.warning.assert_called_with(msg) @@ -985,7 +989,7 @@ class ManagedRBDTestCase(DriverTestCase): self._create_volume_from_image('available', raw=True) self.assertFalse(mock_copy.called) - mock_clone_image.assert_called_once() + self.assertTrue(mock_clone_image.called) self.assertFalse(mock_create.called) def test_create_vol_from_non_raw_image_status_available(self): @@ -1002,10 +1006,10 @@ class ManagedRBDTestCase(DriverTestCase): with mock.patch.object(create_volume.CreateVolumeFromSpecTask, '_copy_image_to_volume') as mock_copy: self._create_volume_from_image('available', raw=False) - mock_copy.assert_called_once() + self.assertTrue(mock_copy.called) - mock_clone_image.assert_called_once() - mock_create.assert_called_once() + self.assertTrue(mock_clone_image.called) + self.assertTrue(mock_create.called) def test_create_vol_from_image_status_error(self): """Fail to clone raw image then verify volume is in error state.""" @@ -1019,7 +1023,7 @@ class ManagedRBDTestCase(DriverTestCase): clone_error=True) self.assertFalse(mock_copy.called) - mock_clone_image.assert_called_once() + self.assertTrue(mock_clone_image.called) self.assertFalse(self.volume.driver.create_volume.called) def test_clone_failure(self): @@ -1047,10 +1051,12 @@ class ManagedRBDTestCase(DriverTestCase): mock_resize: image_loc = ('rbd://fee/fi/fo/fum', None) - actual = driver.clone_image({'name': 'vol1'}, image_loc, + volume = {'name': 'vol1'} + actual = driver.clone_image(volume, image_loc, 'id.foo', {'disk_format': 'raw'}) self.assertEqual(expected, actual) - mock_clone.assert_called_once() - mock_resize.assert_called_once() + mock_clone.assert_called_once_with(volume, + 'fi', 'fo', 'fum') + mock_resize.assert_called_once_with(volume) diff --git a/cinder/tests/test_smbfs.py b/cinder/tests/test_smbfs.py index fa1e1380f75..75fb43188f0 100644 --- a/cinder/tests/test_smbfs.py +++ b/cinder/tests/test_smbfs.py @@ -108,7 +108,7 @@ class SmbFsTestCase(test.TestCase): else: self._smbfs_driver.do_setup(None) self.assertEqual(self._smbfs_driver.shares, {}) - fake_ensure_mounted.assert_called_once() + fake_ensure_mounted.assert_called_once_with() def test_setup_missing_shares_config_option(self): fake_config = copy.copy(self._FAKE_SMBFS_CONFIG) diff --git a/cinder/tests/test_utils.py b/cinder/tests/test_utils.py index 80decf5980f..9381de23970 100644 --- a/cinder/tests/test_utils.py +++ b/cinder/tests/test_utils.py @@ -511,7 +511,7 @@ class GenericUtilsTestCase(test.TestCase): mock_channel = mock.Mock() mock_client.invoke_shell.return_value = mock_channel utils.create_channel(mock_client, test_width, test_height) - mock_client.invoke_shell.assert_called_once() + mock_client.invoke_shell.assert_called_once_with() mock_channel.resize_pty.assert_called_once_with(test_width, test_height) @@ -899,8 +899,6 @@ class SSHPoolTestCase(test.TestCase): @mock.patch('paramiko.SSHClient') def test_single_ssh_connect(self, mock_sshclient, mock_pkey, mock_isfile, mock_open): - mock_sshclient.return_value = FakeSSHClient() - CONF.ssh_hosts_key_file = '/var/lib/cinder/ssh_known_hosts' # create with password @@ -916,7 +914,7 @@ class SSHPoolTestCase(test.TestCase): second_id = ssh.id self.assertEqual(first_id, second_id) - mock_sshclient.connect.assert_called_once() + self.assertEqual(1, mock_sshclient.return_value.connect.call_count) # create with private key sshpool = ssh_utils.SSHPool("127.0.0.1", 22, 10, @@ -924,7 +922,7 @@ class SSHPoolTestCase(test.TestCase): privatekey="test", min_size=1, max_size=1) - mock_sshclient.connect.assert_called_once() + self.assertEqual(2, mock_sshclient.return_value.connect.call_count) # attempt to create with no password or private key self.assertRaises(paramiko.SSHException, diff --git a/cinder/tests/windows/test_vhdutils.py b/cinder/tests/windows/test_vhdutils.py index 19be431d88f..e2974fe921a 100644 --- a/cinder/tests/windows/test_vhdutils.py +++ b/cinder/tests/windows/test_vhdutils.py @@ -302,7 +302,7 @@ class VHDUtilsTestCase(test.TestCase): open_access_mask=vhdutils.VIRTUAL_DISK_ACCESS_GET_INFO) self._vhdutils._get_vhd_info_member.assert_called_with( self._FAKE_FILE_HANDLE, fake_info_member) - self._vhdutils._close.assert_called_once() + self._vhdutils._close.assert_called_once_with(self._FAKE_FILE_HANDLE) def test_parse_vhd_info(self): fake_physical_size = self._FAKE_VHD_SIZE + 1 diff --git a/cinder/tests/zonemanager/test_brcd_fc_zone_client_cli.py b/cinder/tests/zonemanager/test_brcd_fc_zone_client_cli.py index fa3157a475d..18a696adf86 100644 --- a/cinder/tests/zonemanager/test_brcd_fc_zone_client_cli.py +++ b/cinder/tests/zonemanager/test_brcd_fc_zone_client_cli.py @@ -90,25 +90,26 @@ class TestBrcdFCZoneClientCLI(BrcdFCZoneClientCLI, test.TestCase): @mock.patch.object(BrcdFCZoneClientCLI, 'get_active_zone_set') @mock.patch.object(BrcdFCZoneClientCLI, 'apply_zone_change') @mock.patch.object(BrcdFCZoneClientCLI, '_cfg_save') - def test_add_zones_new_zone_no_activate(self, get_active_zs_mock, + def test_add_zones_new_zone_no_activate(self, cfg_save_mock, apply_zone_change_mock, - cfg_save_mock): + get_active_zs_mock): get_active_zs_mock.return_value = active_zoneset self.add_zones(new_zones, False, None) - get_active_zs_mock.assert_called_once() - apply_zone_change_mock.assert_called_twice() - cfg_save_mock.assert_called_once() + get_active_zs_mock.assert_called_once_with() + self.assertEqual(3, apply_zone_change_mock.call_count) + cfg_save_mock.assert_called_once_with() @mock.patch.object(BrcdFCZoneClientCLI, 'get_active_zone_set') @mock.patch.object(BrcdFCZoneClientCLI, 'apply_zone_change') @mock.patch.object(BrcdFCZoneClientCLI, 'activate_zoneset') - def test_add_zones_new_zone_activate(self, get_active_zs_mock, + def test_add_zones_new_zone_activate(self, activate_zoneset_mock, apply_zone_change_mock, - activate_zoneset_mock): + get_active_zs_mock): get_active_zs_mock.return_value = active_zoneset self.add_zones(new_zone, True, active_zoneset) - apply_zone_change_mock.assert_called_once() - activate_zoneset_mock.assert_called_once() + self.assertEqual(2, apply_zone_change_mock.call_count) + activate_zoneset_mock.assert_called_once_with( + active_zoneset['active_zone_config']) @mock.patch.object(BrcdFCZoneClientCLI, '_ssh_execute') def test_activate_zoneset(self, ssh_execute_mock): @@ -124,27 +125,27 @@ class TestBrcdFCZoneClientCLI(BrcdFCZoneClientCLI, test.TestCase): @mock.patch.object(BrcdFCZoneClientCLI, 'apply_zone_change') @mock.patch.object(BrcdFCZoneClientCLI, '_cfg_save') - def test_delete_zones_activate_false(self, apply_zone_change_mock, - cfg_save_mock): - with mock.patch.object(self, '_zone_delete') \ - as zone_delete_mock: + def test_delete_zones_activate_false(self, cfg_save_mock, + apply_zone_change_mock): + with mock.patch.object(self, '_zone_delete') as zone_delete_mock: self.delete_zones(zone_names_to_delete, False, active_zoneset_multiple_zones) - apply_zone_change_mock.assert_called_once() + self.assertEqual(1, apply_zone_change_mock.call_count) zone_delete_mock.assert_called_once_with(zone_names_to_delete) - cfg_save_mock.assert_called_once() + cfg_save_mock.assert_called_once_with() @patch.object(BrcdFCZoneClientCLI, 'apply_zone_change') @patch.object(BrcdFCZoneClientCLI, 'activate_zoneset') - def test_delete_zones_activate_true(self, apply_zone_change_mock, - activate_zs_mock): + def test_delete_zones_activate_true(self, activate_zs_mock, + apply_zone_change_mock): with mock.patch.object(self, '_zone_delete') \ as zone_delete_mock: self.delete_zones(zone_names_to_delete, True, active_zoneset_multiple_zones) - apply_zone_change_mock.assert_called_once() + self.assertEqual(1, apply_zone_change_mock.call_count) zone_delete_mock.assert_called_once_with(zone_names_to_delete) - activate_zs_mock.assert_called_once() + activate_zs_mock.assert_called_once_with( + active_zoneset['active_zone_config']) @patch.object(BrcdFCZoneClientCLI, '_get_switch_info') def test_get_nameserver_info(self, get_switch_info_mock): @@ -180,7 +181,7 @@ class TestBrcdFCZoneClientCLI(BrcdFCZoneClientCLI, test.TestCase): as is_trans_abortable_mock: is_trans_abortable_mock.return_value = True self._cfg_trans_abort() - is_trans_abortable_mock.assert_called_once() + is_trans_abortable_mock.assert_called_once_with() apply_zone_change_mock.assert_called_once_with(cmd_list) @patch.object(BrcdFCZoneClientCLI, '_run_ssh') diff --git a/cinder/tests/zonemanager/test_fc_zone_manager.py b/cinder/tests/zonemanager/test_fc_zone_manager.py index 11cf92e7f7c..f1b2099473d 100644 --- a/cinder/tests/zonemanager/test_fc_zone_manager.py +++ b/cinder/tests/zonemanager/test_fc_zone_manager.py @@ -60,7 +60,7 @@ class TestFCZoneManager(test.TestCase): as add_connection_mock: self.zm.driver.get_san_context.return_value = fabric_map self.zm.add_connection(init_target_map) - self.zm.driver.get_san_context.assert_called_once(target_list) + self.zm.driver.get_san_context.assert_called_once_with(target_list) add_connection_mock.assert_called_once_with(fabric_name, init_target_map)