Honour resetall() arguments for non-callable mocks - #606
Open
dylanpulver wants to merge 1 commit into
Open
Conversation
reset_mock() is defined on NonCallableMock, but resetall() narrowed the kwargs-capable set to (Mock, AsyncMock). NonCallableMagicMock -- what mocker.create_autospec(cls, instance=True) returns -- is not a Mock subclass, so return_value= and side_effect= were silently dropped for it. The guard exists because autospecced functions carry a no-argument reset_mock closure (issue pytest-dev#237); those are plain functions, so they still take the else branch. AsyncMock was redundant: it already subclasses Mock.
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.
reset_mock()is defined onNonCallableMock, butresetall()narrowed the kwargs-capable set to(Mock, AsyncMock).NonCallableMagicMock— whatmocker.create_autospec(cls, instance=True)returns — is not aMocksubclass, so it took theelsebranch andreturn_value=/side_effect=were silently dropped:Calling
m.reset_mock(return_value=True, side_effect=True)directly does reset it, and the callable variantcreate_autospec(cls)already works — so the two disagree today.The guard itself is load-bearing and I kept it. Autospecced functions carry a no-argument
reset_mockclosure (#237), and those are plain functions, notNonCallableMockinstances, so they still take theelsebranch. Removing the isinstance check altogether re-breakstest_spy_resetwith the originalTypeError— I checked.AsyncMockin the old tuple was redundant: it already subclassesMock.Why nothing caught it: #390 added
create_autospecresults to the cache in 2023, which is how non-callable mocks started reaching this loop; the type guard is from #241 (2021) and was not revisited. The test #390 added usescreate_autospec(TestObject)withoutinstance=True— the one variant that takes the working branch.Ran on Python 3.12:
pytest tests/gives 87 passed here vs 86 onmain, same venv and command. Four tests fail identically on both (test_detailed_introspection*,test_assert_called_*_with_introspection) — assertion-output differences under pytest 9.1.1, not related to this change. ruff 0.16.5 check + format, mypy 2.3.1 and rst-lint are clean at the versions.pre-commit-config.yamlpins.How I found it: enumerating the public
mocker.*keyword arguments and checking where each is actually honoured. These two are honoured on one branch only.AI assistance: this change and its test were drafted with Claude Opus 5 (
claude-opus-5). The commands and numbers above were run locally.