<div dir="ltr">(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).</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 21, 2015 at 12:04 PM, Zachary Turner via lldb-dev <span dir="ltr"><<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div><br></div><div>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:</div><div><br></div><div>class MyTest(TestBase):</div><div> def tearDown(self):</div><div> # run the teardown hooks</div><div> # Do the inverse of setUp()</div><div><span style="line-height:1.5"> super.tearDown()</span><br></div><span class=""><div><br> def test_1(self):<br> self.addTearDownHook(lambda: self.foo())<br> raise ValueError<br> def test_2(self):<br> self.addTearDownHook(lambda: self.bar())<br> raise ValueError<br></div><div><br></div></span><div>One possible solution is to shift burden of maintaining the hooks list to the individual test case. E.g.</div><div><br></div><div><div>class MyTest(TestBase):</div><div> self.hooks = []</div><div> def tearDown(self):</div><div> self.runTearDownHooks(self.hooks) # This could be implemented in TestBase, since now we can call it with our list at the right time.</div><div> # Do the inverse of setUp()</div><div><span style="line-height:1.5"> super.tearDown()</span><br></div><div><br> def test_1(self):<br> self.hooks.append(lambda: self.foo())<br> raise ValueError<br> def test_2(self):<br> self.hooks.append(lambda: self.bar())<br> raise ValueError</div></div><div><br></div><div>Almost no extra code to write, and should be bulletproof.</div></div><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Wed, Oct 21, 2015 at 11:41 AM Greg Clayton <<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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?<br>
<br>
I could see it being used to cleanup after a single test case in case you have:<br>
<br>
class MyTest(TestBase):<br>
def test_1(self):<br>
self.addTearDownHook(lambda: self.foo())<br>
raise ValueError<br>
def test_2(self):<br>
self.addTearDownHook(lambda: self.bar())<br>
raise ValueError<br>
<br>
<br>
Are these tearDowns happening per test function, or during class setup/teardown?<br>
<br>
> On Oct 21, 2015, at 9:33 AM, Zachary Turner via lldb-dev <<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a>> wrote:<br>
><br>
> 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.<br>
><br>
> On Wed, Oct 21, 2015 at 2:07 AM Pavel Labath <<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>> wrote:<br>
> I think we can remove these, provided there is a way to mimic the<br>
> functionality they are used for now, which I think shouldn't be hard.<br>
> Anything which was set up in the setUp() method should be undone in<br>
> tearDown(). Anything which was set up in the test method, can be<br>
> undone using a try-finally block. Is there a use case not covered by<br>
> this?<br>
><br>
> pl<br>
><br>
> On 21 October 2015 at 04:47, Zachary Turner via lldb-dev<br>
> <<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a>> wrote:<br>
> > There's a subtle bug that is pervasive throughout the test suite. Consider<br>
> > the following seemingly innocent test class.<br>
> ><br>
> > class MyTest(TestBase);<br>
> > def setUp():<br>
> > TestBase.setUp() #1<br>
> ><br>
> > # Do some stuff #2<br>
> > self.addTearDownHook(lambda: self.foo()) #3<br>
> ><br>
> > def test_interesting_stuff():<br>
> > pass<br>
> ><br>
> > Here's the problem. As a general principle, cleanup needs to happen in<br>
> > reverse order from initialization. That's why, if we had a tearDown()<br>
> > method, it would probably look something like this:<br>
> ><br>
> > def tearDown():<br>
> > # Clean up some stuff #2<br>
> ><br>
> > TestBase.tearDown() #1<br>
> ><br>
> > This follows the pattern in other languages like C++, for example, where<br>
> > construction goes from base -> derived, but destruction goes from derived -><br>
> > base.<br>
> ><br>
> > But if you add these tear down hooks into the mix, it violates that. tear<br>
> > down hooks get invoked as part of TestBase.tearDown(), so in the above<br>
> > example the initialization order is 1 -> 2 -> 3 but the teardown order is 2<br>
> > -> 1 -> 3 (or 2 -> 3 -> 1, or none of the above depending on where inside<br>
> > of TestBase.tearDown() hook the hooks get invoked).<br>
> ><br>
> > To make matters worse, tear down hooks can be added from arbitrary points in<br>
> > a test's run, not just during setup.<br>
> ><br>
> > The only way I can see to fix this is to delete this tearDownHook mechanism<br>
> > entirely. Anyone who wants it can easily reimplement this in the individual<br>
> > test by just keeping their own list of lambdas in the derived class,<br>
> > overriding tearDown(), and running through their own list in reverse order<br>
> > before calling TestBase.tearDown().<br>
> ><br>
> > I don't intend to do this work right now, but I would like to do it in the<br>
> > future, so I want to throw this out there and see if anyone has thoughts on<br>
> > it.<br>
> ><br>
> > _______________________________________________<br>
> > lldb-dev mailing list<br>
> > <a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a><br>
> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br>
> ><br>
> _______________________________________________<br>
> lldb-dev mailing list<br>
> <a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br>
<br>
</blockquote></div>
</div></div><br>_______________________________________________<br>
lldb-dev mailing list<br>
<a href="mailto:lldb-dev@lists.llvm.org">lldb-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">-Todd</div></div>
</div>