Since tearDownClass isn't run if setUpClass fails, there is a need for the class level equivalent of addCleanup. addClassCleanup?

Is this really needed? One can use try/except/raise, and since addClassCleanup() would only be called from setUpClass(), I don't quite see the utility of it.

It would be nice for symmetry. I mean, setUpClass isn't needed either, and we have it ;).

however, we actually have two contexts this would be called from - setUpClass and setUpModule; both share their internals. So we probably need a decoupled cleanups implementation, and two new binding points to it.

I'm not convinced this would be worth the effort required to implement and maintain it.

Can someone find examples from existing test suites where this would clearly be useful? For example, a setUpClass() or setUpModule() function with multiple try/finally clauses.

addCleanup() is helpful because it can be used in test methods. addClassCleanup() and addModuleCleanup() can't be used in test methods, and setUpClass() and setUpModule() are used less than setUp(), therefore the benefit of these methods are less than of addCleanup().

Not having addClassCleanup means that my setUpClass method will have four try/excepts in it, as will my tearDownClass (I have four fixtures I'm setting up for the tests).

So, no, it isn't strictly needed, but it is prettier :). As Robert says, though, it makes for a nice symmetry in the API. Basically, I like to see tearDown deprecated, as I find the setup/addcleanup pattern much easier to write and, more importantly, to read.

Ahh, that makes sense. Sounds good to me!

As further motivation, I actually tried to implement this using try/except. First I wrote a loop in tearDownClass that ran each of the cleanups inside a try/except and printed the exception if there was one. But of course that doesn't run if setUpClass fails. So I started to add a similar try/except loop in setUpClass...and then realized that in order to have the cleanups run correctly, I needed to add each cleanup to a list on the class if and only if the corresponding setup succeeded, and then run only those cleanups in the tearDownClass.

So, I ended up implementing addClassCleanup. However, in order to make it generic (I have two classes where I need it currently, and will probably be adding more), I have a base class with setUpClass that calls a safeSetUpClass inside a try/except, my safeSetUpClass on the actual test class does the setup and calls to addClassCleanUp, and my tearDownClass does the loop over the accumulated cleanups.

So, yeah, it would be really handy to have this as an actual feature :)

contextlib.ExitStack could help you.

That would not make it simpler. In fact it would make the test code more complex. I'd still need the safeSetUpClass dodge (or repeat the try/except/log in every setUpClass method), and I'd have to have a wrapper function that I passed each cleanup to as an argument to wrap it in a try/except/log, since close would just propagate the exceptions.

I think implementing this parallel to addCleanUp is much cleaner, even if the feature isn't (yet :) in the stdlib.

I'm convinced. +1

Here is the patch.

So, thank you for the patch. However - there's no need for a metaclass here, and its actively harmful to comprehending the code.

As I suggested earlier, please decouple the cleanups implementation and then consume it from the two places that it will be needed (testcase and testsuite).

Btw some things to be aware of in addressing this:

• we will need tests that catchable exceptions raised from a setUpModule or setUpClass cause cleanups already registered to run
• if (and I'd need to check this) setUpModule or setUpClass can nest - I think setUpModule may, in package.module contexts) - that non-local cleanups definitively run too

There is a design issue with the order of cleaning up. tearDown() is purposed to release resources acquired in setUp(). Resources acquired in test methods will be released in doCleanup() if use addCleanup(), but since doCleanup() is called after tearDown(), resources will be released not in reversed order of the time of acquiring. This causes issues if resources acquired in a test method depends on the resource acquired in setUp().

Adding doClassCleanups() and doModuleCleanups() with the same semantic will introduce this problem at class and module level.

Serhiy, thats not a design flaw, its a feature.

in a class hierarchy, setup and teardown ordering is undefined: implementors can choose whatever order they want to execute in. e.g.

class A(TestCase):
 def setUp(self):
  super().setUp()
  acquire1()

class B(A):
 def setUp(self):
  acquire2()
  super().setUp()

class C(B):
 def setUp(self):
  super().setUp()
  acquire3()

will acquire 2, then 1, then 3.

tearDown() is similarly ill defined.

Secondly, consider

class Example(TestCase):
 def setUp(self):
  self._1 = acquire()
  self.addCleanUp(acquire())
  self._3 = acquire()

 def tearDown(self):
  self._3.cleanUp()
  self._1.cleanUp()
  super().tearDown()

As such, there is no ordering of cleanup + teardown that is 'right' in all cases.

The question then becomes, which ordering *most facilitates* migration from tearDown to cleanup.

The allowable orders are:

• with a more complex implementation, per-class (no)
• before tearDown starts
• after tearDown finishes

The common case tearDown tends to look like this:

def tearDown(self):
  <local cleanup>
  super().tearDown()

so, by placing doCleanup after tearDown, we make it possible for base classes in a test hierarchy to migrate to cleanups without breaking compatibility with child classes. The cost, as you note is that we make it harder for child classes to migrate until the base class has migrated.

But it is strictly better to permit the base class to migrate, because in projects you cannot be sure of all your subclasses out there in the wild, but you can be sure of all your base classes.

New changeset 0f221d0 by Lisa Roach in branch 'master':
bpo-24412: Adds cleanUps for setUpClass and setUpModule. (GH-9190)
0f221d0

/home/serhiy/py/cpython/Lib/unittest/test/test_runner.py:259: DeprecationWarning: Please use assertEqual instead.
self.assertEquals(e, 'cleanup1')