Skip to content

__test__ = ... abstract helper #12780

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
asottile-sentry opened this issue Sep 5, 2024 · 4 comments
Closed

__test__ = ... abstract helper #12780

asottile-sentry opened this issue Sep 5, 2024 · 4 comments

Comments

@asottile-sentry
Copy link

What's the problem this feature will solve?

it's fairly error prone to remember to set __test__ = True in subclasses of "abstract" tests which are marked with __test__ = False (to prevent execution)

class AbstractTest(TestCase):
    __test__ = False
    def test_something(self):
        assert 1 == 1

class ConcreteTest(AbstractTest):   # or is it?
    pass

Describe the solution you'd like

instead I've written a small helper that assists with this -- and perhaps it could have a home in pytest? though putting on my pytest maintainer hat I would be hesitant to add this as it seems to encourage inheritance-based testing (which I personally actively discourage!)

the usage is fairly simple:

class MyAbstractTestClass:
    __test__ = Abstract(__module__, __qualname__)

it then uses descriptor magic to make that False in the base class only and True after inherited

Alternative Solutions

in a previous framework I maintained (testify) the lookup of __test__ was only done as cls.__dict__.get('__test__', True) such that subclasses were automatically enabled instead of needing to explicitly override as __test__ = True -- I think this is probably too disruptive of a change to implement here in pytest

or really the other reasonable alternative is do nothing -- I don't think this is that dire of a problem that it needs a feature

Additional context

#8612 -- ironically I also rejected this similar idea, this one uses a mixin instead of descriptor magic. the same criticisim there also applies here (what if you have multiple layers of abstract tests?)

@nicoddemus
Copy link
Member

Yeah I have the same sentiments against it:

  1. This is not something we want to encourage
  2. Is not that general of a problem -- usually found in old codebases that migrated from unittest to pytest (been there, done that).

However perhaps adding a link to an external project which implements this? Or perhaps as you said, doing nothing is also a possibility.

@asottile-sentry
Copy link
Author

yeah I'm ok saying "declined" :)

@bluetech
Copy link
Member

bluetech commented Sep 5, 2024

I'm curious (in relation to #12726) whether the AbstractTest actually is (or can be) an abc.ABC (with at least one abstractmethod) or if it just provides some helper methods/setup.

@asottile-sentry
Copy link
Author

it probably could be an abc but that's usually a lot of extra work to get right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants