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

Emit warning on unknown marks via decorator #4935

Merged
merged 4 commits into from
Apr 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions changelog/4826.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
A warning is now emitted when unknown marks are used as a decorator.
This is often due to a typo, which can lead to silently broken tests.
5 changes: 4 additions & 1 deletion doc/en/mark.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ which also serve as documentation.
Raising errors on unknown marks
-------------------------------

Marks can be registered in ``pytest.ini`` like this:
Unknown marks applied with the ``@pytest.mark.name_of_the_mark`` decorator
will always emit a warning, in order to avoid silently doing something
surprising due to mis-typed names. You can disable the warning for custom
marks by registering them in ``pytest.ini`` like this:

.. code-block:: ini

Expand Down
3 changes: 1 addition & 2 deletions src/_pytest/mark/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,7 @@ def pytest_collection_modifyitems(items, config):

def pytest_configure(config):
config._old_mark_config = MARK_GEN._config
if config.option.strict:
MARK_GEN._config = config
MARK_GEN._config = config

empty_parameterset = config.getini(EMPTY_PARAMETERSET_OPTION)

Expand Down
43 changes: 27 additions & 16 deletions src/_pytest/mark/structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from ..compat import MappingMixin
from ..compat import NOTSET
from _pytest.outcomes import fail
from _pytest.warning_types import UnknownMarkWarning

EMPTY_PARAMETERSET_OPTION = "empty_parameter_set_mark"

Expand Down Expand Up @@ -283,28 +284,38 @@ def test_function():
on the ``test_function`` object. """

_config = None
_markers = set()

def __getattr__(self, name):
if name[0] == "_":
raise AttributeError("Marker name must NOT start with underscore")

if self._config is not None:
self._check(name)
return MarkDecorator(Mark(name, (), {}))
# We store a set of markers as a performance optimisation - if a mark
# name is in the set we definitely know it, but a mark may be known and
# not in the set. We therefore start by updating the set!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When can it happen that it is known, but not in the set? (except for the first read of the config of course)

(I've notied that the config is re-processed again for every unknown mark, whereas previously it was "read" just once)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this covers the case where marks can be registered later, for example by pytest_configure hooks in conftests.py not at the root.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, pytest_configure can run later than I had expected.

The re-processing isn't free, but the number of unknown marks should be relatively small and it's necessary for correctness.

if name not in self._markers:
for line in self._config.getini("markers"):
# example lines: "skipif(condition): skip the given test if..."
# or "hypothesis: tests which use Hypothesis", so to get the
# marker name we we split on both `:` and `(`.
marker = line.split(":")[0].split("(")[0].strip()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used maxsplit=1 before (which makes sense to keep).

self._markers.add(marker)

# If the name is not in the set of known marks after updating,
# then it really is time to issue a warning or an error.
if name not in self._markers:
if self._config.option.strict:
nicoddemus marked this conversation as resolved.
Show resolved Hide resolved
fail("{!r} not a registered marker".format(name), pytrace=False)
else:
warnings.warn(
"Unknown pytest.mark.%s - is this a typo? You can register "
"custom marks to avoid this warning - for details, see "
"https://docs.pytest.org/en/latest/mark.html" % name,
UnknownMarkWarning,
)

def _check(self, name):
try:
if name in self._markers:
return
except AttributeError:
pass
self._markers = values = set()
for line in self._config.getini("markers"):
marker = line.split(":", 1)[0]
marker = marker.rstrip()
x = marker.split("(", 1)[0]
values.add(x)
if name not in self._markers:
fail("{!r} not a registered marker".format(name), pytrace=False)
return MarkDecorator(Mark(name, (), {}))


MARK_GEN = MarkGenerator()
Expand Down
9 changes: 9 additions & 0 deletions src/_pytest/warning_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ class PytestWarning(UserWarning):
"""


class UnknownMarkWarning(PytestWarning):
nicoddemus marked this conversation as resolved.
Show resolved Hide resolved
"""
Bases: :class:`PytestWarning`.

Warning emitted on use of unknown markers.
See https://docs.pytest.org/en/latest/mark.html for details.
"""


class PytestDeprecationWarning(PytestWarning, DeprecationWarning):
"""
Bases: :class:`pytest.PytestWarning`, :class:`DeprecationWarning`.
Expand Down
2 changes: 2 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ filterwarnings =
ignore::pytest.PytestExperimentalApiWarning
# Do not cause SyntaxError for invalid escape sequences in py37.
default:invalid escape sequence:DeprecationWarning
# ignore use of unregistered marks, because we use many to test the implementation
ignore::_pytest.warning_types.UnknownMarkWarning
nicoddemus marked this conversation as resolved.
Show resolved Hide resolved
pytester_example_dir = testing/example_scripts
markers =
issue
Expand Down