(And side note: if you're pushing a "lambda: self.foo()" with no arguments, the lambda is unneeded and you can just push "self.foo" --- that cleanup hook pushed on most tests at the end of the file is a perfect example of an unneeded level of lambda indirection).
On Wed, Oct 21, 2015 at 12:04 PM, Zachary Turner via lldb-dev < lldb-dev@lists.llvm.org> wrote: > Well you can see them getting added via self.addTearDownHook(), so that > means they're called through an instance. Specifically, it happens in > Base.tearDown(self), so it's basically identical (in concept) to if the > relevant handlers were called in the implementation of MyTest.tearDown(), > but different in order. > > I agree that it's useful in principle to be able to do what you suggest in > your example, but there's just no way to guarantee the right ordering if > you let the base class run the handlers. If there actually *were* a > tearDown() function in your example, to be correct it would need to look > like this: > > class MyTest(TestBase): > def tearDown(self): > # run the teardown hooks > # Do the inverse of setUp() > super.tearDown() > > def test_1(self): > self.addTearDownHook(lambda: self.foo()) > raise ValueError > def test_2(self): > self.addTearDownHook(lambda: self.bar()) > raise ValueError > > One possible solution is to shift burden of maintaining the hooks list to > the individual test case. E.g. > > class MyTest(TestBase): > self.hooks = [] > def tearDown(self): > self.runTearDownHooks(self.hooks) # This could be implemented in > TestBase, since now we can call it with our list at the right time. > # Do the inverse of setUp() > super.tearDown() > > def test_1(self): > self.hooks.append(lambda: self.foo()) > raise ValueError > def test_2(self): > self.hooks.append(lambda: self.bar()) > raise ValueError > > Almost no extra code to write, and should be bulletproof. > > On Wed, Oct 21, 2015 at 11:41 AM Greg Clayton <gclay...@apple.com> wrote: > >> I think it was mostly done to provide an exception safe way to cleanup >> stuff without having to override TestBase.tearDown(). I am guessing this >> cleanup happens on TestCase.tearDown() and not after the current test case >> right? >> >> I could see it being used to cleanup after a single test case in case you >> have: >> >> class MyTest(TestBase): >> def test_1(self): >> self.addTearDownHook(lambda: self.foo()) >> raise ValueError >> def test_2(self): >> self.addTearDownHook(lambda: self.bar()) >> raise ValueError >> >> >> Are these tearDowns happening per test function, or during class >> setup/teardown? >> >> > On Oct 21, 2015, at 9:33 AM, Zachary Turner via lldb-dev < >> lldb-dev@lists.llvm.org> wrote: >> > >> > Yea, that's what I think too. I think this mechanism was probably >> invented to just to save some code and promote reusability, but in practice >> leads to these kinds of problems. >> > >> > On Wed, Oct 21, 2015 at 2:07 AM Pavel Labath <lab...@google.com> wrote: >> > I think we can remove these, provided there is a way to mimic the >> > functionality they are used for now, which I think shouldn't be hard. >> > Anything which was set up in the setUp() method should be undone in >> > tearDown(). Anything which was set up in the test method, can be >> > undone using a try-finally block. Is there a use case not covered by >> > this? >> > >> > pl >> > >> > On 21 October 2015 at 04:47, Zachary Turner via lldb-dev >> > <lldb-dev@lists.llvm.org> wrote: >> > > There's a subtle bug that is pervasive throughout the test suite. >> Consider >> > > the following seemingly innocent test class. >> > > >> > > class MyTest(TestBase); >> > > def setUp(): >> > > TestBase.setUp() #1 >> > > >> > > # Do some stuff #2 >> > > self.addTearDownHook(lambda: self.foo()) #3 >> > > >> > > def test_interesting_stuff(): >> > > pass >> > > >> > > Here's the problem. As a general principle, cleanup needs to happen >> in >> > > reverse order from initialization. That's why, if we had a tearDown() >> > > method, it would probably look something like this: >> > > >> > > def tearDown(): >> > > # Clean up some stuff #2 >> > > >> > > TestBase.tearDown() #1 >> > > >> > > This follows the pattern in other languages like C++, for example, >> where >> > > construction goes from base -> derived, but destruction goes from >> derived -> >> > > base. >> > > >> > > But if you add these tear down hooks into the mix, it violates that. >> tear >> > > down hooks get invoked as part of TestBase.tearDown(), so in the above >> > > example the initialization order is 1 -> 2 -> 3 but the teardown >> order is 2 >> > > -> 1 -> 3 (or 2 -> 3 -> 1, or none of the above depending on where >> inside >> > > of TestBase.tearDown() hook the hooks get invoked). >> > > >> > > To make matters worse, tear down hooks can be added from arbitrary >> points in >> > > a test's run, not just during setup. >> > > >> > > The only way I can see to fix this is to delete this tearDownHook >> mechanism >> > > entirely. Anyone who wants it can easily reimplement this in the >> individual >> > > test by just keeping their own list of lambdas in the derived class, >> > > overriding tearDown(), and running through their own list in reverse >> order >> > > before calling TestBase.tearDown(). >> > > >> > > I don't intend to do this work right now, but I would like to do it >> in the >> > > future, so I want to throw this out there and see if anyone has >> thoughts on >> > > it. >> > > >> > > _______________________________________________ >> > > lldb-dev mailing list >> > > lldb-dev@lists.llvm.org >> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> > > >> > _______________________________________________ >> > lldb-dev mailing list >> > lldb-dev@lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> >> > _______________________________________________ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > > -- -Todd
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
