classification
Title: Mock(2.0.0).assert_has_calls() raise AssertionError in two same calls
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: cjw296, guboi72, jekin000, jwdevel, mariocj89, michael.foord, rbcollins, xtreak
Priority: normal Keywords:

Created on 2016-04-14 07:43 by jekin000, last changed 2019-08-01 06:00 by xtreak.

Files
File name Uploaded Description Edit
test.py jekin000, 2016-04-14 07:43
Messages (8)
msg263375 - (view) Author: James (jekin000) * Date: 2016-04-14 07:43
>>> 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',)]
msg287728 - (view) Author: John W. (jwdevel) Date: 2017-02-13 21:54
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(!)
msg288222 - (view) Author: John W. (jwdevel) Date: 2017-02-20 17:48
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.
msg306390 - (view) Author: Guillaume Boily (guboi72) Date: 2017-11-16 17:50
As pointed here: https://github.com/testing-cabal/mock/issues/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.
msg336914 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-03-01 10:20
@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:
msg337023 - (view) Author: Mario Corchero (mariocj89) * (Python triager) Date: 2019-03-03 00:57
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 issue17015 (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.
msg337025 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-03-03 05:52
eat_self is also a problem as mentioned and would help in solving issue27715 where self is not ignored. I tried a patch for issue27715 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 (issue25312). 

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.
msg348841 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-08-01 06:00
I proposed a PR for issue36871 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 issue36871 is merged. The issue with error message being cryptic in issue27715 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.
History
Date User Action Args
2019-08-01 06:00:38xtreaksetmessages: + msg348841
components: + Library (Lib), - Tests
versions: + Python 3.9
2019-03-03 05:52:48xtreaksetmessages: + msg337025
2019-03-03 00:57:02mariocj89setmessages: + msg337023
2019-03-01 10:20:29xtreaksetnosy: + cjw296, mariocj89

messages: + msg336914
versions: + Python 3.7, Python 3.8, - Python 3.6
2018-09-23 15:00:12xtreaksetnosy: + xtreak
2017-11-16 17:50:57guboi72setversions: + Python 3.6, - Python 2.7, Python 3.4
nosy: + guboi72

messages: + msg306390

components: + Tests
2017-02-20 17:48:35jwdevelsetmessages: + msg288222
2017-02-13 21:54:01jwdevelsetnosy: + jwdevel

messages: + msg287728
versions: + Python 3.4
2016-04-14 11:46:05SilentGhostsetnosy: + michael.foord
2016-04-14 07:43:24jekin000create