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

diff --git a/HACKING.rst b/HACKING.rst
index 1d1440974f612b28614fb922eaf26b0685b874d8..93e0df93528db7663c7cc952eb216614a1434f7e 100644 (file)
--- 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 d05ad7efb3d2feb827418c23f7b9ea1691a0dc55..8256c0f10587a578777fb33ec69124c59a3b6dbc 100644 (file)
--- 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 ea726c129cf6d5d960ac4b88116780b5c247d018..bc81b1c5e644fcf20fb2cb996edd41fb88cb1b0b 100644 (file)
--- 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 67a2d8f0dbbef5a9385d0c88d771e4b7b2aca668..74ef1aeaeacd37d95d9a0da6ad8aa62cc1edd467 100644 (file)
--- 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 c433f22761e1e8342f57fb86a69a20fb96d0dd54..d06cf69e8804c7fb56fa85e2e4fb6186359280d8 100644 (file)
--- 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 205689bdb1d9f4e14519036fb38800e8bf5ba369..94510ac29b347a8493002dc3ee5b6c806847cb59 100644 (file)
--- 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 9480c6fbeaeb81145b06d7d6eae818949df40759..8c721b24f756343ebca5873a5e653053a56b2e74 100644 (file)
--- 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))
-
-            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()
+                              {'name': dst_name}, {'name': src_name})
+
+            (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):
-        snap = u'snapshot-name'
-
-        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)
+        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)
 
-                with driver.RBDVolumeProxy(self.driver, self.volume_name):
-                    mock_connect_from_rados.assert_called_once()
-                    self.assertFalse(mock_disconnect_from_rados.called)
+        self.assertEqual(1, mock_driver._disconnect_from_rados.call_count)
 
-                mock_disconnect_from_rados.assert_called_once()
+        mock_driver.reset_mock()
 
-                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)
+        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)
 
-                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 fa1e1380f75046d6823808e1b135073ff9bc48ad..75fb43188f0aee551ab599a1619da5ffba6b460a 100644 (file)
--- 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 80decf5980f967ee78316702b8930bc0a34b98ba..9381de239703bf7236010c1d32aa4f3af85047cd 100644 (file)
--- 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 19be431d88f326c0e1224c6b53434a92614029d7..e2974fe921a18acf227d1a492e3236f7aa91b7c3 100644 (file)
--- 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 fa3157a475d110cf54d36d5eeb40ff35279a8bcf..18a696adf86b1a0269085514c82c56c3308f0bf7 100644 (file)
--- 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 11cf92e7f7c6a115ea7f8488fd383626e5f05bb6..f1b2099473db1bb1d4615f77a0c9c63df711af5d 100644 (file)
--- 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)