From 3f71680ac0eb7646707b82e768f1302fcff3a357 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Wed, 10 Mar 2021 13:47:14 +0000 Subject: [PATCH] Warn when a mark is applied to a fixture Fixes #3664 --- changelog/3664.deprecation.rst | 1 + doc/en/how-to/fixtures.rst | 3 +-- src/_pytest/deprecated.py | 2 ++ src/_pytest/fixtures.py | 4 ++++ src/_pytest/mark/structures.py | 4 ++++ testing/python/fixtures.py | 18 ++++++++++++++++++ 6 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 changelog/3664.deprecation.rst diff --git a/changelog/3664.deprecation.rst b/changelog/3664.deprecation.rst new file mode 100644 index 000000000..b12273ef0 --- /dev/null +++ b/changelog/3664.deprecation.rst @@ -0,0 +1 @@ +Deprecate applying a mark to a fixture diff --git a/doc/en/how-to/fixtures.rst b/doc/en/how-to/fixtures.rst index a2eb211e1..420b0d1a5 100644 --- a/doc/en/how-to/fixtures.rst +++ b/doc/en/how-to/fixtures.rst @@ -1667,8 +1667,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 `#3664 `_. + Currently this will generate a deprecation warning. .. _`override fixtures`: diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 596574877..1bdadb343 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -95,6 +95,8 @@ NODE_FSPATH = UnformattedWarning( "see https://docs.pytest.org/en/latest/deprecations.html#node-fspath-in-favor-of-pathlib-and-node-path", ) +MARKED_FIXTURE = PytestDeprecationWarning("Marks cannot be applied to fixtures") + # 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 b0a895a04..bbce454c3 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -54,6 +54,7 @@ from _pytest.config import Config from _pytest.config.argparsing import Parser from _pytest.deprecated import check_ispytest from _pytest.deprecated import FILLFUNCARGS +from _pytest.deprecated import MARKED_FIXTURE from _pytest.deprecated import NODE_FSPATH from _pytest.deprecated import YIELD_FIXTURE from _pytest.mark import Mark @@ -1222,6 +1223,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 d2f21e168..737a3ad05 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -29,6 +29,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 @@ -399,6 +400,9 @@ def store_mark(obj, mark: Mark) -> None: assert isinstance(mark, Mark), mark # Always reassign name to avoid updating pytestmark in a reference that # was only borrowed. + if hasattr(obj, "_pytestfixturefunction"): + warnings.warn(MARKED_FIXTURE, stacklevel=2) + obj.pytestmark = get_unpacked_marks(obj) + [mark] diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 3eb4b80f3..3d52c1c60 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -3612,6 +3612,24 @@ class TestShowFixtures: def foo(): raise NotImplementedError() + def test_fixture_disallow_on_marked_functions(self): + """Test that applying @pytest.fixture to a marked function warns (#3364).""" + with pytest.warns(pytest.PytestDeprecationWarning): + + @pytest.fixture + @pytest.mark.usefixtures("tmp_path") + def foo(): + raise NotImplementedError() + + def test_fixture_disallow_marks_on_fixtures(self): + """Test that applying a mark to a fixture warns (#3364).""" + with pytest.warns(pytest.PytestDeprecationWarning): + + @pytest.mark.usefixtures("tmp_path") + @pytest.fixture + def foo(): + raise NotImplementedError() + class TestContextManagerFixtureFuncs: def test_simple(self, pytester: Pytester) -> None: