Skip to content

Fix autospec's behavior on method-bound builtin functions#389

Merged
cjw296 merged 1 commit intotesting-cabal:masterfrom
habnabit:fix-builtin-methods-for-cython
Nov 30, 2018
Merged

Fix autospec's behavior on method-bound builtin functions#389
cjw296 merged 1 commit intotesting-cabal:masterfrom
habnabit:fix-builtin-methods-for-cython

Conversation

@habnabit
Copy link
Contributor

Cython will, in the right circumstances, offer a MethodType instance where im_func is a builtin function. Any instance of MethodType is automatically assumed to be a python-defined function (more specifically, a function that has an inspectable signature), but _set_signature was still conservative in its assumptions. As a result _set_signature would return early with None instead of a mock since the im_func had no inspectable signature. This causes problems deeper inside mock, as _set_signature is assumed to always return a mock, and nothing checked its return value.

In similar corner cases, autospec will simply not check the spec of the function, so _set_signature is amended to now return early with the original, not-wrapped mock object.

@habnabit
Copy link
Contributor Author

There is another None early return in _set_signature, but it seems.. harder to trigger? It seems more appropriate to raise an exception there if this is being fixed.

@habnabit
Copy link
Contributor Author

That travis failure seems to be not my fault?

@cjw296
Copy link
Collaborator

cjw296 commented Nov 28, 2018

@habnabit - perhaps look at getting this fixed in https://github.com/python/cpython? After that, I'd imagine a backport to this repo would get merged.

@cjw296 cjw296 closed this Nov 29, 2018
@habnabit
Copy link
Contributor Author

Yes, it was merged upstream: python/cpython#3

Cython will, in the right circumstances, offer a MethodType instance
where im_func is a builtin function. Any instance of MethodType is
automatically assumed to be a python-defined function (more
specifically, a function that has an inspectable signature), but
_set_signature was still conservative in its assumptions. As a result
_set_signature would return early with None instead of a mock since the
im_func had no inspectable signature. This causes problems deeper inside
mock, as _set_signature is assumed to _always_ return a mock, and
nothing checked its return value.

In similar corner cases, autospec will simply not check the spec of the
function, so _set_signature is amended to now return early with the
original, not-wrapped mock object.
@cjw296 cjw296 force-pushed the fix-builtin-methods-for-cython branch from b7023f2 to 6db8d96 Compare November 29, 2018 21:35
@cjw296 cjw296 merged commit 78dd97a into testing-cabal:master Nov 30, 2018
@habnabit habnabit deleted the fix-builtin-methods-for-cython branch December 28, 2018 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants