From c33a3007f6448c254f35d47c57e2055fdbabe9d9 Mon Sep 17 00:00:00 2001 From: Jack Wong Date: Mon, 21 Dec 2020 16:07:16 -0800 Subject: [PATCH 1/6] Warn when a mock is used as a context manager Instead of raising an exception, warn when a pytest-mock mock is used as a context manager. This lets pytest-mock mock objects that are used as context managers, like threading.Lock, while still letting users know that mocks returned from pytest-mock do not need their __enter__ method called to take effect. Create a PytestMockWarning class so that the warning is easy to pinpoint and handle. --- src/pytest_mock/__init__.py | 1 + src/pytest_mock/plugin.py | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/pytest_mock/__init__.py b/src/pytest_mock/__init__.py index 97cdc58..169ece5 100644 --- a/src/pytest_mock/__init__.py +++ b/src/pytest_mock/__init__.py @@ -5,6 +5,7 @@ __all__ = [ "MockerFixture", "MockFixture", + "PytestMockWarning", "pytest_addoption", "pytest_configure", "session_mocker", diff --git a/src/pytest_mock/plugin.py b/src/pytest_mock/plugin.py index 1218d93..41b5aa8 100644 --- a/src/pytest_mock/plugin.py +++ b/src/pytest_mock/plugin.py @@ -13,6 +13,7 @@ import asyncio import functools import inspect +import warnings import pytest @@ -39,6 +40,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, @@ -183,9 +188,11 @@ def _start_patch( # 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( + mocked.__enter__.side_effect = lambda: warnings.warn( "Using mocker in a with context is not supported. " - "https://github.com/pytest-dev/pytest-mock#note-about-usage-as-context-manager" + "https://github.com/pytest-dev/pytest-mock#note-about-usage-as-context-manager", + PytestMockWarning, + stacklevel=4, ) return mocked From e40e97cbc931cbf6dd2b38eeda4a2587d4509cb9 Mon Sep 17 00:00:00 2001 From: Jack Wong Date: Mon, 21 Dec 2020 16:13:29 -0800 Subject: [PATCH 2/6] Update unit tests for context manager warnings Test that warnings are issued when mocks are used as context manager. Verify that the warnings have the expected message and that they point to the correct code. --- tests/test_pytest_mock.py | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/test_pytest_mock.py b/tests/test_pytest_mock.py index b2e69cb..01eae7e 100644 --- a/tests/test_pytest_mock.py +++ b/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,40 @@ 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 = ( + expected_warning_msg = ( "Using mocker in a with context is not supported. " "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 = ( +def test_warn_patch_context_manager(mocker: MockerFixture) -> None: + expected_warning_msg = ( "Using mocker in a with context is not supported. " "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: From 3220c964a5b6510cf6c0a2e0330d2565eeea9879 Mon Sep 17 00:00:00 2001 From: Jack Wong Date: Mon, 21 Dec 2020 17:35:32 -0800 Subject: [PATCH 3/6] Fix stacklevel for Python 3.8 and higher Use a stacklevel=5 for Python 3.8 and up so that the emitted warning points to the correct line of code. At stacklevel=4, it was incorrectly pointing to code in the standard library. --- src/pytest_mock/plugin.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/pytest_mock/plugin.py b/src/pytest_mock/plugin.py index 41b5aa8..f352aab 100644 --- a/src/pytest_mock/plugin.py +++ b/src/pytest_mock/plugin.py @@ -14,6 +14,7 @@ import functools import inspect import warnings +import sys import pytest @@ -188,11 +189,15 @@ def _start_patch( # check if `mocked` is actually a mock object, as depending on autospec or target # parameters `mocked` can be anything if hasattr(mocked, "__enter__"): + if sys.version_info >= (3, 8): + depth = 5 + else: + depth = 4 mocked.__enter__.side_effect = lambda: warnings.warn( "Using mocker in a with context is not supported. " "https://github.com/pytest-dev/pytest-mock#note-about-usage-as-context-manager", PytestMockWarning, - stacklevel=4, + stacklevel=depth, ) return mocked From 45d8a205d2e2db81bdb2b551e413aacaf54b5a44 Mon Sep 17 00:00:00 2001 From: Jack Wong Date: Wed, 23 Dec 2020 18:13:31 -0800 Subject: [PATCH 4/6] Make context manager warning more detailed Make the warning message when a mock is used as a context manager more detailed. In the message, describe the use case to avoid and when the warning can be safely ignored. --- src/pytest_mock/plugin.py | 4 +++- tests/test_pytest_mock.py | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/pytest_mock/plugin.py b/src/pytest_mock/plugin.py index f352aab..9c32a32 100644 --- a/src/pytest_mock/plugin.py +++ b/src/pytest_mock/plugin.py @@ -194,7 +194,9 @@ def _start_patch( else: depth = 4 mocked.__enter__.side_effect = lambda: warnings.warn( - "Using mocker in a with context is not supported. " + "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, diff --git a/tests/test_pytest_mock.py b/tests/test_pytest_mock.py index 01eae7e..6cde9f0 100644 --- a/tests/test_pytest_mock.py +++ b/tests/test_pytest_mock.py @@ -815,7 +815,9 @@ def doIt(self): a = A() expected_warning_msg = ( - "Using mocker in a with context is not supported. " + "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" ) @@ -830,7 +832,9 @@ def doIt(self): def test_warn_patch_context_manager(mocker: MockerFixture) -> None: expected_warning_msg = ( - "Using mocker in a with context is not supported. " + "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" ) From 838d3841433d5b537f37e1b51f5f871075b2995c Mon Sep 17 00:00:00 2001 From: Jack Wong Date: Wed, 23 Dec 2020 19:31:38 -0800 Subject: [PATCH 5/6] Create patch.context_manager method Create patch.context_manager method that allows the patched object to be used as a context manager without issuing a warning on __enter__. Otherwise, the method is functionally equivalent to patch.object. --- src/pytest_mock/plugin.py | 38 ++++++++++++++++++++++++++++++++++++-- tests/test_pytest_mock.py | 17 +++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/pytest_mock/plugin.py b/src/pytest_mock/plugin.py index 9c32a32..2926b74 100644 --- a/src/pytest_mock/plugin.py +++ b/src/pytest_mock/plugin.py @@ -175,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 @@ -188,7 +188,7 @@ 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__"): + if hasattr(mocked, "__enter__") and warn_on_mock_enter: if sys.version_info >= (3, 8): depth = 5 else: @@ -220,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: object, + attribute: str, + new: object = DEFAULT, + spec: Optional[object] = None, + create: bool = False, + spec_set: Optional[object] = None, + autospec: Optional[object] = None, + new_callable: 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, @@ -244,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, @@ -263,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, @@ -342,6 +375,7 @@ def __call__( new = self.mock_module.DEFAULT return self._start_patch( self.mock_module.patch, + True, target, new=new, spec=spec, diff --git a/tests/test_pytest_mock.py b/tests/test_pytest_mock.py index 6cde9f0..f9fa520 100644 --- a/tests/test_pytest_mock.py +++ b/tests/test_pytest_mock.py @@ -867,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)""" From 58402351bb5be236deea690c13774b13b4c4e2b5 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 4 Jan 2021 16:17:56 -0300 Subject: [PATCH 6/6] Fix linting --- src/pytest_mock/plugin.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pytest_mock/plugin.py b/src/pytest_mock/plugin.py index 2926b74..3be8667 100644 --- a/src/pytest_mock/plugin.py +++ b/src/pytest_mock/plugin.py @@ -234,14 +234,14 @@ def object( def context_manager( self, - target: object, + target: builtins.object, attribute: str, - new: object = DEFAULT, - spec: Optional[object] = None, + new: builtins.object = DEFAULT, + spec: Optional[builtins.object] = None, create: bool = False, - spec_set: Optional[object] = None, - autospec: Optional[object] = None, - new_callable: object = None, + 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