bpo-35226: Fix equality for nested unittest.mock.call objects. (#10555)

Also refactor the call recording imolementation and add some notes
about its limitations.

3 files changed, 99 insertions, 23 deletions

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 9547b1a..d13f749 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -977,46 +977,51 @@ class CallableMixin(Base):
         self = _mock_self
         self.called = True
         self.call_count += 1
-        _new_name = self._mock_new_name
-        _new_parent = self._mock_new_parent
 
+        # handle call_args
         _call = _Call((args, kwargs), two=True)
         self.call_args = _call
         self.call_args_list.append(_call)
-        self.mock_calls.append(_Call(('', args, kwargs)))
 
         seen = set()
-        skip_next_dot = _new_name == '()'
+
+        # initial stuff for method_calls:
         do_method_calls = self._mock_parent is not None
-        name = self._mock_name
-        while _new_parent is not None:
-            this_mock_call = _Call((_new_name, args, kwargs))
-            if _new_parent._mock_new_name:
-                dot = '.'
-                if skip_next_dot:
-                    dot = ''
+        method_call_name = self._mock_name
 
-                skip_next_dot = False
-                if _new_parent._mock_new_name == '()':
-                    skip_next_dot = True
+        # initial stuff for mock_calls:
+        mock_call_name = self._mock_new_name
+        is_a_call = mock_call_name == '()'
+        self.mock_calls.append(_Call(('', args, kwargs)))
 
-                _new_name = _new_parent._mock_new_name + dot + _new_name
+        # follow up the chain of mocks:
+        _new_parent = self._mock_new_parent
+        while _new_parent is not None:
 
+            # handle method_calls:
             if do_method_calls:
-                if _new_name == name:
-                    this_method_call = this_mock_call
-                else:
-                    this_method_call = _Call((name, args, kwargs))
-                _new_parent.method_calls.append(this_method_call)
-
+                _new_parent.method_calls.append(_Call((method_call_name, args, kwargs)))
                 do_method_calls = _new_parent._mock_parent is not None
                 if do_method_calls:
-                    name = _new_parent._mock_name + '.' + name
+                    method_call_name = _new_parent._mock_name + '.' + method_call_name
 
+            # handle mock_calls:
+            this_mock_call = _Call((mock_call_name, args, kwargs))
             _new_parent.mock_calls.append(this_mock_call)
+
+            if _new_parent._mock_new_name:
+                if is_a_call:
+                    dot = ''
+                else:
+                    dot = '.'
+                is_a_call = _new_parent._mock_new_name == '()'
+                mock_call_name = _new_parent._mock_new_name + dot + mock_call_name
+
+            # follow the parental chain:
             _new_parent = _new_parent._mock_new_parent
 
-            # use ids here so as not to call __hash__ on the mocks
+            # check we're not in an infinite loop:
+            # ( use ids here so as not to call __hash__ on the mocks)
             _new_parent_id = id(_new_parent)
             if _new_parent_id in seen:
                 break
@@ -2054,6 +2059,10 @@ class _Call(tuple):
         else:
             self_name, self_args, self_kwargs = self
 
+        if (getattr(self, 'parent', None) and getattr(other, 'parent', None)
+                and self.parent != other.parent):
+            return False
+
         other_name = ''
         if len_other == 0:
             other_args, other_kwargs = (), {}
diff --git a/Lib/unittest/test/testmock/testhelpers.py b/Lib/unittest/test/testmock/testhelpers.py
index 9edebf5..9388311 100644
--- a/Lib/unittest/test/testmock/testhelpers.py
+++ b/Lib/unittest/test/testmock/testhelpers.py
@@ -270,6 +270,22 @@ class CallTest(unittest.TestCase):
         self.assertEqual(mock.mock_calls, last_call.call_list())
 
 
+    def test_extended_not_equal(self):
+        a = call(x=1).foo
+        b = call(x=2).foo
+        self.assertEqual(a, a)
+        self.assertEqual(b, b)
+        self.assertNotEqual(a, b)
+
+
+    def test_nested_calls_not_equal(self):
+        a = call(x=1).foo().bar
+        b = call(x=2).foo().bar
+        self.assertEqual(a, a)
+        self.assertEqual(b, b)
+        self.assertNotEqual(a, b)
+
+
     def test_call_list(self):
         mock = MagicMock()
         mock(1)
diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py
index ac6eea3..e46ef7b 100644
--- a/Lib/unittest/test/testmock/testmock.py
+++ b/Lib/unittest/test/testmock/testmock.py
@@ -925,6 +925,57 @@ class MockTest(unittest.TestCase):
                              call().__int__().call_list())
 
 
+    def test_child_mock_call_equal(self):
+        m = Mock()
+        result = m()
+        result.wibble()
+        # parent looks like this:
+        self.assertEqual(m.mock_calls, [call(), call().wibble()])
+        # but child should look like this:
+        self.assertEqual(result.mock_calls, [call.wibble()])
+
+
+    def test_mock_call_not_equal_leaf(self):
+        m = Mock()
+        m.foo().something()
+        self.assertNotEqual(m.mock_calls[1], call.foo().different())
+        self.assertEqual(m.mock_calls[0], call.foo())
+
+
+    def test_mock_call_not_equal_non_leaf(self):
+        m = Mock()
+        m.foo().bar()
+        self.assertNotEqual(m.mock_calls[1], call.baz().bar())
+        self.assertNotEqual(m.mock_calls[0], call.baz())
+
+
+    def test_mock_call_not_equal_non_leaf_params_different(self):
+        m = Mock()
+        m.foo(x=1).bar()
+        # This isn't ideal, but there's no way to fix it without breaking backwards compatibility:
+        self.assertEqual(m.mock_calls[1], call.foo(x=2).bar())
+
+
+    def test_mock_call_not_equal_non_leaf_attr(self):
+        m = Mock()
+        m.foo.bar()
+        self.assertNotEqual(m.mock_calls[0], call.baz.bar())
+
+
+    def test_mock_call_not_equal_non_leaf_call_versus_attr(self):
+        m = Mock()
+        m.foo.bar()
+        self.assertNotEqual(m.mock_calls[0], call.foo().bar())
+
+
+    def test_mock_call_repr(self):
+        m = Mock()
+        m.foo().bar().baz.bob()
+        self.assertEqual(repr(m.mock_calls[0]), 'call.foo()')
+        self.assertEqual(repr(m.mock_calls[1]), 'call.foo().bar()')
+        self.assertEqual(repr(m.mock_calls[2]), 'call.foo().bar().baz.bob()')
+
+
     def test_subclassing(self):
         class Subclass(Mock):
             pass