Merge pull request #3459 from RonnyPfannschmidt/mark-iter-name-filter

introduce name filtering for marker iteration again

12 files changed, 134 insertions, 44 deletions

diff --git a/_pytest/deprecated.py b/_pytest/deprecated.py
index 2be6b7300..c85588d79 100644
--- a/_pytest/deprecated.py
+++ b/_pytest/deprecated.py
@@ -32,8 +32,9 @@ RESULT_LOG = (
 )
 
 MARK_INFO_ATTRIBUTE = RemovedInPytest4Warning(
-    "MarkInfo objects are deprecated as they contain the merged marks.\n"
-    "Please use node.iter_markers to iterate over markers correctly"
+    "MarkInfo objects are deprecated as they contain merged marks which are hard to deal with correctly.\n"
+    "Please use node.get_closest_marker(name) or node.iter_markers(name).\n"
+    "Docs: https://docs.pytest.org/en/latest/mark.html#updating-code"
 )
 
 MARK_PARAMETERSET_UNPACKING = RemovedInPytest4Warning(
diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py
index ab524cc71..7b109ec11 100644
--- a/_pytest/fixtures.py
+++ b/_pytest/fixtures.py
@@ -988,7 +988,7 @@ class FixtureManager(object):
             argnames = getfuncargnames(func, cls=cls)
         else:
             argnames = ()
-        usefixtures = flatten(mark.args for mark in node.iter_markers() if mark.name == "usefixtures")
+        usefixtures = flatten(mark.args for mark in node.iter_markers(name="usefixtures"))
         initialnames = argnames
         initialnames = tuple(usefixtures) + initialnames
         fm = node.session._fixturemanager
diff --git a/_pytest/mark/evaluate.py b/_pytest/mark/evaluate.py
index c89b4933a..0afbc56e7 100644
--- a/_pytest/mark/evaluate.py
+++ b/_pytest/mark/evaluate.py
@@ -35,7 +35,7 @@ class MarkEvaluator(object):
         return not hasattr(self, 'exc')
 
     def _get_marks(self):
-        return [x for x in self.item.iter_markers() if x.name == self._mark_name]
+        return list(self.item.iter_markers(name=self._mark_name))
 
     def invalidraise(self, exc):
         raises = self.get('raises')
diff --git a/_pytest/nodes.py b/_pytest/nodes.py
index 799ee078a..43e81da9b 100644
--- a/_pytest/nodes.py
+++ b/_pytest/nodes.py
@@ -183,30 +183,46 @@ class Node(object):
         self.keywords[marker.name] = marker
         self.own_markers.append(marker)
 
-    def iter_markers(self):
+    def iter_markers(self, name=None):
         """
+        :param name: if given, filter the results by the name attribute
+
         iterate over all markers of the node
         """
-        return (x[1] for x in self.iter_markers_with_node())
+        return (x[1] for x in self.iter_markers_with_node(name=name))
 
-    def iter_markers_with_node(self):
+    def iter_markers_with_node(self, name=None):
         """
+        :param name: if given, filter the results by the name attribute
+
         iterate over all markers of the node
         returns sequence of tuples (node, mark)
         """
         for node in reversed(self.listchain()):
             for mark in node.own_markers:
-                yield node, mark
+                if name is None or getattr(mark, 'name', None) == name:
+                    yield node, mark
+
+    def get_closest_marker(self, name, default=None):
+        """return the first marker matching the name, from closest (for example function) to farther level (for example
+        module level).
+
+        :param default: fallback return value of no marker was found
+        :param name: name to filter by
+        """
+        return next(self.iter_markers(name=name), default)
 
     def get_marker(self, name):
         """ get a marker object from this node or None if
         the node doesn't have a marker with that name.
 
-        ..warning::
-
-          deprecated
+        .. deprecated:: 3.6
+            This function has been deprecated in favor of
+            :meth:`Node.get_closest_marker <_pytest.nodes.Node.get_closest_marker>` and
+            :meth:`Node.iter_markers <_pytest.nodes.Node.iter_markers>`, see :ref:`update marker code`
+            for more details.
         """
-        markers = [x for x in self.iter_markers() if x.name == name]
+        markers = list(self.iter_markers(name=name))
         if markers:
             return MarkInfo(markers)
 
diff --git a/_pytest/python.py b/_pytest/python.py
index 821f844eb..8bcb051a2 100644
--- a/_pytest/python.py
+++ b/_pytest/python.py
@@ -118,9 +118,8 @@ def pytest_generate_tests(metafunc):
         if hasattr(metafunc.function, attr):
             msg = "{0} has '{1}', spelling should be 'parametrize'"
             raise MarkerError(msg.format(metafunc.function.__name__, attr))
-    for marker in metafunc.definition.iter_markers():
-        if marker.name == 'parametrize':
-            metafunc.parametrize(*marker.args, **marker.kwargs)
+    for marker in metafunc.definition.iter_markers(name='parametrize'):
+        metafunc.parametrize(*marker.args, **marker.kwargs)
 
 
 def pytest_configure(config):
diff --git a/_pytest/skipping.py b/_pytest/skipping.py
index f62edcf9a..36eb4a337 100644
--- a/_pytest/skipping.py
+++ b/_pytest/skipping.py
@@ -64,9 +64,7 @@ def pytest_runtest_setup(item):
         item._skipped_by_mark = True
         skip(eval_skipif.getexplanation())
 
-    for skip_info in item.iter_markers():
-        if skip_info.name != 'skip':
-            continue
+    for skip_info in item.iter_markers(name='skip'):
         item._skipped_by_mark = True
         if 'reason' in skip_info.kwargs:
             skip(skip_info.kwargs['reason'])
diff --git a/_pytest/warnings.py b/_pytest/warnings.py
index d8b9fc460..2179328dc 100644
--- a/_pytest/warnings.py
+++ b/_pytest/warnings.py
@@ -60,10 +60,9 @@ def catch_warnings_for_item(item):
         for arg in inifilters:
             _setoption(warnings, arg)
 
-        for mark in item.iter_markers():
-            if mark.name == 'filterwarnings':
-                for arg in mark.args:
-                    warnings._setoption(arg)
+        for mark in item.iter_markers(name='filterwarnings'):
+            for arg in mark.args:
+                warnings._setoption(arg)
 
         yield
 
diff --git a/changelog/3317.feature b/changelog/3317.feature
index b7f947af8..84bf679f7 100644
--- a/changelog/3317.feature
+++ b/changelog/3317.feature
@@ -1,3 +1,4 @@
-Revamp the internals of the ``pytest.mark`` implementation with correct per node handling and introduce a new ``Node.iter_markers``
-API for mark iteration over nodes which fixes a number of long standing bugs caused by the old approach. More details can be
-found in `the marks documentation <https://docs.pytest.org/en/latest/mark.html#marker-revamp-and-iteration>`_.
+Revamp the internals of the ``pytest.mark`` implementation with correct per node handling which fixes a number of
+long standing bugs caused by the old design. This introduces new ``Node.iter_markers(name)`` and ``Node.get_closest_mark(name)`` APIs.
+Users are **strongly encouraged** to read the `reasons for the revamp in the docs <https://docs.pytest.org/en/latest/mark.html#marker-revamp-and-iteration>`_,
+or jump over to details about `updating existing code to use the new APIs <https://docs.pytest.org/en/latest/mark.html#updating-code>`_.
diff --git a/doc/en/example/markers.rst b/doc/en/example/markers.rst
index b162c938c..5b049d463 100644
--- a/doc/en/example/markers.rst
+++ b/doc/en/example/markers.rst
@@ -330,7 +330,7 @@ specifies via named environments::
             "env(name): mark test to run only on named environment")
 
     def pytest_runtest_setup(item):
-        envnames = [mark.args[0] for mark in item.iter_markers() if mark.name == "env"]
+        envnames = [mark.args[0] for mark in item.iter_markers(name='env')]
         if envnames:
             if item.config.getoption("-E") not in envnames:
                 pytest.skip("test requires env in %r" % envnames)
@@ -402,10 +402,9 @@ Below is the config file that will be used in the next examples::
     import sys
 
     def pytest_runtest_setup(item):
-        for marker in item.iter_markers():
-            if marker.name == 'my_marker':
-                print(marker)
-                sys.stdout.flush()
+        for marker in item.iter_markers(name='my_marker'):
+            print(marker)
+            sys.stdout.flush()
 
 A custom marker can have its argument set, i.e. ``args`` and ``kwargs`` properties, defined by either invoking it as a callable or using ``pytest.mark.MARKER_NAME.with_args``. These two methods achieve the same effect most of the time.
 
@@ -458,10 +457,9 @@ test function.  From a conftest file we can read it like this::
     import sys
 
     def pytest_runtest_setup(item):
-        for mark in item.iter_markers():
-            if mark.name == 'glob':
-                print ("glob args=%s kwargs=%s" %(mark.args, mark.kwargs))
-                sys.stdout.flush()
+        for mark in item.iter_markers(name='glob'):
+            print ("glob args=%s kwargs=%s" %(mark.args, mark.kwargs))
+            sys.stdout.flush()
 
 Let's run this without capturing output and see what we get::
 
diff --git a/doc/en/mark.rst b/doc/en/mark.rst
index bd416479d..f07917653 100644
--- a/doc/en/mark.rst
+++ b/doc/en/mark.rst
@@ -28,8 +28,8 @@ which also serve as documentation.
 
 .. currentmodule:: _pytest.mark.structures
 .. autoclass:: Mark
-	:members:
-	:noindex:
+    :members:
+    :noindex:
 
 
 .. `marker-iteration`
@@ -51,8 +51,60 @@ in fact, markers where only accessible in functions, even if they where declared
 
 A new API to access markers has been introduced in pytest 3.6 in order to solve the problems with the initial design, providing :func:`_pytest.nodes.Node.iter_markers` method to iterate over markers in a consistent manner and reworking the internals, which solved great deal of problems with the initial design.
 
-Here is a non-exhaustive list of issues fixed by the new implementation:
 
+.. _update marker code:
+
+Updating code
+~~~~~~~~~~~~~
+
+The old ``Node.get_marker(name)`` function is considered deprecated because it returns an internal ``MarkerInfo`` object
+which contains the merged name, ``*args`` and ``**kwargs**`` of all the markers which apply to that node.
+
+In general there are two scenarios on how markers should be handled:
+
+1. Marks overwrite each other. Order matters but you only want to think of your mark as a single item. E.g.
+``log_level('info')`` at a module level can be overwritten by ``log_level('debug')`` for a specific test.
+
+    In this case replace use ``Node.get_closest_marker(name)``:
+
+    .. code-block:: python
+
+        # replace this:
+        marker = item.get_marker('log_level')
+        if marker:
+            level = marker.args[0]
+
+        # by this:
+        marker = item.get_closest_marker('log_level')
+        if marker:
+            level = marker.args[0]
+
+2. Marks compose additive. E.g. ``skipif(condition)`` marks means you just want to evaluate all of them,
+order doesn't even matter. You probably want to think of your marks as a set here.
+
+   In this case iterate over each mark and handle their ``*args`` and ``**kwargs`` individually.
+
+   .. code-block:: python
+
+        # replace this
+        skipif = item.get_marker('skipif')
+        if skipif:
+            for condition in skipif.args:
+                # eval condition
+
+        # by this:
+        for skipif in item.iter_markers('skipif'):
+            condition = skipif.args[0]
+            # eval condition
+
+
+If you are unsure or have any questions, please consider opening
+`an issue <https://github.com/pytest-dev/pytest/issues>`_.
+
+Related issues
+~~~~~~~~~~~~~~
+
+Here is a non-exhaustive list of issues fixed by the new implementation:
 
 * Marks don't pick up nested classes (`#199 <https://github.com/pytest-dev/pytest/issues/199>`_).
 
diff --git a/doc/en/usage.rst b/doc/en/usage.rst
index b9b8059f7..667eaf8ee 100644
--- a/doc/en/usage.rst
+++ b/doc/en/usage.rst
@@ -274,10 +274,9 @@ Alternatively, you can integrate this functionality with custom markers:
 
     def pytest_collection_modifyitems(session, config, items):
         for item in items:
-            for marker in item.iter_markers():
-                if marker.name == 'test_id':
-                    test_id = marker.args[0]
-                    item.user_properties.append(('test_id', test_id))
+            for marker in item.iter_markers(name='test_id'):
+                test_id = marker.args[0]
+                item.user_properties.append(('test_id', test_id))
 
 And in your tests:
 
diff --git a/testing/test_mark.py b/testing/test_mark.py
index 31d3af3e5..764678ab4 100644
--- a/testing/test_mark.py
+++ b/testing/test_mark.py
@@ -553,7 +553,6 @@ class TestFunctional(object):
         self.assert_markers(items, test_foo=('a', 'b'), test_bar=('a',))
 
     @pytest.mark.issue568
-    @pytest.mark.xfail(reason="markers smear on methods of base classes")
     def test_mark_should_not_pass_to_siebling_class(self, testdir):
         p = testdir.makepyfile("""
             import pytest
@@ -573,8 +572,16 @@ class TestFunctional(object):
         """)
         items, rec = testdir.inline_genitems(p)
         base_item, sub_item, sub_item_other = items
-        assert not hasattr(base_item.obj, 'b')
-        assert not hasattr(sub_item_other.obj, 'b')
+        print(items, [x.nodeid for x in items])
+        # legacy api smears
+        assert hasattr(base_item.obj, 'b')
+        assert hasattr(sub_item_other.obj, 'b')
+        assert hasattr(sub_item.obj, 'b')
+
+        # new api seregates
+        assert not list(base_item.iter_markers(name='b'))
+        assert not list(sub_item_other.iter_markers(name='b'))
+        assert list(sub_item.iter_markers(name='b'))
 
     def test_mark_decorator_baseclasses_merged(self, testdir):
         p = testdir.makepyfile("""
@@ -598,6 +605,26 @@ class TestFunctional(object):
         self.assert_markers(items, test_foo=('a', 'b', 'c'),
                             test_bar=('a', 'b', 'd'))
 
+    def test_mark_closest(self, testdir):
+        p = testdir.makepyfile("""
+            import pytest
+
+            @pytest.mark.c(location="class")
+            class Test:
+                @pytest.mark.c(location="function")
+                def test_has_own():
+                    pass
+
+                def test_has_inherited():
+                    pass
+
+        """)
+        items, rec = testdir.inline_genitems(p)
+        has_own, has_inherited = items
+        assert has_own.get_closest_marker('c').kwargs == {'location': 'function'}
+        assert has_inherited.get_closest_marker('c').kwargs == {'location': 'class'}
+        assert has_own.get_closest_marker('missing') is None
+
     def test_mark_with_wrong_marker(self, testdir):
         reprec = testdir.inline_runsource("""
                 import pytest