Consider MRO when obtaining marks for classes

[untitaker]

When using pytest markers in two baseclasses Foo and Bar, inheriting from both of those baseclasses will lose the markers of one of those classes. This behavior is present in pytest 3-6, and I think it may as well have been intended. I am still filing it as a bug because I am not sure if this edge case was ever explicitly considered.

If it is widely understood that all markers are part of a single attribute, I guess you could say that this is just expected behavior as per MRO. However, I'd argue that it would be more intuitive to attempt to merge marker values into one, possibly deduplicating marker names by MRO.

import itertools
import pytest

class BaseMeta(type):
    @property
    def pytestmark(self):
        return (
            getattr(self, "_pytestmark", []) +
            list(itertools.chain.from_iterable(getattr(x, "_pytestmark", []) for x in self.__mro__))
        )

    @pytestmark.setter
    def pytestmark(self, value):
        self._pytestmark = value

class Base(object):
    # Without this metaclass, foo and bar markers override each other, and test_dings
    # will only have one marker
    # With the metaclass, test_dings will have both
    __metaclass__ = BaseMeta

@pytest.mark.foo
class Foo(Base):
    pass

@pytest.mark.bar
class Bar(Base):
    pass

class TestDings(Foo, Bar):
    def test_dings(self):
        # This test should have both markers, foo and bar.
        # In practice markers are resolved using MRO (so foo wins), unless the
        # metaclass is applied
        pass

I'd expect foo and bar to be markers for test_dings, but this only actually is the case with this metaclass.

Please note that the repro case is Python 2/3 compatible excluding how metaclasses are added to Base (this needs to be taken care of to repro this issue on pytest 6)

[untitaker]

ronny has already refactored this multiple times iirc, but I wonder if it would make sense to store markers as pytestmark_foo and pytestmark_bar on the class instead of in one pytestmark array, that way you can leverage regular inheritance rules

[RonnyPfannschmidt]

Thanks for bringing this to attention, pytest show walk the mro of a class to get all markers

It hadn't done before, but it is a logical conclusion

It's potentially a breaking change.

Cc @nicoddemus @bluetech @asottile

[RonnyPfannschmidt]

As for storing as normal attributes, that has issues when combining same name marks from diamond structures, so it doesn't buy anything that isn't already solved

[untitaker]

so it doesn't buy anything that isn't already solved

So I mean it absolves you from explicitly walking MRO because you just sort of rely on the attributes being propagated by Python to subclasses like pytest currently expects it to.

[nicoddemus]

It's potentially a breaking change.

Strictly yes, so if we were to fix this it should go into 7.0 only.

[radkujawa]

And are there plans to include it to 7.0? The metaclass workaround does not work for me. :/ I use pytest 6.2.4, python 3.7.9

[The-Compiler]

@radkujawa Nobody has been working on this so far, and 7.0 has been delayed for a long time (we haven't had a feature release this year yet!) for various reasons. Even if someone worked on this, it'd likely have to wait for 8.0 (at least in my eyes).

[untitaker]

Re workaround: The way the metaclass is declared needs to be ported from Python 2 to 3 for it to work.

On Fri, Jun 4, 2021, at 11:15, radkujawa wrote:

And are there plans to include it to 7.0? The metaclass workaround does not work for me. :/ I use pytest 6.2.4, python 3.7.9

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pytest-dev/pytest/issues/7792#issuecomment-854515753, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGMPRKY3A7P2EBKQN5KHY3TRCKU5ANCNFSM4RYY25OA.

[radkujawa]

@radkujawa Nobody has been working on this so far, and 7.0 has been delayed for a long time (we haven't had a feature release this year yet!) for various reasons. Even if someone worked on this, it'd likely have to wait for 8.0 (at least in my eyes).

thanks!

[Orenef11]

I can not understand the solution proposed by @untitaker. In my opinion, the purpose of test inheritance is that the new test class will contain all tests from parent classes. Also, I do not think it is necessary to mark the tests in the new class with the markers from parent classes. In my opinion, every test in the new class is separate and should be explicitly marked by the user.

Example:

@pytest.mark.mark1
class Test1:
    @pytest.mark.mark2
    def test_a(self):
        ...

    @pytest.mark.mark3
    def test_b(self):
        ...

@pytest.mark4
class Test2:
    @pytest.mark.mark5
    def test_c(self):
        ...

class Test3(Test1, Test):
    def test_d(self):
        ...

Pytest will run these tests Test3:

• Test3.test_a - The value of variable pytestmark cotians [Mark(name="mark1", ...), Mark(name="mark2", ...)]
• Test3.test_b - The value of variable pytestmark cotians [Mark(name="mark1", ...), Mark(name="mark3", ...)]
• Test3.test_c - The value of variable pytestmark cotians [Mark(name="mark4", ...), Mark(name="mark5", ...)]
• Test3.test_d - The value of variable pytestmark is empty

@RonnyPfannschmidt What do you think?

[RonnyPfannschmidt]

The marks have to transfer with the mro, its a well used feature and its a bug that it doesn't extend to multiple inheritance

[Orenef11]

The marks have to transfer with the mro, its a well used feature and its a bug that it doesn't extend to multiple inheritance

After fixing the problem with mro, the goal is that each test will contain all the marks it inherited from parent classes? According to my example, the marks of test_d should be [Mark(name="mark1", ...), Mark(name="mark4", ...)]?

[RonnyPfannschmidt]

Correct