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

Also refactor the call recording implementation and add some notes about its limitations. (cherry picked from commit 8ca0fa9) Co-authored-by: Chris Withers <chris@withers.org>

Author: miss-islington

Doc/library/unittest.mock-examples.rst

@@ -153,6 +153,15 @@ You use the :data:`call` object to construct lists for comparing with
  >>> mock.mock_calls == expected
  True
 
+However, parameters to calls that return mocks are not recorded, which means it is not
+possible to track nested calls where the parameters used to create ancestors are important:
+
+ >>> m = Mock()
+ >>> m.factory(important=True).deliver()
+ <Mock name='mock.factory().deliver()' id='...'>
+ >>> m.mock_calls[-1] == call.factory(important=False).deliver()
+ True
+
 
 Setting Return Values and Attributes
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Doc/library/unittest.mock.rst

@@ -680,6 +680,19 @@ the *new_callable* argument to :func:`patch`.
  unpacked as tuples to get at the individual arguments. See
  :ref:`calls as tuples <calls-as-tuples>`.
 
+ .. note::
+
+ The way :attr:`mock_calls` are recorded means that where nested
+ calls are made, the parameters of ancestor calls are not recorded
+ and so will always compare equal:
+
+ >>> mock = MagicMock()
+ >>> mock.top(a=3).bottom()
+ <MagicMock name='mock.top().bottom()' id='...'>
+ >>> mock.mock_calls
+ [call.top(a=3), call.top().bottom()]
+ >>> mock.mock_calls[-1] == call.top(a=-1).bottom()
+ True
 
  .. attribute:: __class__
 

Lib/unittest/mock.py

@@ -943,46 +943,51 @@ def _mock_call(_mock_self, *args, **kwargs):
  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
@@ -2018,6 +2023,10 @@ def __eq__(self, other):
  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 = (), {}

Lib/unittest/test/testmock/testhelpers.py

@@ -269,6 +269,22 @@ def test_extended_call(self):
  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)

Lib/unittest/test/testmock/testmock.py

@@ -916,6 +916,57 @@ def test_mock_calls(self):
  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

Misc/NEWS.d/next/Library/2018-11-15-07-14-32.bpo-35226.wJPEEe.rst

@@ -0,0 +1,3 @@
+Recursively check arguments when testing for equality of
+:class:`unittest.mock.call` objects and add note that tracking of parameters
+used to create ancestors of mocks in ``mock_calls`` is not possible.