Repository navigation
Fix spy on inherited classmethod/staticmethod called through an instance - #661
Open
abhishekcpp007 wants to merge 2 commits into
Open
abhishekcpp007 wants to merge 2 commits into
abhishekcpp007 wants to merge 2 commits into
Conversation
Since pytest-dev#399, mocker.spy() autospecs every function or method it spies. mock.patch.object only recognises a classmethod or staticmethod that is defined on the patched class itself. For one inherited from a base class it autospecs the plain function, which then binds to the instance when called through one and fails with TypeError: too many positional arguments. Pass the classmethod/staticmethod descriptor as autospec, which is what mock does on its own for one defined on the class.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mocker.spy()breaks for aclassmethodorstaticmethodthat the spied class inherits from a base class, as soon as the method is called through an instance. The call fails insideinspectwith aTypeErrorbefore the spied code even runs.A
classmethodfails the same way. Calling through the class works, which is why the existingtest_class_method_subclassandtest_static_method_subclasstests don't catch it. Ifdoubleis defined onFooitself, both ways work.Root cause
src/pytest_mock/plugin.py:247setsautospec=Truefor any function or method.mock.patch.objectthen autospecstarget.__dict__[name]when it's there, which keeps thestaticmethod/classmethoddescriptor and gives a non-binding mock. For an inherited one it falls back togetattr(target, name), which is already the plain or bound function, so it creates an autospecced function and sets it onFoo. A function on a class binds to the instance, soFoo().double(1)passes the instance as an extra argument and the signature check rejects it.Before #399 the spy checked
inspect.getattr_static(obj, name)forclassmethod/staticmethodand didn't autospec them. That PR removed the check as an old Python workaround and mentioned it shouldn't affect users. This is the case it did affect, so it's a regression since 3.13.0. Plainmock.patch.object(Foo, "double", autospec=True)has the same problem, but the spy picksautospecitself, so it can avoid it.Fix
When spying on a class, I look up the attribute with
inspect.getattr_staticand, if it's aclassmethodorstaticmethod, pass that descriptor asautospec. That's exactly whatmockdoes on its own when the method is defined on the class, so inherited and local methods are now specced the same way, signature checks included.Tests
I added
test_class_method_subclass_called_on_instanceandtest_static_method_subclass_called_on_instance. They call the spy through an instance, check the recorded call and return value, check that a bad call still raisesTypeError, and check thatmocker.stop()removes the patch from the subclass again.Run in Docker (
python:3.12, package installed withcoverage mock pytest-asynciolike tox does,pytest tests):168 passed, 9 warnings170 passed, 9 warningssrc/:2 failed, 168 passed, 9 warnings(the two new tests)pre-commit (rst, ruff, ruff-format, mypy) passes on the changed files.