Fix failures introduced by the new version of mock
=== 'assert_has_calls' that have changed behavior in the new version (no longer accepts single calls). Calls to non-existent assert methods that did not exist and were silently passing. Use of autospec on a class with decorated functions. Conflicts: neutron/tests/base.py (cherry picked from commit3a871a73b8
) === Fix dhcp _test_sync_state_helper asserting calls wrong It was using a non-existing method that did nothing and that masked other problems with the tests that used it. Changed to use assert_has_calls() and fixed the problems that fell out. (cherry picked from commitb85cfa9611
) === Closes-Bug: #1473369 Change-Id: I164a9af2a7f9ac0f0229ec3c5071f7a470445c98
This commit is contained in:
parent
aea84e8bd7
commit
5eff0019dc
@ -107,14 +107,22 @@ def check_assert_called_once_with(logical_line, filename):
|
|||||||
# Try to detect unintended calls of nonexistent mock methods like:
|
# Try to detect unintended calls of nonexistent mock methods like:
|
||||||
# assert_called_once
|
# assert_called_once
|
||||||
# assertCalledOnceWith
|
# assertCalledOnceWith
|
||||||
|
# assert_has_called
|
||||||
if 'neutron/tests/' in filename:
|
if 'neutron/tests/' in filename:
|
||||||
if '.assert_called_once_with(' in logical_line:
|
if '.assert_called_once_with(' in logical_line:
|
||||||
return
|
return
|
||||||
if '.assertcalledonce' in logical_line.lower().replace('_', ''):
|
uncased_line = logical_line.lower().replace('_', '')
|
||||||
|
|
||||||
|
if '.assertcalledonce' in uncased_line:
|
||||||
msg = ("N322: Possible use of no-op mock method. "
|
msg = ("N322: Possible use of no-op mock method. "
|
||||||
"please use assert_called_once_with.")
|
"please use assert_called_once_with.")
|
||||||
yield (0, msg)
|
yield (0, msg)
|
||||||
|
|
||||||
|
if '.asserthascalled' in uncased_line:
|
||||||
|
msg = ("N322: Possible use of no-op mock method. "
|
||||||
|
"please use assert_has_calls.")
|
||||||
|
yield (0, msg)
|
||||||
|
|
||||||
|
|
||||||
def check_oslo_namespace_imports(logical_line):
|
def check_oslo_namespace_imports(logical_line):
|
||||||
if re.match(oslo_namespace_imports_from_dot, logical_line):
|
if re.match(oslo_namespace_imports_from_dot, logical_line):
|
||||||
|
@ -336,7 +336,9 @@ class TestDhcpAgent(base.BaseTestCase):
|
|||||||
trace_level='warning',
|
trace_level='warning',
|
||||||
expected_sync=False)
|
expected_sync=False)
|
||||||
|
|
||||||
def _test_sync_state_helper(self, known_networks, active_networks):
|
def _test_sync_state_helper(self, known_net_ids, active_net_ids):
|
||||||
|
active_networks = set(mock.Mock(id=netid) for netid in active_net_ids)
|
||||||
|
|
||||||
with mock.patch(DHCP_PLUGIN) as plug:
|
with mock.patch(DHCP_PLUGIN) as plug:
|
||||||
mock_plugin = mock.Mock()
|
mock_plugin = mock.Mock()
|
||||||
mock_plugin.get_active_networks_info.return_value = active_networks
|
mock_plugin.get_active_networks_info.return_value = active_networks
|
||||||
@ -344,23 +346,18 @@ class TestDhcpAgent(base.BaseTestCase):
|
|||||||
|
|
||||||
dhcp = dhcp_agent.DhcpAgent(HOSTNAME)
|
dhcp = dhcp_agent.DhcpAgent(HOSTNAME)
|
||||||
|
|
||||||
attrs_to_mock = dict(
|
attrs_to_mock = dict([(a, mock.DEFAULT)
|
||||||
[(a, mock.DEFAULT) for a in
|
for a in ['disable_dhcp_helper', 'cache',
|
||||||
['refresh_dhcp_helper', 'disable_dhcp_helper', 'cache']])
|
'safe_configure_dhcp_for_network']])
|
||||||
|
|
||||||
with mock.patch.multiple(dhcp, **attrs_to_mock) as mocks:
|
with mock.patch.multiple(dhcp, **attrs_to_mock) as mocks:
|
||||||
mocks['cache'].get_network_ids.return_value = known_networks
|
mocks['cache'].get_network_ids.return_value = known_net_ids
|
||||||
dhcp.sync_state()
|
dhcp.sync_state()
|
||||||
|
|
||||||
exp_refresh = [
|
diff = set(known_net_ids) - set(active_net_ids)
|
||||||
mock.call(net_id) for net_id in active_networks]
|
|
||||||
|
|
||||||
diff = set(known_networks) - set(active_networks)
|
|
||||||
exp_disable = [mock.call(net_id) for net_id in diff]
|
exp_disable = [mock.call(net_id) for net_id in diff]
|
||||||
|
|
||||||
mocks['cache'].assert_has_calls([mock.call.get_network_ids()])
|
mocks['cache'].assert_has_calls([mock.call.get_network_ids()])
|
||||||
mocks['refresh_dhcp_helper'].assert_has_called(exp_refresh)
|
mocks['disable_dhcp_helper'].assert_has_calls(exp_disable)
|
||||||
mocks['disable_dhcp_helper'].assert_has_called(exp_disable)
|
|
||||||
|
|
||||||
def test_sync_state_initial(self):
|
def test_sync_state_initial(self):
|
||||||
self._test_sync_state_helper([], ['a'])
|
self._test_sync_state_helper([], ['a'])
|
||||||
@ -372,19 +369,10 @@ class TestDhcpAgent(base.BaseTestCase):
|
|||||||
self._test_sync_state_helper(['b'], ['a'])
|
self._test_sync_state_helper(['b'], ['a'])
|
||||||
|
|
||||||
def test_sync_state_waitall(self):
|
def test_sync_state_waitall(self):
|
||||||
class mockNetwork(object):
|
|
||||||
id = '0'
|
|
||||||
admin_state_up = True
|
|
||||||
subnets = []
|
|
||||||
|
|
||||||
def __init__(self, id):
|
|
||||||
self.id = id
|
|
||||||
with mock.patch.object(dhcp_agent.eventlet.GreenPool, 'waitall') as w:
|
with mock.patch.object(dhcp_agent.eventlet.GreenPool, 'waitall') as w:
|
||||||
active_networks = [mockNetwork('1'), mockNetwork('2'),
|
active_net_ids = ['1', '2', '3', '4', '5']
|
||||||
mockNetwork('3'), mockNetwork('4'),
|
known_net_ids = ['1', '2', '3', '4', '5']
|
||||||
mockNetwork('5')]
|
self._test_sync_state_helper(known_net_ids, active_net_ids)
|
||||||
known_networks = ['1', '2', '3', '4', '5']
|
|
||||||
self._test_sync_state_helper(known_networks, active_networks)
|
|
||||||
w.assert_called_once_with()
|
w.assert_called_once_with()
|
||||||
|
|
||||||
def test_sync_state_plugin_error(self):
|
def test_sync_state_plugin_error(self):
|
||||||
|
@ -59,7 +59,7 @@ class TestAsyncProcess(base.BaseTestCase):
|
|||||||
with mock.patch.object(self.proc, '_kill') as kill:
|
with mock.patch.object(self.proc, '_kill') as kill:
|
||||||
self.proc._handle_process_error()
|
self.proc._handle_process_error()
|
||||||
|
|
||||||
kill.assert_has_calls(mock.call(respawning=False))
|
kill.assert_has_calls([mock.call(respawning=False)])
|
||||||
|
|
||||||
def test__handle_process_error_kills_without_respawn(self):
|
def test__handle_process_error_kills_without_respawn(self):
|
||||||
self.proc.respawn_interval = 1
|
self.proc.respawn_interval = 1
|
||||||
@ -68,8 +68,8 @@ class TestAsyncProcess(base.BaseTestCase):
|
|||||||
with mock.patch('eventlet.sleep') as sleep:
|
with mock.patch('eventlet.sleep') as sleep:
|
||||||
self.proc._handle_process_error()
|
self.proc._handle_process_error()
|
||||||
|
|
||||||
kill.assert_has_calls(mock.call(respawning=True))
|
kill.assert_has_calls([mock.call(respawning=True)])
|
||||||
sleep.assert_has_calls(mock.call(self.proc.respawn_interval))
|
sleep.assert_has_calls([mock.call(self.proc.respawn_interval)])
|
||||||
spawn.assert_called_once_with()
|
spawn.assert_called_once_with()
|
||||||
|
|
||||||
def _test__watch_process(self, callback, kill_event):
|
def _test__watch_process(self, callback, kill_event):
|
||||||
|
@ -50,7 +50,7 @@ class TestPrivileges(base.BaseTestCase):
|
|||||||
with mock.patch.object(daemon.LOG, 'critical') as log_critical:
|
with mock.patch.object(daemon.LOG, 'critical') as log_critical:
|
||||||
self.assertRaises(exceptions.FailToDropPrivilegesExit,
|
self.assertRaises(exceptions.FailToDropPrivilegesExit,
|
||||||
daemon.setuid, '321')
|
daemon.setuid, '321')
|
||||||
log_critical.assert_once_with(mock.ANY)
|
log_critical.assert_called_once_with(mock.ANY)
|
||||||
|
|
||||||
def test_setgid_with_name(self):
|
def test_setgid_with_name(self):
|
||||||
with mock.patch('grp.getgrnam', return_value=FakeEntry('gr_gid', 123)):
|
with mock.patch('grp.getgrnam', return_value=FakeEntry('gr_gid', 123)):
|
||||||
@ -68,7 +68,7 @@ class TestPrivileges(base.BaseTestCase):
|
|||||||
with mock.patch.object(daemon.LOG, 'critical') as log_critical:
|
with mock.patch.object(daemon.LOG, 'critical') as log_critical:
|
||||||
self.assertRaises(exceptions.FailToDropPrivilegesExit,
|
self.assertRaises(exceptions.FailToDropPrivilegesExit,
|
||||||
daemon.setgid, '321')
|
daemon.setgid, '321')
|
||||||
log_critical.assert_once_with(mock.ANY)
|
log_critical.assert_called_once_with(mock.ANY)
|
||||||
|
|
||||||
def test_drop_no_privileges(self):
|
def test_drop_no_privileges(self):
|
||||||
with contextlib.nested(
|
with contextlib.nested(
|
||||||
@ -115,7 +115,7 @@ class TestPrivileges(base.BaseTestCase):
|
|||||||
with mock.patch.object(daemon.LOG, 'critical') as log_critical:
|
with mock.patch.object(daemon.LOG, 'critical') as log_critical:
|
||||||
self.assertRaises(exceptions.FailToDropPrivilegesExit,
|
self.assertRaises(exceptions.FailToDropPrivilegesExit,
|
||||||
daemon.drop_privileges, 'user')
|
daemon.drop_privileges, 'user')
|
||||||
log_critical.assert_once_with(mock.ANY)
|
log_critical.assert_called_once_with(mock.ANY)
|
||||||
|
|
||||||
|
|
||||||
class TestPidfile(base.BaseTestCase):
|
class TestPidfile(base.BaseTestCase):
|
||||||
|
@ -171,8 +171,9 @@ class TestProcessManager(base.BaseTestCase):
|
|||||||
|
|
||||||
with mock.patch.object(ep, 'utils') as utils:
|
with mock.patch.object(ep, 'utils') as utils:
|
||||||
manager.disable()
|
manager.disable()
|
||||||
utils.assert_has_calls(
|
utils.assert_has_calls([
|
||||||
mock.call.execute(['kill', '-9', 4], run_as_root=True))
|
mock.call.execute(['kill', '-9', 4],
|
||||||
|
run_as_root=True)])
|
||||||
|
|
||||||
def test_disable_namespace(self):
|
def test_disable_namespace(self):
|
||||||
with mock.patch.object(ep.ProcessManager, 'pid') as pid:
|
with mock.patch.object(ep.ProcessManager, 'pid') as pid:
|
||||||
@ -184,8 +185,9 @@ class TestProcessManager(base.BaseTestCase):
|
|||||||
|
|
||||||
with mock.patch.object(ep, 'utils') as utils:
|
with mock.patch.object(ep, 'utils') as utils:
|
||||||
manager.disable()
|
manager.disable()
|
||||||
utils.assert_has_calls(
|
utils.assert_has_calls([
|
||||||
mock.call.execute(['kill', '-9', 4], run_as_root=True))
|
mock.call.execute(['kill', '-9', 4],
|
||||||
|
run_as_root=True)])
|
||||||
|
|
||||||
def test_disable_not_active(self):
|
def test_disable_not_active(self):
|
||||||
with mock.patch.object(ep.ProcessManager, 'pid') as pid:
|
with mock.patch.object(ep.ProcessManager, 'pid') as pid:
|
||||||
|
@ -663,4 +663,4 @@ class TestMidonetInterfaceDriver(TestBase):
|
|||||||
self.ip_dev.assert_has_calls([
|
self.ip_dev.assert_has_calls([
|
||||||
mock.call(self.device_name, namespace=self.namespace),
|
mock.call(self.device_name, namespace=self.namespace),
|
||||||
mock.call().link.delete()])
|
mock.call().link.delete()])
|
||||||
self.ip.assert_has_calls(mock.call().garbage_collect_namespace())
|
self.ip.assert_has_calls([mock.call().garbage_collect_namespace()])
|
||||||
|
@ -32,8 +32,8 @@ class TestGetPollingManager(base.BaseTestCase):
|
|||||||
with polling.get_polling_manager(minimize_polling=True) as pm:
|
with polling.get_polling_manager(minimize_polling=True) as pm:
|
||||||
self.assertEqual(pm.__class__,
|
self.assertEqual(pm.__class__,
|
||||||
polling.InterfacePollingMinimizer)
|
polling.InterfacePollingMinimizer)
|
||||||
mock_stop.assert_has_calls(mock.call())
|
mock_stop.assert_has_calls([mock.call()])
|
||||||
mock_start.assert_has_calls(mock.call())
|
mock_start.assert_has_calls([mock.call()])
|
||||||
|
|
||||||
|
|
||||||
class TestInterfacePollingMinimizer(base.BaseTestCase):
|
class TestInterfacePollingMinimizer(base.BaseTestCase):
|
||||||
|
@ -84,7 +84,7 @@ class TestDhcpRpcCallback(base.BaseTestCase):
|
|||||||
def _test__port_action_good_action(self, action, port, expected_call):
|
def _test__port_action_good_action(self, action, port, expected_call):
|
||||||
self.callbacks._port_action(self.plugin, mock.Mock(),
|
self.callbacks._port_action(self.plugin, mock.Mock(),
|
||||||
port, action)
|
port, action)
|
||||||
self.plugin.assert_has_calls(expected_call)
|
self.plugin.assert_has_calls([expected_call])
|
||||||
|
|
||||||
def test_port_action_create_port(self):
|
def test_port_action_create_port(self):
|
||||||
self._test__port_action_good_action(
|
self._test__port_action_good_action(
|
||||||
@ -219,8 +219,8 @@ class TestDhcpRpcCallback(base.BaseTestCase):
|
|||||||
host='foo_host',
|
host='foo_host',
|
||||||
port_id='foo_port_id',
|
port_id='foo_port_id',
|
||||||
port=port)
|
port=port)
|
||||||
self.plugin.assert_has_calls(
|
self.plugin.assert_has_calls([
|
||||||
mock.call.update_port(mock.ANY, 'foo_port_id', expected_port))
|
mock.call.update_port(mock.ANY, 'foo_port_id', expected_port)])
|
||||||
|
|
||||||
def test_get_dhcp_port_existing(self):
|
def test_get_dhcp_port_existing(self):
|
||||||
port_retval = dict(id='port_id', fixed_ips=[dict(subnet_id='a')])
|
port_retval = dict(id='port_id', fixed_ips=[dict(subnet_id='a')])
|
||||||
|
@ -92,11 +92,21 @@ class HackingTestCase(base.BaseTestCase):
|
|||||||
mock.method(1, 2, 3, test='wow')
|
mock.method(1, 2, 3, test='wow')
|
||||||
mock.method.assertCalledOnceWith()
|
mock.method.assertCalledOnceWith()
|
||||||
"""
|
"""
|
||||||
|
fail_code3 = """
|
||||||
|
mock = Mock()
|
||||||
|
mock.method(1, 2, 3, test='wow')
|
||||||
|
mock.method.assert_has_called()
|
||||||
|
"""
|
||||||
pass_code = """
|
pass_code = """
|
||||||
mock = Mock()
|
mock = Mock()
|
||||||
mock.method(1, 2, 3, test='wow')
|
mock.method(1, 2, 3, test='wow')
|
||||||
mock.method.assert_called_once_with()
|
mock.method.assert_called_once_with()
|
||||||
"""
|
"""
|
||||||
|
pass_code2 = """
|
||||||
|
mock = Mock()
|
||||||
|
mock.method(1, 2, 3, test='wow')
|
||||||
|
mock.method.assert_has_calls()
|
||||||
|
"""
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
1, len(list(checks.check_assert_called_once_with(fail_code1,
|
1, len(list(checks.check_assert_called_once_with(fail_code1,
|
||||||
"neutron/tests/test_assert.py"))))
|
"neutron/tests/test_assert.py"))))
|
||||||
@ -106,6 +116,12 @@ class HackingTestCase(base.BaseTestCase):
|
|||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
0, len(list(checks.check_assert_called_once_with(pass_code,
|
0, len(list(checks.check_assert_called_once_with(pass_code,
|
||||||
"neutron/tests/test_assert.py"))))
|
"neutron/tests/test_assert.py"))))
|
||||||
|
self.assertEqual(
|
||||||
|
1, len(list(checks.check_assert_called_once_with(fail_code3,
|
||||||
|
"neutron/tests/test_assert.py"))))
|
||||||
|
self.assertEqual(
|
||||||
|
0, len(list(checks.check_assert_called_once_with(pass_code2,
|
||||||
|
"neutron/tests/test_assert.py"))))
|
||||||
|
|
||||||
def test_check_oslo_namespace_imports(self):
|
def test_check_oslo_namespace_imports(self):
|
||||||
def check(s, fail=True):
|
def check(s, fail=True):
|
||||||
|
@ -63,7 +63,7 @@ class TestMeteringOperations(base.BaseTestCase):
|
|||||||
self.metering_rpc_patch = mock.patch(metering_rpc, return_value=[])
|
self.metering_rpc_patch = mock.patch(metering_rpc, return_value=[])
|
||||||
self.metering_rpc_patch.start()
|
self.metering_rpc_patch.start()
|
||||||
|
|
||||||
self.driver_patch = mock.patch(self.noop_driver, autospec=True)
|
self.driver_patch = mock.patch(self.noop_driver, spec=True)
|
||||||
self.driver_patch.start()
|
self.driver_patch.start()
|
||||||
|
|
||||||
loopingcall_patch = mock.patch(
|
loopingcall_patch = mock.patch(
|
||||||
|
Loading…
Reference in New Issue
Block a user