diff --git a/changelog/3664.deprecation.rst b/changelog/3664.deprecation.rst new file mode 100644 index 000000000..0a00e26c1 --- /dev/null +++ b/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. diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 810f0062b..e73c1a18e 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -380,6 +380,25 @@ conflicts (such as :class:`pytest.File` now taking ``path`` instead of ``fspath``, as :ref:`outlined above `), 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`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/doc/en/how-to/fixtures.rst b/doc/en/how-to/fixtures.rst index d8517c2c8..c6047af12 100644 --- a/doc/en/how-to/fixtures.rst +++ b/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`: diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index b9c10df7a..3fcf99ba4 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -122,6 +122,11 @@ HOOK_LEGACY_MARKING = UnformattedWarning( "#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): diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 0462504ef..66722da27 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -53,6 +53,7 @@ from _pytest.config import _PluggyPlugin 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 @@ class FixtureFunctionMarker: "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__ diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index 8dbff1dc9..406da5d9d 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -28,6 +28,7 @@ from ..compat import NOTSET 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] diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 50eedb83c..f4197a1f6 100644 --- a/testing/deprecated_test.py +++ b/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")