[lldb-dev] lldb tests and tear down hooks

[Zachary Turner via lldb-dev]

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 <gclayton at 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 at 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 <labath at 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 at 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 at lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> > >
> > _______________________________________________
> > lldb-dev mailing list
> > lldb-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20151021/c0a15c88/attachment.html>