dotest: make inline tests compatible with -f

[labath]

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.

[JDevlieghere]

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

139 ↗

(On Diff #149257)

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

[labath]

• obliterate using_dsym property
• reduce confusion in the MakeInlineTest function (variable "test" used for multiple things)

[labath]

In D47579#1117457, @JDevlieghere wrote:

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

139 ↗

(On Diff #149257)

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]

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! :-)

lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py

def handle_breakpoint(self, test, breakpoint_id):
             if breakpoint['breakpoint'].GetID() == breakpoint_id:
                 test.execute_user_command(breakpoint['command'])
                 return
 
 
 class InlineTest(TestBase):
     # Internal implementation
 
-    def getRerunArgs(self):
-        # The -N option says to NOT run a if it matches the option argument, so
-        # if we are using dSYM we say to NOT run dwarf (-N dwarf) and vice
-        # versa.
-        if self.using_dsym is None:
-            # The test was skipped altogether.
-            return ""
-        elif self.using_dsym:
-            return "-N dwarf " + self.mydir
-        else:
-            return "-N dsym " + self.mydir
-
     def BuildMakefile(self):
         makefilePath = self.getBuildArtifact("Makefile")
         if os.path.exists(makefilePath):
             return
 
         categories = {}
 
         for f in os.listdir(self.getSourceDir()):
def BuildMakefile(self):
         if ('CXX_SOURCES' in list(categories.keys())):
             makefile.write("CXXFLAGS += -std=c++11\n")
 
         makefile.write("include $(LEVEL)/Makefile.rules\n")
         makefile.write("\ncleanup:\n\trm -f Makefile *.d\n\n")
         makefile.flush()
         makefile.close()
 
-    @add_test_categories(["dsym"])
-    def __test_with_dsym(self):
-        self.using_dsym = True
-        self.BuildMakefile()
-        self.build()
-        self.do_test()
-    __test_with_dsym.debug_info = "dsym"
-
-    @add_test_categories(["dwarf"])
-    def __test_with_dwarf(self):
-        self.using_dsym = False
-        self.BuildMakefile()
-        self.build()
-        self.do_test()
-    __test_with_dwarf.debug_info = "dwarf"
-
-    @add_test_categories(["dwo"])
-    def __test_with_dwo(self):
-        self.using_dsym = False
-        self.BuildMakefile()
-        self.build()
-        self.do_test()
-    __test_with_dwo.debug_info = "dwo"
-
-    @add_test_categories(["gmodules"])
-    def __test_with_gmodules(self):
-        self.using_dsym = False
+    def _test(self):
         self.BuildMakefile()
         self.build()
         self.do_test()
-    __test_with_gmodules.debug_info = "gmodules"
 
     def execute_user_command(self, __command):
         exec(__command, globals(), locals())
 
     def do_test(self):
         exe = self.getBuildArtifact("a.out")
         source_files = [f for f in os.listdir(self.getSourceDir())
                         if source_type(f)]
if __file is not None and __file.endswith(".pyc"):
         # Strip the trailing "c"
         __file = __file[0:-1]
 
     # Derive the test name from the current file name
     file_basename = os.path.basename(__file)
     InlineTest.mydir = TestBase.compute_mydir(__file)
 
     test_name, _ = os.path.splitext(file_basename)
-    # Build the test case
-    test = type(test_name, (InlineTest,), {'using_dsym': None})
-    test.name = test_name
 
-    test.test_with_dsym = ApplyDecoratorsToFunction(
-        test._InlineTest__test_with_dsym, decorators)
-    test.test_with_dwarf = ApplyDecoratorsToFunction(
-        test._InlineTest__test_with_dwarf, decorators)
-    test.test_with_dwo = ApplyDecoratorsToFunction(
-        test._InlineTest__test_with_dwo, decorators)
-    test.test_with_gmodules = ApplyDecoratorsToFunction(
-        test._InlineTest__test_with_gmodules, decorators)
+    test_func = ApplyDecoratorsToFunction(InlineTest._test, decorators)
+    # Build the test case
+    test_class = type(test_name, (InlineTest,), dict(test=test_func, name=test_name))
 
     # Add the test case to the globals, and hide InlineTest
-    __globals.update({test_name: test})
+    __globals.update({test_name: test_class})
 
     # Keep track of the original test filename so we report it
     # correctly in test results.
-    test.test_filename = __file
-    return test
+    test_class.test_filename = __file
+    return test_class