Merge pull request #8428 from graingert/warn-when-a-mark-is-applied-to-a-fixture

Author: Zac-HD

changelog/3664.deprecation.rst

@@ -0,0 +1,3 @@
+Applying a mark to a fixture function now issues a warning: marks in fixtures never had any effect, but it is a common user error to apply a mark to a fixture (for example ``usefixtures``) and expect it to work.
+
+This will become an error in the future.

doc/en/deprecations.rst

@@ -380,6 +380,25 @@ conflicts (such as :class:`pytest.File` now taking ``path`` instead of
 ``fspath``, as :ref:`outlined above <node-ctor-fspath-deprecation>`), a
 deprecation warning is now raised.
 
+Applying a mark to a fixture function
+-------------------------------------
+
+.. deprecated:: 7.4
+
+Applying a mark to a fixture function never had any effect, but it is a common user error.
+
+.. code-block:: python
+
+    @pytest.mark.usefixtures("clean_database")
+    @pytest.fixture
+    def user() -> User:
+        ...
+
+Users expected in this case that the ``usefixtures`` mark would have its intended effect of using the ``clean_database`` fixture when ``user`` was invoked, when in fact it has no effect at all.
+
+Now pytest will issue a warning when it encounters this problem, and will raise an error in the future versions.
+
+
 Backward compatibilities in ``Parser.addoption``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

doc/en/how-to/fixtures.rst

@@ -1752,8 +1752,7 @@ into an ini-file:
         def my_fixture_that_sadly_wont_use_my_other_fixture():
             ...
 
-    Currently this will not generate any error or warning, but this is intended
-    to be handled by :issue:`3664`.
+    This generates a deprecation warning, and will become an error in Pytest 8.
 
 .. _`override fixtures`:
 

src/_pytest/deprecated.py

@@ -122,6 +122,11 @@
     "#configuring-hook-specs-impls-using-markers",
 )
 
+MARKED_FIXTURE = PytestRemovedIn8Warning(
+    "Marks applied to fixtures have no effect\n"
+    "See docs: https://docs.pytest.org/en/stable/deprecations.html#applying-a-mark-to-a-fixture-function"
+)
+
 # You want to make some `__init__` or function "private".
 #
 #   def my_private_function(some, args):

src/_pytest/fixtures.py

@@ -53,6 +53,7 @@
 from _pytest.config import Config
 from _pytest.config.argparsing import Parser
 from _pytest.deprecated import check_ispytest
+from _pytest.deprecated import MARKED_FIXTURE
 from _pytest.deprecated import YIELD_FIXTURE
 from _pytest.mark import Mark
 from _pytest.mark import ParameterSet
@@ -1199,6 +1200,9 @@ def __call__(self, function: FixtureFunction) -> FixtureFunction:
                 "fixture is being applied more than once to the same function"
             )
 
+        if hasattr(function, "pytestmark"):
+            warnings.warn(MARKED_FIXTURE, stacklevel=2)
+
         function = wrap_function_to_error_out_if_called_directly(function, self)
 
         name = self.name or function.__name__

src/_pytest/mark/structures.py

@@ -28,6 +28,7 @@
 from ..compat import NotSetType
 from _pytest.config import Config
 from _pytest.deprecated import check_ispytest
+from _pytest.deprecated import MARKED_FIXTURE
 from _pytest.outcomes import fail
 from _pytest.warning_types import PytestUnknownMarkWarning
 
@@ -412,6 +413,12 @@ def store_mark(obj, mark: Mark) -> None:
     This is used to implement the Mark declarations/decorators correctly.
     """
     assert isinstance(mark, Mark), mark
+
+    from ..fixtures import getfixturemarker
+
+    if getfixturemarker(obj) is not None:
+        warnings.warn(MARKED_FIXTURE, stacklevel=2)
+
     # Always reassign name to avoid updating pytestmark in a reference that
     # was only borrowed.
     obj.pytestmark = [*get_unpacked_marks(obj, consider_mro=False), mark]

testing/deprecated_test.py

@@ -281,6 +281,57 @@ def test_importing_instance_is_deprecated(pytester: Pytester) -> None:
         from _pytest.python import Instance  # noqa: F401
 
 
+def test_fixture_disallow_on_marked_functions():
+    """Test that applying @pytest.fixture to a marked function warns (#3364)."""
+    with pytest.warns(
+        pytest.PytestRemovedIn8Warning,
+        match=r"Marks applied to fixtures have no effect",
+    ) as record:
+
+        @pytest.fixture
+        @pytest.mark.parametrize("example", ["hello"])
+        @pytest.mark.usefixtures("tmp_path")
+        def foo():
+            raise NotImplementedError()
+
+    # it's only possible to get one warning here because you're already prevented
+    # from applying @fixture twice
+    # ValueError("fixture is being applied more than once to the same function")
+    assert len(record) == 1
+
+
+def test_fixture_disallow_marks_on_fixtures():
+    """Test that applying a mark to a fixture warns (#3364)."""
+    with pytest.warns(
+        pytest.PytestRemovedIn8Warning,
+        match=r"Marks applied to fixtures have no effect",
+    ) as record:
+
+        @pytest.mark.parametrize("example", ["hello"])
+        @pytest.mark.usefixtures("tmp_path")
+        @pytest.fixture
+        def foo():
+            raise NotImplementedError()
+
+    assert len(record) == 2  # one for each mark decorator
+
+
+def test_fixture_disallowed_between_marks():
+    """Test that applying a mark to a fixture warns (#3364)."""
+    with pytest.warns(
+        pytest.PytestRemovedIn8Warning,
+        match=r"Marks applied to fixtures have no effect",
+    ) as record:
+
+        @pytest.mark.parametrize("example", ["hello"])
+        @pytest.fixture
+        @pytest.mark.usefixtures("tmp_path")
+        def foo():
+            raise NotImplementedError()
+
+    assert len(record) == 2  # one for each mark decorator
+
+
 @pytest.mark.filterwarnings("default")
 def test_nose_deprecated_with_setup(pytester: Pytester) -> None:
     pytest.importorskip("nose")