Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn when a mark is applied to a fixture #8428

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3f71680
Warn when a mark is applied to a fixture
graingert Mar 10, 2021
12efc58
document deprecation in deprecations.rst
graingert Mar 19, 2021
a7e0ae2
update MARKED_FIXTURE deprecation message
graingert Mar 19, 2021
5227279
Update changelog/3664.deprecation.rst
graingert Mar 19, 2021
faac197
Merge branch 'main' into warn-when-a-mark-is-applied-to-a-fixture
graingert Oct 9, 2022
5c286d1
Update doc/en/deprecations.rst
graingert Oct 9, 2022
aa9cc7e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 9, 2022
a1b10b5
use getfixturemarker
graingert Oct 9, 2022
24fd292
Update changelog/3664.deprecation.rst
graingert Oct 10, 2022
d86df89
Update doc/en/deprecations.rst
graingert Oct 10, 2022
7759a9d
Update src/_pytest/deprecated.py
graingert Oct 10, 2022
0de2aa9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 10, 2022
f7542f6
move tests into deprecated_test, and test for number of warnings issued
graingert Oct 10, 2022
7f73722
add a test for fixture between mark decorators
graingert Oct 10, 2022
2998b59
Merge remote-tracking branch 'graingert/warn-when-a-mark-is-applied-t…
graingert Oct 10, 2022
2fd1601
Merge branch 'main' into warn-when-a-mark-is-applied-to-a-fixture
graingert Oct 10, 2022
246ceb8
Update doc/en/deprecations.rst
graingert Oct 10, 2022
4cd95ee
Merge branch 'main' into warn-when-a-mark-is-applied-to-a-fixture
graingert Jun 25, 2023
45f1a46
Apply suggestions from code review
graingert Jun 25, 2023
518ca37
Update doc/en/deprecations.rst
graingert Jun 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/3664.deprecation.rst
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 19 additions & 0 deletions doc/en/deprecations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
graingert marked this conversation as resolved.
Show resolved Hide resolved

@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``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
3 changes: 1 addition & 2 deletions doc/en/how-to/fixtures.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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`:

Expand Down
5 changes: 5 additions & 0 deletions src/_pytest/deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 4 additions & 0 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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__
Expand Down
7 changes: 7 additions & 0 deletions src/_pytest/mark/structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
graingert marked this conversation as resolved.
Show resolved Hide resolved

# Always reassign name to avoid updating pytestmark in a reference that
# was only borrowed.
obj.pytestmark = [*get_unpacked_marks(obj, consider_mro=False), mark]
Expand Down
51 changes: 51 additions & 0 deletions testing/deprecated_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down