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