bpo-32092: Consume self when autospeccing unbound instance methods - #4476
bpo-32092: Consume self when autospeccing unbound instance methods#4476claudiubelu wants to merge 2 commits into
Conversation
d59fe18 to
797f190
Compare
|
Or, just merge the 2 and close out the older one? |
|
Hm, tbh, I'd like to keep it simple, since they're both addressing different issues. Anyways, I guess I'll just separate this patch from the other. We'll deal with the merge conflict when it happens. |
|
Actually, nevermind, the unit tests are not passing without the 1st patch. :) |
|
alright then. |
797f190 to
60bbe51
Compare
|
@claudiubelu, please resolve the merge conflicts. Thank you! |
|
@claudiubelu - it looks like #1982 is with @voidspace and this PR depends on it? Would it be okay to close that one until #1982 is resolved? |
|
This PR is stale because it has been open for 30 days with no activity. |
Currently, when patching instance / class methods with autospec, their
self / cls arguments are not consumed, causing call asserts to fail
(they expect an instance / class reference as the first argument).
Example:
from unittest import mock
class Something(object):
def foo(self, a, b, c, d):
pass
with mock.patch.object(Something, 'foo', autospec=True):
s = Something()
s.foo()
Fix this by skipping the first argument when presented with a method.
Based on python#4476
Signed-off-by: Stephen Finucane <stephen@that.guru>
Co-authored-by: Claudiu Belu <cbelu@cloudbasesolutions.com>
Currently, when patching instance / class methods with autospec, their
self / cls arguments are not consumed, causing call asserts to fail
(they expect an instance / class reference as the first argument).
Example:
from unittest import mock
class Something(object):
def foo(self, a, b, c, d):
pass
with mock.patch.object(Something, 'foo', autospec=True):
s = Something()
s.foo()
Fix this by skipping the first argument when presented with a method.
Based on python#4476
Signed-off-by: Stephen Finucane <stephen@that.guru>
Co-authored-by: Claudiu Belu <cbelu@cloudbasesolutions.com>
|
This PR is stale because it has been open for 30 days with no activity. |
60bbe51 to
18deb31
Compare
Currently, when patching methods with autospec, their self
arguments are not consumed, causing call asserts to fail (they
expect an instance reference as the first argument).
Example:
```
from unittest import mock
class Something(object):
def foo(self, a):
pass
patcher = mock.patch.object(Something, 'foo', autospec=True)
patcher.start()
s = Something()
s.foo("foo")
s.foo.assert_called_once_with("foo")
```
The assertion above will fail, expecting an instance as a first
argument. This issue **only** occurs in this scenario, and **not**
in any of the following scenarios:
- `autospec=False`
- if the autospecced method is a `classmethod` or a `staticmethod`
- if an instance is patched instead of a class
- if `mock.create_autospec(Something)` is used instead
- if `mock.Mock(autospec=Something)` is used instead
This issue affects all patch variants.
self is now consumed by the generated wrapper and excluded from call
recording and signature checking, in both the sync and async paths.
wraps and side_effect still get `self` bound for the call if needed,
since they need to proxy through to a potentially unbound callable (e.g.:
`wraps=getattr(cls, name)`.
Adds test coverage across instance methods, classmethods,
staticmethods, sync and async, for both wraps and side_effect,
including a recursive-wraps regression test.
Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>
18deb31 to
3f8c8ff
Compare
Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>
Documentation build overview
|
bpo-32092: Fixes mock.patch autospec self / cls argument consumption issue
https://bugs.python.org/issue32092
#76273
Currently, when patching methods with
autospec, theirselfarguments are not consumed, causing call asserts to fail (theyexpect an instance reference as the first argument).
Example:
The assertion above will fail, expecting an instance as a first argument. This issue only occurs in this scenario, and not in any of the following scenarios:
autospec=Falseclassmethodor astaticmethodmock.create_autospec(Something)is used insteadmock.Mock(autospec=Something)is used instead (if the PR adding it gets accepted)This issue affects all patch variants.
selfis now consumed by the generated wrapper and excluded from call recording and signature checking, in both the sync and async paths.wrapsandside_effectstill getselfbound for the call if needed, since they need to proxy through to a potentially unbound callable (e.g.:wraps=getattr(cls, name).Adds test coverage across instance methods, classmethods, staticmethods, sync and async, for both
wrapsandside_effect, including a recursive-wraps regression test.