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

Mixin for abstract test classes #8612

Open
Ginkss opened this issue Apr 30, 2021 · 2 comments
Open

Mixin for abstract test classes #8612

Ginkss opened this issue Apr 30, 2021 · 2 comments
Labels
type: docs documentation improvement, missing or needing clarification type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@Ginkss
Copy link

Ginkss commented Apr 30, 2021

What's the problem this feature will solve?

Abstract test classes that use __test__=False to avoid collection require that the subclasses set __test__=True to be collected. If a user forgets to reset the attribute, the test they thought was running will be silently ignored. Difficult to catch in large projects.

Describe the solution you'd like

Have subclasses collected by pytest regardless of superclass attribute. I use a mixin class to achieve this:

class NotATest:
    def __init_subclass__(cls):
        cls.__test__ = NotATest not in cls.__bases__

class AbstractTest(NotATest):
    pass

class RealTest(AbstractTest):
    pass

Mixin clearly indicates what's not a test. No need to unset on subclasses. Safe to mixin on subclasses if they are also abstract. What I'm really asking is, would you like to add this mixin class to pytest.__init__?

Alternative Solutions

  • Set abstract test class name so pytest will ignore it. Pycharm will also not reckonise it as a pytest class which means the IDE features like type hinting, navigating to the fixture function definition and find usage, don't work.
  • Change pytest.Class.collect to access class __dict__ instead of using getattr. This ensures that the __test__ attribute is only considered if it is explicitly set for that class definition. However, I'm assuming that another use case for __test__=False is when an existing test set has classes who's name cause their collection even though they are completely unrelated and there subclasses should also never match.

Additional context

No

@Zac-HD Zac-HD added type: docs documentation improvement, missing or needing clarification type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature labels Apr 30, 2021
@Zac-HD
Copy link
Member

Zac-HD commented Apr 30, 2021

What I'm really asking is, would you like to add this mixin class to pytest.__init__?

IMO this would be great as a 'recipe' in the documentation for the __test__ attribute, but doesn't quite meet the bar of generality-and-difficulty to justify adding it to our public API. Would you like to open a PR to add it to the docs?

@asottile
Copy link
Member

asottile commented May 4, 2021

mixins with side-effects are usually a smell, and I think this is not an exception here -- what if you have two levels of "abstract" you'd have to mixin the class twice (?) -- it's also just as much if not the same amount of typing as just putting __test__ = False -- admittedly, pytest is designed around avoiding test base classes and instead using fixtures and function-tests -- I usually consider inheritance-based testing a smell because it makes tests significantly harder to reason about and maintain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: docs documentation improvement, missing or needing clarification type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

No branches or pull requests

3 participants