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

@@ -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

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)

cinder/tests/keymgr/test_barbican.py

@@ -124,7 +124,7 @@ def test_copy_key(self):
             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 @@ def test_store_key_plaintext(self):
                                             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

cinder/tests/test_backup_driver_base.py

@@ -236,4 +236,4 @@ def test_is_not_serializable(self):
         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)

cinder/tests/test_glusterfs.py

@@ -1858,7 +1858,7 @@ def test_copy_volume_to_image_raw_image(self):
             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 @@ def test_copy_volume_to_image_qcow2_image(self):
                 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 @@ def test_copy_volume_to_image_snapshot_exists(self):
                 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)

cinder/tests/test_hp_msa.py

@@ -496,7 +496,7 @@ def test_initialize_connection(self, mock_login, mock_map, mock_ports,
                                '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')