This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: cleanUp stack for unittest
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: michael.foord Nosy List: ezio.melotti, gregory.p.smith, jml, kumar303, michael.foord, ngie, pitrou, rbcollins, vdupras
Priority: normal Keywords: patch

Created on 2009-04-03 19:56 by michael.foord, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
unittest-cleanup.diff michael.foord, 2009-04-04 16:10
unittest-no-exit.patch michael.foord, 2009-04-25 23:17
unittest-cleanup.patch michael.foord, 2009-04-26 21:17
Messages (45)
msg85321 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-03 19:56
Proposal to add a cleanUp stack to unittest.TestCase. This is a list of
callables to be called (LIFO) to cleanup resources. If there are items
on the stack it should be called even if setUp fails. Otherwise it
should be called after tearDown.

Similar functionality is in the testing frameworks used by bzr, Twisted,
and Zope.

I will create a patch similar to the bzr implementation.

The public API is via a new method:

    def addCleanUpItem(self, callable, *args, **kwargs):

Usage is:

    self.db = self.createDB()
    self.addCleanUpItem(self.db.close)
msg85323 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-04-03 20:09
I'm used to doing this using a finally clause in the setUp() method (if 
I do it at all, I'm not used to setUp failing).

I guess this would be a convenience to avoid the need for this pattern?  
I'm not sure we really need a list of cleanup callbacks.  Got pointers 
to good examples of this in bzr/twisted/zope?


def setUp(self):
  try:
    do_a_bunch_of_stuff_that_could_fail()
  finally:
    conditionally_undo_setup()
  do_more()

def conditionally_undo_setup(self):
  if self.foo: 
    self.foo.close()
  if self.bar:
    shutil.rmtree(self.bar)

def tearDown(self):
  conditionally_undo_setup()
  undo_more()


fwiw, in your self.db.close example, normally i'd also want to remove 
and unlink the database as well as just closing it for a unittest.  
maybe not the best example.
msg85324 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-03 20:16
This is a nice (simple to use and understand) pattern for resource
allocation / deallocation.

Supporting the cleaning up of resources when setUp fails (without
duplicating clean up code) is just one use case. (I agree setUp failure
is unusual.)

It provides a clean way to clean up just the resources you have
allocated in the event of test failure, without having to keep track
yourself of how far the test has got. Manually tracking resource usage
is easy to get wrong.

bzr:

http://bazaar.launchpad.net/~jml/testtools/trunk/annotate/head%3A/testtools/testcase.py


And from zope.testing:

The documentation:
http://svn.zope.org/zope.testing/trunk/src/zope/testing/setupstack.txt?rev=92340&view=auto


The module:
http://svn.zope.org/zope.testing/trunk/src/zope/testing/setupstack.py?rev=92340&view=auto
msg85325 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-03 20:18
From your example this would completely remove the need for the
conditional checking in the cleanup code. Only resources actually
allocated would be in the stack.
msg85344 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-03 23:50
And actually your conditionally_undo_setup() call in setUp is incorrect.
It should in an except block that re-raises rather than a finally block
(which will do the cleanup even in non-exceptional circumstances). Easy
code to get wrong...
msg85404 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-04 16:10
Patch attached.

No docs, if it is agreed I can apply I'll write docs.

After a long discussion we arrived at some semblance of consensus on the
Testing In Python mailing list that this was a good thing (tm).

Only one -1 (thought that cleanUp should be a method) and Holger Krekel
was only +0 because he thinks changes in unittest should be conservative
(but he isn't actually using it).

Many others were +1 - Fred Drake, Titus Brown, Laura Creighton, Robert
Collins, Jonathan Lange, Kumar McMillan and others.

It provides a clean and simple pattern for the cleaning up of resources
- avoiding the need for nested try finallys and the manual tracking of
resources. It can also avoid the need for duplication of cleanup code.
msg85407 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-04 16:42
I'm not sure why it is called after tearDown. It would be better to call
it before tearDown, so that test cases can also use it to initialize
additional resources and finalize them in a LIFO way wrt the main tearDown.
msg85413 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-04 17:11
I'm agnostic on before / after tearDown, so happy to make that change.
msg85432 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-04 21:53
It should run after tearDown so that teardown can do actions that may
require cleanup; because the cleanups run in LIFO you can acquire
resources in setUp and have cleanups clean them up,
msg85434 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-04 21:56
Actually let me phrase that differently.
standard practice for setUp is
super.setUp()
my_setup_code()

and tearDown is
my_teardown_code()
super.tearDown()

because of the LIFO need.

If you imagine that clean ups are being done in the base class teardown,
at the end of that method, then it becomes a lot more obvious why it
should run after tearDown - because thats the right place, and because
many users may be failing to call the base class tearDown which has
historically done nothing.
msg85437 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-04 22:09
teardown

Why should they? It's only an implementation choice, and not a wise one
I would say (precisely because people are used to the fact that the
standard tearDown() method does nothing, and doesn't need to be called).

I explained my proposal in terms of actual use cases, but I don't see
any actual use case of addCleanup() in your argument.
msg85439 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-04 22:14
Sorry, roundup screwed the quoting. I was responding to the following
sentence:
“If you imagine that clean ups are being done in the base class teardown”.
msg85443 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-04 22:57
On Sat, 2009-04-04 at 22:09 +0000, Antoine Pitrou wrote:
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> teardown
> 
> Why should they? It's only an implementation choice, and not a wise one
> I would say (precisely because people are used to the fact that the
> standard tearDown() method does nothing, and doesn't need to be called).
> 
> I explained my proposal in terms of actual use cases, but I don't see
> any actual use case of addCleanup() in your argument.

I was arguing by analogy: if we were to implement addCleanup as
something called at the end of the base class tearDown, then it would
clearly support LIFO tearing down of anything people have done [except
that we can't rely on them having called the base tearDown]. The next
best thing then is to call it from run() after calling tearDown.

If you want a worked example:
---
class TestSample(TestCase):

    def setUp(self):
        dirname = mkdtemp()
        self.addCleanup(shutils.rmtree, dirname, ignore_errors=True)
        db = make_db(dirname)
        self.addCleanup(db.tearDown)

    ....
---

This depends on db being torn down before the rmtree, or else the db
teardown will blow up (and it must be torn down to release locks
correctly on windows).

-Rob
msg85444 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-04 23:02
Antoine, Robert suggests calling it after tearDown so that tearDown can
also perform actions that need clean up. Not a common use case but at
least a use case. What do you mean by: 

"so that test cases can also use it to initialize
additional resources and finalize them in a LIFO way wrt the main tearDown"

Can you give an example? You seem to imply that clean up be used to
initialize resources which seems very odd.
msg85445 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-04 23:02
>    def setUp(self):
>         dirname = mkdtemp()
>         self.addCleanup(shutils.rmtree, dirname, ignore_errors=True)
>         db = make_db(dirname)
>         self.addCleanup(db.tearDown)

Sure, but that's an example of doing something which is already doable
without addCleanup (resource allocation in setUp).
I was talking about doing resource allocation in the test methods
themselves, which is /only/ possible using addCleanup, and needs
cleanups to be run before tearDown, not after.
msg85446 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-04 23:04
The main use case for addCleanup is resource allocation in tests. Why
does this require clean ups to be executed before tearDown?
msg85447 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-04 23:06
> The main use case for addCleanup is resource allocation in tests. Why
> does this require clean ups to be executed before tearDown?

If your cleanup relies on something which has been set up during setUp
and will be dropped during tearDown (a database connection, a temp dir,
an ssh session, whatever), then the cleanup must be run before the
teardown.
msg85463 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-05 04:46
On Sat, 2009-04-04 at 23:06 +0000, Antoine Pitrou wrote:
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> > The main use case for addCleanup is resource allocation in tests. Why
> > does this require clean ups to be executed before tearDown?
> 
> If your cleanup relies on something which has been set up during setUp
> and will be dropped during tearDown (a database connection, a temp dir,
> an ssh session, whatever), then the cleanup must be run before the
> teardown.

I don't think you can sensibly define an ordering between setup/teardown
and cleanups that works for all cases.

Either cleanups happen after teardown, and then you can use them when
allocating things in setup/run/teardown, or they happen before teardown
and you cannot safely use them except in run because things setup by a
child will not have been torn down when cleanups run. [see the two
where-it-breaks sequences below]. The issue is that cleanups are not
connected to the inheritance heirarchy. They are safe and trivial to use
with resources that are not connected to each other *however* they are
defined, but the interaction with things being freed in tearDown cannot
meet all cases unless you have per-class-in the-MRO-queues (which is
overly complex).

I assert that its better to be able to use cleanups in all three test
methods rather than only in run, all other things being equal.

Our experience in bzr (we use this heavily, and migrated to it
incrementally across our 17K fixture suite) is that we rarely need to
use cleanups on dependent resources, and when we need to it has been
very easy to migrate the dependent resource to use cleanups as well.

-Rob

sequence 1: cleanup after teardowns prevents using cleanups on things
freed in teardown:

setup
  self.foo = thing()
run
  self.bar = something(self.foo)
  self.addCleanUp(free, self.bar)
teardown
  unthing(self.foo)
cleanups
  free(self.bar) *boom* self.foo is already gone

sequence 2: cleanup before teardown prevents using cleanups in base
class setup methods

base.setup
  self.foo = thing()
  self.addCleanup(unthing, self.foo)
setup
  super.setup()
  self.bar = something(self.foo)
run
  assertWhatever
cleanups
  unthing(self.foo)
teardown
  free(self.bar) *boom* self.foo is already gone
msg85465 - (view) Author: Enji Cooper (ngie) * Date: 2009-04-05 07:25
I think some perspective is required on this enhancement request. I
originally filed this issue -- http://bugs.python.org/issue5538 --
because of the unneeded complexity involved with duplicating
teardown-related code in setUp because of a step in setUp failing.

From my perspective, there are two issues:

- setUp failing doesn't cleanup on failure unless the test writer
explicitly adds cleanup logic.
- cleanup shouldn't partially replace tearDown -- either supplement it
or completely replace it longterm. Otherwise the unittest code and
expectations associated with it will potentially confuse end users.

Another thought: Why not have an option for defining a method called
`incrementalTearDown', which replaces `tearDown' from a functional
standpoint? A method like that would clearly convey that this is
designed to replace tearDown, it's not the same functionally, and would
ease migration over the long-term if people chose to use this design
when writing testcases.

I personally think that doing something like this would be trivial (yet
novel) functionality as it makes more sense than the current
implementation of setUp->test->tearDown.
msg85466 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-05 08:03
On Sun, 2009-04-05 at 07:25 +0000, Garrett Cooper wrote:
> Garrett Cooper <yanegomi@gmail.com> added the comment:
> 
> I think some perspective is required on this enhancement request. I
> originally filed this issue -- http://bugs.python.org/issue5538 --
> because of the unneeded complexity involved with duplicating
> teardown-related code in setUp because of a step in setUp failing.

Indeed, and in bzr and other unittest extension libraries one of the
first things done is to make tearDown be called always.

> >From my perspective, there are two issues:
> 
> - setUp failing doesn't cleanup on failure unless the test writer
> explicitly adds cleanup logic.

the proposed patch in 5679 runs cleanups always, so this shouldn't be an
issue with the addCleanup functionality.

> - cleanup shouldn't partially replace tearDown -- either supplement it
> or completely replace it longterm. Otherwise the unittest code and
> expectations associated with it will potentially confuse end users.

I'm conceptually fine with this, certainly I've not written a tearDown
for tests in bzr in a very long time.

> Another thought: Why not have an option for defining a method called
> `incrementalTearDown', which replaces `tearDown' from a functional
> standpoint? A method like that would clearly convey that this is
> designed to replace tearDown, it's not the same functionally, and would
> ease migration over the long-term if people chose to use this design
> when writing testcases.

Its much more complex than addCleanup [it will be tied to the MRO], not
as flexible (and in big suites that starts to matter a lot), and unable
to trivially hook into actually used resources [which addCleanup does].

-Rob
msg85471 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-05 10:15
> Our experience in bzr (we use this heavily, and migrated to it
> incrementally across our 17K fixture suite) is that we rarely need to
> use cleanups on dependent resources, and when we need to it has been
> very easy to migrate the dependent resource to use cleanups as well.

I'm baffled. If you say you don't care about the order, why are you
arguing at all?

[...]
> sequence 2: cleanup before teardown prevents using cleanups in base
> class setup methods

The point is that sequence 2 can already be emulated using careful
"try...finally" in tearDown, while sequence 1 cannot. That is, sequence
1 *needs* the addCleanup, while for sequence 2 it is a mere additional
convenience.
msg85494 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-05 13:49
On Sun, 2009-04-05 at 10:15 +0000, Antoine Pitrou wrote:
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> > Our experience in bzr (we use this heavily, and migrated to it
> > incrementally across our 17K fixture suite) is that we rarely need to
> > use cleanups on dependent resources, and when we need to it has been
> > very easy to migrate the dependent resource to use cleanups as well.
> 
> I'm baffled. If you say you don't care about the order, why are you
> arguing at all?

I didn't say I don't care; I do - I care that it is robust and hard to
misuse. Having addCleanup() when called from a tearDown lead to cleanups
not being called would be an easy route to misuse.

> [...]
> > sequence 2: cleanup before teardown prevents using cleanups in base
> > class setup methods
> 
> The point is that sequence 2 can already be emulated using careful
> "try...finally" in tearDown, while sequence 1 cannot. That is, sequence
> 1 *needs* the addCleanup, while for sequence 2 it is a mere additional
> convenience.

I don't understand; neither sequence works - they are showing how any
choice [that retains the current simple proposed mechanism] cannot
interact without some failure modes with tearDown. Whichever point we
choose to have cleanups execute can be entirely emulated using careful
try:finally: in tearDown methods, so surely this is not an argument for
either order.

-Rob
msg85497 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-05 13:59
> I don't understand; neither sequence works

????

>  - they are showing how any
> choice [that retains the current simple proposed mechanism] cannot
> interact without some failure modes with tearDown.

And I'm telling you one failure mode is more desireable than the other.
msg85562 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-05 20:35
I'm in favour of running clean ups afterwards on the basis that it makes
things possible that would otherwise not be possible.

> If your cleanup relies on something which has been set up during setUp
> and will be dropped during tearDown (a database connection, a temp dir,
> an ssh session, whatever), then the cleanup must be run before the
> teardown.

But this is a function of whichever way we do it - and so not an
argument for one way or the other. Conversely if you write a tearDown
that relies on resources existing that will later be removed by a clean
up then clean ups must be run afterwards.

> The point is that sequence 2 can already be emulated using careful
> "try...finally" in tearDown, while sequence 1 cannot. That is, sequence
> 1 *needs* the addCleanup, while for sequence 2 it is a mere additional
> convenience.

Which is an argument in favour of running clean ups afterwards.
msg85565 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-05 21:19
A use case for running clean ups before tearDown (in which case
addCleanUp would need to raise an exception if called during tearDown).

You have existing code which (for example) creates an SSH connection in
setUp and removes it in tearDown.

If clean ups are run after tearDown then you can't use addCleanup inside
tests to deallocate resources that rely on the SSH connection being
there (i.e. issue a remote command).

Switching to use clean ups would require rewriting the existing code.
Running clean ups before tearDown is more likely to fit in with existing
test code.

The downsides are that it makes adding clean ups in tearDown impossible
(but why would you want to do that?) and you can't use clean ups for any
resource accessed in tearDown (which you may want to do but won't be the
case for any code that currently exists...).

We probably need a third party arbiter to pick which is the most sane. :-)
msg85567 - (view) Author: Virgil Dupras (vdupras) (Python triager) Date: 2009-04-05 21:26
So, we are talking about adding a feature that could cause problem whether 
cleanup is performed before tearDown or after tearDown. Don't we risk 
confusing developers who are not familiar with the cleanup order?

Do we really want to add this feature? The added functionality doesn't 
seem so attractive compared to the risk of confusion. We might as well 
continue to let the developers implement this feature in their own 
TestCase subclasses (for example, my own subclass already implement a 
TestCase.create_tmpdir(name) (which does cleanup, of course), which is 
easier to use than the rmtree example that was given as a use case). At 
least, when they do, they know about the cleanup order.
msg85568 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009-04-05 21:31
> So, we are talking about adding a feature that could cause problem whether 
> cleanup is performed before tearDown or after tearDown. Don't we risk 
> confusing developers who are not familiar with the cleanup order?

Well, we could do both. Call cleanups before tearDown (all the while
popping them), call tearDown, and call the new cleanups.

If the cleanup machinery is written carefully enough, you may even be
able to add another cleanup during a cleanup :-)
msg85569 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-05 21:34
This is actually a minor point about the order things happen in. I don't
think it will cause confusion in practise once we have made a decision
and documented it.

Particularly if we decide to call clean ups before tearDown then I will
make adding new ones during tearDown raise an exception.

I did think about having two separate clean up stacks - one for those
added during setUp and one for those added during tests, but that is
starting to get silly. 

The point about this patch is that it makes the deallocation of
resources a lot simpler without having to manually track them inside
your test (nested try finallys for this are not uncommon in the tests
I've seen) and the fact that this mechanism is already in use in the
test frameworks of three major projects indicate that it is definitely
worthwhile.
msg85570 - (view) Author: Enji Cooper (ngie) * Date: 2009-04-05 21:48
I see the validity in the concern raised by Virgil:

I think this can be simplified from a user perspective by saying `I'm
electing to use cleanUp over tearDown'. The two systems are similar, but
also very different semantically as tearDown gets executed in the event
of test completion (regardless of whether or not it passed) whereas
cleanUp gets executed based on what occurs in setUp.

Or am I missing the boat here on what's trying to be accomplished?

We really shouldn't be mixing and matching both implementations because
that will confuse people...
msg85572 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-05 21:51
On Sun, 2009-04-05 at 21:31 +0000, Antoine Pitrou wrote:
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> > So, we are talking about adding a feature that could cause problem whether 
> > cleanup is performed before tearDown or after tearDown. Don't we risk 
> > confusing developers who are not familiar with the cleanup order?
> 
> Well, we could do both. Call cleanups before tearDown (all the while
> popping them), call tearDown, and call the new cleanups.

I think this makes cleanups harder to explain, but yes we could do it.
(Its harder because its no longer clearly a LIFO as cleanups added
before teardown at not executed last).

> If the cleanup machinery is written carefully enough, you may even be
> able to add another cleanup during a cleanup :-)

I've been meaning to tweak the patch - 
rather than
+        for function, args, kwargs in reversed(self._cleanups):
it should be
+        while self._cleanups:
+            function, args, kwargs = self._cleanups.pop(-1)

precisely to allow this.

The point I've been trying to make isn't just that after everything is
clearly best (which is subjective), but that dropping cleanups from user
code is clearly wrong (something I hope we all agree on) and that its
harder to have bugs like that if cleanups run after teardown.

@Antoine I appreciate that you feel very strongly about this; I'm at a
bit of a loss why, given that I've been working with this feature for
over 3 *years* and we've not once had headaches or confusion related to
the discussed sequencing failure modes - and our code runs it after all
the user code. I've tried my level best to document why it works best
after running after all the user code, but ultimately whoever is
committing this code to python needs to make a call about what you think
is best. It would be nice if you can learn from our experience, but I'm
about out of energy to discuss this; certainly when we're down to
authority by posturing it seems like fruitful discussion is rather over.

@Michael - you may have a convenience function that uses addCleanup - if
that gets called during tearDown then while its a little odd its still
correct for cleanups the convenience function added to get called. It's
only in the light of having to really explain why it should be
afterwards that I've realised we had a latent bug - fixed in the change
above [and a test for it is pretty obvious].

-Rob
msg85589 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-05 23:49
@Garrett / Virgil

I don't think anything other than very short term confusion is possible.
Whichever decision is made a developer who assumes the opposite will
actually get an error. In both cases the downside is that in certain
circumstances you could attempt to use a resource that has already been
disposed of.

As an interesting data point, the Bzr code does clean ups *before* tearDown.
msg85590 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-05 23:54
On Sun, 2009-04-05 at 23:49 +0000, Michael Foord wrote:
> 
> 
> As an interesting data point, the Bzr code does clean ups *before*
> tearDown.

No it doesn't:

We subclass unittest.TestCase. We also override run() to make tearDown
run always.

Our base test case class has it's tearDown:

    def tearDown(self):
        self._bzr_test_tearDown_run = True
        self._runCleanups()
        self._log_contents = ''
        unittest.TestCase.tearDown(self)

(which is to say, _runCleanups runs after any child classes tearDown,
even though we implement it by calling it from our base-most tearDown).

-Rob
msg85592 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-05 23:56
My apologies - the jml code on launchpad runs clean ups before taerDown.

http://bazaar.launchpad.net/~jml/testtools/trunk/annotate/head%3A/testtools/testcase.py
msg85593 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-06 00:06
On Sun, 2009-04-05 at 23:57 +0000, Michael Foord wrote:
> Michael Foord <michael@voidspace.org.uk> added the comment:
> 
> My apologies - the jml code on launchpad runs clean ups before taerDown.
> 
> http://bazaar.launchpad.net/~jml/testtools/trunk/annotate/head%3A/testtools/testcase.py

Ah, indeed it does. Well, FWIW my use has nearly all been with bzr's
that does it after :)

Jono - was there some particular reason testtools does it after? (We're
discussing the merits of pre-and-post tearDown, ad-nauseum :)).

-Rob
msg85594 - (view) Author: Jonathan Lange (jml) Date: 2009-04-06 00:14
On Mon, Apr 6, 2009 at 10:06 AM, Robert Collins
<robertc@robertcollins.net> wrote:
> On Sun, 2009-04-05 at 23:57 +0000, Michael Foord wrote:
>> Michael Foord <michael@voidspace.org.uk> added the comment:
>>
>> My apologies - the jml code on launchpad runs clean ups before taerDown.
>>
>> http://bazaar.launchpad.net/~jml/testtools/trunk/annotate/head%3A/testtools/testcase.py
>
> Ah, indeed it does. Well, FWIW my use has nearly all been with bzr's
> that does it after :)
>
> Jono - was there some particular reason testtools does it after? (We're
> discussing the merits of pre-and-post tearDown, ad-nauseum :)).
>

No particular reason.
msg85609 - (view) Author: Kumar McMillan (kumar303) Date: 2009-04-06 04:16
I like this patch.  However, a nice-to-have would be that _doCleanups()
prints a traceback from function(*args, **kwargs) (if there is one) the
same way that atexit does.  That would aid in debugging mis-written
cleanup functions yet would not intrude on your test program.
msg85612 - (view) Author: Jonathan Lange (jml) Date: 2009-04-06 04:53
On Mon, Apr 6, 2009 at 2:16 PM, Kumar McMillan <report@bugs.python.org> wrote:
>
> Kumar McMillan <kumar.mcmillan@gmail.com> added the comment:
>
> I like this patch.  However, a nice-to-have would be that _doCleanups()
> prints a traceback from function(*args, **kwargs) (if there is one) the
> same way that atexit does.  That would aid in debugging mis-written
> cleanup functions yet would not intrude on your test program.
>

I think reporting the errors via addError is the right thing. If your
cleanups fail, then your test suite becomes unreliable, so they should
be treated like tests that raise unexpected errors.
msg85613 - (view) Author: Jonathan Lange (jml) Date: 2009-04-06 04:56
FWIW, I kind of like the tests here:
http://bazaar.launchpad.net/~jml/testtools/trunk/annotate/head%3A/testtools/tests/test_testtools.py#L221
msg86568 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-25 23:16
Damn - proper patch without extraneous stuff this time.
msg86569 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-25 23:17
Proper patch and proper issue this time! Not my evening.
msg86576 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2009-04-26 00:03
On Sat, 2009-04-25 at 23:17 +0000, Michael Foord wrote:
> Michael Foord <michael@voidspace.org.uk> added the comment:
> 
> Proper patch and proper issue this time! Not my evening.
> 
> ----------
> Added file: http://bugs.python.org/file13787/unittest-no-exit.patch

Did this want a new issue perhaps? [it looks fine, just unrelated].

-Rob
msg86607 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-04-26 21:17
Updated patch with docs. My intention is to apply this in the next 
couple of days.

I've settled on calling doCleanups *after* tearDown. The issues and 
reasoning explained below.

One point of view concerns using addCleanups with existing tests. 

If the setUp allocates resources that are deallocated by tearDown, the 
natural LIFO pattern would seem to be setUp -> tests that create cleanup 
functions -> do cleanups (which may use resources allocated by setUp) -> 
tearDown.

This pattern doesn't allow tearDown to create cleanup functions and 
doesn't permit setUp to create cleanup functions if tearDown need to 
access those resources (unless tearDown itself becomes one big cleanup 
function).

If you look at the situation with new tests then it is perfectly natural  
for setUp, tests and tearDown to all create cleanup functions. The 
natural LIFO order is for cleanups to be run after tearDown.

The solution I've opted for is to make doCleanups public. If you are 
adding the use of cleanups to old tests and need the cleanup functions 
run before tearDown then you are free to decorate the test with a 
function that calls doCleanups on success or failure.

This takes into account the experience of those already using test 
frameworks that provide this mechanism.
msg86978 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2009-05-02 20:22
Committed in revision 72219.
msg86998 - (view) Author: Enji Cooper (ngie) * Date: 2009-05-03 04:27
Cool! Thanks for all of the hard work Michael :D.
msg130459 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-03-09 19:32
Apparently during a merge from trunk (r72477) the addCleanup and other methods ended up in 3.1rc1, even if they are documented as new in 3.2. I'll update the doc to say "new in 3.1".
History
Date User Action Args
2022-04-11 14:56:47adminsetgithub: 49929
2011-03-09 19:32:54ezio.melottisetnosy: + ezio.melotti
messages: + msg130459
2009-05-03 04:27:28ngiesetmessages: + msg86998
2009-05-02 20:22:01michael.foordsetstatus: open -> closed

messages: + msg86978
2009-04-26 21:17:08michael.foordsetfiles: + unittest-cleanup.patch

messages: + msg86607
2009-04-26 00:03:39rbcollinssetmessages: + msg86576
2009-04-25 23:17:22michael.foordsetfiles: + unittest-no-exit.patch

messages: + msg86569
2009-04-25 23:16:39michael.foordsetfiles: - unittest-no-exit.patch
2009-04-25 23:16:29michael.foordsetfiles: + unittest-no-exit.patch

messages: + msg86568
2009-04-06 04:56:37jmlsetmessages: + msg85613
2009-04-06 04:53:43jmlsetmessages: + msg85612
2009-04-06 04:16:24kumar303setmessages: + msg85609
2009-04-06 04:04:44kumar303setnosy: + kumar303
2009-04-06 00:14:30jmlsetnosy: + jml
messages: + msg85594
2009-04-06 00:06:49rbcollinssetmessages: + msg85593
2009-04-05 23:56:58michael.foordsetmessages: + msg85592
2009-04-05 23:54:59rbcollinssetmessages: + msg85590
2009-04-05 23:49:50michael.foordsetmessages: + msg85589
2009-04-05 21:51:58rbcollinssetmessages: + msg85572
2009-04-05 21:48:25ngiesetmessages: + msg85570
2009-04-05 21:34:58michael.foordsetmessages: + msg85569
2009-04-05 21:31:38pitrousetmessages: + msg85568
2009-04-05 21:26:30vduprassetnosy: + vdupras
messages: + msg85567
2009-04-05 21:19:02michael.foordsetmessages: + msg85565
2009-04-05 20:35:59michael.foordsetmessages: + msg85562
2009-04-05 13:59:02pitrousetmessages: + msg85497
2009-04-05 13:49:58rbcollinssetmessages: + msg85494
2009-04-05 10:15:03pitrousetmessages: + msg85471
2009-04-05 08:03:50rbcollinssetmessages: + msg85466
2009-04-05 07:25:52ngiesetnosy: + ngie
messages: + msg85465
2009-04-05 04:46:43rbcollinssetmessages: + msg85463
2009-04-04 23:06:27pitrousetmessages: + msg85447
2009-04-04 23:04:13michael.foordsetmessages: + msg85446
2009-04-04 23:02:27pitrousetmessages: + msg85445
2009-04-04 23:02:14michael.foordsetmessages: + msg85444
2009-04-04 22:57:29rbcollinssetmessages: + msg85443
2009-04-04 22:14:09pitrousetmessages: + msg85439
2009-04-04 22:09:12pitrousetmessages: + msg85437
2009-04-04 21:56:49rbcollinssetmessages: + msg85434
2009-04-04 21:53:27rbcollinssetnosy: + rbcollins
messages: + msg85432
2009-04-04 17:11:17michael.foordsetmessages: + msg85413
2009-04-04 16:42:23pitrousetnosy: + pitrou
messages: + msg85407
2009-04-04 16:10:43michael.foordsetfiles: + unittest-cleanup.diff
keywords: + patch
messages: + msg85404
2009-04-03 23:50:15michael.foordsetmessages: + msg85344
2009-04-03 21:09:02michael.foordsettype: behavior -> enhancement
2009-04-03 20:18:04michael.foordsetmessages: + msg85325
2009-04-03 20:16:33michael.foordsetmessages: + msg85324
2009-04-03 20:09:45gregory.p.smithsetnosy: + gregory.p.smith
messages: + msg85323
2009-04-03 19:56:46michael.foordcreate