unittest should not try to run abstract classes

[ricPiel]

BPO	17519
Nosy	@ezio-melotti, @bitdancer, @voidspace, @nathanielmanistaatgoogle
Files	test_abc.py: Example of usage of unittest and ABC, which fails fake_abc.py: Example of workaround by faking an abstract class

^{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 = <Date 2013-03-23.00:33:23.093>
created_at = <Date 2013-03-22.11:24:26.163>
labels = ['type-feature', 'library']
title = 'unittest should not try to run abstract classes'
updated_at = <Date 2021-04-01.21:05:32.272>
user = 'https://bugs.python.org/ricPiel'

bugs.python.org fields:

activity = <Date 2021-04-01.21:05:32.272>
actor = 'sthorne'
assignee = 'michael.foord'
closed = True
closed_date = <Date 2013-03-23.00:33:23.093>
closer = 'michael.foord'
components = ['Library (Lib)']
creation = <Date 2013-03-22.11:24:26.163>
creator = '\xc3\x89ric.Piel'
dependencies = []
files = ['29544', '29545']
hgrepos = []
issue_num = 17519
keywords = []
message_count = 6.0
messages = ['184959', '185017', '185022', '388510', '389640', '390011']
nosy_count = 6.0
nosy_names = ['ezio.melotti', 'r.david.murray', 'michael.foord', '\xc3\x89ric.Piel', 'Nathaniel Manista', 'sthorne']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17519'
versions = ['Python 3.4']

[ricPiel]

Since Python 2.6 there is the notion if abstract class (ABC). It could be useful to use it for test cases, but unittest doesn't support it. Typically, I'd like to test a bunch of classes which all should behave similarly (at least for some cases). So I'd like to have one abstract class containing many test cases, and a separate real tests classes which inherit from this abstract class.

Unfortunately, for now unittest tries to instantiate the abstract class, which fails.

Note that I'm not the only one thinking of this, here is a mention of the same idea on stack overflow:
http://stackoverflow.com/questions/4566910/abstract-test-case-using-python-unittest

Attached are two small examples of test cases. test_abs.py shows what I think is a good usage of ABC, with unittest. It fails to run with this error:
TypeError: Can't instantiate abstract class VirtualTest with abstract methods important_num
fake_abc.py is typically what people end up doing for using abstract classes with unittests (that's what people used to do before ABC exists). It does work, but it's not really beautiful as VirtualTest uses self.assertGreater() and self.important_num which are not explicitly part of the class.

My guess is that the following patch to Lib/unittest/loader.py should be enough (but it's untested):
diff -r a2128cb22372 Lib/unittest/loader.py
--- a/Lib/unittest/loader.py	Thu Mar 21 23:04:45 2013 -0500
+++ b/Lib/unittest/loader.py	Fri Mar 22 12:22:46 2013 +0100
@@ -6,6 +6,7 @@
 import traceback
 import types
 import functools
+import inspect
 
 from fnmatch import fnmatch
 
@@ -74,7 +75,8 @@
         tests = []
         for name in dir(module):
             obj = getattr(module, name)
-            if isinstance(obj, type) and issubclass(obj, case.TestCase):
+            if (isinstance(obj, type) and issubclass(obj, case.TestCase) and
+                not inspect.isabstract(test_class)):
                 tests.append(self.loadTestsFromTestCase(obj))
 
         load_tests = getattr(module, 'load_tests', None)

[bitdancer]

I leave it to Michael to decide if your suggestion is a good addition :)

I personally don't see anything wrong with fake_abc.py...the stdlib test suite uses that idom extensively. Other developers insist it should be called a "mixin" instead of a base class, but I don't see the point of the distinction myself.

If you want an ABC so that certain required methods are supplied by the actual TestCases, you could omit TestCase in the ABC and only supply it in the actual TestCase. I'm guessing you will find that ugly too :)

[voidspace]

As David says, the current workaround is to provide a mixin (base) class that inherits from object. Because this doesn't inherit from TestCase there is no confusion.

We *may* add a class marker that allows you to provide TestCase subclasses that won't be run. I don't think we'll do this with the abstract class machinery as it is additional complication for no benefit.

[nathanielmanistaatgoogle]

In the years since this was considered and declined, I wonder if the facts have changed sufficiently to make it now worth doing?

I often find myself writing TestCases for interfaces, and those define test_* methods that call the interface under test, but of course my TestCase needs to be abstract because I'm only testing an interface and not a concrete implementation of that interface. It's also the case when I'm writing this kind of test that I wish to use a type-checker, and if I can have my abstract TestCase inherit from unittest.TestCase, that will satisfy my type-checker's questions about why I believe my TestCase has all kinds of assert* methods defined that it doesn't otherwise see.

I currently have the impression that if this is cheap enough to do, it may be worth doing just for the ergonomics alone? It mightn't make anything impossible become possible to do, but I forecast that it would make something difficult to do much more straightforward to do.

(I remain a fan of the all-powerful load_tests protocol, but... often it's nice to escape all the responsibility that comes with use of it.)

[nathanielmanistaatgoogle]

michael.foord: I am now persuaded that the feature requested here ought be reconsidered (since my last comment there's been a lot of chatter about it behind closed doors at work, but I can at least cite abseil/abseil-py#166 as a public example of the question coming up).

Would it be appropriate to file a new issue? My issue tracker training brought me up to believe that it's better to reopen an existing closed issue in a circumstance like this, but I respect that that may not be the custom in this issue tracker, and besides I lack the permission in this issue tracker to reopen this issue. 😛

[sthorne]

I have done some experimentation here and thought through this feature request.

The concept we are trying to deliver is: "I would like to share functionality between test classes, by having an abstract parent, with concrete leaves"

The metaclass abc.ABCMeta provides functionality that means two things:

• any class with this metaclass (so the class and all its subclasses, typically) that have @abc.abstractmethod or @abc.abstractproperty decorated methods will be treated as abstract
• any class that is treated as abstract will raise an exception immediately, to make it clear to the programmer (and unit tests) that a programming error has occured.

Following this through, we end up with two ways in which this can go wrong in unit testing if we ask our unit testing framework to not test abstract classes.

This is a complete example, with both failure modes illustrated:

Consider:

class AbstractTestCase(unittest.TestCase, metaclass=abc.ABCMeta):
  ...

class FooTest(AbstractTestCase):
  def foo(self):
    return 1

In this case, AbstractTestCase will not be skipped: this is because without any abstract methods inside it: it's not actually considered 'abstract', and is a concrete class.

In the second case:

class AbstractTestCase(unittest.TestCase, metaclass=abc.ABCMeta):
  @abc.abstractmethod
  def foo(self):
    ...

  @abc.abstractmethod
   def bar(self):
    ...

class FooTest(AbstractTestCase):
  def foo(self):
    return 1

In this case, because AbstractTestCase has 2 abstract methods, it will be skipped. No tests run. But also FooTest will be skipped because it has 1 abstract method, and is therefore also abstract.

If this were a 'normal' program, we would see an exception raised when FooTest is instanciated, but because we're skipping tests in abstract classes, we skip all the tests and exit with success.

My gut feeling on this is that what we really want is a decorator that says: Skip this class, and only this class, explicitly. All subclasses are concrete, only this one is abstract.

[ptc-rdar]

@voidspace Please re-consider this.
Using the fake ABC is not a good solution, since:

1. if you use static code scanners they will yell at you on every call to unknown methods (such as assert...). Causing you to add ignore instructions on the entire file, which is bad
2. You cannot chain inheritance of abstract classes, and override correctly OOB hooks like setUp, tearDown
3. I don't see the downside of this implementation. Specifically, if you have an ABC class that inherits TestCase, it fails the entire suite, nothing runs. Why would we want this to happen. That doesn't make sense