[Twisted-Python] Remove setupClass branch of trial - what is the motivation?
Hi, Still haven't gotten an answer to this; why is the motivation for deprecating a perfectly reasonable feature?
On Tue, 1 Nov 2005 12:38:21 -0500, Jean-Paul Calderone <exarkun@divmod.com> wrote:
Yeah, uh... As I understand it the implementation was previously horrendously broken. Now it is not. Given that it is now no longer broken, and some quite reasonable tests I've seen make use of it (in particular: tests for Divmod ClickChronicle do some very expensive setup in a setUpClass), I'd like to see it stick around.
On 02/11/05, glyph@divmod.com <glyph@divmod.com> wrote:
Thanks everyone for your patience while I make the time to answer this question. I'd like to deprecate setUpClass, but I'm not totally sold on it. The biggest problem I have with it is that it encourages (nay, forces) users to share state between tests. This may be a problem inherent with any setup optimisation -- but I'd like to experiment first. My other problems are to do with implementation -- and these are the problems I feel most keenly. Trial only knows about classes in the test loader. After that, it only knows about things to call run() on. Sometimes these are collections of tests (suites), sometimes these are actual tests. setUpClass mandates a ClassSuite -- a suite that runs setUpClass at the start and tearDownClass at the end. One big implication is that a TestCase isn't (necessarily) a self-contained TestCase. It has to be wrapped in a ClassSuite to make sure that it gets properly set up and torn down. Running a single test shouldn't require a suite. ClassSuite also means that the test loader needs to have two suite factories. This makes it more difficult to extend (which you'd want to do for alternative runners (ala GUIs) and also if you have a cool way to optimise your tests locally -- see below). In addition, ClassSuite makes "what happens when tests are run" a fair bit more complicated. This is particularly confusing wrt KeyboardInterrupt and reactor cleanup. Oh yeah, and it makes things incompatible with unittest. The example below shows what I'm considering at the moment. This is the testresources system by Robert Collins. One of the reasons I hesitated on this post is that I wanted to have chewed over this more. But here 'tis. Instead of: class FooTest(unittest.TestCase): def setUpClass(self): self.db = ExpensiveAcquisition() def tearDownClass(self): self.db.expensiveDismissal() def test_foo(self): self.failUnless(self.db.isAwesome()) we have: class DBResource(resources.Resource): def makeResource(self): cls.db = ExpensiveAcquisition() return cls.db makeResource = classmethod(makeResource) def cleanResource(self): cls.db.expensiveDismissal() cleanResource = classmethod(cleanResource) class FooTest(unittest.ResourcedTestCase): resources = [ ('db', DBResource) ] def test_foo(self): self.failUnless(self.db.isAwesome()) One of the advantages of this is that it can be made faster than setUpClass / tearDownClass because the OptimizingTestSuite can look at all the tests and resources underneath is to establish the best order to run the tests, and it can preserve resources between TestCase classes. cheers, jml PS. Jp, I hope your day continues as wonderfully as you thought it would start. :)
On Thu, 2005-11-03 at 16:46 +1100, Jonathan Lange wrote:
I don't see why you need a ClassSuite; couldn't a TestCase have optional pre- and post- hooks called along with the test?
Oh yeah, and it makes things incompatible with unittest.
A correctly running trial (not what we have now, cause of all the tests that do wait() and reactor.iterate()) would call reactor.run() once, and tests would only support returning Deferreds as a mean for making the reactor continue. Could we preserve unittest compatability while still working this way? If not, there's no point in bothering. E.g. it seems unlikely that a random unittest.py runner program will be able to run trial tests once we switch to the correct reactor-running strategy.
Any reason we need a whole complex new system when the current one works?
On 02/11/05, glyph@divmod.com <glyph@divmod.com> wrote:
Another thing I totally forgot. setUpClass assumes that there is only one instance of each TestCase subclass. This is a broken assumption, because a TestCase instance represents a single test. Currently, the implentation is horrendous (take a look). I'd be much happier if setUpClass were a class method. jml
On Tue, 1 Nov 2005 12:38:21 -0500, Jean-Paul Calderone <exarkun@divmod.com> wrote:
Yeah, uh... As I understand it the implementation was previously horrendously broken. Now it is not. Given that it is now no longer broken, and some quite reasonable tests I've seen make use of it (in particular: tests for Divmod ClickChronicle do some very expensive setup in a setUpClass), I'd like to see it stick around.
On 02/11/05, glyph@divmod.com <glyph@divmod.com> wrote:
Thanks everyone for your patience while I make the time to answer this question. I'd like to deprecate setUpClass, but I'm not totally sold on it. The biggest problem I have with it is that it encourages (nay, forces) users to share state between tests. This may be a problem inherent with any setup optimisation -- but I'd like to experiment first. My other problems are to do with implementation -- and these are the problems I feel most keenly. Trial only knows about classes in the test loader. After that, it only knows about things to call run() on. Sometimes these are collections of tests (suites), sometimes these are actual tests. setUpClass mandates a ClassSuite -- a suite that runs setUpClass at the start and tearDownClass at the end. One big implication is that a TestCase isn't (necessarily) a self-contained TestCase. It has to be wrapped in a ClassSuite to make sure that it gets properly set up and torn down. Running a single test shouldn't require a suite. ClassSuite also means that the test loader needs to have two suite factories. This makes it more difficult to extend (which you'd want to do for alternative runners (ala GUIs) and also if you have a cool way to optimise your tests locally -- see below). In addition, ClassSuite makes "what happens when tests are run" a fair bit more complicated. This is particularly confusing wrt KeyboardInterrupt and reactor cleanup. Oh yeah, and it makes things incompatible with unittest. The example below shows what I'm considering at the moment. This is the testresources system by Robert Collins. One of the reasons I hesitated on this post is that I wanted to have chewed over this more. But here 'tis. Instead of: class FooTest(unittest.TestCase): def setUpClass(self): self.db = ExpensiveAcquisition() def tearDownClass(self): self.db.expensiveDismissal() def test_foo(self): self.failUnless(self.db.isAwesome()) we have: class DBResource(resources.Resource): def makeResource(self): cls.db = ExpensiveAcquisition() return cls.db makeResource = classmethod(makeResource) def cleanResource(self): cls.db.expensiveDismissal() cleanResource = classmethod(cleanResource) class FooTest(unittest.ResourcedTestCase): resources = [ ('db', DBResource) ] def test_foo(self): self.failUnless(self.db.isAwesome()) One of the advantages of this is that it can be made faster than setUpClass / tearDownClass because the OptimizingTestSuite can look at all the tests and resources underneath is to establish the best order to run the tests, and it can preserve resources between TestCase classes. cheers, jml PS. Jp, I hope your day continues as wonderfully as you thought it would start. :)
On Thu, 2005-11-03 at 16:46 +1100, Jonathan Lange wrote:
I don't see why you need a ClassSuite; couldn't a TestCase have optional pre- and post- hooks called along with the test?
Oh yeah, and it makes things incompatible with unittest.
A correctly running trial (not what we have now, cause of all the tests that do wait() and reactor.iterate()) would call reactor.run() once, and tests would only support returning Deferreds as a mean for making the reactor continue. Could we preserve unittest compatability while still working this way? If not, there's no point in bothering. E.g. it seems unlikely that a random unittest.py runner program will be able to run trial tests once we switch to the correct reactor-running strategy.
Any reason we need a whole complex new system when the current one works?
On 02/11/05, glyph@divmod.com <glyph@divmod.com> wrote:
Another thing I totally forgot. setUpClass assumes that there is only one instance of each TestCase subclass. This is a broken assumption, because a TestCase instance represents a single test. Currently, the implentation is horrendous (take a look). I'd be much happier if setUpClass were a class method. jml
participants (4)
-
glyph＠divmod.com -
Itamar Shtull-Trauring -
Jean-Paul Calderone -
Jonathan Lange