diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d116714..a2f610b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,11 @@ Releases ======== +UNRELEASED +---------- + +* `#646 `_: ``mocker.resetall(side_effect=True)`` no longer removes the warning about using mocks returned by ``mocker.patch`` and friends as context managers. + 3.17.0 ------ diff --git a/src/pytest_mock/plugin.py b/src/pytest_mock/plugin.py index 71e5c2b..f448e51 100644 --- a/src/pytest_mock/plugin.py +++ b/src/pytest_mock/plugin.py @@ -56,6 +56,10 @@ class MockCacheItem: is_spy: bool = False # The ``side_effect`` a spy uses to call the spied code, restored by ``resetall``. spy_side_effect: Callable[..., Any] | None = None + # ``mock.__enter__`` and its ``side_effect`` that warns about using the mock + # as a context manager, restored by ``resetall``. + enter_mock: Any | None = None + enter_side_effect: Callable[[], None] | None = None # The objects ``resetall`` resets instead of ``mock``, for patches such as # ``patch.multiple`` whose ``mock`` is a dict of the patched values. reset_mocks: tuple[Any, ...] | None = None @@ -168,6 +172,10 @@ def resetall( # it means going back to that, not to ``None``. if side_effect and mock_item.spy_side_effect is not None: mock_item.mock.side_effect = mock_item.spy_side_effect + # Resetting ``side_effect`` also clears it on ``__enter__``, which + # would drop the warning about using the mock as a context manager. + if side_effect and mock_item.enter_mock is not None: + mock_item.enter_mock.side_effect = mock_item.enter_side_effect def _reset_mock(self, mock: Any, *, return_value: bool, side_effect: bool) -> None: # See issue #237. @@ -307,7 +315,7 @@ def _start_patch( """ p = mock_func(*args, **kwargs) mocked: MockType = p.start() - self.__mock_cache.add(mock=mocked, patch=p) + mock_item = self.__mock_cache.add(mock=mocked, patch=p) if hasattr(mocked, "reset_mock"): # noqa:SIM102 # check if `mocked` is actually a mock object, as depending on autospec or target # parameters `mocked` can be anything @@ -320,6 +328,8 @@ def _start_patch( PytestMockWarning, stacklevel=5, ) + mock_item.enter_mock = mocked.__enter__ + mock_item.enter_side_effect = mocked.__enter__.side_effect return mocked def object( diff --git a/tests/test_pytest_mock.py b/tests/test_pytest_mock.py index 8921e6d..e74ca8d 100644 --- a/tests/test_pytest_mock.py +++ b/tests/test_pytest_mock.py @@ -1818,6 +1818,52 @@ def test_patch(self, mocker: MockerFixture) -> None: assert warn_record[0].filename == __file__ + def test_patch_after_resetall(self, mocker: MockerFixture) -> None: + """``resetall(side_effect=True)`` keeps the warning (#637).""" + mocked = mocker.patch("json.loads") + mocker.resetall(return_value=True, side_effect=True) + + with pytest.warns(PytestMockWarning) as warn_record, mocked: + pass + + assert warn_record[0].filename == __file__ + + def test_patch_deleted_enter_after_resetall(self, mocker: MockerFixture) -> None: + """``resetall(side_effect=True)`` leaves a deleted ``__enter__`` alone.""" + mocked = mocker.patch("json.loads") + del mocked.__enter__ + mocker.resetall(side_effect=True) + + assert not hasattr(mocked, "__enter__") + + def test_spy_after_resetall(self, mocker: MockerFixture) -> None: + """Spies backed by a ``MagicMock`` also keep the warning after + ``resetall(side_effect=True)`` (#637).""" + expected = os.getcwd() + spy = mocker.spy(os, "getcwd") + mocker.resetall(side_effect=True) + + with pytest.warns(PytestMockWarning), spy: + pass + assert os.getcwd() == expected + + def test_patch_context_manager_after_resetall(self, mocker: MockerFixture) -> None: + """``resetall`` does not add the warning to ``patch.context_manager``.""" + + class A: + def doIt(self): + return False + + a = A() + mocked = mocker.patch.context_manager(a, "doIt", return_value=True) + mocker.resetall(side_effect=True) + + with warnings.catch_warnings(record=True) as warn_record: # noqa:SIM117 + with mocked: + pass + + assert len(warn_record) == 0 + def test_patching_a_context_manager(self, mocker: MockerFixture) -> None: """Our message about misusing mocker as a context manager should not affect mocking context managers (see #192)"""