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

diff --git a/HACKING.rst b/HACKING.rst
index 43dd4540ba216ac817dac79b7ddffe522d26154e..e08eb54224bab9d7234cdc583e578b6a1fea7756 100644 (file)
--- 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 8474ee533bb4a98d9d5fcca69f7056eafe6da0c4..e0ff75197d31741987aee954ad70bf8d8fe4f2c9 100644 (file)
--- 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 9d23b78e36ceaebd5975e59407e517d5a658c3ff..10ecbe69af716b1ba50dcd87c601697982802b87 100644 (file)
--- 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 0a8fa0de625eb201bc36b679c96d05e2d71daaa0..2d2ffde256f4c0c082bdabe4c46c6fde9ce55376 100644 (file)
--- 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(
+            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(pass_code,
+            0, len(list(checks.check_assert_called_once_with(pass_code,
                                             "neutron/tests/test_assert.py"))))