test_dhcp_agent: Fix no-op tests
Fix some uses of assertCalledOnceWith, which seems like a mistake of assert_called_once_with. Also, add a hacking check to prevent the mistake. Closes-Bug: #1397184 Change-Id: I12d077e2724d52eff65d55aff1130fbbb69671b1
This commit is contained in:
parent
738430d6a4
commit
3da8b8d381
@ -13,7 +13,7 @@ Neutron Specific Commandments
|
|||||||
- [N321] Validate that jsonutils module is used instead of json
|
- [N321] Validate that jsonutils module is used instead of json
|
||||||
- [N322] We do not use @authors tags in source files. We have git to track
|
- [N322] We do not use @authors tags in source files. We have git to track
|
||||||
authorship.
|
authorship.
|
||||||
- [N323] assert_called_once() is not a valid method
|
- [N323] Detect common errors with assert_called_once_with
|
||||||
|
|
||||||
Creating Unit Tests
|
Creating Unit Tests
|
||||||
-------------------
|
-------------------
|
||||||
|
@ -127,20 +127,22 @@ def no_translate_debug_logs(logical_line, filename):
|
|||||||
yield(0, "N319 Don't translate debug level logs")
|
yield(0, "N319 Don't translate debug level logs")
|
||||||
|
|
||||||
|
|
||||||
def check_assert_called_once(logical_line, filename):
|
def check_assert_called_once_with(logical_line, filename):
|
||||||
msg = ("N323: assert_called_once is a no-op. please use "
|
# Try to detect unintended calls of nonexistent mock methods like:
|
||||||
"assert_called_once_with to test with explicit parameters or an "
|
# assert_called_once
|
||||||
"assertEqual with call_count.")
|
# assertCalledOnceWith
|
||||||
|
|
||||||
if 'neutron/tests/' in filename:
|
if 'neutron/tests/' in filename:
|
||||||
pos = logical_line.find('.assert_called_once(')
|
if '.assert_called_once_with(' in logical_line:
|
||||||
if pos != -1:
|
return
|
||||||
yield (pos, msg)
|
if '.assertcalledonce' in logical_line.lower().replace('_', ''):
|
||||||
|
msg = ("N323: Possible use of no-op mock method. "
|
||||||
|
"please use assert_called_once_with.")
|
||||||
|
yield (0, msg)
|
||||||
|
|
||||||
|
|
||||||
def factory(register):
|
def factory(register):
|
||||||
register(validate_log_translations)
|
register(validate_log_translations)
|
||||||
register(use_jsonutils)
|
register(use_jsonutils)
|
||||||
register(no_author_tags)
|
register(no_author_tags)
|
||||||
register(check_assert_called_once)
|
register(check_assert_called_once_with)
|
||||||
register(no_translate_debug_logs)
|
register(no_translate_debug_logs)
|
||||||
|
@ -805,26 +805,26 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
|
|||||||
|
|
||||||
with mock.patch.object(self.dhcp, 'enable_dhcp_helper') as enable:
|
with mock.patch.object(self.dhcp, 'enable_dhcp_helper') as enable:
|
||||||
self.dhcp.network_create_end(None, payload)
|
self.dhcp.network_create_end(None, payload)
|
||||||
enable.assertCalledOnceWith(fake_network.id)
|
enable.assert_called_once_with(fake_network.id)
|
||||||
|
|
||||||
def test_network_update_end_admin_state_up(self):
|
def test_network_update_end_admin_state_up(self):
|
||||||
payload = dict(network=dict(id=fake_network.id, admin_state_up=True))
|
payload = dict(network=dict(id=fake_network.id, admin_state_up=True))
|
||||||
with mock.patch.object(self.dhcp, 'enable_dhcp_helper') as enable:
|
with mock.patch.object(self.dhcp, 'enable_dhcp_helper') as enable:
|
||||||
self.dhcp.network_update_end(None, payload)
|
self.dhcp.network_update_end(None, payload)
|
||||||
enable.assertCalledOnceWith(fake_network.id)
|
enable.assert_called_once_with(fake_network.id)
|
||||||
|
|
||||||
def test_network_update_end_admin_state_down(self):
|
def test_network_update_end_admin_state_down(self):
|
||||||
payload = dict(network=dict(id=fake_network.id, admin_state_up=False))
|
payload = dict(network=dict(id=fake_network.id, admin_state_up=False))
|
||||||
with mock.patch.object(self.dhcp, 'disable_dhcp_helper') as disable:
|
with mock.patch.object(self.dhcp, 'disable_dhcp_helper') as disable:
|
||||||
self.dhcp.network_update_end(None, payload)
|
self.dhcp.network_update_end(None, payload)
|
||||||
disable.assertCalledOnceWith(fake_network.id)
|
disable.assert_called_once_with(fake_network.id)
|
||||||
|
|
||||||
def test_network_delete_end(self):
|
def test_network_delete_end(self):
|
||||||
payload = dict(network_id=fake_network.id)
|
payload = dict(network_id=fake_network.id)
|
||||||
|
|
||||||
with mock.patch.object(self.dhcp, 'disable_dhcp_helper') as disable:
|
with mock.patch.object(self.dhcp, 'disable_dhcp_helper') as disable:
|
||||||
self.dhcp.network_delete_end(None, payload)
|
self.dhcp.network_delete_end(None, payload)
|
||||||
disable.assertCalledOnceWith(fake_network.id)
|
disable.assert_called_once_with(fake_network.id)
|
||||||
|
|
||||||
def test_refresh_dhcp_helper_no_dhcp_enabled_networks(self):
|
def test_refresh_dhcp_helper_no_dhcp_enabled_networks(self):
|
||||||
network = dhcp.NetModel(True, dict(id='net-id',
|
network = dhcp.NetModel(True, dict(id='net-id',
|
||||||
|
@ -86,20 +86,28 @@ class HackingTestCase(base.BaseTestCase):
|
|||||||
self.assertEqual(2, checks.no_author_tags("# Author: pele")[0])
|
self.assertEqual(2, checks.no_author_tags("# Author: pele")[0])
|
||||||
self.assertEqual(3, checks.no_author_tags(".. moduleauthor:: pele")[0])
|
self.assertEqual(3, checks.no_author_tags(".. moduleauthor:: pele")[0])
|
||||||
|
|
||||||
def test_assert_called_once(self):
|
def test_assert_called_once_with(self):
|
||||||
fail_code = """
|
fail_code1 = """
|
||||||
mock = Mock()
|
mock = Mock()
|
||||||
mock.method(1, 2, 3, test='wow')
|
mock.method(1, 2, 3, test='wow')
|
||||||
mock.method.assert_called_once()
|
mock.method.assert_called_once()
|
||||||
"""
|
"""
|
||||||
|
fail_code2 = """
|
||||||
|
mock = Mock()
|
||||||
|
mock.method(1, 2, 3, test='wow')
|
||||||
|
mock.method.assertCalledOnceWith()
|
||||||
|
"""
|
||||||
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()
|
||||||
"""
|
"""
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
1, len(list(checks.check_assert_called_once(fail_code,
|
1, len(list(checks.check_assert_called_once_with(fail_code1,
|
||||||
"neutron/tests/test_assert.py"))))
|
"neutron/tests/test_assert.py"))))
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
0, len(list(checks.check_assert_called_once(pass_code,
|
1, len(list(checks.check_assert_called_once_with(fail_code2,
|
||||||
|
"neutron/tests/test_assert.py"))))
|
||||||
|
self.assertEqual(
|
||||||
|
0, len(list(checks.check_assert_called_once_with(pass_code,
|
||||||
"neutron/tests/test_assert.py"))))
|
"neutron/tests/test_assert.py"))))
|
||||||
|
Loading…
x
Reference in New Issue
Block a user