Mock(2.0.0).assert_has_calls() raise AssertionError in two same calls

[jekin000]

BPO	26752
Nosy	@rbtcollins, @cjw296, @voidspace, @mariocj89, @guboi72, @tirkarthi
Files	test.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2016-04-14.07:43:24.290>
labels = ['3.7', '3.8', 'type-bug', 'library', '3.9']
title = 'Mock(2.0.0).assert_has_calls() raise AssertionError in two same calls'
updated_at = <Date 2019-08-01.06:00:38.882>
user = 'https://bugs.python.org/jekin000'

bugs.python.org fields:

activity = <Date 2019-08-01.06:00:38.882>
actor = 'xtreak'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2016-04-14.07:43:24.290>
creator = 'jekin000'
dependencies = []
files = ['42460']
hgrepos = []
issue_num = 26752
keywords = []
message_count = 8.0
messages = ['263375', '287728', '288222', '306390', '336914', '337023', '337025', '348841']
nosy_count = 8.0
nosy_names = ['rbcollins', 'cjw296', 'michael.foord', 'jekin000', 'jwdevel', 'mariocj89', 'guboi72', 'xtreak']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26752'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

[jekin000]

>>> import mock
>>> print mock.__version__
2.0.0
>>> 

=================
test.py
from mock import Mock,call

class BB(object):
    def __init__(self):pass
    def print_b(self):pass
    def print_bb(self,tsk_id):pass


bMock = Mock(return_value=Mock(spec=BB))
bMock().print_bb(20)
bMock().assert_has_calls([call.print_bb(20)])

===================

Traceback (most recent call last):
  File "test.py", line 11, in <module>
    bMock().assert_has_calls([call.print_bb(20)])
  File "/usr/lib/python2.7/site-packages/mock/mock.py", line 969, in assert_has_calls
    ), cause)
  File "/usr/lib/python2.7/site-packages/six.py", line 718, in raise_from
    raise value
AssertionError: Calls not found.
Expected: [call.print_bb(20)]
Actual: [call.print_bb(20)]

=======
print expected in mock.py assert_has_calls()

result is:
[TypeError('too many positional arguments',)]

[jwdevel]

This also seems to apply to unittest.mock in Python3.4.

I described my similar issue on SO: http://stackoverflow.com/questions/42212433/what-is-wrong-with-this-simple-py-test-use-case

It seems like it may be the same issue described here.

For reference, this is my repro case:

from unittest.mock import patch, call

    class Foo:
        def __init__(self):
            pass
        def my_method(self, value):
            pass
    
    def test_foo():
        with patch('test.Foo', autospec=True) as MockFoo:
            m = MockFoo()
            m.my_method(123)
            MockFoo.assert_has_calls([call(), call().my_method(123)])

It fails with:

...
E               AssertionError: Calls not found.
E               Expected: [call(), call().my_method(123)]
E               Actual:   [call(), call().my_method(123)]

Which seems nonsensical to me.

Removing the value parameter and the 123 arguments in the test makes it pass(!)

[jwdevel]

This got a little discussion over at http://lists.idyll.org/pipermail/testing-in-python/2017-February/007012.html

The current evidence seem to indicate this is indeed a bug in the implementation of assert_has_calls.

[guboi72]

As pointed here: testing-cabal/mock#353,

this issue is related to the method assert_has_calls or probably any calls that use the method _call_matcher to match calls. Given that you mock a class and spec it, since we always bind to the _spec_signature (e.g. the constructor signature), when it is a method call then it bind() throws a TypeError looking like missing a require argument. A possible solution would be to include method signatures into the spec.

[tirkarthi]

@guboi72 is right that signature for class constructor is used also for methods. Another possible solution would be that mock stores it's children in _mock_children dictionary so when a method is called then name can be used to get the relevant child mock which would have the signature set that can be used.

In the below program call().foo also uses the signature (a, b) of __init__ and in the call_matcher check name "foo" can be used to get the child mock from mock_class._mock_children that will have the signature set during create_autospec. This will give the signature (a) but it's little difficult to construct the name. It also needs to handle cases for inner classes like Foo.Bar.foo() where Bar is an inner class inside Foo.

from unittest.mock import *

class Foo:

    def __init__(self, a, b):
        pass

    def foo(self, a):
        pass


mock_class = create_autospec(Foo)
mock = mock_class(1, 2)
mock.foo(1)
print(mock_class._mock_children)
print(mock._mock_children)
mock_class.assert_has_calls([call(1, 2), call().foo(1)])
mock.assert_has_calls([call.foo(1)])

$ python3.7 ../backups/unittest_mock_spec_conflict.py
{'foo': <MagicMock name='mock.foo' spec='function' id='4534528096'>}
{'foo': <MagicMock name='mock().foo' spec='function' id='4534099584'>}
Traceback (most recent call last):
  File "../backups/unittest_mock_spec_conflict.py", line 17, in <module>
    mock_class.assert_has_calls([call(1, 2), call().foo(1)])
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/unittest/mock.py", line 852, in assert_has_calls
    ) from cause
AssertionError: Calls not found.
Expected: [call(1, 2), call().foo(1)]
Actual: [call(1, 2), call().foo(1)]

A very rough hack that fixes the above case and explains my approach but not so robust.

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 2ccf0d82ce..f0e917d57e 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -777,7 +777,17 @@ class NonCallableMock(Base):
             else:
                 name, args, kwargs = _call
             try:
-                return name, sig.bind(*args, **kwargs)
+                if name:
+                    if name.startswith("()"):
+                        mock_name = "mock" + name # Handle call().foo where name is ().foo
+                    else:
+                        mock_name = "mock." + name # Handle call.foo where name is foo
+                    sig = self._mock_children.get(mock_name)
+
+                if sig:
+                    return name, sig.bind(*args, **kwargs)
+                else:
+                    return _call
             except TypeError as e:
                 return e.with_traceback(None)
         else:

[mariocj89]

Quite a tricky bug!

Indeed @XTreak the _call_matcher is using the __init__ signature. I think this is a bug introduced in https://bugs.python.org/issue17015. There is a mix of the fact that spec in Mock also can accept classes (not instances) whilst spec requires the user to say whether what you are passing is a class or an instance. This gets messed up when validating the calls as at validation time it tries to match the signature as done in bpo-17015 (something that is useful for other cases as outlined in the issue).

You can see how this is clearly a bug with the following reproducer:

from unittest.mock import Mock, call
class X(object):
    def __init__(self):pass
    def foo(self, a):pass
x = Mock(spec=X)
x.foo(20)
x.assert_has_calls(x.mock_calls)

Using an instance (X()) of the class "hides" the issue, as the right signature is used for validation.

I am not sure if there is any easy fix here :/, but it is broken that the validation happens in different ways when a class is used in the spec (when calling it vs when validating it). Things "work" as if you use a spec of a class and then construct it, that passes fine as it does not get validated as attribute lookup and then there is no further validation.

Maybe something like this would work:

--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -384,8 +384,10 @@ class NonCallableMock(Base):
     def __init__(
             self, spec=None, wraps=None, name=None, spec_set=None,
             parent=None, _spec_state=None, _new_name='', _new_parent=None,
-            _spec_as_instance=False, _eat_self=None, unsafe=False, **kwargs
+            _spec_as_instance=None, _eat_self=None, unsafe=False, **kwargs
         ):
+        if _spec_as_instance is None:
+            _spec_as_instance = isinstance(spec, type) # We might need to play with eat_self as well here.
         if _new_parent is None:
             _new_parent = parent

@@ -2205,8 +2207,8 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None,
     elif spec is None:
         # None we mock with a normal mock without a spec
         _kwargs = {}
-    if _kwargs and instance:
-        _kwargs['_spec_as_instance'] = True
+    if _kwargs:
+        _kwargs['_spec_as_instance'] = instance

Basically, being explicit on auto_spec and inferring it on the creation of Mocks, but that might break some people who (probably badly) rely on the signature of the class.

This issue probably needs some further work.

[tirkarthi]

eat_self is also a problem as mentioned and would help in solving bpo-27715 where self is not ignored. I tried a patch for bpo-27715 but that would require changing the output of some functions that take _eat_self as a parameter to return _eat_self since the functions themselves change the value. So some call sites need to be updated to receive actual value for _eat_self when the signature is created.

It's a hard issue and there are less tests in this area so it's even harder to figure out all the existing cases supported and if the solution is breaking existing code. Also the general repr during failure could be improved since it just says prints same call object for expected and actual with an assertion error making it more confusing (bpo-25312).

I think there are currently 3-4 different variants of this issue in the tracker due to wrong signature that could be resolved if _call_matcher becomes more robust using correct signature.

[tirkarthi]

I proposed a PR for bpo-36871 which has the same issue with constructor signature being used for method calls when specced. I also checked the patch with the examples reported here regarding signature mismatch and they seem to be fixed. So perhaps this can be closed once bpo-36871 is merged. The issue with error message being cryptic in bpo-27715 when self is passed or not for sig.bind is not yet solved though.

Please let me know if there is anything missed with the PR 13261 so that I can add relevant tests. Thanks.