From 9121138a1b469b97d544c38967a8859d8112ed14 Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Mon, 1 Apr 2019 10:40:18 +1100 Subject: [PATCH 1/4] Emit warning for unknown marks --- changelog/4826.feature.rst | 2 ++ doc/en/mark.rst | 5 ++++- src/_pytest/mark/__init__.py | 3 +-- src/_pytest/mark/structures.py | 38 +++++++++++++++++++++------------- 4 files changed, 31 insertions(+), 17 deletions(-) create mode 100644 changelog/4826.feature.rst diff --git a/changelog/4826.feature.rst b/changelog/4826.feature.rst new file mode 100644 index 00000000000..2afcba1ad8e --- /dev/null +++ b/changelog/4826.feature.rst @@ -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. diff --git a/doc/en/mark.rst b/doc/en/mark.rst index 0ce9cb31ba8..5801dccde7e 100644 --- a/doc/en/mark.rst +++ b/doc/en/mark.rst @@ -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 diff --git a/src/_pytest/mark/__init__.py b/src/_pytest/mark/__init__.py index e7f08e163f6..ef81784f447 100644 --- a/src/_pytest/mark/__init__.py +++ b/src/_pytest/mark/__init__.py @@ -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) diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index 5186e754588..dfb35805296 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -11,6 +11,7 @@ from ..compat import MappingMixin from ..compat import NOTSET from _pytest.outcomes import fail +from _pytest.warning_types import PytestWarning EMPTY_PARAMETERSET_OPTION = "empty_parameter_set_mark" @@ -283,28 +284,37 @@ 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) + self._update_markers(name) + if name not in self._markers: + warnings.warn( + "Unknown mark %r. You can register custom marks to avoid this " + "warning, without risking typos that break your tests. See " + "https://docs.pytest.org/en/latest/mark.html for details." % name, + PytestWarning, + ) + if self._config.option.strict: + fail("{!r} not a registered marker".format(name), pytrace=False) + return MarkDecorator(Mark(name, (), {})) - 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) + def _update_markers(self, name): + # We store a set of registered markers as a performance optimisation, + # but more could be added to `self._config` by other plugins at runtime. + # If we see an unknown marker, we therefore update the set and try again! if name not in self._markers: - fail("{!r} not a registered marker".format(name), pytrace=False) + 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() + self._markers.add(marker) MARK_GEN = MarkGenerator() From cda9ce198ad10d7d82addf55c23e0c8f2e7fbb6d Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Mon, 1 Apr 2019 10:52:43 +1100 Subject: [PATCH 2/4] Register marks from self-tests --- tox.ini | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tox.ini b/tox.ini index 2b87199c874..131bacae6a2 100644 --- a/tox.ini +++ b/tox.ini @@ -168,6 +168,12 @@ filterwarnings = pytester_example_dir = testing/example_scripts markers = issue + nothing + foo + bar + baz + xyz + XYZ [flake8] max-line-length = 120 From 4f6c67658c95ae45f4b5bdc5967aa9faab38640d Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Mon, 1 Apr 2019 12:38:33 +1100 Subject: [PATCH 3/4] Use mark-specific warning type So that we can ignore it in self-tests. --- src/_pytest/mark/structures.py | 10 +++++----- src/_pytest/warning_types.py | 9 +++++++++ tox.ini | 8 ++------ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index dfb35805296..d96c0b126e9 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -11,7 +11,7 @@ from ..compat import MappingMixin from ..compat import NOTSET from _pytest.outcomes import fail -from _pytest.warning_types import PytestWarning +from _pytest.warning_types import UnknownMarkWarning EMPTY_PARAMETERSET_OPTION = "empty_parameter_set_mark" @@ -294,10 +294,10 @@ def __getattr__(self, name): self._update_markers(name) if name not in self._markers: warnings.warn( - "Unknown mark %r. You can register custom marks to avoid this " - "warning, without risking typos that break your tests. See " - "https://docs.pytest.org/en/latest/mark.html for details." % name, - PytestWarning, + "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, ) if self._config.option.strict: fail("{!r} not a registered marker".format(name), pytrace=False) diff --git a/src/_pytest/warning_types.py b/src/_pytest/warning_types.py index 55e1f037ae5..ffc6e69d6fb 100644 --- a/src/_pytest/warning_types.py +++ b/src/_pytest/warning_types.py @@ -9,6 +9,15 @@ class PytestWarning(UserWarning): """ +class UnknownMarkWarning(PytestWarning): + """ + 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`. diff --git a/tox.ini b/tox.ini index 131bacae6a2..d0dd95ea357 100644 --- a/tox.ini +++ b/tox.ini @@ -165,15 +165,11 @@ 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 pytester_example_dir = testing/example_scripts markers = issue - nothing - foo - bar - baz - xyz - XYZ [flake8] max-line-length = 120 From cab4069f42e9bd2debfc8f71b8805a334194a9cf Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Tue, 2 Apr 2019 12:31:42 +1100 Subject: [PATCH 4/4] Clarify mark.__getattr__ --- src/_pytest/mark/structures.py | 39 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index d96c0b126e9..f3602b2d56f 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -291,31 +291,32 @@ def __getattr__(self, name): raise AttributeError("Marker name must NOT start with underscore") if self._config is not None: - self._update_markers(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! + 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() + 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: - 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, - ) if self._config.option.strict: 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, + ) return MarkDecorator(Mark(name, (), {})) - def _update_markers(self, name): - # We store a set of registered markers as a performance optimisation, - # but more could be added to `self._config` by other plugins at runtime. - # If we see an unknown marker, we therefore update the set and try again! - 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() - self._markers.add(marker) - MARK_GEN = MarkGenerator()