Make debug info specification use categories system

[zturner]

Summary

I'm not sure if I did this right, but the end result of this CL is removal of the -N command line option, and making people go through the categories system instead.

[zturner]

Actually remove the -N command line option as well.

[labath]

The interaction between the add_test_categories and the "magic test multiplier" is causing problems, which is evident in the fact how you needed to add the @skip decorator to make it work.

It sounds like we need to decide what will be the "correct" interaction of the "test multiplier" and tests manually marked with a specific debug info.

I think the most sensible behavior would be to treat a manual debuginfo mark as a signal that you don't want your test to be auto-multiplied, the test multiplier could then check whether the test is already marked with one of the debuginfo annotations, and if it is, then avoid touching it.

What do you think?

packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py

120 ↗

(On Diff #42452)

You need [] around the "dwarf". Otherwise, you will get errors here, when the skipIf below does not actually skip.

packages/Python/lldbsuite/test/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py

10 ↗

(On Diff #42452)

It should not be necessary to annotate and skip at the same time.

packages/Python/lldbsuite/test/lldbtest.py

2226 ↗

(On Diff #42452)

Here, we would then also check getCategoriesForTest(attrvalue) for presence of debug info markers. It looks like you will need to move that function from test_result.py to somewhere else though...

2230 ↗

(On Diff #42452)

might as well use getDarwinOSTriples() here

2233 ↗

(On Diff #42452)

This annotation will overwrite any previous category annotations that the user has previously added. I wasn't sure whether we wanted this behavior when I was writing the decorator, but now that we have these automatic categorizations, it definitely sounds like a bad idea. add_test_categories should really "add" categories to the test.

Since this is quite orthogonal to your patch, I'll whip up a separate patch to do that.

[labath]

packages/Python/lldbsuite/test/lldbtest.py

2226 ↗

(On Diff #42452)

Actually, getCategoriesForTest(attrvalue) will not work here, as the test is not running and the class is not constructed yet. After D15451 you should be able to do something like this here:

categories = getattr(attrvalue, "categories", [])
if not set(categories).intersection(["dwarf", "dsym", "dwo"]):
  categories = ["dwarf", "dsym", "dwo"]
dont_do_dsym_test = any(platform in target_platform for platform in ["linux", "freebsd", "windows"]) or ("dsym" not in categories)
dont_do_dwo_test = any(platform in target_platform for platform in getDarwinOSTriples()) or ("dwo" not in categories)
dont_do_dwarf_test = "dwarf" not in categories

This way, if the test is categorized with a debug info marker, we use that info to choose which tests to generate. If the test is not categorized, we just use the default platform rules.

Zach, Tamas, how does that sound?

2233 ↗

(On Diff #42452)

I have this up it D15451.

[zturner]

In D15428#308131, @labath wrote:

The interaction between the add_test_categories and the "magic test multiplier" is causing problems, which is evident in the fact how you needed to add the @skip decorator to make it work.

It sounds like we need to decide what will be the "correct" interaction of the "test multiplier" and tests manually marked with a specific debug info.

I think the most sensible behavior would be to treat a manual debuginfo mark as a signal that you don't want your test to be auto-multiplied, the test multiplier could then check whether the test is already marked with one of the debuginfo annotations, and if it is, then avoid touching it.

In hindsight I wonder if I even needed to manually mark the test with a debug info. Maybe we can say that tests should *never* be manually marked with a debug info category, and only use the skip decorators. That should still work, because it will be auto-multiplied to every debug format, and then some of them will be skipped.

[zturner]

Updated with comments from previous review. Also change the multiplication logic to use a loop instead of writing each one separately.

[tberghammer]

packages/Python/lldbsuite/test/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py

9 ↗

(On Diff #42550)

Considering your change in lldbtest.py:2227 I think you should annotate the function with add_test_categories(["dwarf"]) instead

packages/Python/lldbsuite/test/lldbtest.py

2227–2228 ↗

(On Diff #42550)

I think this won't work if the list of debug info formats are specified on class level instead of at function level.

[labath]

In D15428#308401, @zturner wrote:

In hindsight I wonder if I even needed to manually mark the test with a debug info. Maybe we can say that tests should *never* be manually marked with a debug info category, and only use the skip decorators. That should still work, because it will be auto-multiplied to every debug format, and then some of them will be skipped.

I quite like that. @dwarf_test essentially means "skip if debug info is not dwarf", so why not state that explicitly... I'm not going to ask you to rewrite it again, but if you feel like it, then by all means go for it.

packages/Python/lldbsuite/test/lldbtest.py

2227–2228 ↗

(On Diff #42550)

That's true, but given that it's difficult to access the class annotations from this context, I'm fine with living with this "defect", at least for now. Although, it sounds like that if we go with the option Zachary proposes, this problem would go away...

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

if platform.system() == "Darwin":
         setCrashInfoHook = __setCrashInfoHook_Mac
 
     else:
         pass
 
 # The test suite.
 suite = unittest2.TestSuite()
 
-dont_do_dsym_test = False
-dont_do_dwarf_test = False
-dont_do_dwo_test = False
-
 # The list of categories we said we care about
 categoriesList = None
 # set to true if we are going to use categories for cherry-picking test cases
 useCategories = False
 # Categories we want to skip
 skipCategories = []
 # use this to track per-category failures
 failuresPerCategory = {}

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

def parseOptionsAndInitTestdirs():
 
     if args.skipCategories:
         configuration.skipCategories = test_categories.validate(args.skipCategories, False)
 
     if args.E:
         cflags_extras = args.E
         os.environ['CFLAGS_EXTRAS'] = cflags_extras
 
-    # argparse makes sure we have correct options
-    if args.N == 'dwarf':
-        configuration.dont_do_dwarf_test = True
-    elif args.N == 'dwo':
-        configuration.dont_do_dwo_test = True
-    elif args.N == 'dsym':
-        configuration.dont_do_dsym_test = True
-
     if args.d:
         sys.stdout.write("Suspending the process %d to wait for debugger to attach...\n" % os.getpid())
         sys.stdout.flush()
         os.kill(os.getpid(), signal.SIGSTOP)
 
     if args.f:
         if any([x.startswith('-') for x in args.f]):
             usage(parser)
if configuration.lldb_platform_working_dir:
         lldb.DBG.SetSelectedPlatform(lldb.remote_platform)
     else:
         lldb.remote_platform = None
         configuration.lldb_platform_working_dir = None
         configuration.lldb_platform_url = None
 
     target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
 
-    # By default, both dsym, dwarf and dwo tests are performed.
-    # Use @dsym_test, @dwarf_test or @dwo_test decorators, defined in lldbtest.py, to mark a test as
-    # a dsym, dwarf or dwo test.  Use '-N dsym', '-N dwarf' or '-N dwo' to exclude dsym, dwarf or
-    # dwo tests from running.
-    configuration.dont_do_dsym_test = configuration.dont_do_dsym_test \
-        or any(platform in target_platform for platform in ["linux", "freebsd", "windows"])
-    configuration.dont_do_dwo_test = configuration.dont_do_dwo_test \
-        or any(platform in target_platform for platform in ["darwin", "macosx", "ios"])
-
     # Don't do debugserver tests on everything except OS X.
     configuration.dont_do_debugserver_test = "linux" in target_platform or "freebsd" in target_platform or "windows" in target_platform
 
     # Don't do lldb-server (llgs) tests on anything except Linux.
     configuration.dont_do_llgs_test = not ("linux" in target_platform)
 
     #
     # Walk through the testdirs while collecting tests.

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

def create_parser():
     if sys.platform == 'darwin':
         group.add_argument('--apple-sdk', metavar='apple_sdk', dest='apple_sdk', help=textwrap.dedent('''Specify the name of the Apple SDK (macosx, macosx.internal, iphoneos, iphoneos.internal, or path to SDK) and use the appropriate tools from that SDK's toolchain.'''))
     # FIXME? This won't work for different extra flags according to each arch.
     group.add_argument('-E', metavar='extra-flags', help=textwrap.dedent('''Specify the extra flags to be passed to the toolchain when building the inferior programs to be debugged
                                                            suggestions: do not lump the "-A arch1 -A arch2" together such that the -E option applies to only one of the architectures'''))
 
     # Test filtering options
     group = parser.add_argument_group('Test filtering options')
-    group.add_argument('-N', choices=['dwarf', 'dwo', 'dsym'], help="Don't do test cases marked with the @dsym_test/@dwarf_test/@dwo_test decorator by passing dsym/dwarf/dwo as the option arg")
     group.add_argument('-f', metavar='filterspec', action='append', help='Specify a filter, which consists of the test class name, a dot, followed by the test method, to only admit such test into the test suite')  # FIXME: Example?
     X('-l', "Don't skip long running tests")
     group.add_argument('-p', metavar='pattern', help='Specify a regexp filename pattern for inclusion in the test suite')
     group.add_argument('-G', '--category', metavar='category', action='append', dest='categoriesList', help=textwrap.dedent('''Specify categories of test cases of interest. Can be specified more than once.'''))
     group.add_argument('--skip-category', metavar='category', action='append', dest='skipCategories', help=textwrap.dedent('''Specify categories of test cases to skip. Takes precedence over -G. Can be specified more than once.'''))
 
     # Configuration options
     group = parser.add_argument_group('Configuration options')

lldb/trunk/packages/Python/lldbsuite/test/functionalities/unwind/standard/TestStandardUnwind.py

else:
         dirname = os.path.join(os.path.dirname(__file__), d)
 
     for root, _, files in os.walk(dirname):
         test_source_files = test_source_files | set(os.path.abspath(os.path.join(root, f)) for f in files)
 
 # Generate test cases based on the collected source files
 for f in test_source_files:
     if f.endswith(".cpp") or f.endswith(".c"):
-        @dwarf_test
+        @add_test_categories(["dwarf"])
         @unittest2.skipIf(TestBase.skipLongRunningTest(), "Skip this long running test")
         def test_function_dwarf(self, f=f):
             if f.endswith(".cpp"):
                 d = {'CXX_SOURCES': f}
             elif f.endswith(".c"):
                 d = {'C_SOURCES': f}
 
             # If we can't compile the inferior just skip the test instead of failing it.

lldb/trunk/packages/Python/lldbsuite/test/lang/c/typedef/Testtypedef.py

 """Look up type information for typedefs of same name at different lexical scope and check for correct display."""
 
 from __future__ import print_function
 
 
 
 import os, time
 import lldb
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
 class TypedefTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @skipUnlessDarwin
-    @dsym_test
-    @expectedFailureClang("llvm.org/pr19238")
-    def test_with_dsym(self):
+    @expectedFailureAll(bugnumber="llvm.org/pr19238", compiler="clang")
+    @expectedFailureAll(bugnumber="llvm.org/pr25626 expectedFailureClang fails on FreeBSD", oslist=["freebsd"])
+    def test_typedef(self):
         """Test 'image lookup -t a' and check for correct display at different scopes."""
-        self.buildDsym()
-        self.image_lookup_for_multiple_typedefs()
-
-    @dwarf_test
-    @expectedFailureClang("llvm.org/pr19238")
-    @expectedFailureFreeBSD("llvm.org/pr25626 expectedFailureClang fails on FreeBSD")
-    def test_with_dwarf(self):
-        """Test 'image lookup -t a' and check for correct display at different scopes."""
-        self.buildDwarf()
+        self.build()
         self.image_lookup_for_multiple_typedefs()
 
     def image_lookup_for_multiple_typedefs(self):
         """Test 'image lookup -t a' at different scopes and check for correct display."""
         exe = os.path.join(os.getcwd(), "a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
         typearray = ("float", "float", "char", "double *", "float", "int", "double", "float", "float")
         arraylen = len(typearray)+1

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py

 import lldb
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
 class TestWithLimitDebugInfo(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @dwarf_test
-    def test_with_dwarf(self):
-        self.buildDwarf()
+    @skipIf(debug_info=not_in(["dwarf"]))
+    def test_limit_debug_info(self):
+        self.build()
 
         cwd = os.getcwd()
 
         src_file = os.path.join(cwd, "main.cpp")
         src_file_spec = lldb.SBFileSpec(src_file)
         self.assertTrue(src_file_spec.IsValid(), "breakpoint file")
 
         # Get the path of the executable

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

def no_debug_info_test(func):
     @wraps(func)
     def wrapper(self, *args, **kwargs):
         return func(self, *args, **kwargs)
 
     # Mark this function as such to separate them from the regular tests.
     wrapper.__no_debug_info_test__ = True
     return wrapper
 
-def dsym_test(func):
-    """Decorate the item as a dsym test."""
-    if isinstance(func, type) and issubclass(func, unittest2.TestCase):
-        raise Exception("@dsym_test can only be used to decorate a test method")
-    @wraps(func)
-    def wrapper(self, *args, **kwargs):
-        if configuration.dont_do_dsym_test:
-            self.skipTest("dsym tests")
-        return func(self, *args, **kwargs)
-
-    # Mark this function as such to separate them from the regular tests.
-    wrapper.__dsym_test__ = True
-    return wrapper
-
-def dwarf_test(func):
-    """Decorate the item as a dwarf test."""
-    if isinstance(func, type) and issubclass(func, unittest2.TestCase):
-        raise Exception("@dwarf_test can only be used to decorate a test method")
-    @wraps(func)
-    def wrapper(self, *args, **kwargs):
-        if configuration.dont_do_dwarf_test:
-            self.skipTest("dwarf tests")
-        return func(self, *args, **kwargs)
-
-    # Mark this function as such to separate them from the regular tests.
-    wrapper.__dwarf_test__ = True
-    return wrapper
-
-def dwo_test(func):
-    """Decorate the item as a dwo test."""
-    if isinstance(func, type) and issubclass(func, unittest2.TestCase):
-        raise Exception("@dwo_test can only be used to decorate a test method")
-    @wraps(func)
-    def wrapper(self, *args, **kwargs):
-        if configuration.dont_do_dwo_test:
-            self.skipTest("dwo tests")
-        return func(self, *args, **kwargs)
-
-    # Mark this function as such to separate them from the regular tests.
-    wrapper.__dwo_test__ = True
-    return wrapper
-
 def debugserver_test(func):
     """Decorate the item as a debugserver test."""
     if isinstance(func, type) and issubclass(func, unittest2.TestCase):
         raise Exception("@debugserver_test can only be used to decorate a test method")
     @wraps(func)
     def wrapper(self, *args, **kwargs):
         if configuration.dont_do_debugserver_test:
             self.skipTest("debugserver tests")

 # We change the test methods to create a new test method for each test for each debug info we are
 # testing. The name of the new test method will be '<original-name>_<debug-info>' and with adding
 # the new test method we remove the old method at the same time.
 class LLDBTestCaseFactory(type):
     def __new__(cls, name, bases, attrs):
         newattrs = {}
         for attrname, attrvalue in attrs.items():
             if attrname.startswith("test") and not getattr(attrvalue, "__no_debug_info_test__", False):
-                @dsym_test
-                @wraps(attrvalue)
-                def dsym_test_method(self, attrvalue=attrvalue):
-                    self.debug_info = "dsym"
-                    return attrvalue(self)
-                dsym_method_name = attrname + "_dsym"
-                dsym_test_method.__name__ = dsym_method_name
-                newattrs[dsym_method_name] = dsym_test_method
-
-                @dwarf_test
-                @wraps(attrvalue)
-                def dwarf_test_method(self, attrvalue=attrvalue):
-                    self.debug_info = "dwarf"
-                    return attrvalue(self)
-                dwarf_method_name = attrname + "_dwarf"
-                dwarf_test_method.__name__ = dwarf_method_name
-                newattrs[dwarf_method_name] = dwarf_test_method
-                
-                @dwo_test
+                target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
+                # If any debug info categories were explicitly tagged, assume that list to be
+                # authoritative.  If none were specified, try with all debug info formats.
+                all_dbginfo_categories = set(test_categories.debug_info_categories)
+                categories = set(getattr(attrvalue, "categories", [])) & all_dbginfo_categories
+                if not categories:
+                    categories = all_dbginfo_categories
+
+                supported_categories = [x for x in categories 
+                                        if test_categories.is_supported_on_platform(x, target_platform)]
+                for category in supported_categories:
+                    @add_test_categories([category])
                     @wraps(attrvalue)
-                def dwo_test_method(self, attrvalue=attrvalue):
-                    self.debug_info = "dwo"
+                    def test_method(self, attrvalue=attrvalue):
+                        self.debug_info = category
                         return attrvalue(self)
-                dwo_method_name = attrname + "_dwo"
-                dwo_test_method.__name__ = dwo_method_name
-                newattrs[dwo_method_name] = dwo_test_method
+                    method_name = attrname + "_" + category
+                    test_method.__name__ = method_name
+                    newattrs[method_name] = test_method
             else:
                 newattrs[attrname] = attrvalue
         return super(LLDBTestCaseFactory, cls).__new__(cls, name, bases, newattrs)
 
 # Setup the metaclass for this class to change the list of the test methods when a new class is loaded
 @add_metaclass(LLDBTestCaseFactory)
 class TestBase(Base):
     """

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

 """
 Provides definitions for various lldb test categories
 """
 
 from __future__ import absolute_import
 from __future__ import print_function
 
 # System modules
 import sys
 
 # Third-party modules
 
 # LLDB modules
 
+debug_info_categories = [
+    'dwarf', 'dwo', 'dsym'
+]
+
 all_categories = {
     'dataformatters': 'Tests related to the type command and the data formatters subsystem',
+    'dwarf'         : 'Tests that can be run with DWARF debug information',
+    'dwo'           : 'Tests that can be run with DWO debug information',
+    'dsym'          : 'Tests that can be run with DSYM debug information',
     'expression'    : 'Tests related to the expression parser',
     'objc'          : 'Tests related to the Objective-C programming language support',
     'pyapi'         : 'Tests related to the Python API',
     'basic_process' : 'Basic process execution sniff tests.',
     'cmdline'       : 'Tests related to the LLDB command-line interface',
     'dyntype'       : 'Tests related to dynamic type support',
     'stresstest'    : 'Tests related to stressing lldb limits',
     'flakey'        : 'Flakey test cases, i.e. tests that do not reliably pass at each execution',
     'lldb-mi'       : 'lldb-mi tests'
 }
 
 def unique_string_match(yourentry, list):
     candidate = None
     for item in list:
         if not item.startswith(yourentry):
             continue
         if candidate:
             return None
         candidate = item
     return candidate
 
+def is_supported_on_platform(category, platform):
+    if category == "dwo":
+        return platform in ["linux", "freebsd", "windows"]
+    elif category == "dsym":
+        return platform in ["darwin", "macosx", "ios"]
+    return True
+
 def validate(categories, exact_match):
     """
     For each category in categories, ensure that it's a valid category (if exact_match is false,
     unique prefixes are also accepted). If a category is invalid, print a message and quit.
        If all categories are valid, return the list of categories. Prefixes are expanded in the
        returned list.
     """
     result = []