Skip to content

Odd/incorrect behaviour when marking a test superclass #842

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

Closed
xmo-odoo opened this issue Jul 14, 2015 · 5 comments
Closed

Odd/incorrect behaviour when marking a test superclass #842

xmo-odoo opened this issue Jul 14, 2015 · 5 comments
Assignees
Labels
type: bug problem that needs to be addressed

Comments

@xmo-odoo
Copy link

Currently looking into pytest to port over a unittest-based test system with quite a bunch of customisations, we have a number of base "test superclass" for opt-in capabilities and setup/teardown code reuse.

While in the future we'll probably phase them out for regular fixtures & markers, right now I'm trying to "softly" add pytest and these superclasses seemed like a nice seam for a few default markers, fixtures and the like, and then things started breaking down completely: class-level markers (and fixtures since I understand fixtures are enabled through markers? I may be mistaken) are shared between classes rather than only copied/shared from superclass to subclass.

More precisely, if a class has a marker, any marker apparently set on a subclass will actually be set on the superclass (and on all "sibling" subclasses"):

@pytest.mark.a
class Base(unittest.TestCase):
    pass

@pytest.mark.b
class TestBar(Base):
    def test_foo(self):
        pass

class TestBaz(Base):
    def test_qux(self):
        self.fail("should not execute")

Selecting using -m b, one would expect only test_foo would be collected…

> py.test -m b test_foo.py --collect-only
= test session starts =
platform darwin -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.2
rootdir: /Users/masklinn/projects/scratchpad/test, inifile: 
collected 2 items 
<Module 'test_foo.py'>
  <UnitTestCase 'TestBar'>
    <TestCaseFunction 'test_foo'>
  <UnitTestCase 'TestBaz'>
    <TestCaseFunction 'test_qux'>

=  in 0.02 seconds =

Digging in the code, the problem is in MarkDecorator:

                if hasattr(func, '__bases__'):
                    if hasattr(func, 'pytestmark'):
                        l = func.pytestmark
                        if not isinstance(l, list):
                            func.pytestmark = [l, self]
                        else:
                            l.append(self)
                    else:
                        func.pytestmark = [self]

so if func is a class and it has a pytestmark attribute which is already a list then append the new MarkDecorator to the list. This is problematic if the existing pytestmark comes from a superclass rather than being set on the current class.

I see a number of other possible problems with the code in case pytestmark is set explicitly (rather than implicitly via mark decorators)

  • if pytestmark is an iterable but not a list (tuples and sets are seem easy/possible) things may completely blow up
  • pytestmarks from multiple bases aren't merged, one is picked depending on the order of bases
  • if a pytestmark is defined explicitly on the subclass, superclass pytestmarks are ignored

These seem like somewhat less problematic issue though as they're explicit and follow fairly normal Python inheritance concerns. The oddity with MarkDecorator is it's completely implicit, and seems desirable in no situation whatsoever.

Also why look for __bases__ instead of seeing if func is an instance of type or types.ClassType?

@RonnyPfannschmidt
Copy link
Member

the real problem is a bug in marker transfer from test classes to functions, i recall we have a open bug about that topic

@xmo-odoo
Copy link
Author

There might be an other issue in function transfer, but as far as I can see the application of markers themselves is already problematic (and I tried to outline how), I don't see how changes happening after that would fix it, the function transfer is going to transfer an incorrect set of markers either way.

@RonnyPfannschmidt
Copy link
Member

true, i missed that detail due to thinking of the old issue

thanks for bringing it up in a direct form again

@nicoddemus
Copy link
Member

Currently looking into pytest to port over a unittest-based test system with quite a bunch of customisations, we have a number of base "test superclass" for opt-in capabilities and setup/teardown code reuse.

Been there, done that. 😄

class-level markers (and fixtures since I understand fixtures are enabled through markers? I may be mistaken)

Actually fixtures use another mechanism than markers.

Thanks for the report, I'll take a look into this.

@nicoddemus nicoddemus added the type: bug problem that needs to be addressed label Jul 15, 2015
@nicoddemus nicoddemus self-assigned this Jul 15, 2015
nicoddemus added a commit to nicoddemus/pytest that referenced this issue Jul 16, 2015
@nicoddemus
Copy link
Member

@RonnyPfannschmidt

the real problem is a bug in marker transfer from test classes to functions, i recall we have a open bug about that topic

#725?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

3 participants