From c6b11b9f6279fa6ee3ea91ae4f1d5ecfae67ce28 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 31 Jul 2018 22:09:19 -0300 Subject: [PATCH] Refactor direct fixture call warning to avoid incompatibility with plugins This refactors the code so we have the real function object right during collection. This avoids having to unwrap it later and lose attached information such as "async" functions. Fix #3747 --- changelog/3747.bugfix.rst | 1 + src/_pytest/compat.py | 15 +++++++++++++++ src/_pytest/fixtures.py | 27 +++++++++++++-------------- testing/deprecated_test.py | 1 - testing/python/fixture.py | 7 +++---- 5 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 changelog/3747.bugfix.rst diff --git a/changelog/3747.bugfix.rst b/changelog/3747.bugfix.rst new file mode 100644 index 000000000..ab579d5a8 --- /dev/null +++ b/changelog/3747.bugfix.rst @@ -0,0 +1 @@ +Fix compatibility problem with plugins and the the warning code issued by fixture functions when they are called directly. diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index a1cd3bd4e..52051ff23 100644 --- a/src/_pytest/compat.py +++ b/src/_pytest/compat.py @@ -249,6 +249,21 @@ def get_real_func(obj): return obj +def get_real_method(obj, holder): + """ + Attempts to obtain the real function object that might be wrapping ``obj``, while at the same time + returning a bound method to ``holder`` if the original object was a bound method. + """ + try: + is_method = hasattr(obj, "__func__") + obj = get_real_func(obj) + except Exception: + return obj + if is_method and hasattr(obj, "__get__") and callable(obj.__get__): + obj = obj.__get__(holder) + return obj + + def getfslineno(obj): # xxx let decorators etc specify a sane ordering obj = get_real_func(obj) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 93557fa04..818c5b81f 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -30,6 +30,7 @@ from _pytest.compat import ( getfuncargnames, safe_getattr, FuncargnamesCompatAttr, + get_real_method, ) from _pytest.deprecated import FIXTURE_FUNCTION_CALL, RemovedInPytest4Warning from _pytest.outcomes import fail, TEST_OUTCOME @@ -931,13 +932,6 @@ def pytest_fixture_setup(fixturedef, request): request._check_scope(argname, request.scope, fixdef.scope) kwargs[argname] = result - # if function has been defined with @pytest.fixture, we want to - # pass the special __being_called_by_pytest parameter so we don't raise a warning - # this is an ugly hack, see #3720 for an opportunity to improve this - defined_using_fixture_decorator = hasattr(fixturedef.func, "_pytestfixturefunction") - if defined_using_fixture_decorator: - kwargs["__being_called_by_pytest"] = True - fixturefunc = resolve_fixture_function(fixturedef, request) my_cache_key = request.param_index try: @@ -973,9 +967,7 @@ def wrap_function_to_warning_if_called_directly(function, fixture_marker): @functools.wraps(function) def result(*args, **kwargs): __tracebackhide__ = True - __being_called_by_pytest = kwargs.pop("__being_called_by_pytest", False) - if not __being_called_by_pytest: - warnings.warn(warning, stacklevel=3) + warnings.warn(warning, stacklevel=3) for x in function(*args, **kwargs): yield x @@ -984,9 +976,7 @@ def wrap_function_to_warning_if_called_directly(function, fixture_marker): @functools.wraps(function) def result(*args, **kwargs): __tracebackhide__ = True - __being_called_by_pytest = kwargs.pop("__being_called_by_pytest", False) - if not __being_called_by_pytest: - warnings.warn(warning, stacklevel=3) + warnings.warn(warning, stacklevel=3) return function(*args, **kwargs) if six.PY2: @@ -1279,9 +1269,9 @@ class FixtureManager(object): # The attribute can be an arbitrary descriptor, so the attribute # access below can raise. safe_getatt() ignores such exceptions. obj = safe_getattr(holderobj, name, None) + marker = getfixturemarker(obj) # fixture functions have a pytest_funcarg__ prefix (pre-2.3 style) # or are "@pytest.fixture" marked - marker = getfixturemarker(obj) if marker is None: if not name.startswith(self._argprefix): continue @@ -1303,6 +1293,15 @@ class FixtureManager(object): name = marker.name assert not name.startswith(self._argprefix), FIXTURE_MSG.format(name) + # during fixture definition we wrap the original fixture function + # to issue a warning if called directly, so here we unwrap it in order to not emit the warning + # when pytest itself calls the fixture function + if six.PY2 and unittest: + # hack on Python 2 because of the unbound methods + obj = get_real_func(obj) + else: + obj = get_real_method(obj, holderobj) + fixture_def = FixtureDef( self, nodeid, diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 2a9b4be91..41907503e 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -267,7 +267,6 @@ def test_pytest_plugins_in_non_top_level_conftest_deprecated_no_false_positives( ) -# @pytest.mark.skipif(six.PY2, reason="We issue the warning in Python 3 only") def test_call_fixture_function_deprecated(): """Check if a warning is raised if a fixture function is called directly (#3661)""" diff --git a/testing/python/fixture.py b/testing/python/fixture.py index 70d79ab71..a3c6b95b0 100644 --- a/testing/python/fixture.py +++ b/testing/python/fixture.py @@ -1470,10 +1470,9 @@ class TestFixtureManagerParseFactories(object): print (faclist) assert len(faclist) == 3 - kwargs = {'__being_called_by_pytest': True} - assert faclist[0].func(item._request, **kwargs) == "conftest" - assert faclist[1].func(item._request, **kwargs) == "module" - assert faclist[2].func(item._request, **kwargs) == "class" + assert faclist[0].func(item._request) == "conftest" + assert faclist[1].func(item._request) == "module" + assert faclist[2].func(item._request) == "class" """ ) reprec = testdir.inline_run("-s")