Warn instead of raising exception in context manager (#221)

Co-authored-by: Bruno Oliveira <[email protected]>

Author: iforapsy

src/pytest_mock/__init__.py

@@ -5,6 +5,7 @@
 __all__ = [
     "MockerFixture",
     "MockFixture",
+    "PytestMockWarning",
     "pytest_addoption",
     "pytest_configure",
     "session_mocker",

src/pytest_mock/plugin.py

@@ -13,6 +13,8 @@
 import asyncio
 import functools
 import inspect
+import warnings
+import sys
 
 import pytest
 
@@ -39,6 +41,10 @@ def _get_mock_module(config):
     return _get_mock_module._module
 
 
+class PytestMockWarning(UserWarning):
+    """Base class for all warnings emitted by pytest-mock."""
+
+
 class MockerFixture:
     """
     Fixture that provides the same interface to functions in the mock module,
@@ -169,7 +175,7 @@ def __init__(self, patches, mocks, mock_module):
             self.mock_module = mock_module
 
         def _start_patch(
-            self, mock_func: Any, *args: Any, **kwargs: Any
+            self, mock_func: Any, warn_on_mock_enter: bool, *args: Any, **kwargs: Any
         ) -> unittest.mock.MagicMock:
             """Patches something by calling the given function from the mock
             module, registering the patch to stop it later and returns the
@@ -182,10 +188,18 @@ def _start_patch(
                 self._mocks.append(mocked)
                 # check if `mocked` is actually a mock object, as depending on autospec or target
                 # parameters `mocked` can be anything
-                if hasattr(mocked, "__enter__"):
-                    mocked.__enter__.side_effect = ValueError(
-                        "Using mocker in a with context is not supported. "
-                        "https://github.com/pytest-dev/pytest-mock#note-about-usage-as-context-manager"
+                if hasattr(mocked, "__enter__") and warn_on_mock_enter:
+                    if sys.version_info >= (3, 8):
+                        depth = 5
+                    else:
+                        depth = 4
+                    mocked.__enter__.side_effect = lambda: warnings.warn(
+                        "Mocks returned by pytest-mock do not need to be used as context managers. "
+                        "The mocker fixture automatically undoes mocking at the end of a test. "
+                        "This warning can be ignored if it was triggered by mocking a context manager. "
+                        "https://github.com/pytest-dev/pytest-mock#note-about-usage-as-context-manager",
+                        PytestMockWarning,
+                        stacklevel=depth,
                     )
             return mocked
 
@@ -206,6 +220,37 @@ def object(
                 new = self.mock_module.DEFAULT
             return self._start_patch(
                 self.mock_module.patch.object,
+                True,
+                target,
+                attribute,
+                new=new,
+                spec=spec,
+                create=create,
+                spec_set=spec_set,
+                autospec=autospec,
+                new_callable=new_callable,
+                **kwargs
+            )
+
+        def context_manager(
+            self,
+            target: builtins.object,
+            attribute: str,
+            new: builtins.object = DEFAULT,
+            spec: Optional[builtins.object] = None,
+            create: bool = False,
+            spec_set: Optional[builtins.object] = None,
+            autospec: Optional[builtins.object] = None,
+            new_callable: builtins.object = None,
+            **kwargs: Any
+        ) -> unittest.mock.MagicMock:
+            """This is equivalent to mock.patch.object except that the returned mock
+            does not issue a warning when used as a context manager."""
+            if new is self.DEFAULT:
+                new = self.mock_module.DEFAULT
+            return self._start_patch(
+                self.mock_module.patch.object,
+                False,
                 target,
                 attribute,
                 new=new,
@@ -230,6 +275,7 @@ def multiple(
             """API to mock.patch.multiple"""
             return self._start_patch(
                 self.mock_module.patch.multiple,
+                True,
                 target,
                 spec=spec,
                 create=create,
@@ -249,6 +295,7 @@ def dict(
             """API to mock.patch.dict"""
             return self._start_patch(
                 self.mock_module.patch.dict,
+                True,
                 in_dict,
                 values=values,
                 clear=clear,
@@ -328,6 +375,7 @@ def __call__(
                 new = self.mock_module.DEFAULT
             return self._start_patch(
                 self.mock_module.patch,
+                True,
                 target,
                 new=new,
                 spec=spec,

tests/test_pytest_mock.py

@@ -1,12 +1,13 @@
 import os
 import platform
+import re
 import sys
 from contextlib import contextmanager
 from typing import Callable, Any, Tuple, Generator, Type
 from unittest.mock import MagicMock
 
 import pytest
-from pytest_mock import MockerFixture
+from pytest_mock import MockerFixture, PytestMockWarning
 
 pytest_plugins = "pytester"
 
@@ -806,36 +807,44 @@ def test_get_random_number(mocker):
     assert "RuntimeError" not in result.stderr.str()
 
 
-def test_abort_patch_object_context_manager(mocker: MockerFixture) -> None:
+def test_warn_patch_object_context_manager(mocker: MockerFixture) -> None:
     class A:
         def doIt(self):
             return False
 
     a = A()
 
-    with pytest.raises(ValueError) as excinfo:
-        with mocker.patch.object(a, "doIt", return_value=True):
-            assert a.doIt() is True
-
-    expected_error_msg = (
-        "Using mocker in a with context is not supported. "
+    expected_warning_msg = (
+        "Mocks returned by pytest-mock do not need to be used as context managers. "
+        "The mocker fixture automatically undoes mocking at the end of a test. "
+        "This warning can be ignored if it was triggered by mocking a context manager. "
         "https://github.com/pytest-dev/pytest-mock#note-about-usage-as-context-manager"
     )
 
-    assert str(excinfo.value) == expected_error_msg
+    with pytest.warns(
+        PytestMockWarning, match=re.escape(expected_warning_msg)
+    ) as warn_record:
+        with mocker.patch.object(a, "doIt", return_value=True):
+            assert a.doIt() is True
 
+    assert warn_record[0].filename == __file__
 
-def test_abort_patch_context_manager(mocker: MockerFixture) -> None:
-    with pytest.raises(ValueError) as excinfo:
-        with mocker.patch("json.loads"):
-            pass
 
-    expected_error_msg = (
-        "Using mocker in a with context is not supported. "
+def test_warn_patch_context_manager(mocker: MockerFixture) -> None:
+    expected_warning_msg = (
+        "Mocks returned by pytest-mock do not need to be used as context managers. "
+        "The mocker fixture automatically undoes mocking at the end of a test. "
+        "This warning can be ignored if it was triggered by mocking a context manager. "
         "https://github.com/pytest-dev/pytest-mock#note-about-usage-as-context-manager"
     )
 
-    assert str(excinfo.value) == expected_error_msg
+    with pytest.warns(
+        PytestMockWarning, match=re.escape(expected_warning_msg)
+    ) as warn_record:
+        with mocker.patch("json.loads"):
+            pass
+
+    assert warn_record[0].filename == __file__
 
 
 def test_context_manager_patch_example(mocker: MockerFixture) -> None:
@@ -858,6 +867,23 @@ def my_func():
     assert isinstance(my_func(), mocker.MagicMock)
 
 
+def test_patch_context_manager_with_context_manager(mocker: MockerFixture) -> None:
+    """Test that no warnings are issued when an object patched with
+    patch.context_manager is used as a context manager (#221)"""
+
+    class A:
+        def doIt(self):
+            return False
+
+    a = A()
+
+    with pytest.warns(None) as warn_record:
+        with mocker.patch.context_manager(a, "doIt", return_value=True):
+            assert a.doIt() is True
+
+    assert len(warn_record) == 0
+
+
 def test_abort_patch_context_manager_with_stale_pyc(testdir: Any) -> None:
     """Ensure we don't trigger an error in case the frame where mocker.patch is being
     used doesn't have a 'context' (#169)"""