Test cases not garbage collected after run

I actually created a bug entry for this (http://bugs.python.org/issue11798) and just later it occurred that I should've asked in the list first :) So, here's the text for opinions: Right now, when doing a test case, one must clear all the variables created in the test class, and I believe this shouldn't be needed... E.g.: class Test(TestCase): def setUp(self): self.obj1 = MyObject() ... def tearDown(self): del self.obj1 Ideally (in my view), right after running the test, it should be garbage-collected and the explicit tearDown just for deleting the object wouldn't be needed (as the test would be garbage-collected, that reference would automatically die), because this is currently very error prone... (and probably a source of leaks for any sufficiently big test suite). If that's accepted, I can provide a patch. Thanks, Fabio

On 07/04/2011 17:18, Fabio Zadrozny wrote:
You mean that the test run keeps the test instances alive for the whole test run so instance attributes are also kept alive. How would you solve this - by having calling a TestSuite (which is how a test run is executed) remove members from themselves after each test execution? (Any failure tracebacks etc stored by the TestResult would also have to not keep the test alive.) My only concern would be backwards compatibility due to the change in behaviour. All the best, Michael Foord
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On Fri, Apr 8, 2011 at 4:49 AM, Michael Foord <fuzzyman@voidspace.org.uk> wrote:
An alternative is in TestCase.run() / TestCase.__call__(), make a copy and immediately delegate to it; that leaves the original untouched, permitting run-in-a-loop style helpers to still work. Testtools did something to address this problem, but I forget what it was offhand. -Rob

On 07/04/2011 20:18, Robert Collins wrote:
That doesn't sound like a general solution as not everything is copyable and I don't think we should make that a requirement of tests. The proposed "fix" is to make test suite runs destructive, either replacing TestCase instances with None or pop'ing tests after they are run (the latter being what twisted Trial does). run-in-a-loop helpers could still repeatedly iterate over suites, just not call the suite. All the best, Michael
-Rob
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On Fri, Apr 8, 2011 at 8:12 AM, Michael Foord <fuzzyman@voidspace.org.uk> wrote:
Thats quite expensive - repeating discovery etc from scratch. If you don't repeat discovery then you're assuming copyability. What I suggested didn't /require/ copying - it delegates it to the test, an uncopyable test would simply not do this. -Rob

On 08/04/2011 02:10, Robert Collins wrote:
Nope, just executing the tests by iterating over the suite and calling them individually - no need to repeat discovery. With the fix in place executing tests by calling the suite would be destructive, but iterating over the suite wouldn't be destructive - so the contained tests can still be executed repeatedly by copying and executing. point is that frameworks that wish to do that would still be able to do this, but they'd have to iterate over the suite themselves rather than calling it directly.
Ok, so you're not suggesting tests copy themselves by default? In which case I don't see that you're offering a fix for the problem. (Or at least not a built-in one.) All the best, Michael Foord
-Rob
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On 07/04/2011, Michael Foord <fuzzyman@voidspace.org.uk> wrote:
Some issues were worked around, but I don't remember any comprehensive solution.
Just pop-ing is unlikely to be sufficient in practice. The Bazaar test suite (which uses testtools nowadays) has code that pops during the run, but still keeps every case alive for the duration. That trebles the runtime on my memory-constrained box unless I add a hack that clears the __dict__ of every testcase after it's run. Martin

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 4/14/2011 1:23 AM, Martin (gzlist) wrote:
I think we would be ok with merging the __dict__ clearing as long as it doesn't do it for failed tests, etc. John =:-> -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Cygwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk2m1oAACgkQJdeBCYSNAAPHmwCfQSNW8Pk7V7qx6Jl/gYthFVxE p0cAn0XRvRR+Rqb+yiJnaVEzUOBdwOpf =19YJ -----END PGP SIGNATURE-----

On 14/04/2011 00:23, Martin (gzlist) wrote: that would do that. It's either a general problem that unittest can fix, or it is a problem *caused* by the bazaar test suite and should be fixed there. Bazaar does some funky stuff copying tests to run them with different backends, so it is possible that this is the cause of the problem (and it isn't a general problem). All the best, Michael Foord
Martin
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On 14/04/2011, Michael Foord <fuzzyman@voidspace.org.uk> wrote:
The main cause is some handy code for collecting and filtering tests by name, which unintentionally keeps alive a list outside the TestSuite instance. There's also the problem of reference cycles involving exc_info, bound methods, and so on that make the lifetimes of test cases unpredictable. That's mostly a problem for systems with a very limited allotment of certain resources such as socket handles. However it also makes ensuring the code doesn't regress back to leaking-the-universe more complicated as tests may still survive past the pop.
The fact it's easy to accidentally keep objects alive is a general problem. If every project that writes their own little test loader risks reverting to immortal cases, that's not really progress. The Bazaar example is a warning because the intention was the same as yours, but ended up being a behaviour regression that went unnoticed by most of the developers while crippling others. And as John mentioned, the fix hasn't yet landed, mostly because the hack is good enough for me and the right thing is too complicated. Martin

On 15/04/2011 17:49, Martin (gzlist) wrote:
I can't remember if I replied to this or not. Sorry. Anyway, so long as *unittest* doesn't keep your test cases alive (which currently it does) then if any individual test framework keeps the tests alive that is a bug in the framework (and it can be tested for). If we stomp on the test instance dictionaries then legitimate use cases may be prevented (like test cases copying themselves and executing a copy when run - a use case described by Robert Collins in a previous email). Although other test frameworks may implement additional measures required specifically by them, the duty of unittest is just to ensure that it doesn't make disposing of test cases *impossible* during normal use. All the best, Michael Foord
Martin
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On 07/04/2011 17:18, Fabio Zadrozny wrote:
You mean that the test run keeps the test instances alive for the whole test run so instance attributes are also kept alive. How would you solve this - by having calling a TestSuite (which is how a test run is executed) remove members from themselves after each test execution? (Any failure tracebacks etc stored by the TestResult would also have to not keep the test alive.) My only concern would be backwards compatibility due to the change in behaviour. All the best, Michael Foord
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On Fri, Apr 8, 2011 at 4:49 AM, Michael Foord <fuzzyman@voidspace.org.uk> wrote:
An alternative is in TestCase.run() / TestCase.__call__(), make a copy and immediately delegate to it; that leaves the original untouched, permitting run-in-a-loop style helpers to still work. Testtools did something to address this problem, but I forget what it was offhand. -Rob

On 07/04/2011 20:18, Robert Collins wrote:
That doesn't sound like a general solution as not everything is copyable and I don't think we should make that a requirement of tests. The proposed "fix" is to make test suite runs destructive, either replacing TestCase instances with None or pop'ing tests after they are run (the latter being what twisted Trial does). run-in-a-loop helpers could still repeatedly iterate over suites, just not call the suite. All the best, Michael
-Rob
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On Fri, Apr 8, 2011 at 8:12 AM, Michael Foord <fuzzyman@voidspace.org.uk> wrote:
Thats quite expensive - repeating discovery etc from scratch. If you don't repeat discovery then you're assuming copyability. What I suggested didn't /require/ copying - it delegates it to the test, an uncopyable test would simply not do this. -Rob

On 08/04/2011 02:10, Robert Collins wrote:
Nope, just executing the tests by iterating over the suite and calling them individually - no need to repeat discovery. With the fix in place executing tests by calling the suite would be destructive, but iterating over the suite wouldn't be destructive - so the contained tests can still be executed repeatedly by copying and executing. point is that frameworks that wish to do that would still be able to do this, but they'd have to iterate over the suite themselves rather than calling it directly.
Ok, so you're not suggesting tests copy themselves by default? In which case I don't see that you're offering a fix for the problem. (Or at least not a built-in one.) All the best, Michael Foord
-Rob
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On 07/04/2011, Michael Foord <fuzzyman@voidspace.org.uk> wrote:
Some issues were worked around, but I don't remember any comprehensive solution.
Just pop-ing is unlikely to be sufficient in practice. The Bazaar test suite (which uses testtools nowadays) has code that pops during the run, but still keeps every case alive for the duration. That trebles the runtime on my memory-constrained box unless I add a hack that clears the __dict__ of every testcase after it's run. Martin

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 4/14/2011 1:23 AM, Martin (gzlist) wrote:
I think we would be ok with merging the __dict__ clearing as long as it doesn't do it for failed tests, etc. John =:-> -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Cygwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk2m1oAACgkQJdeBCYSNAAPHmwCfQSNW8Pk7V7qx6Jl/gYthFVxE p0cAn0XRvRR+Rqb+yiJnaVEzUOBdwOpf =19YJ -----END PGP SIGNATURE-----

On 14/04/2011 00:23, Martin (gzlist) wrote: that would do that. It's either a general problem that unittest can fix, or it is a problem *caused* by the bazaar test suite and should be fixed there. Bazaar does some funky stuff copying tests to run them with different backends, so it is possible that this is the cause of the problem (and it isn't a general problem). All the best, Michael Foord
Martin
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html

On 14/04/2011, Michael Foord <fuzzyman@voidspace.org.uk> wrote:
The main cause is some handy code for collecting and filtering tests by name, which unintentionally keeps alive a list outside the TestSuite instance. There's also the problem of reference cycles involving exc_info, bound methods, and so on that make the lifetimes of test cases unpredictable. That's mostly a problem for systems with a very limited allotment of certain resources such as socket handles. However it also makes ensuring the code doesn't regress back to leaking-the-universe more complicated as tests may still survive past the pop.
The fact it's easy to accidentally keep objects alive is a general problem. If every project that writes their own little test loader risks reverting to immortal cases, that's not really progress. The Bazaar example is a warning because the intention was the same as yours, but ended up being a behaviour regression that went unnoticed by most of the developers while crippling others. And as John mentioned, the fix hasn't yet landed, mostly because the hack is good enough for me and the right thing is too complicated. Martin

On 15/04/2011 17:49, Martin (gzlist) wrote:
I can't remember if I replied to this or not. Sorry. Anyway, so long as *unittest* doesn't keep your test cases alive (which currently it does) then if any individual test framework keeps the tests alive that is a bug in the framework (and it can be tested for). If we stomp on the test instance dictionaries then legitimate use cases may be prevented (like test cases copying themselves and executing a copy when run - a use case described by Robert Collins in a previous email). Although other test frameworks may implement additional measures required specifically by them, the duty of unittest is just to ensure that it doesn't make disposing of test cases *impossible* during normal use. All the best, Michael Foord
Martin
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html
participants (5)
-
Fabio Zadrozny
-
John Arbash Meinel
-
Martin (gzlist)
-
Michael Foord
-
Robert Collins