From ef8ec01e3979aff992d63540a0e36a957e133c76 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 4 Aug 2018 15:14:00 -0300 Subject: [PATCH 1/2] Fix issue where fixtures would lose the decorated functionality Fix #3774 --- src/_pytest/compat.py | 7 ++++ src/_pytest/fixtures.py | 7 ++-- testing/acceptance_test.py | 7 ++++ .../acceptance/fixture_mock_integration.py | 17 ++++++++++ testing/test_compat.py | 32 +++++++++++++++++++ 5 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 testing/example_scripts/acceptance/fixture_mock_integration.py diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index 52051ff2326..57ad4fdd84d 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -234,6 +234,13 @@ def get_real_func(obj): """ start_obj = obj for i in range(100): + # __pytest_wrapped__ is set by @pytest.fixture when wrapping the fixture function + # to trigger a warning if it gets called directly instead of by pytest: we don't + # want to unwrap further than this otherwise we lose useful wrappings like @mock.patch (#3774) + new_obj = getattr(obj, "__pytest_wrapped__", None) + if new_obj is not None: + obj = new_obj + break new_obj = getattr(obj, "__wrapped__", None) if new_obj is None: break diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 818c5b81f74..0d63b151fd7 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -954,9 +954,6 @@ def _ensure_immutable_ids(ids): def wrap_function_to_warning_if_called_directly(function, fixture_marker): """Wrap the given fixture function so we can issue warnings about it being called directly, instead of used as an argument in a test function. - - The warning is emitted only in Python 3, because I didn't find a reliable way to make the wrapper function - keep the original signature, and we probably will drop Python 2 in Pytest 4 anyway. """ is_yield_function = is_generator(function) msg = FIXTURE_FUNCTION_CALL.format(name=fixture_marker.name or function.__name__) @@ -982,6 +979,10 @@ def result(*args, **kwargs): if six.PY2: result.__wrapped__ = function + # keep reference to the original function in our own custom attribute so we don't unwrap + # further than this point and lose useful wrappings like @mock.patch (#3774) + result.__pytest_wrapped__ = function + return result diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 6f12cc3351a..bc4e3bed85f 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -1044,3 +1044,10 @@ def test2(): ) result = testdir.runpytest_subprocess() result.stdout.fnmatch_lines(["*1 failed, 1 passed in*"]) + + +def test_fixture_mock_integration(testdir): + """Test that decorators applied to fixture are left working (#3774)""" + p = testdir.copy_example("acceptance/fixture_mock_integration.py") + result = testdir.runpytest(p) + result.stdout.fnmatch_lines("*1 passed*") diff --git a/testing/example_scripts/acceptance/fixture_mock_integration.py b/testing/example_scripts/acceptance/fixture_mock_integration.py new file mode 100644 index 00000000000..51f46f82cae --- /dev/null +++ b/testing/example_scripts/acceptance/fixture_mock_integration.py @@ -0,0 +1,17 @@ +"""Reproduces issue #3774""" + +import mock + +import pytest + +config = {"mykey": "ORIGINAL"} + + +@pytest.fixture(scope="function") +@mock.patch.dict(config, {"mykey": "MOCKED"}) +def my_fixture(): + return config["mykey"] + + +def test_foobar(my_fixture): + assert my_fixture == "MOCKED" diff --git a/testing/test_compat.py b/testing/test_compat.py index 399b0d342b5..0663fa1496d 100644 --- a/testing/test_compat.py +++ b/testing/test_compat.py @@ -1,5 +1,8 @@ from __future__ import absolute_import, division, print_function import sys +from functools import wraps + +import six import pytest from _pytest.compat import is_generator, get_real_func, safe_getattr @@ -26,6 +29,8 @@ def __repr__(self): return "".format(left=self.left) def __getattr__(self, attr): + if attr == "__pytest_wrapped__": + raise AttributeError if not self.left: raise RuntimeError("its over") self.left -= 1 @@ -38,6 +43,33 @@ def __getattr__(self, attr): print(res) +def test_get_real_func(): + """Check that get_real_func correctly unwraps decorators until reaching the real function""" + + def decorator(f): + @wraps(f) + def inner(): + pass + + if six.PY2: + inner.__wrapped__ = f + return inner + + def func(): + pass + + wrapped_func = decorator(decorator(func)) + assert get_real_func(wrapped_func) is func + + wrapped_func2 = decorator(decorator(wrapped_func)) + assert get_real_func(wrapped_func2) is func + + # special case for __pytest_wrapped__ attribute: used to obtain the function up until the point + # a function was wrapped by pytest itself + wrapped_func2.__pytest_wrapped__ = wrapped_func + assert get_real_func(wrapped_func2) is wrapped_func + + @pytest.mark.skipif( sys.version_info < (3, 4), reason="asyncio available in Python 3.4+" ) From 67106f056b0633b35dd4a080ef120fa61b55cf37 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 9 Aug 2018 09:20:37 -0300 Subject: [PATCH 2/2] Use a custom holder class so we can be sure __pytest_wrapper__ was set by us --- src/_pytest/compat.py | 16 ++++++++++++++-- src/_pytest/fixtures.py | 3 ++- testing/test_compat.py | 6 ++---- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index 57ad4fdd84d..c3ecaf9121d 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -228,6 +228,18 @@ def ascii_escaped(val): return val.encode("unicode-escape") +class _PytestWrapper(object): + """Dummy wrapper around a function object for internal use only. + + Used to correctly unwrap the underlying function object + when we are creating fixtures, because we wrap the function object ourselves with a decorator + to issue warnings when the fixture function is called directly. + """ + + def __init__(self, obj): + self.obj = obj + + def get_real_func(obj): """ gets the real function object of the (possibly) wrapped object by functools.wraps or functools.partial. @@ -238,8 +250,8 @@ def get_real_func(obj): # to trigger a warning if it gets called directly instead of by pytest: we don't # want to unwrap further than this otherwise we lose useful wrappings like @mock.patch (#3774) new_obj = getattr(obj, "__pytest_wrapped__", None) - if new_obj is not None: - obj = new_obj + if isinstance(new_obj, _PytestWrapper): + obj = new_obj.obj break new_obj = getattr(obj, "__wrapped__", None) if new_obj is None: diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 0d63b151fd7..a6634cd1162 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -31,6 +31,7 @@ safe_getattr, FuncargnamesCompatAttr, get_real_method, + _PytestWrapper, ) from _pytest.deprecated import FIXTURE_FUNCTION_CALL, RemovedInPytest4Warning from _pytest.outcomes import fail, TEST_OUTCOME @@ -981,7 +982,7 @@ def result(*args, **kwargs): # keep reference to the original function in our own custom attribute so we don't unwrap # further than this point and lose useful wrappings like @mock.patch (#3774) - result.__pytest_wrapped__ = function + result.__pytest_wrapped__ = _PytestWrapper(function) return result diff --git a/testing/test_compat.py b/testing/test_compat.py index 0663fa1496d..a6249d14b2e 100644 --- a/testing/test_compat.py +++ b/testing/test_compat.py @@ -5,7 +5,7 @@ import six import pytest -from _pytest.compat import is_generator, get_real_func, safe_getattr +from _pytest.compat import is_generator, get_real_func, safe_getattr, _PytestWrapper from _pytest.outcomes import OutcomeException @@ -29,8 +29,6 @@ def __repr__(self): return "".format(left=self.left) def __getattr__(self, attr): - if attr == "__pytest_wrapped__": - raise AttributeError if not self.left: raise RuntimeError("its over") self.left -= 1 @@ -66,7 +64,7 @@ def func(): # special case for __pytest_wrapped__ attribute: used to obtain the function up until the point # a function was wrapped by pytest itself - wrapped_func2.__pytest_wrapped__ = wrapped_func + wrapped_func2.__pytest_wrapped__ = _PytestWrapper(wrapped_func) assert get_real_func(wrapped_func2) is wrapped_func