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

@@ -0,0 +1 @@
+Fix compatibility problem with plugins and the the warning code issued by fixture functions when they are called directly.

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)

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)
+            warnings.warn(warning, stacklevel=3)
-            if not __being_called_by_pytest:
-                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)
+            warnings.warn(warning, stacklevel=3)
-            if not __being_called_by_pytest:
-                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,

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)"""
 

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) == "conftest"
-                    assert faclist[0].func(item._request, **kwargs) == "conftest"
+                    assert faclist[1].func(item._request) == "module"
-                    assert faclist[1].func(item._request, **kwargs) == "module"
+                    assert faclist[2].func(item._request) == "class"
-                    assert faclist[2].func(item._request, **kwargs) == "class"
         """
         )
         reprec = testdir.inline_run("-s")