A number of improvements to decorator conditionals

[zturner]

Summary

The goal here is to try to improve consistency across the variety of decorators we support. This patch makes the following changes:

• Change the not_in function to be a class object called no_match. This indicates that it can be used for more than just checking list membership, and that instead it negates whatever condition is used to normally match.
• Change the name of _check_list_or_lambda to match_decorator_property. This indicates that it could be more than a list or a lambda.
• Always use a regex match when matching strings in match_decorator_property. This allows any string on any decoratory property to automatically support regex matching.
• Also support the use of compiled regexes on decorator properties.
• Fix a bug in the compiler check used by _decorateTest. The two arguments were reversed, so that check was not matching when it should have matched.
• Change one test that uses skipUnlessArch to use skipIf. Now that skipIf supports regex matching on the architecture property, this works, and as a followup skipUnlessArch will be removed entirely.

Note that you won't be able to apply this patch unless you apply it on top of my previous review D16936.

[zturner]

Minor change that allows any iterable to be used (not just lists), and also recurses through the _match_decorator_property function for each item in the iterable, instead of just doing a simple list membership. Combined with the regex change and the no_match change from function to object, this allows complex specifications such as archs=['mips.*', 'i[0-9]86', no_match('x86_x64)'] (not that that particular condition makes sense, but might as well support it).

[tfiala]

I'll have a look at this after I get through D16936.

Concept sounds great.

[labath]

Looks good, after applying one fix.

packages/Python/lldbsuite/test/decorators.py

148 ↗

(On Diff #47068)

The actual value should be simply lldb.remote_platform is not None. When remote_platform is None, we are local, when it is not None, we are remote.

Btw, since this flag can only have three values (None, True, False), the _match_decorator property dance is not really necessary. Although, I guess it won't hurt either...

[tfiala]

LGTM.

Regarding the section Pavel was looking at, on the Apple side we have a simulator target that is not technically remote, but isn't really local either. I'll have to look at that if/when we actually want to mark a simulator test as failed.

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

 from . import test_categories
 from .result_formatter import EventBuilder
 from lldbsuite.support import funcutils
 from lldbsuite.test import lldbplatformutil
 
 class DecorateMode:
     Skip, Xfail = range(2)
 
     
+# You can use no_match to reverse the test of the conditional that is used to match keyword
+# arguments in the skip / xfail decorators.  If oslist=["windows", "linux"] skips windows
+# and linux, oslist=no_match(["windows", "linux"]) skips *unless* windows or linux.
+class no_match:
+    def __init__(self, item):
+        self.item = item
+
 def _check_expected_version(comparison, expected, actual):
     def fn_leq(x,y): return x <= y
     def fn_less(x,y): return x < y
     def fn_geq(x,y): return x >= y
     def fn_greater(x,y): return x > y
     def fn_eq(x,y): return x == y
     def fn_neq(x,y): return x != y
 
     op_lookup = {
         "==": fn_eq,
         "=": fn_eq,
         "!=": fn_neq,
         "<>": fn_neq,
         ">": fn_greater,
         "<": fn_less,
         ">=": fn_geq,
         "<=": fn_leq
         }
     expected_str = '.'.join([str(x) for x in expected])
     actual_str = '.'.join([str(x) for x in actual])
 
     return op_lookup[comparison](LooseVersion(actual_str), LooseVersion(expected_str))
 
-def _check_list_or_lambda(list_or_lambda, value):
-    if six.callable(list_or_lambda):
-        return list_or_lambda(value)
-    elif isinstance(list_or_lambda, list):
-        for item in list_or_lambda:
-            if value in item:
-                return True
-        return False
-    elif isinstance(list_or_lambda, str):
-        return value is None or value in list_or_lambda
+def _match_decorator_property(expected, actual):
+    if actual is None or expected is None:
+
+    if isinstance(expected, no_match):
+        return not _match_decorator_property(expected.item, actual)
+    elif isinstance(expected, (str, re._pattern_type)):
+        return re.search(expected, actual) is not None
+    elif hasattr(expected, "__iter__"):
+        return any([x is not None and _match_decorator_property(x, actual) for x in expected])
     else:
-        return list_or_lambda is None or value is None or list_or_lambda == value
+        return expected == actual
 
 def expectedFailure(expected_fn, bugnumber=None):
     def expectedFailure_impl(func):
         if isinstance(func, type) and issubclass(func, unittest2.TestCase):
             raise Exception("Decorator can only be used to decorate a test method")
         @wraps(func)
         def wrapper(*args, **kwargs):
             from unittest2 import case

 def _decorateTest(mode,
                  bugnumber=None, oslist=None, hostoslist=None,
                  compiler=None, compiler_version=None,
                  archs=None, triple=None,
                  debug_info=None,
                  swig_version=None, py_version=None,
                  remote=None):
     def fn(self):
-        skip_for_os = _check_list_or_lambda(oslist, self.getPlatform())
-        skip_for_hostos = _check_list_or_lambda(hostoslist, lldbplatformutil.getHostPlatform())
-        skip_for_compiler = _check_list_or_lambda(self.getCompiler(), compiler) and self.expectedCompilerVersion(compiler_version)
-        skip_for_arch = _check_list_or_lambda(archs, self.getArchitecture())
-        skip_for_debug_info = _check_list_or_lambda(debug_info, self.debug_info)
-        skip_for_triple = triple is None or re.match(triple, lldb.DBG.GetSelectedPlatform().GetTriple())
+        skip_for_os = _match_decorator_property(oslist, self.getPlatform())
+        skip_for_hostos = _match_decorator_property(hostoslist, lldbplatformutil.getHostPlatform())
+        skip_for_compiler = _match_decorator_property(compiler, self.getCompiler()) and self.expectedCompilerVersion(compiler_version)
+        skip_for_arch = _match_decorator_property(archs, self.getArchitecture())
+        skip_for_debug_info = _match_decorator_property(debug_info, self.debug_info)
+        skip_for_triple = _match_decorator_property(triple, lldb.DBG.GetSelectedPlatform().GetTriple())
+        skip_for_remote = _match_decorator_property(remote, lldb.remote_platform is not None)
+
         skip_for_swig_version = (swig_version is None) or (not hasattr(lldb, 'swig_version')) or (_check_expected_version(swig_version[0], swig_version[1], lldb.swig_version))
         skip_for_py_version = (py_version is None) or _check_expected_version(py_version[0], py_version[1], sys.version_info)
-        skip_for_remote = (remote is None) or (remote == (lldb.remote_platform is not None))
 
         # For the test to be skipped, all specified (e.g. not None) parameters must be True.
         # An unspecified parameter means "any", so those are marked skip by default.  And we skip
         # the final test if all conditions are True.
         conditions = [(oslist, skip_for_os, "target o/s"),
                       (hostoslist, skip_for_hostos, "host o/s"),
                       (compiler, skip_for_compiler, "compiler or version"),
                       (archs, skip_for_arch, "architecture"),
def llgs_test(func):
     return skipTestIfFn(should_skip_llgs_tests)(func)
 
 def not_remote_testsuite_ready(func):
     """Decorate the item as a test which is not ready yet for remote testsuite."""
     def is_remote():
         return "Not ready for remote testsuite" if lldb.remote_platform else None
     return skipTestIfFn(is_remote)(func)
 
-# You can also pass not_in(list) to reverse the sense of the test for the arguments that
-# are simple lists, namely oslist, compiler, and debug_info.
-
-def not_in(iterable):
-    return lambda x : x not in iterable
-
 def _match_architectures(archs, actual_arch):
     retype = type(re.compile('hello, world'))
     list_passes = isinstance(archs, list) and actual_arch in archs
     basestring_passes = isinstance(archs, six.string_types) and actual_arch == archs
     regex_passes = isinstance(archs, retype) and re.match(archs, actual_arch)
 
     return (list_passes or basestring_passes or regex_passes)
 
def skipIfHostIncompatibleWithRemote(func):
     return skipTestIfFn(is_host_incompatible_with_remote)(func)
 
 def skipIfHostPlatform(oslist):
     """Decorate the item to skip tests if running on one of the listed host platforms."""
     return skipIf(hostoslist=oslist)
 
 def skipUnlessHostPlatform(oslist):
     """Decorate the item to skip tests unless running on one of the listed host platforms."""
-    return skipIf(hostoslist=not_in(oslist))
+    return skipIf(hostoslist=no_match(oslist))
 
 def skipUnlessArch(archs):
     """Decorate the item to skip tests unless running on one of the listed architectures."""
     # This decorator cannot be ported to `skipIf` yet because it is uused with regular
     # expressions, which the common matcher does not yet support.
     def myImpl(func):
         if isinstance(func, type) and issubclass(func, unittest2.TestCase):
             raise Exception("@skipUnlessArch can only be used to decorate a test method")

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_in_delayslot/TestAvoidBreakpointInDelaySlot.py

 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 class AvoidBreakpointInDelaySlotAPITestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @skipUnlessArch(archs=re.compile('mips*'))
+    @skipIf(archs=no_match(re.compile('mips*')))
     def test(self):
         self.build()
         exe = os.path.join(os.getcwd(), "a.out")
         self.expect("file " + exe,
                     patterns = [ "Current executable set to .*a.out.*" ])
         
         # Create a target by the debugger.
         target = self.dbg.CreateTarget(exe)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/debugbreak/TestDebugBreak.py

 """
 Test embedded breakpoints, like `asm int 3;` in x86 or or `__debugbreak` on Windows.
 """
 
 from __future__ import print_function
 
 import os
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 class DebugBreakTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @skipIf(archs=not_in(["i386", "i686"]))
+    @skipIf(archs=no_match(["i386", "i686"]))
     @no_debug_info_test
     def test_asm_int_3(self):
         """Test that intrinsics like `__debugbreak();` and `asm {"int3"}` are treated like breakpoints."""
         self.build()
         exe = os.path.join(os.getcwd(), "a.out")
 
         # Run the program.
         target = self.dbg.CreateTarget(exe)

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

 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 class TestWithLimitDebugInfo(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @skipIf(debug_info=not_in(["dwarf"]))
+    @skipIf(debug_info=no_match(["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")