diff --git a/HACKING.rst b/HACKING.rst index c32b6db09357..f1a7f1ed1b7d 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -39,6 +39,7 @@ Nova Specific Commandments - [N324] Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s - [N325] str() cannot be used on an exception. Remove use or use six.text_type() - [N326] Translated messages cannot be concatenated. String should be included in translated message. +- [N327] assert_called_once() is not a valid method Creating Unit Tests ------------------- diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index 5a3e2319f79a..22133f579b61 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -347,6 +347,17 @@ def use_jsonutils(logical_line, filename): yield (pos, msg % {'fun': f[:-1]}) +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 'nova/tests/' in filename: + pos = logical_line.find('.assert_called_once(') + if pos != -1: + yield (pos, msg) + + class CheckForStrExc(BaseASTChecker): """Checks for the use of str() on an exception. @@ -423,5 +434,6 @@ def factory(register): register(no_mutable_default_args) register(check_explicit_underscore_import) register(use_jsonutils) + register(check_assert_called_once) register(CheckForStrExc) register(CheckForTransAdd) diff --git a/nova/tests/test_hacking.py b/nova/tests/test_hacking.py index f42af1847254..bfd2cef8854f 100644 --- a/nova/tests/test_hacking.py +++ b/nova/tests/test_hacking.py @@ -248,20 +248,34 @@ class HackingTestCase(test.NoDBTestCase): # installed. @mock.patch('pep8._checks', {'physical_line': {}, 'logical_line': {}, 'tree': {}}) - def _run_check(self, code, checker): + def _run_check(self, code, checker, filename=None): pep8.register_check(checker) lines = textwrap.dedent(code).strip().splitlines(True) - checker = pep8.Checker(lines=lines) + checker = pep8.Checker(filename=filename, lines=lines) checker.check_all() checker.report._deferred_print.sort() return checker.report._deferred_print - def _assert_has_errors(self, code, checker, expected_errors=None): - actual_errors = [e[:3] for e in self._run_check(code, checker)] + def _assert_has_errors(self, code, checker, expected_errors=None, + filename=None): + actual_errors = [e[:3] for e in + self._run_check(code, checker, filename)] self.assertEqual(expected_errors or [], actual_errors) + def test_assert_called_once(self): + + checker = checks.check_assert_called_once + code = """ + mock = Mock() + mock.method(1, 2, 3, test='wow') + mock.method.assert_called_once() + """ + errors = [(3, 11, 'N327')] + self._assert_has_errors(code, checker, expected_errors=errors, + filename='nova/tests/test_assert.py') + def test_str_exception(self): checker = checks.CheckForStrExc diff --git a/nova/tests/virt/hyperv/test_ioutils.py b/nova/tests/virt/hyperv/test_ioutils.py index 786d6363e4ef..570d6a6722f5 100644 --- a/nova/tests/virt/hyperv/test_ioutils.py +++ b/nova/tests/virt/hyperv/test_ioutils.py @@ -50,7 +50,7 @@ class IOThreadTestCase(test.NoDBTestCase): self._iothread._copy(self._FAKE_SRC, self._FAKE_DEST) fake_dest.write.assert_called_once_with(fake_data) - fake_dest.close.assert_called_once() + fake_dest.close.assert_called_once_with() fake_rename.assert_called_once_with( self._iothread._dest, self._iothread._dest_archive) fake_remove.assert_called_once_with( diff --git a/nova/tests/virt/ironic/test_driver.py b/nova/tests/virt/ironic/test_driver.py index a356bc375bae..d33d6234ca08 100644 --- a/nova/tests/virt/ironic/test_driver.py +++ b/nova/tests/virt/ironic/test_driver.py @@ -529,7 +529,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper): instance) fake_looping_call.start.assert_called_once_with( interval=CONF.ironic.api_retry_interval) - fake_looping_call.wait.assert_called_once() + fake_looping_call.wait.assert_called_once_with() @mock.patch.object(loopingcall, 'FixedIntervalLoopingCall') @mock.patch.object(FAKE_CLIENT, 'node') @@ -1117,7 +1117,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper): instance) fake_looping_call.start.assert_called_once_with( interval=CONF.ironic.api_retry_interval) - fake_looping_call.wait.assert_called_once() + fake_looping_call.wait.assert_called_once_with() def test_rebuild_preserve_ephemeral(self): self._test_rebuild(preserve=True) diff --git a/nova/tests/virt/libvirt/test_driver.py b/nova/tests/virt/libvirt/test_driver.py index d193d42bb421..08fecec5ce88 100644 --- a/nova/tests/virt/libvirt/test_driver.py +++ b/nova/tests/virt/libvirt/test_driver.py @@ -3968,7 +3968,10 @@ class LibvirtConnTestCase(test.TestCase, mock_lookupByName.assert_called_once_with("instance-00000001") mock_volume_info.assert_has_calls(mock_volume_info_calls) - mock_convert_image.assert_called_once() + mock_convert_image.assert_called_once_with('/dev/nova-vg/lv', + mock.ANY, + 'raw', + run_as_root=True) snapshot = image_service.show(context, recv_meta['id']) mock_update_task_state.assert_has_calls(update_task_state_calls) self.assertEqual('available', snapshot['properties']['image_state']) @@ -4186,7 +4189,10 @@ class LibvirtConnTestCase(test.TestCase, mock_lookupByName.assert_called_once_with("instance-00000001") mock_volume_info.assert_has_calls(mock_volume_info_calls) - mock_convert_image.assert_called_once() + mock_convert_image.assert_called_once_with('/dev/nova-vg/lv', + mock.ANY, + 'qcow2', + run_as_root=True) snapshot = image_service.show(context, recv_meta['id']) mock_update_task_state.assert_has_calls(update_task_state_calls) self.assertEqual('available', snapshot['properties']['image_state']) @@ -9870,7 +9876,7 @@ Active: 8381604 kB "host1.example.com", lambda x: x, lambda x: x) - mock_spawn.assert_called_once() + self.assertEqual(1, mock_spawn.call_count) @mock.patch.object(greenthread, "spawn") @mock.patch.object(fake_libvirt_utils, "is_valid_hostname")