[issue5679] cleanUp stack for unittest
Ezio Melotti ezio.melo...@gmail.com added the comment: 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. -- nosy: +ezio.melotti ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: Committed in revision 72219. -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Garrett Cooper yaneg...@gmail.com added the comment: Cool! Thanks for all of the hard work Michael :D. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: 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. -- Added file: http://bugs.python.org/file13791/unittest-cleanup.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Changes by Michael Foord mich...@voidspace.org.uk: Removed file: http://bugs.python.org/file13786/unittest-no-exit.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@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 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: On Sat, 2009-04-25 at 23:17 +, Michael Foord wrote: Michael Foord mich...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Garrett Cooper yaneg...@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. 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. -- nosy: +yaneurabeya ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: On Sun, 2009-04-05 at 07:25 +, Garrett Cooper wrote: Garrett Cooper yaneg...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Antoine Pitrou pit...@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? [...] 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: On Sun, 2009-04-05 at 10:15 +, Antoine Pitrou wrote: Antoine Pitrou pit...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Antoine Pitrou pit...@free.fr added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: 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. :-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Virgil Dupras hs...@hardcoded.net 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? 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. -- nosy: +vdupras ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Antoine Pitrou pit...@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. If the cleanup machinery is written carefully enough, you may even be able to add another cleanup during a cleanup :-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Garrett Cooper yaneg...@gmail.com added the comment: 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... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: On Sun, 2009-04-05 at 21:31 +, Antoine Pitrou wrote: Antoine Pitrou pit...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: @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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: On Sun, 2009-04-05 at 23:49 +, 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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: On Sun, 2009-04-05 at 23:57 +, Michael Foord wrote: Michael Foord mich...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Jonathan Lange j...@mumak.net added the comment: On Mon, Apr 6, 2009 at 10:06 AM, Robert Collins robe...@robertcollins.net wrote: On Sun, 2009-04-05 at 23:57 +, Michael Foord wrote: Michael Foord mich...@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. -- nosy: +jml ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Changes by Kumar McMillan kumar.mcmil...@gmail.com: -- nosy: +kumar303 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Kumar McMillan kumar.mcmil...@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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Jonathan Lange j...@mumak.net added the comment: On Mon, Apr 6, 2009 at 2:16 PM, Kumar McMillan rep...@bugs.python.org wrote: Kumar McMillan kumar.mcmil...@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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Jonathan Lange j...@mumak.net added the comment: FWIW, I kind of like the tests here: http://bazaar.launchpad.net/~jml/testtools/trunk/annotate/head%3A/testtools/tests/test_testtools.py#L221 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: 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. -- keywords: +patch Added file: http://bugs.python.org/file13612/unittest-cleanup.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: I'm agnostic on before / after tearDown, so happy to make that change. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: 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, -- nosy: +rbcollins ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Antoine Pitrou pit...@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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Antoine Pitrou pit...@free.fr added the comment: 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”. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: On Sat, 2009-04-04 at 22:09 +, Antoine Pitrou wrote: Antoine Pitrou pit...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Antoine Pitrou pit...@free.fr added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk 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? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Antoine Pitrou pit...@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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Robert Collins robe...@robertcollins.net added the comment: On Sat, 2009-04-04 at 23:06 +, Antoine Pitrou wrote: Antoine Pitrou pit...@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 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
New submission from Michael Foord mich...@voidspace.org.uk: 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) -- assignee: michael.foord components: Library (Lib) messages: 85321 nosy: michael.foord severity: normal status: open title: cleanUp stack for unittest type: behavior versions: Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Gregory P. Smith g...@krypto.org added the comment: 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. -- nosy: +gregory.p.smith ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: 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=92340view=auto The module: http://svn.zope.org/zope.testing/trunk/src/zope/testing/setupstack.py?rev=92340view=auto -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Changes by Michael Foord mich...@voidspace.org.uk: -- type: behavior - feature request ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5679] cleanUp stack for unittest
Michael Foord mich...@voidspace.org.uk added the comment: 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... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5679 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
