[issue39485] Bug in mock running on PyPy3

[Chris Withers]

Chris Withers  added the comment:

Thank you very much for this, that was a really good catch!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39485] Bug in mock running on PyPy3

[Chris Withers]

Chris Withers  added the comment:


New changeset 696d2324cf2a54e20e8d6a6739fa97ba815a8be9 by Miss Islington (bot) 
in branch '3.8':
bpo-39485: fix corner-case in method-detection of mock (GH-18255)
https://github.com/python/cpython/commit/696d2324cf2a54e20e8d6a6739fa97ba815a8be9


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39485] Bug in mock running on PyPy3

[Chris Withers]

Chris Withers  added the comment:


New changeset cf0645a17acbc0c4dbbf82434e37637965748bbb by Miss Islington (bot) 
in branch '3.7':
bpo-39485: fix corner-case in method-detection of mock (GH-18256)
https://github.com/python/cpython/commit/cf0645a17acbc0c4dbbf82434e37637965748bbb


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39485] Bug in mock running on PyPy3

[Chris Withers]

Chris Withers  added the comment:


New changeset a327677905956ae0b239ff430a1346dfe265709e by Carl Friedrich 
Bolz-Tereick in branch 'master':
bpo-39485: fix corner-case in method-detection of mock (GH-18252)
https://github.com/python/cpython/commit/a327677905956ae0b239ff430a1346dfe265709e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39485] Bug in mock running on PyPy3

[miss-islington]

Change by miss-islington :


--
pull_requests: +17633
pull_request: https://github.com/python/cpython/pull/18256

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39485] Bug in mock running on PyPy3

[miss-islington]

Change by miss-islington :


--
pull_requests: +17632
pull_request: https://github.com/python/cpython/pull/18255

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39485] Bug in mock running on PyPy3

[Chris Withers]

Change by Chris Withers :


--
assignee:  -> cjw296

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39485] Bug in mock running on PyPy3

[Carl Friedrich Bolz-Tereick]

Change by Carl Friedrich Bolz-Tereick :


--
keywords: +patch
pull_requests: +17629
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/18252

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39485] Bug in mock running on PyPy3

[Carl Friedrich Bolz-Tereick]

New submission from Carl Friedrich Bolz-Tereick :

One of the new-in-3.8 tests for unittest.mock, 
test_spec_has_descriptor_returning_function, is failing on PyPy. This exposes a 
bug in unittest.mock. The bug is most noticeable on PyPy, where it can be 
triggered by simply writing a slightly weird descriptor (CrazyDescriptor in the 
test). Getting it to trigger on CPython would be possible too, by implementing 
the same descriptor in C, but I did not actually do that.

The relevant part of the test looks like this:

from unittest.mock import create_autospec

class CrazyDescriptor(object):
def __get__(self, obj, type_):
if obj is None:
return lambda x: None

class MyClass(object):

some_attr = CrazyDescriptor()

mock = create_autospec(MyClass)
mock.some_attr(1)

On CPython this just works, on PyPy it fails with:

Traceback (most recent call last):
  File "x.py", line 13, in 
mock.some_attr(1)
  File 
"/home/cfbolz/bin/.pyenv/versions/pypy3.6-7.2.0/lib-python/3/unittest/mock.py", 
line 938, in __call__
_mock_self._mock_check_sig(*args, **kwargs)
  File 
"/home/cfbolz/bin/.pyenv/versions/pypy3.6-7.2.0/lib-python/3/unittest/mock.py", 
line 101, in checksig
sig.bind(*args, **kwargs)
  File 
"/home/cfbolz/bin/.pyenv/versions/pypy3.6-7.2.0/lib-python/3/inspect.py", line 
3034, in bind
return args[0]._bind(args[1:], kwargs)
  File 
"/home/cfbolz/bin/.pyenv/versions/pypy3.6-7.2.0/lib-python/3/inspect.py", line 
2955, in _bind
raise TypeError('too many positional arguments') from None
TypeError: too many positional arguments

The reason for this problem is that mock deduced that MyClass.some_attr is a 
method on PyPy. Since mock thinks the lambda returned by the descriptor is a 
method, it adds self as an argument, which leads to the TypeError.

Checking whether something is a method is done by _must_skip in mock.py. The 
relevant condition is this one:

elif isinstance(getattr(result, '__get__', None), MethodWrapperTypes):
# Normal method => skip if looked up on type
# (if looked up on instance, self is already skipped)
return is_type
else:
return False

MethodWrapperTypes is defined as:

MethodWrapperTypes = (
type(ANY.__eq__.__get__),
)

which is just types.MethodType on PyPy, because there is no such thing as a 
method wrapper (the builtin types look pretty much like python-defined types in 
PyPy).

On PyPy the condition isinstance(getattr...) is thus True for all descriptors! 
so as soon as result has a __get__, it counts as a method, even in the above 
case where it's a custom descriptor.


Now even on CPython the condition makes no sense to me. It would be True for a 
C-defined version of CrazyDescriptor, it's just not a good way to check whether 
result is a method.

I would propose to replace the condition with the much more straightforward 
check:

elif isinstance(result, FunctionTypes):
...

something is a method if it's a function on the class. Doing that change makes 
the test pass on PyPy, and doesn't introduce any test failures on CPython 
either.

Will open a pull request.

--
messages: 360961
nosy: Carl.Friedrich.Bolz, cjw296
priority: normal
severity: normal
status: open
title: Bug in mock running on PyPy3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com