Reject non existent mock assert calls
assert_called and assert_not_called are not asserting the state of the mock object but considered as mocked calls so mock will never raise exception but always executed successfully This change patches the Mock class during unit test to raise an exception if a function called on a mock object that's name starts with 'assert' and does not one of the supported Mock assert calls. This change also fix the unit test to call only the supported assert function on mock object. This change also removes hacking rule N327 as the mock change renders this hacking rule obsolete. Change-Id: Id75def84bd4bdc1c1fb06221b4223322a99251eb
This commit is contained in:
parent
0fef477e3b
commit
e0d22ba4de
|
@ -379,17 +379,6 @@ 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)
|
||||
|
||||
|
||||
def check_api_version_decorator(logical_line, blank_before, filename):
|
||||
msg = ("N332: the api_version decorator must be the first decorator"
|
||||
" on a method.")
|
||||
|
@ -474,7 +463,6 @@ def factory(register):
|
|||
register(no_mutable_default_args)
|
||||
register(check_explicit_underscore_import)
|
||||
register(use_jsonutils)
|
||||
register(check_assert_called_once)
|
||||
register(check_api_version_decorator)
|
||||
register(CheckForStrUnicodeExc)
|
||||
register(CheckForTransAdd)
|
||||
|
|
24
nova/test.py
24
nova/test.py
|
@ -26,6 +26,7 @@ eventlet.monkey_patch(os=False)
|
|||
|
||||
import copy
|
||||
import inspect
|
||||
import mock
|
||||
import logging
|
||||
import os
|
||||
|
||||
|
@ -157,6 +158,29 @@ class skipXmlTest(skipIf):
|
|||
reason)
|
||||
|
||||
|
||||
def _patch_mock_to_raise_for_invalid_assert_calls():
|
||||
def raise_for_invalid_assert_calls(wrapped):
|
||||
def wrapper(_self, name):
|
||||
valid_asserts = [
|
||||
'assert_called_with',
|
||||
'assert_called_once_with',
|
||||
'assert_has_calls',
|
||||
'assert_any_calls']
|
||||
|
||||
if name.startswith('assert') and name not in valid_asserts:
|
||||
raise AttributeError('%s is not a valid mock assert method'
|
||||
% name)
|
||||
|
||||
return wrapped(_self, name)
|
||||
return wrapper
|
||||
mock.Mock.__getattr__ = raise_for_invalid_assert_calls(
|
||||
mock.Mock.__getattr__)
|
||||
|
||||
# NOTE(gibi): needs to be called only once at import time
|
||||
# to patch the mock lib
|
||||
_patch_mock_to_raise_for_invalid_assert_calls()
|
||||
|
||||
|
||||
class TestCase(testtools.TestCase):
|
||||
"""Test case base class for all unit tests.
|
||||
|
||||
|
|
|
@ -5017,7 +5017,7 @@ class ComputeTestCase(BaseTestCase):
|
|||
self.context, instance, bdms='fake_bdms')
|
||||
mock_terminate_vol_conn.assert_called_once_with(self.context,
|
||||
instance, 'fake_bdms')
|
||||
mock_get_power_off_values.assert_caleld_once_with(self.context,
|
||||
mock_get_power_off_values.assert_called_once_with(self.context,
|
||||
instance, clean_shutdown)
|
||||
self.assertEqual(migration.dest_compute, instance.host)
|
||||
self.compute.terminate_instance(self.context, instance, [], [])
|
||||
|
|
|
@ -291,18 +291,6 @@ class HackingTestCase(test.NoDBTestCase):
|
|||
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_unicode_exception(self):
|
||||
|
||||
checker = checks.CheckForStrUnicodeExc
|
||||
|
|
|
@ -8513,7 +8513,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
|||
result = drvr._get_disk_over_committed_size_total()
|
||||
self.assertEqual(result, 10653532160)
|
||||
mock_list.assert_called_with()
|
||||
mock_info.assert_called()
|
||||
self.assertTrue(mock_info.called)
|
||||
|
||||
@mock.patch.object(libvirt_driver.LibvirtDriver,
|
||||
"_list_instance_domains")
|
||||
|
@ -9631,20 +9631,17 @@ Active: 8381604 kB
|
|||
dom_mock.ID.assert_called_once_with()
|
||||
lookup_mock.assert_called_once_with(instance['name'])
|
||||
|
||||
@mock.patch.object(fake_libvirt_utils, 'get_instance_path')
|
||||
@mock.patch.object(encodeutils, 'safe_decode')
|
||||
def test_create_domain(self, mock_safe_decode, mock_get_inst_path):
|
||||
def test_create_domain(self, mock_safe_decode):
|
||||
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
|
||||
mock_domain = mock.MagicMock()
|
||||
mock_instance = mock.MagicMock()
|
||||
mock_get_inst_path.return_value = '/tmp/'
|
||||
|
||||
domain = conn._create_domain(domain=mock_domain,
|
||||
instance=mock_instance)
|
||||
|
||||
self.assertEqual(mock_domain, domain)
|
||||
mock_get_inst_path.assertHasCalls([mock.call(mock_instance)])
|
||||
mock_domain.createWithFlags.assertHasCalls([mock.call(0)])
|
||||
mock_domain.createWithFlags.assert_has_calls([mock.call(0)])
|
||||
self.assertEqual(2, mock_safe_decode.call_count)
|
||||
|
||||
@mock.patch('nova.virt.disk.api.clean_lxc_namespace')
|
||||
|
@ -9682,7 +9679,7 @@ Active: 8381604 kB
|
|||
mock_instance, [], None)
|
||||
|
||||
self.assertEqual('/dev/nbd0', inst_sys_meta['rootfs_device_name'])
|
||||
mock_instance.save.assert_not_called()
|
||||
self.assertFalse(mock_instance.called)
|
||||
mock_get_inst_path.assert_has_calls([mock.call(mock_instance)])
|
||||
mock_ensure_tree.assert_has_calls([mock.call('/tmp/rootfs')])
|
||||
conn.image_backend.image.assert_has_calls([mock.call(mock_instance,
|
||||
|
@ -9745,7 +9742,7 @@ Active: 8381604 kB
|
|||
mock_instance, [], None)
|
||||
|
||||
self.assertEqual('/dev/nbd0', inst_sys_meta['rootfs_device_name'])
|
||||
mock_instance.save.assert_not_called()
|
||||
self.assertFalse(mock_instance.called)
|
||||
mock_get_inst_path.assert_has_calls([mock.call(mock_instance)])
|
||||
mock_ensure_tree.assert_has_calls([mock.call('/tmp/rootfs')])
|
||||
conn.image_backend.image.assert_has_calls([mock.call(mock_instance,
|
||||
|
@ -9794,7 +9791,7 @@ Active: 8381604 kB
|
|||
mock_instance, [], None)
|
||||
|
||||
self.assertEqual('/dev/nbd0', inst_sys_meta['rootfs_device_name'])
|
||||
mock_instance.save.assert_not_called()
|
||||
self.assertFalse(mock_instance.called)
|
||||
mock_get_inst_path.assert_has_calls([mock.call(mock_instance)])
|
||||
mock_ensure_tree.assert_has_calls([mock.call('/tmp/rootfs')])
|
||||
conn.image_backend.image.assert_has_calls([mock.call(mock_instance,
|
||||
|
|
Loading…
Reference in New Issue