Add means to mark unittest.TestCases as "do not load".

[bitdancer]

BPO	14534
Nosy	@pitrou, @kristjanvalur, @rbtcollins, @ezio-melotti, @merwok, @bitdancer, @voidspace, @4kir4, @vadmium, @zware, @remilapeyre
Files	issue14534.diff: Patch by Victor Zhong and Lisha Ruan unittest_do_not_run.diff: Patch for marking unittest as do not run issue14534.v2.diff: Combined patch issue14534.v3.diff: combined patch against default tip

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/voidspace'
closed_at = None
created_at = <Date 2012-04-09.13:10:52.390>
labels = ['easy', 'type-feature', 'library']
title = 'Add means to mark unittest.TestCases as "do not load".'
updated_at = <Date 2019-02-24.22:49:24.668>
user = 'https://github.com/bitdancer'

bugs.python.org fields:

activity = <Date 2019-02-24.22:49:24.668>
actor = 'remi.lapeyre'
assignee = 'michael.foord'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2012-04-09.13:10:52.390>
creator = 'r.david.murray'
dependencies = []
files = ['35792', '35797', '36062', '36086']
hgrepos = ['262', '263', '266']
issue_num = 14534
keywords = ['patch', 'easy']
message_count = 41.0
messages = ['157842', '157867', '157868', '157869', '157870', '157894', '157896', '157898', '157910', '158222', '158252', '220643', '220648', '220844', '220845', '220854', '220864', '220911', '220926', '220928', '220930', '220931', '220933', '220938', '221019', '221311', '221686', '221687', '221800', '223841', '223959', '224793', '224798', '224972', '224977', '225000', '225004', '248924', '248925', '248926', '249112']
nosy_count = 13.0
nosy_names = ['pitrou', 'kristjan.jonsson', 'rbcollins', 'stutzbach', 'ezio.melotti', 'eric.araujo', 'r.david.murray', 'michael.foord', 'akira', 'martin.panter', 'zach.ware', 'vzhong', 'remi.lapeyre']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue14534'
versions = ['Python 3.6']

[bitdancer]

A common pattern in testing is to have a base test class that implements test methods, and subclasses that provide various data that drives the tests to be run in different ways. It is convenient for the base class to inherit from TestCase, but this causes the normal test loader to load it as a test to be run, when most times it can't run by itself.

My proposed solution is to make test case loading depend on an attribute of the class rather than the fact that it subclasses TestCase. TestCase would then have the attribute set to True by default. A decorator would be provided that sets the attribute to False, since that would make it visually obvious which TestCases are base classes and not to be loaded.

(Note: once this is available the 'best practices' description of test file construction in the documentation for the stdlib 'test' module should be updated to use it.)

[pitrou]

A decorator would be provided that sets the attribute to False, since
that would make it visually obvious which TestCases are base classes
and not to be loaded.

What's the point? Just derive from TestCase in the derived classes, not the base classes.
-1 on useless complication.

[stutzbach]

Wouldn't the subclass inherit the False value? Then the user would need to remember to override the value again in the subclass, which is error prone.

Antoine: I've used the pattern you describe on a couple of occasions, and it routinely confuses my code reviewers.

[bitdancer]

Antoine: I don't have any problem with that personally, but Michael did, and he's the maintainer :)

But there is a small advantage: it means you don't have to keep repeating the 'unittest.TestCase' boilerplate in each subclass declaration, you only have to decorate the base class once.

Daniel: Oops, you are right. Michael seemed to have some idea on how to implement the decorator. The class attribute was my contribution, and obviously that isn't going to work. I wanted something more transparent than a magic decorator, but it looks like magic will be required.

Which, IMO, is a point in Antoine's favor. Let's see what Michael has to say.

[pitrou]

Antoine: I've used the pattern you describe on a couple of occasions,
and it routinely confuses my code reviewers.

Really? What is confusing about it?
Perhaps we should simply document it.

[voidspace]

So the technique I suggested is that the TestLoader checks classes for the "testbase" (or whatever we call it) *in the class dict*. So inheritance doesn't matter - a class is only excluded from test loading if it has the attribute directly.

[voidspace]

Here are my objections to the standard (but not widely used outside our own tests) mixin pattern for base testcases (copied and pasted from bpo-14408):

Because you then have classes that inherit from object calling methods that clearly don't exist (until you subclass them *and* TestCase). It looks weird and also means the classes can't be tested in isolation.

With a class decorator the code would *look* straightforward, and the hidden attribute is just an implementation detail.

It still looks weird to see code calling methods that obviously don't exist, and with no indication *at the call site* where they come from. Making it clearer with naming would help: "TestThingMixin" or similar.

There are classes like this in the unittest test suite, and I was very confused by them initially until I found where and how they were used. It is obviously *not* a pattern that is widely known for test base classes, as we have this problem of it not being done even in the standard library tests.

In contrast I think code similar to the following would be clear and readable without knowing about multiple inheritance and the mixin trick:

    @test_base_class
    class SomeTestBase(TestCase):
        ...

Besides which, the mixin pattern won't *stop* working if we provide this extra functionality - it would just be an alternative for those (like myself) who think it impedes code readability. :-)

At this point we're off topic for the *specific issue*, and I'm fine with our own standard library tests moving to use mixins to support standard unittest invocation. I would suggest the base test cases include Mixin in their name to make it clear how they should be used.

[pitrou]

So the technique I suggested is that the TestLoader checks classes for
the "testbase" (or whatever we call it) *in the class dict*. So
inheritance doesn't matter - a class is only excluded from test
loading if it has the attribute directly.

Why not document the official recommendation in the docs?
Adding another gimmick isn't very useful. It doesn't add a
functionality. It doesn't even make it significantly easier to write
tests (unless you have more test classes than test methods). And it
means that people have to remember two idioms instead of one.

[bitdancer]

Note that I did just document the mixin idiom in the Lib/test docs. Which core developers probably don't read :)

[merwok]

FWIW I use the mixin approach too and find it simple and clean. I don’t have a problem with a method in the mixin class calling methods from TestCase.

[kristjanvalur]

+1, we already have such decorators for individual test cases. Code should be obvious, particularly testing code and mixins often aren't. Magic such as identifying classes to run by their type should be over rideable. All magic should.