Allow for tests to be disabled at runtime

[fjricci]

Summary

The current implementation of the test suite allows the user to run
a certain subset of tests using '-p', but does not allow the inverse,
where a user wants to run all but some number of known failing tests.
Implement this functionality.

[zturner]

If a set of tests is failing, wouldn't you just want to xfail them?

[fjricci]

The issue is that you can only commit a patch to xfail a test that fails when you run the test suite on master with no local changes.

The problem is that if you run into test failures on other branches or in unconventional configurations, there is no good way to disable failing tests, other than carrying local patches to xfail the tests which fail. Carrying these sorts of local patches is tedious, prone to breakages, and requires many manual changes whenever test suite sources changes.

I'm particular, we run into this with ds2, since it fails some tests passed by lldb-server (and passes some tests xfail-ed by lldb-server).

I also find that I fail different tests on master (with lldb-server) between Ubuntu and CentOS, for example, and I'm not sure that it makes sense to xfail in those cases.

[labath]

I don't think this is a totally bad idea. In fact we already had something like this (nobody used it though), before it was removed in rL255040. If it goes in, we might start using it actually -- e.g., currently we have watchpoint tests which fail on some devices which do not support watchpoints. There is no reasonable thing we can base the expectation as the exact same device with a different cpu revision could support watchpoints just fine, so we could just define the list of these tests externally (in this case, I would probably annotate them with the watchpoint category and then do the skips based on categories instead).

That said, I do have slightly mixed feelings about it, as it is increasing the complexity of an already complex system, and there are other possible ways to solve the watchpoint problem (have the tests detect whether the device supports watchpoints, and self-skip when appropriate).

packages/Python/lldbsuite/test/dotest.py

803 ↗

(On Diff #71551)

We should just import re at top level. A lot of tests already do that, so it's not likely it will break anyone.

[fjricci]

Refactor re

[fjricci]

I do understand the complexity problem, and it was one of my concerns with this as well. For my cases, the complexity here is significantly less than the alternatives, but I also do understand if you don't think that's generally true.

It probably comes down to how often we think that people are running the test suite in cases where this sort of functionality would be useful. I don't really have a good sense for how other people tend to use the test suite, so I'm personally not sure. For our case, it's a big deal, but if we're the only people who this patch helps, I know it doesn't make sense to merge it.

[tfiala]

There is no reasonable thing we can base the expectation as the exact same device with a different cpu revision could support watchpoints just fine, so we could just define the list of these tests externally (in this case, I would probably annotate them with the watchpoint category and then do the skips based on categories instead).

Tangential: most chips I've worked on that had hardware watchpoint support had an instruction that could be called to find out if such a feature exists. I think ARM does this. I would think we could expose an API that says whether watchpoints are supported or not, and use that info in LLDB and the test suite to enable or disable them.

I'll look at the rest of the change here. I'm not opposed to the general idea, although if it encourages people to skip running tests, then check in code that breaks those tests, "unbeknownst to them" (* only because they were intentionally not running them), then I'd say that's bad news.

[tfiala]

I am accepting this with one strong reservation which I will explicitly call out here:

• If somebody checks in changes that are broken, and claims they missed it because they have an xfail exclusion file and didn't catch it, I will rip this out. If the xfails are hard to setup, it is likely that this is a code smell for needing better decorators to more precisely home in on the cases that are failing. Often times version checks are helpful.

I do get the utility this would afford for bring-up of different scenarios, though. Hence I see that being useful enough to have it as an escape hatch.

packages/Python/lldbsuite/test/configuration.py

107–108 ↗

(On Diff #71651)

The skip seems okay. The xfail seems *very* dangerous. Nobody else is going to get these xfails. We're setting ourselves up for having people check in tests that are broken. It allows for a workflow where the user "thinks they're done", when they're not.

[fjricci]

In D24629#550823, @tfiala wrote:

There is no reasonable thing we can base the expectation as the exact same device with a different cpu revision could support watchpoints just fine, so we could just define the list of these tests externally (in this case, I would probably annotate them with the watchpoint category and then do the skips based on categories instead).

Tangential: most chips I've worked on that had hardware watchpoint support had an instruction that could be called to find out if such a feature exists. I think ARM does this. I would think we could expose an API that says whether watchpoints are supported or not, and use that info in LLDB and the test suite to enable or disable them.

I believe that PTRACE_GETHBPREGS with a value of 0 returns the hardware stoppoint info on arm, and the byte representing the number of available hardware watchpoints will be 0 if they aren't supported. Not sure if there's a simpler way.

[fjricci]

Ok. Barring objections from anyone else, I'll merge this later on today then, with the understanding that if it causes issues like the ones you describe, it should be reverted.

[jingham]

As long as the only way you can specify the black-list is explicitly on the command line, I think this is fine. There should never be implicit searches for a backlist file. You must have to supply it each time you run the testsuite. That way somebody would have to willfully decide not to run the full testsuite on their patch, and that's a human not a tech problem, since they could just as well check it in with failures they are ignoring, and not need this fancy mechanism...

[labath]

In D24629#550841, @fjricci wrote:

In D24629#550823, @tfiala wrote:

There is no reasonable thing we can base the expectation as the exact same device with a different cpu revision could support watchpoints just fine, so we could just define the list of these tests externally (in this case, I would probably annotate them with the watchpoint category and then do the skips based on categories instead).

Tangential: most chips I've worked on that had hardware watchpoint support had an instruction that could be called to find out if such a feature exists. I think ARM does this. I would think we could expose an API that says whether watchpoints are supported or not, and use that info in LLDB and the test suite to enable or disable them.

I believe that PTRACE_GETHBPREGS with a value of 0 returns the hardware stoppoint info on arm, and the byte representing the number of available hardware watchpoints will be 0 if they aren't supported. Not sure if there's a simpler way.

It's a bit trickier than that. In some cases that call will still return non-zero as the number of supported watchpoints, but the "watchpoint size" field will be zero, and it will still mean that watchpoints don't work. This is probably a kernel bug, though it is pretty easy to work around. The more boring part would be plumbing that information all the way to the test suite - Nothing that can't be done, it's just a bit laborious, so I haven't done that yet.

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

 # Parsable mode silences headers, and any other output this script might generate, and instead
 # prints machine-readable output similar to what clang tests produce.
 parsable = False
 
 # The regular expression pattern to match against eligible filenames as
 # our test cases.
 regexp = None
 
+# Sets of tests which are excluded at runtime
+skip_files = None
+skip_methods = None
+xfail_files = None
+xfail_methods = None
+
 # By default, recorded session info for errored/failed test are dumped into its
 # own file under a session directory named after the timestamp of the test suite
 # run.  Use '-s session-dir-name' to specify a specific dir name.
 sdir_name = None
 
 # Valid options:
 # f - test file name (without extension)
 # n - test class name

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

 from __future__ import absolute_import
 from __future__ import print_function
 
 # System modules
 import atexit
 import os
 import errno
 import platform
+import re
 import signal
 import socket
 import subprocess
 import sys
 
 # Third-party modules
 import six
 import unittest2

 o GDB_REMOTE_LOG: if defined, specifies the log file pathname for the
   'process.gdb-remote' subsystem with a default option of 'packets' if
   GDB_REMOTE_LOG_OPTION is not defined.
 
 """)
     sys.exit(0)
 
 
+def parseExclusion(exclusion_file):
+    """Parse an exclusion file, of the following format, where
+       'skip files', 'skip methods', 'xfail files', and 'xfail methods'
+       are the possible list heading values:
+
+       skip files
+       <file name>
+       <file name>
+
+       xfail methods
+       <method name>
+    """
+    excl_type = None
+    case_type = None
+
+    with open(exclusion_file) as f:
+        for line in f:
+            if not excl_type:
+                [excl_type, case_type] = line.split()
+                continue
+
+            line = line.strip()
+            if not line:
+                excl_type = None
+            elif excl_type == 'skip' and case_type == 'files':
+                if not configuration.skip_files:
+                    configuration.skip_files = []
+                configuration.skip_files.append(line)
+            elif excl_type == 'skip' and case_type == 'methods':
+                if not configuration.skip_methods:
+                    configuration.skip_methods = []
+                configuration.skip_methods.append(line)
+            elif excl_type == 'xfail' and case_type == 'files':
+                if not configuration.xfail_files:
+                    configuration.xfail_files = []
+                configuration.xfail_files.append(line)
+            elif excl_type == 'xfail' and case_type == 'methods':
+                if not configuration.xfail_methods:
+                    configuration.xfail_methods = []
+                configuration.xfail_methods.append(line)
+
+
 def parseOptionsAndInitTestdirs():
     """Initialize the list of directories containing our unittest scripts.
 
     '-h/--help as the first option prints out usage info and exit the program.
     """
 
     do_help = False
 
if args.l:
         configuration.skip_long_running_test = False
 
     if args.framework:
         configuration.lldbFrameworkPath = args.framework
 
     if args.executable:
         lldbtest_config.lldbExec = os.path.realpath(args.executable)
 
+    if args.excluded:
+        parseExclusion(args.excluded)
+
     if args.p:
         if args.p.startswith('-'):
             usage(parser)
         configuration.regexp = args.p
 
     if args.q:
         configuration.parsable = True
 
if lldbPythonDir:
         # This is to locate the lldb.py module.  Insert it right after
         # sys.path[0].
         sys.path[1:1] = [lldbPythonDir]
 
 
 def visit_file(dir, name):
     # Try to match the regexp pattern, if specified.
     if configuration.regexp:
-        import re
         if not re.search(configuration.regexp, name):
             # We didn't match the regex, we're done.
             return
 
+    if configuration.skip_files:
+        for file_regexp in configuration.skip_files:
+            if re.search(file_regexp, name):
+                return
+
     # We found a match for our test.  Add it to the suite.
 
     # Update the sys.path first.
     if not sys.path.count(dir):
         sys.path.insert(0, dir)
     base = os.path.splitext(name)[0]
 
     # Thoroughly check the filterspec against the base module and admit

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

group.add_argument(
         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('--excluded', metavar='exclusion-file', help=textwrap.dedent(
+        '''Specify a file for tests to exclude. File should contain lists of regular expressions for test files or methods,
+                                with each list under a matching header (xfail files, xfail methods, skip files, skip methods)'''))
     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(

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

 
 # System modules
 import inspect
 import os
 
 # Third-party modules
 import unittest2
 
+from unittest2.util import strclass
+
 # LLDB Modules
 from . import configuration
 from lldbsuite.test_event.event_builder import EventBuilder
 from lldbsuite.test_event import build_exception
 
 
 class LLDBTestResult(unittest2.TextTestResult):
     """
def getCategoriesForTest(self, test):
         return test_categories
 
     def hardMarkAsSkipped(self, test):
         getattr(test, test._testMethodName).__func__.__unittest_skip__ = True
         getattr(
             test,
             test._testMethodName).__func__.__unittest_skip_why__ = "test case does not fall in any category of interest for this run"
 
+    def checkExclusion(self, exclusion_list, name):
+        if exclusion_list:
+            import re
+            for item in exclusion_list:
+                if re.search(item, name):
+                    return True
+        return False
+
     def startTest(self, test):
         if configuration.shouldSkipBecauseOfCategories(
                 self.getCategoriesForTest(test)):
             self.hardMarkAsSkipped(test)
+        if self.checkExclusion(
+                configuration.skip_methods,
+                test._testMethodName):
+            self.hardMarkAsSkipped(test)
+
         configuration.setCrashInfoHook(
             "%s at %s" %
             (str(test), inspect.getfile(
                 test.__class__)))
         self.counter += 1
         # if self.counter == 4:
         #    import crashinfo
         #    crashinfo.testCrashReporterDescription(None)
         test.test_number = self.counter
         if self.showAll:
             self.stream.write(self.fmt % self.counter)
         super(LLDBTestResult, self).startTest(test)
         if self.results_formatter:
             self.results_formatter.handle_event(
                 EventBuilder.event_for_start(test))
 
     def addSuccess(self, test):
+        if self.checkExclusion(
+                configuration.xfail_files,
+                strclass(
+                    test.__class__)) or self.checkExclusion(
+                configuration.xfail_methods,
+                test._testMethodName):
+            self.addUnexpectedSuccess(test, None)
+            return
+
         super(LLDBTestResult, self).addSuccess(test)
         if configuration.parsable:
             self.stream.write(
                 "PASS: LLDB (%s) :: %s\n" %
                 (self._config_string(test), str(test)))
         if self.results_formatter:
             self.results_formatter.handle_event(
                 EventBuilder.event_for_success(test))
def addCleanupError(self, test, err):
                 "CLEANUP ERROR: LLDB (%s) :: %s\n" %
                 (self._config_string(test), str(test)))
         if self.results_formatter:
             self.results_formatter.handle_event(
                 EventBuilder.event_for_cleanup_error(
                     test, err))
 
     def addFailure(self, test, err):
+        if self.checkExclusion(
+                configuration.xfail_files,
+                strclass(
+                    test.__class__)) or self.checkExclusion(
+                configuration.xfail_methods,
+                test._testMethodName):
+            self.addExpectedFailure(test, err, None)
+            return
+
         configuration.sdir_has_content = True
         super(LLDBTestResult, self).addFailure(test, err)
         method = getattr(test, "markFailure", None)
         if method:
             method()
         if configuration.parsable:
             self.stream.write(
                 "FAIL: LLDB (%s) :: %s\n" %