Skip to content

Conversation

@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Jan 29, 2020

Replace check for whether something is a method in the mock module. The
previous version fails on PyPy, because there no method wrappers exist
(everything looks like a regular Python-defined function). Thus the
isinstance(getattr(result, 'get', None), MethodWrapperTypes) check
returns True for any descriptor, not just methods.

This condition could also return erroneously True in CPython for
C-defined descriptors.

Instead to decide whether something is a method, just check directly
whether it's a function defined on the class. This passes all tests on
CPython and fixes the bug on PyPy.
(cherry picked from commit a327677)

Co-authored-by: Carl Friedrich Bolz-Tereick [email protected]

https://bugs.python.org/issue39485

Replace check for whether something is a method in the mock module. The
previous version fails on PyPy, because there no method wrappers exist
(everything looks like a regular Python-defined function). Thus the
isinstance(getattr(result, '__get__', None), MethodWrapperTypes) check
returns True for any descriptor, not just methods.

This condition could also return erroneously True in CPython for
C-defined descriptors.

Instead to decide whether something is a method, just check directly
whether it's a function defined on the class. This passes all tests on
CPython and fixes the bug on PyPy.
(cherry picked from commit a327677)

Co-authored-by: Carl Friedrich Bolz-Tereick <[email protected]>
@miss-islington
Copy link
Contributor Author

@cfbolz and @cjw296: Status check is done, and it's a success ✅ .

1 similar comment
@miss-islington
Copy link
Contributor Author

@cfbolz and @cjw296: Status check is done, and it's a success ✅ .

@cjw296 cjw296 merged commit cf0645a into python:3.7 Jan 29, 2020
@miss-islington miss-islington deleted the backport-a327677-3.7 branch January 29, 2020 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants