Thing: <class '__main__.Thing'>
mock_thing.method sig=(a=None, b=0)
F
======================================================================
FAIL: test_has_calls_on_thing (main.MockCallTest)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/usr/local/google/home/gps/mock_call_test.py", line 25, in test_has_calls_on_thing
    mock_thing.assert_has_calls([
  File "/usr/local/google/home/gps/oss/cpython/gpshead/Lib/unittest/mock.py", line 843, in assert_has_calls
    raise AssertionError(
AssertionError: Calls not found.
Expected: [call.method(0.5, b=3000), call.method(0.6, b=6000), call.method(0.7, b=9000)]
Actual: [call.method(0.5, b=3000), call.method(0.6, b=6000), call.method(0.7, b=9000)].

See the attached mock_call_test.py.

If you use a debugger on this, you'll discover that what is happening inside of mock's assert_has_calls is that it is catching and swallowing an exception around the inspect.Signature instances bind() call complaining about 'b' being an unexpected keyword argument. Diving within the signature object being _used_ at the time, you find it checking against the signature of the class *constructor* rather than the methods. (!)

the real error here is that the mock.create_autospec(Thing) has created a mock of the class. but the mock class (mock_thing) was never constructed. Adding a call to the constructor makes this code work:

 mock_thing = mock.create_autospec(Thing)(constructor_param="spam")

But debugging this is insane, the error message is entirely misleading and shows two equal lists yet claiming they are different.

This is little tricky to fix since the call-matcher needs the signature and also the information over whether to add self or not for some cases. Similar open issues.

https://bugs.python.org/issue27715
https://bugs.python.org/issue26752

An approach suggested by Mario : https://bugs.python.org/issue26752#msg337023

I have created a PR for this. My approach is that when mock_thing.assert_has_calls(mock.call.method1(1, 2)) is made the assert_has_calls is made against mock_thing whose spec_signature (constructor signature) is always used. But there is _mock_children, a dictionary which has signature for the methods. Thus method1 can be used as key for _mock_children to get correct signature.

Where it gets tricky is that if there is a nested class whose method is called like Foo.Bar.meth1 as below then the dictionary has only 'Bar' from which the children has to be obtained to get meth1 signature like {'Foo': {'bar1': signature}} and it's not stored with the key 'Foo.Bar.meth1' resulting in iteration. There could be a better way or some edge case not covered so I have opened up PR for review but if someone else has better approach then that would be great too since this is a long standing issue resulting autospec needing to be turned off.

class Foo:
    class Bar:
        def meth1(self, a): pass

This PR also solves the case at https://bugs.python.org/issue26752#msg287728. There is a test failure caught by doctest for nested calls without spec and not by unittest :) I have converted the doctest as a unittest.

New changeset c961278 by Chris Withers (Xtreak) in branch 'master':
bpo-36871: Ensure method signature is used when asserting mock calls to a method (GH13261)
c961278

New changeset be310e0 by Chris Withers (Miss Islington (bot)) in branch '3.7':
bpo-36871: Ensure method signature is used when asserting mock calls to a method (GH15577)
be310e0

New changeset 38d311d by Chris Withers (Miss Islington (bot)) in branch '3.8':
bpo-36871: Ensure method signature is used when asserting mock calls to a method (GH15578)
38d311d

Closing this as fixed since all PRs are merged. Thank you all :)

This is still not totally fixed. This solves the underlying error with method specs, but not the bug that was causing the error-swallowing in assert_has_calls:

expected = [self._call_matcher(c) for c in calls]
cause = expected if isinstance(expected, Exception) else None

isinstance(expected, Exception) is never true, because expected is always a list. It should instead be:

cause = next((e for e in expected if isinstance(e, Exception)), None)

It was a conscious decision over not to do the proposed change in the issue. There are other issues over signature difference during construction of mock and in the call_matcher over presence/absence of self. It needs a more careful fix.

Sure, but the bug in error-handling should still be fixed. Currently, if _call_matcher returns an exception, that's ignored by assert_has_calls, and the error message generated as a result is extremely misleading!

i reopened this without diving into the code to better understand based on Samuels comment.

We could really do with a testcase that demonstrates the misleading error message problem for some test driven development here.

The code around whether or not to swallow self is hairy. Even if the original spec object is a class we may still be mocking an instance of the class (we don't want users to have to create an instance just to be able to use it as a spec). So we have to carry metadata about whether or not we're mocking an instance. But we also have to support the use case of when users are mocking an actual class object.

This gets potentially recursive in the case of autospec and has to apply to the class object itself. If we're mocking an instance that is callable then the signature on the top level mock should be taken from __call__. If we're mocking the constructor the signature comes from __init__.

So it's all complicated. And when I originally wrote the code it was worse as it predated inspect.Signature (and was one of the driving use cases for it) and created functions with the right signature by exec'ing code.

So it's better code than it used to be, but I'm still scared of it and that particular bug came in the switch to use sig.bind which I didn't write.

Check out my PR, which solves a much smaller issue: It fixes a bug in the exception raising logic in assert_has_calls (and _awaits) which makes complicated problems like the one you mention much harder to debug.

Also it has tests!

(The code that generated functions was originally borrowed from the decorator module by Michele Simionato. When I first started in Python, around 2002, I was impressed by the Python community as it had two very prominent women amongst the part of the community I inhabited. Nicola Larosa and Michele Simionato. It turned out they were both Italian men.)

New changeset b5a7a4f by Miss Islington (bot) (Samuel Freilich) in branch 'master':
bpo-36871: Handle spec errors in assert_has_calls (GH-16005)
b5a7a4f

New changeset 2180f6b by Gregory P. Smith (Samuel Freilich) in branch 'master':
bpo-36871: Avoid duplicated 'Actual:' in assertion message (GH-16361)
2180f6b

New changeset 1a17a05 by Miss Islington (bot) in branch '3.8':
[3.8] bpo-36871: Handle spec errors in assert_has_calls (GH-16005) (GH-16364)
1a17a05

New changeset 4042e1a by Gregory P. Smith in branch '3.7':
[3.7] bpo-36871: Handle spec errors in assert_has_calls (GH-16364) (GH-16374)
4042e1a

i believe the issues surfaces in this are fixed at this point.

of note, my mock_call_test.py example now passes.

i'm not entirely sure that it _should_ pass though, but that depends on what we want create_autospec of a class to do. should that autospec'd class require instantiation before methods are called? _that_ is outside the scope of this issue. As is, the current behavior is likely what people expect to find convenient.