diff --git a/HACKING.rst b/HACKING.rst index 43dd4540ba..e08eb54224 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -13,7 +13,7 @@ Neutron Specific Commandments - [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 authorship. -- [N323] assert_called_once() is not a valid method +- [N323] Detect common errors with assert_called_once_with Creating Unit Tests ------------------- diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index 8474ee533b..e0ff75197d 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -127,20 +127,22 @@ def no_translate_debug_logs(logical_line, filename): yield(0, "N319 Don't translate debug level logs") -def check_assert_called_once(logical_line, filename): - msg = ("N323: assert_called_once is a no-op. please use " - "assert_called_once_with to test with explicit parameters or an " - "assertEqual with call_count.") - +def check_assert_called_once_with(logical_line, filename): + # Try to detect unintended calls of nonexistent mock methods like: + # assert_called_once + # assertCalledOnceWith if 'neutron/tests/' in filename: - pos = logical_line.find('.assert_called_once(') - if pos != -1: - yield (pos, msg) + if '.assert_called_once_with(' in logical_line: + return + 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): register(validate_log_translations) register(use_jsonutils) register(no_author_tags) - register(check_assert_called_once) + register(check_assert_called_once_with) register(no_translate_debug_logs) diff --git a/neutron/tests/unit/test_dhcp_agent.py b/neutron/tests/unit/test_dhcp_agent.py index 9d23b78e36..10ecbe69af 100644 --- a/neutron/tests/unit/test_dhcp_agent.py +++ b/neutron/tests/unit/test_dhcp_agent.py @@ -805,26 +805,26 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): with mock.patch.object(self.dhcp, 'enable_dhcp_helper') as enable: 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): payload = dict(network=dict(id=fake_network.id, admin_state_up=True)) with mock.patch.object(self.dhcp, 'enable_dhcp_helper') as enable: 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): payload = dict(network=dict(id=fake_network.id, admin_state_up=False)) with mock.patch.object(self.dhcp, 'disable_dhcp_helper') as disable: 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): payload = dict(network_id=fake_network.id) with mock.patch.object(self.dhcp, 'disable_dhcp_helper') as disable: 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): network = dhcp.NetModel(True, dict(id='net-id', diff --git a/neutron/tests/unit/test_hacking.py b/neutron/tests/unit/test_hacking.py index 0a8fa0de62..2d2ffde256 100644 --- a/neutron/tests/unit/test_hacking.py +++ b/neutron/tests/unit/test_hacking.py @@ -86,20 +86,28 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual(2, checks.no_author_tags("# Author: pele")[0]) self.assertEqual(3, checks.no_author_tags(".. moduleauthor:: pele")[0]) - def test_assert_called_once(self): - fail_code = """ + def test_assert_called_once_with(self): + fail_code1 = """ mock = Mock() mock.method(1, 2, 3, test='wow') mock.method.assert_called_once() """ + fail_code2 = """ + mock = Mock() + mock.method(1, 2, 3, test='wow') + mock.method.assertCalledOnceWith() + """ pass_code = """ mock = Mock() mock.method(1, 2, 3, test='wow') mock.method.assert_called_once_with() """ 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")))) 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"))))