classification
Title: unittest mock create_autospec doesn't correctly replace mocksignature
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: michael.foord Nosy List: cjw296, mariocj89, michael.foord, pablogsal, xtreak
Priority: normal Keywords: patch

Created on 2013-02-11 22:31 by cjw296, last changed 2018-12-12 08:58 by cjw296. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 11048 merged xtreak, 2018-12-09 11:37
PR 11125 merged miss-islington, 2018-12-12 07:55
Messages (9)
msg181934 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2013-02-11 22:31
Sticking an issue in at Michael's request...

Older versions of mock had a helper called mocksignature.
In newer versions, create_autospec replaces this, but doesn't get it right sometimes:

>>> from inspect import getargspec
>>> from mock import create_autospec
>>> def myfunc(x, y): pass
...
>>> getargspec(myfunc)
ArgSpec(args=['x', 'y'], varargs=None, keywords=None, defaults=None)
>>> getargspec(create_autospec(myfunc))
ArgSpec(args=[], varargs='args', keywords='kwargs', defaults=None)

mocksignature gets it right:

>>> from mock import mocksignature
>>> getargspec(mocksignature(myfunc))
ArgSpec(args=['x', 'y'], varargs=None, keywords=None, defaults=None)
msg223380 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-17 22:20
Just curiosity why such a name change?
msg223405 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2014-07-18 10:16
It was a functionality change, not just a name change.
msg329345 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2018-11-06 05:38
Is this being worked on or can I try fixing this?

My analysis so far is as below : 

1. For functions : inspect.signature looks for attribute __signature__ for functions and while creating the mock for a function since we already have the signature we can set __signature__ attribute during _set_signature. I don't know why __signature__ was set. I downloaded mock from GitHub to look at the actual mocksignature implementation which uses lambda signature: _mock_signature to form a mock function which I hope is done here too with a function constructed and evald with sig.bind used for parameter checking. I am still new to the mocksignature internals so I couldn't understand how mocksignature worked.


2. For class and partial functions _check_signature is called and __call__ is overriden in the mock class to check for signature during initialization acting like a constructor. The actual signature will have self along with rest of the parameters but _get_signature_object checks for __init__ and skips the self thus the constructor signature skips self to return a partial function which makes comparing actual constructor call with self and the partial function signature without self to fail.


Attaching a sample patch with tests. Hope I am on the right track and guidance would help. I am changing the version to 3.8

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index a9c82dcb5d..8cbef0e514 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -103,6 +103,7 @@ def _check_signature(func, mock, skipfirst, instance=False):
         sig.bind(*args, **kwargs)
     _copy_func_details(func, checksig)
     type(mock)._mock_check_sig = checksig
+    type(mock).__signature__ = sig


 def _copy_func_details(func, funcopy):
@@ -172,11 +173,11 @@ def _set_signature(mock, original, instance=False):
     return mock(*args, **kwargs)""" % name
     exec (src, context)
     funcopy = context[name]
-    _setup_func(funcopy, mock)
+    _setup_func(funcopy, mock, sig)
     return funcopy


-def _setup_func(funcopy, mock):
+def _setup_func(funcopy, mock, sig):
     funcopy.mock = mock

     # can't use isinstance with mocks
@@ -224,6 +225,7 @@ def _setup_func(funcopy, mock):
     funcopy.assert_called = assert_called
     funcopy.assert_not_called = assert_not_called
     funcopy.assert_called_once = assert_called_once
+    funcopy.__signature__ = sig

     mock._mock_delegate = funcopy


Initial set of tests where partial function and class test fails : 

def test_spec_inspect_signature(self):
    def foo(a: int, b: int=10, *, c:int) -> int:
        return a  b  c

    mock = create_autospec(foo)
    assert inspect.getfullargspec(mock) == inspect.getfullargspec(foo)
    self.assertRaises(TypeError, mock, 1)

def test_spec_inspect_signature_partial(self):
    def foo(a: int, b: int=10, *, c:int) -> int:
        return a  b  c

    import functools

    partial_object = functools.partial(foo, 1)
    mock = create_autospec(partial_object)
    assert inspect.getfullargspec(mock) == inspect.getfullargspec(partial_object) # Fails
    self.assertRaises(TypeError, partial_object)

def test_spec_inspect_signature_class(self):
    class Bar:
        def __init__(self, a: int):
            self.a = a

    mock = create_autospec(Bar)
    assert inspect.getfullargspec(mock) == inspect.getfullargspec(Bar) # Fails since mock signature has no self
    self.assertRaises(TypeError, mock)
    self._check_someclass_mock(mock)
msg330987 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2018-12-03 22:05
xtreak - great to see action on this! First step would be to add a unit test for the failure case I reported. I like the tests you have too, but would be good to see the specific failure case covered too.

Beyond that, if we can get all the the new tests passing, then I'd say you're there!
msg331149 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2018-12-05 19:42
While working on partials test case failure I found two more cases along the way. 

1. When we call create_autospec it calls _get_signature_object that gets the signature for the given parameter. With functools.partial it returns a partial object and hence while getting the signature it returns the signature for the constructor of partial instead of the underlying function passed to functools.partial. I think a check needs to be added to make sure not to use func.__init__ when it's a partial object.

2. When we call create_autospect on a class that has a partialmethod the self parameter is not skipped in the signature and thus it creates a signature with self causing error. The fix would be to handle partialmethod also in _must_skip that determines whether to skip self or not.

3. It also seems that inspect.getfullargspec doesn't work for magic mocks which I hope is now fixed with __signature__ set in my patch

To be honest I don't if my getting hacky at this point I will clean up the code in a couple of days and raise a PR for initial feedback. Current changes are at https://github.com/python/cpython/compare/master...tirkarthi:bpo17185. I am adding @mariocj89 to the issue.


Sample reproducer : 

from functools import partial, partialmethod
from unittest.mock import create_autospec
import inspect

def foo(a, b):
    pass

p = partial(foo, 1)
m = create_autospec(p)
m(1, 2, 3) # passes since signature is set as (*args, **kwargs) the signature of functools.partial constructor. This should throw TypeError under autospec


class A:

    def f(self, a, b):
        print(a, b)

    g = partialmethod(f, 1)

m = create_autospec(A)
m().g(1, 2) # passes since signature is set as (self, b) and self is not skipped in _must_skip thus self=1, b=2. This should throw TypeError under autospec since the valid call is m().g(2)
msg331344 - (view) Author: Mario Corchero (mariocj89) * (Python triager) Date: 2018-12-07 18:33
Agree that it sounds reasonable to just set `__signature__` to tell `inspect` where to look at (thanks PEP 362 :P). Not an expert here though.

For the partials bug, I'll add Pablo as we were speaking today about something similar :) but that might be unrelated (though interesting and worth fixing!) from the issue Chris brought up.
msg331673 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2018-12-12 07:54
New changeset f7fa62ef4422c9deee050a794fd8504640d9f8f4 by Chris Withers (Xtreak) in branch 'master':
bpo-17185: Add __signature__ to mock that can be used by inspect for signature (GH11048)
https://github.com/python/cpython/commit/f7fa62ef4422c9deee050a794fd8504640d9f8f4
msg331682 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2018-12-12 08:58
New changeset 6a12931c9cb5d472fe6370dbcd2bde72f34dddb4 by Chris Withers (Miss Islington (bot)) in branch '3.7':
bpo-17185: Add __signature__ to mock that can be used by inspect for signature (GH11125)
https://github.com/python/cpython/commit/6a12931c9cb5d472fe6370dbcd2bde72f34dddb4
History
Date User Action Args
2018-12-12 08:58:40cjw296setmessages: + msg331682
2018-12-12 07:58:18cjw296setstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-12-12 07:55:39miss-islingtonsetpull_requests: + pull_request10356
2018-12-12 07:54:58cjw296setmessages: + msg331673
2018-12-09 11:37:00xtreaksetkeywords: + patch
stage: patch review
pull_requests: + pull_request10284
2018-12-07 18:33:23mariocj89setnosy: + pablogsal
messages: + msg331344
2018-12-05 19:42:15xtreaksetnosy: + mariocj89
messages: + msg331149
2018-12-03 22:05:04cjw296setmessages: + msg330987
versions: + Python 3.6, Python 3.7
2018-11-06 05:38:27xtreaksetmessages: + msg329345
versions: + Python 3.8, - Python 3.4, Python 3.5
2018-09-23 08:51:32BreamoreBoysetnosy: - BreamoreBoy
2018-09-23 06:33:11xtreaksetnosy: + xtreak
2014-07-18 10:16:47michael.foordsetmessages: + msg223405
2014-07-17 22:20:00BreamoreBoysettype: behavior
title: create_autospec -> unittest mock create_autospec doesn't correctly replace mocksignature
components: + Tests
versions: + Python 3.4, Python 3.5
nosy: + BreamoreBoy

messages: + msg223380
2013-02-11 22:31:40cjw296create