From 3f71680ac0eb7646707b82e768f1302fcff3a357 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Wed, 10 Mar 2021 13:47:14 +0000 Subject: [PATCH 01/16] 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: From 12efc584791ee70151bbc1d6c10fa6faa330cd2c Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 19 Mar 2021 09:43:22 +0000 Subject: [PATCH 02/16] document deprecation in deprecations.rst --- doc/en/deprecations.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 6ecb37b38..482c5e18d 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -29,6 +29,14 @@ As pytest tries to move off `py.path.local Date: Fri, 19 Mar 2021 09:44:39 +0000 Subject: [PATCH 03/16] update MARKED_FIXTURE deprecation message --- src/_pytest/deprecated.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 1bdadb343..a0498a3d0 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -95,7 +95,7 @@ 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") +MARKED_FIXTURE = PytestDeprecationWarning("Marks applied to fixtures have no effect") # You want to make some `__init__` or function "private". # From 5227279c150cfc23a4c73c9538ed81a141a641b6 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 19 Mar 2021 09:47:19 +0000 Subject: [PATCH 04/16] Update changelog/3664.deprecation.rst --- changelog/3664.deprecation.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/3664.deprecation.rst b/changelog/3664.deprecation.rst index b12273ef0..6436910a6 100644 --- a/changelog/3664.deprecation.rst +++ b/changelog/3664.deprecation.rst @@ -1 +1 @@ -Deprecate applying a mark to a fixture +Applying a mark to a fixture function is deprecated. Doing so has no effect. From 5c286d19d80f60e9f1f607f517e9df7cda2ff6b1 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 9 Oct 2022 20:11:51 +0100 Subject: [PATCH 05/16] Update doc/en/deprecations.rst --- doc/en/deprecations.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 88978b6fe..8ff0fe30d 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -276,7 +276,7 @@ deprecation warning is now raised. Applying a mark to a fixture function ------------------------------------- -.. deprecated:: 6.3 +.. deprecated:: 7.2 Applying a mark to a fixture function is deprecated. Doing so has no effect, and will raise an error in the next version. From aa9cc7e8b4f57e1c2fc15c5f7f2212c45c3ae267 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 9 Oct 2022 19:11:56 +0000 Subject: [PATCH 06/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_pytest/fixtures.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 04035a642..3cb152eeb 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -53,9 +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 FILLFUNCARGS from _pytest.deprecated import MARKED_FIXTURE -from _pytest.deprecated import NODE_FSPATH from _pytest.deprecated import YIELD_FIXTURE from _pytest.mark import Mark from _pytest.mark import ParameterSet From a1b10b552abeec24a859a4596eef530d1c6f46da Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 9 Oct 2022 20:23:53 +0100 Subject: [PATCH 07/16] use getfixturemarker --- src/_pytest/mark/structures.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index 32137e8fd..31797f563 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -387,11 +387,14 @@ def store_mark(obj, mark: Mark) -> None: This is used to implement the Mark declarations/decorators correctly. """ assert isinstance(mark, Mark), mark - # Always reassign name to avoid updating pytestmark in a reference that - # was only borrowed. - if hasattr(obj, "_pytestfixturefunction"): + + 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), mark] From 24fd2928780ea1f073d26a9ba415d62ff4101138 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 10 Oct 2022 12:28:14 +0100 Subject: [PATCH 08/16] Update changelog/3664.deprecation.rst Co-authored-by: Bruno Oliveira --- changelog/3664.deprecation.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/changelog/3664.deprecation.rst b/changelog/3664.deprecation.rst index 6436910a6..0a00e26c1 100644 --- a/changelog/3664.deprecation.rst +++ b/changelog/3664.deprecation.rst @@ -1 +1,3 @@ -Applying a mark to a fixture function is deprecated. Doing so has no effect. +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. From d86df89a9291c29e19af3a762bd38bd959f1325c Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 10 Oct 2022 12:28:22 +0100 Subject: [PATCH 09/16] Update doc/en/deprecations.rst Co-authored-by: Bruno Oliveira --- doc/en/deprecations.rst | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 8ff0fe30d..25be02d41 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -278,7 +278,18 @@ Applying a mark to a fixture function .. deprecated:: 7.2 -Applying a mark to a fixture function is deprecated. Doing so has no effect, and will raise an error in the next version. +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`` From 7759a9d3c2d6805f1e080512f40a3ada9207e66c Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 10 Oct 2022 12:28:30 +0100 Subject: [PATCH 10/16] Update src/_pytest/deprecated.py Co-authored-by: Bruno Oliveira --- src/_pytest/deprecated.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index d6b48a3c9..fb528e38b 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -107,7 +107,10 @@ HOOK_LEGACY_MARKING = UnformattedWarning( "#configuring-hook-specs-impls-using-markers", ) -MARKED_FIXTURE = PytestDeprecationWarning("Marks applied to fixtures have no effect") +MARKED_FIXTURE = PytestDeprecationWarning( + "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". # From 0de2aa93f0e214341af40f63f498a0498cc07832 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 10 Oct 2022 11:30:13 +0000 Subject: [PATCH 11/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- doc/en/deprecations.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 25be02d41..e61bf5567 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -281,7 +281,6 @@ Applying a mark to a fixture function 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: From f7542f629232633b8c9bfeee5c14de19e1070d7f Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 10 Oct 2022 13:48:26 +0100 Subject: [PATCH 12/16] move tests into deprecated_test, and test for number of warnings issued --- testing/deprecated_test.py | 32 ++++++++++++++++++++++++++++++++ testing/python/fixtures.py | 18 ------------------ 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index d468b4435..01b91ce6d 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -279,3 +279,35 @@ def test_importing_instance_is_deprecated(pytester: Pytester) -> None: match=re.escape("The pytest.Instance collector type is deprecated"), ): 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.PytestDeprecationWarning, + 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() + + 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.PytestDeprecationWarning, + 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 diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 4f53ffddc..3ce5cb34d 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -3620,24 +3620,6 @@ 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: From 7f73722c4adedd3b9da6d7efe5b699a5bbc0ee0d Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 10 Oct 2022 13:52:26 +0100 Subject: [PATCH 13/16] add a test for fixture between mark decorators --- testing/deprecated_test.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 01b91ce6d..2934e69cc 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -294,6 +294,9 @@ def test_fixture_disallow_on_marked_functions(): 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 @@ -310,4 +313,20 @@ def test_fixture_disallow_marks_on_fixtures(): def foo(): raise NotImplementedError() - assert len(record) == 2 + 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.PytestDeprecationWarning, + 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 From 246ceb84bdd7999225b44e10f220ede1d75c21d5 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Mon, 10 Oct 2022 13:58:05 +0100 Subject: [PATCH 14/16] Update doc/en/deprecations.rst --- doc/en/deprecations.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index e5fe61ab6..17055dfb9 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -388,6 +388,7 @@ Applying a mark to a fixture function 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: From 45f1a462d505438548a4dda2072093a22fbd1cb3 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 25 Jun 2023 16:08:53 +0100 Subject: [PATCH 15/16] Apply suggestions from code review Co-authored-by: Zac Hatfield-Dodds --- doc/en/how-to/fixtures.rst | 2 +- src/_pytest/deprecated.py | 2 +- testing/deprecated_test.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/en/how-to/fixtures.rst b/doc/en/how-to/fixtures.rst index e9f7f2d4b..c6047af12 100644 --- a/doc/en/how-to/fixtures.rst +++ b/doc/en/how-to/fixtures.rst @@ -1752,7 +1752,7 @@ into an ini-file: def my_fixture_that_sadly_wont_use_my_other_fixture(): ... - Currently this will generate a deprecation warning. + 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 5874eeb99..3fcf99ba4 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -122,7 +122,7 @@ HOOK_LEGACY_MARKING = UnformattedWarning( "#configuring-hook-specs-impls-using-markers", ) -MARKED_FIXTURE = PytestDeprecationWarning( +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" ) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index cf067bac9..f4197a1f6 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -284,7 +284,7 @@ def test_importing_instance_is_deprecated(pytester: Pytester) -> None: def test_fixture_disallow_on_marked_functions(): """Test that applying @pytest.fixture to a marked function warns (#3364).""" with pytest.warns( - pytest.PytestDeprecationWarning, + pytest.PytestRemovedIn8Warning, match=r"Marks applied to fixtures have no effect", ) as record: @@ -303,7 +303,7 @@ def test_fixture_disallow_on_marked_functions(): def test_fixture_disallow_marks_on_fixtures(): """Test that applying a mark to a fixture warns (#3364).""" with pytest.warns( - pytest.PytestDeprecationWarning, + pytest.PytestRemovedIn8Warning, match=r"Marks applied to fixtures have no effect", ) as record: @@ -319,7 +319,7 @@ def test_fixture_disallow_marks_on_fixtures(): def test_fixture_disallowed_between_marks(): """Test that applying a mark to a fixture warns (#3364).""" with pytest.warns( - pytest.PytestDeprecationWarning, + pytest.PytestRemovedIn8Warning, match=r"Marks applied to fixtures have no effect", ) as record: From 518ca37caeda845376f71648714d53f26b218e82 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 25 Jun 2023 16:09:04 +0100 Subject: [PATCH 16/16] Update doc/en/deprecations.rst --- doc/en/deprecations.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 3e15f892e..e73c1a18e 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -383,7 +383,7 @@ deprecation warning is now raised. Applying a mark to a fixture function ------------------------------------- -.. deprecated:: 7.2 +.. deprecated:: 7.4 Applying a mark to a fixture function never had any effect, but it is a common user error.