Merge pull request pytest-dev#3705 from nicoddemus/deprecate-call-fixture-func

 Deprecate calling fixture functions directly

Author: nicoddemus

changelog/3661.removal.rst

@@ -0,0 +1,3 @@
+Calling a fixture function directly, as opposed to request them in a test function, now issues a ``RemovedInPytest4Warning``. It will be changed into an error in pytest ``4.0``.
+
+This is a great source of confusion to new users, which will often call the fixture functions and request them from test functions interchangeably, which breaks the fixture resolution model.

src/_pytest/compat.py

@@ -84,6 +84,7 @@ def iscoroutinefunction(func):
 
 
 def getlocation(function, curdir):
+    function = get_real_func(function)
     fn = py.path.local(inspect.getfile(function))
     lineno = function.__code__.co_firstlineno
     if fn.relto(curdir):

src/_pytest/deprecated.py

@@ -22,6 +22,12 @@ class RemovedInPytest4Warning(DeprecationWarning):
     "Please remove the prefix and use the @pytest.fixture decorator instead."
 )
 
+FIXTURE_FUNCTION_CALL = (
+    "Fixture {name} called directly. Fixtures are not meant to be called directly, "
+    "are created automatically when test functions request them as parameters. "
+    "See https://docs.pytest.org/en/latest/fixture.html for more information."
+)
+
 CFG_PYTEST_SECTION = (
     "[pytest] section in {filename} files is deprecated, use [tool:pytest] instead."
 )

src/_pytest/fixtures.py

@@ -6,6 +6,8 @@
 import sys
 import warnings
 from collections import OrderedDict, deque, defaultdict
+
+import six
 from more_itertools import flatten
 
 import attr
@@ -29,6 +31,7 @@
     safe_getattr,
     FuncargnamesCompatAttr,
 )
+from _pytest.deprecated import FIXTURE_FUNCTION_CALL, RemovedInPytest4Warning
 from _pytest.outcomes import fail, TEST_OUTCOME
 
 FIXTURE_MSG = 'fixtures cannot have "pytest_funcarg__" prefix and be decorated with @pytest.fixture:\n{}'
@@ -798,7 +801,7 @@ def call_fixture_func(fixturefunc, request, kwargs):
 
 def _teardown_yield_fixture(fixturefunc, it):
     """Executes the teardown of a fixture function by advancing the iterator after the
-    yield and ensure the iteration ends (if not it means there is more than one yield in the function"""
+    yield and ensure the iteration ends (if not it means there is more than one yield in the function)"""
     try:
         next(it)
     except StopIteration:
@@ -928,6 +931,13 @@ 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:
@@ -947,6 +957,44 @@ def _ensure_immutable_ids(ids):
     return tuple(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__)
+    warning = RemovedInPytest4Warning(msg)
+
+    if is_yield_function:
+
+        @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)
+            for x in function(*args, **kwargs):
+                yield x
+
+    else:
+
+        @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)
+            return function(*args, **kwargs)
+
+    if six.PY2:
+        result.__wrapped__ = function
+
+    return result
+
+
 @attr.s(frozen=True)
 class FixtureFunctionMarker(object):
     scope = attr.ib()
@@ -964,6 +1012,8 @@ def __call__(self, function):
                 "fixture is being applied more than once to the same function"
             )
 
+        function = wrap_function_to_warning_if_called_directly(function, self)
+
         function._pytestfixturefunction = self
         return function
 

testing/deprecated_test.py

@@ -1,4 +1,6 @@
 from __future__ import absolute_import, division, print_function
+
+
 import pytest
 
 
@@ -263,3 +265,15 @@ def test_func():
         str(PYTEST_PLUGINS_FROM_NON_TOP_LEVEL_CONFTEST).splitlines()[0]
         not in res.stderr.str()
     )
+
+
+# @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)"""
+
+    @pytest.fixture
+    def fix():
+        return 1
+
+    with pytest.deprecated_call():
+        assert fix() == 1

testing/example_scripts/tmpdir/tmpdir_fixture.py

@@ -0,0 +1,7 @@
+import pytest
+
+
+@pytest.mark.parametrize("a", [r"qwe/\abc"])
+def test_fixture(tmpdir, a):
+    tmpdir.check(dir=1)
+    assert tmpdir.listdir() == []

testing/python/fixture.py

@@ -1456,6 +1456,7 @@ def test_parsefactories_conftest_and_module_and_class(self, testdir):
         testdir.makepyfile(
             """
             import pytest
+            import six
 
             @pytest.fixture
             def hello(request):
@@ -1468,9 +1469,11 @@ def test_hello(self, item, fm):
                     faclist = fm.getfixturedefs("hello", item.nodeid)
                     print (faclist)
                     assert len(faclist) == 3
-                    assert faclist[0].func(item._request) == "conftest"
-                    assert faclist[1].func(item._request) == "module"
-                    assert faclist[2].func(item._request) == "class"
+
+                    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"
         """
         )
         reprec = testdir.inline_run("-s")

testing/python/metafunc.py

@@ -212,6 +212,9 @@ class A(object):
     @hypothesis.settings(
         deadline=400.0
     )  # very close to std deadline and CI boxes are not reliable in CPU power
+    @pytest.mark.xfail(
+        sys.platform.startswith("win32"), reason="flaky #3707", strict=False
+    )
     def test_idval_hypothesis(self, value):
         from _pytest.python import _idval
 

testing/test_assertion.py

@@ -12,7 +12,6 @@
 PY3 = sys.version_info >= (3, 0)
 
 
-@pytest.fixture
 def mock_config():
     class Config(object):
         verbose = False
@@ -768,15 +767,15 @@ def test_rewritten():
     assert testdir.runpytest().ret == 0
 
 
-def test_reprcompare_notin(mock_config):
-    detail = plugin.pytest_assertrepr_compare(
-        mock_config, "not in", "foo", "aaafoobbb"
-    )[1:]
+def test_reprcompare_notin():
+    config = mock_config()
+    detail = plugin.pytest_assertrepr_compare(config, "not in", "foo", "aaafoobbb")[1:]
     assert detail == ["'foo' is contained here:", "  aaafoobbb", "?    +++"]
 
 
-def test_reprcompare_whitespaces(mock_config):
-    detail = plugin.pytest_assertrepr_compare(mock_config, "==", "\r\n", "\n")
+def test_reprcompare_whitespaces():
+    config = mock_config()
+    detail = plugin.pytest_assertrepr_compare(config, "==", "\r\n", "\n")
     assert detail == [
         r"'\r\n' == '\n'",
         r"Strings contain only whitespace, escaping them using repr()",

testing/test_conftest.py

@@ -10,9 +10,7 @@
 
 @pytest.fixture(scope="module", params=["global", "inpackage"])
 def basedir(request, tmpdir_factory):
-    from _pytest.tmpdir import tmpdir
-
-    tmpdir = tmpdir(request, tmpdir_factory)
+    tmpdir = tmpdir_factory.mktemp("basedir", numbered=True)
     tmpdir.ensure("adir/conftest.py").write("a=1 ; Directory = 3")
     tmpdir.ensure("adir/b/conftest.py").write("b=2 ; a = 1.5")
     if request.param == "inpackage":