Page MenuHomePhabricator

dotest: make inline tests compatible with -f
ClosedPublic

Authored by labath on May 31 2018, 3:22 AM.

Details

Summary

This is split off from D47265 where I needed to be able to invoke every test
with -f. That patch is kinda dead now, but this part seems like a good
cleanup anyway.

The problem with inline tests was in the way we were adding methods to
the class, which left them with an incorrect name property. This
prevented dotest from finding them with -f.

I fix this with (what I think is) the correct way of dynamically
creating classes -- passing the list of methods during type construction
instead of fixing up the class afterwards. Among other things this has
the advantage of not needing to do anything special for debug info
variants. As our test method will be visible to the metaclass, it will
automagically to the multiplication for us.

Event Timeline

labath created this revision.May 31 2018, 3:22 AM

If I correctly understand this change, this might make it possible to apply the add_test_categories decorator to an inline test. We had issues with this when introducing the swiftpr category because the methods were part of the inline test class. From the description I get the idea that now they become part of the individual instantiations? I really hope this is the case :-)

Regardless of my (potential) excitement, this looks good to me!

packages/Python/lldbsuite/test/lldbinline.py
125–126

Will the decorators ensure that this value is properly set (or ignored) for the non-dsym tests?

labath updated this revision to Diff 149269.May 31 2018, 5:40 AM
  • obliterate using_dsym property
  • reduce confusion in the MakeInlineTest function (variable "test" used for multiple things)

If I correctly understand this change, this might make it possible to apply the add_test_categories decorator to an inline test. We had issues with this when introducing the swiftpr category because the methods were part of the inline test class. From the description I get the idea that now they become part of the individual instantiations? I really hope this is the case :-)

I don't think this will have any affect on that as this still applies the decorators to the base class. However, I have an idea what might solve that.

Can you try inserting something like this instead of the ApplyDecoratorsToFunction line and see if your problems go away?

@wraps(InlineTest._test)
def test_func(*args, **kwargs):
    return InlineTest._test(*args, **kwargs)

test_func = ApplyDecoratorsToFunction(test_func, decorators)

This should make sure each test class gets a fresh function object with an independent set of categories.

packages/Python/lldbsuite/test/lldbinline.py
125–126

Ah, good catch. No the decorators won't do that. However, this property is only used in getRerunArgs.
In fact it turns out that after this patch we don't even need to override getRerunArgs for inline tests as the default implementation (which just appends -f class.method) will work just fine for inline tests as well (or at least, it won't be any worse).

So I just obliterate every mention of this property.

JDevlieghere accepted this revision.Jun 4 2018, 9:30 AM

LGTM

Can you try inserting something like this instead of the ApplyDecoratorsToFunction line and see if your problems go away?

@wraps(InlineTest._test)
def test_func(*args, **kwargs):
    return InlineTest._test(*args, **kwargs)

test_func = ApplyDecoratorsToFunction(test_func, decorators)

This should make sure each test class gets a fresh function object with an independent set of categories.

Alright, I'll give that a shot. Thanks for the suggestion! :-)

This revision is now accepted and ready to land.Jun 4 2018, 9:30 AM
This revision was automatically updated to reflect the committed changes.