Add a decorator option to skip tests based on a default setting

[aprantl]

Summary

This patch allows skipping a test based on a default setting, which is useful when running the testsuite in different "modes" based on a default setting. This is a feature I need for the Swift testsuite, but I think it's generally useful.

See also https://reviews.llvm.org/D72662 for context.

[JDevlieghere]

lldb/test/API/sanity/TestSettingSkipping.py

29

This won't trip if the tests doesn't run. If you make the assert trip and XFAIL the test it should catch it.

[labath]

If a test requires a specific value of a setting, would it make more sense to just (re)set its value at the start of a test?

[aprantl]

In D75864#1913876, @labath wrote:

If a test requires a specific value of a setting, would it make more sense to just (re)set its value at the start of a test?

This is not meant for requiring a setting, but to run the testsuite once in normal mode, and then again with --setting "experimental-feature-that-doesn't-work-for-all-tests-yet=true" and have a way to mark tests as not yet supported with that feature.

[aprantl]

lldb/test/API/sanity/TestSettingSkipping.py

29

I'm not sure how that would work. Can you post an example? I want to test the skipIf decorator — how would I XFAIL it?

[JDevlieghere]

LGTM but let's give Pavel another chance to take a look.

lldb/test/API/sanity/TestSettingSkipping.py

29

Yeah I think it has the same problem, skipping takes priority over XFAIL, so it wouldn't matter.

[labath]

In D75864#1914795, @aprantl wrote:

In D75864#1913876, @labath wrote:

If a test requires a specific value of a setting, would it make more sense to just (re)set its value at the start of a test?

This is not meant for requiring a setting, but to run the testsuite once in normal mode, and then again with --setting "experimental-feature-that-doesn't-work-for-all-tests-yet=true" and have a way to mark tests as not yet supported with that feature.

Right, that makes sense, though it makes me cry a bit every time we add a new option to the test decorators...

lldb/test/API/sanity/TestSettingSkipping.py

29

At one point, Todd added some tests for the test harness. There are still some remnants of it in packages/Python/lldbsuite/test/test_runner/ but they're hopelessly out of date by now (and even when they were new they only seemed to work for Todd).

If we wanted to write self-tests something like that would seem appropriate. I'm not sure these tests are very useful as they stand now because they could pass or fail depending on what kind of --setting values I pass to dotest.

[aprantl]

In D75864#1914916, @labath wrote:

Right, that makes sense, though it makes me cry a bit every time we add a new option to the test decorators...

We call it the Christmas tree approach to testing!

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

 
 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,
                   macos_version=None,
-                  remote=None, dwarf_version=None):
+                  remote=None, dwarf_version=None,
+                  setting=None):
     def fn(self):
         skip_for_os = _match_decorator_property(
             lldbplatform.translate(oslist), self.getPlatform())
         skip_for_hostos = _match_decorator_property(
             lldbplatform.translate(hostoslist),
             lldbplatformutil.getHostPlatform())
         skip_for_compiler = _match_decorator_property(
             compiler, self.getCompiler()) and self.expectedCompilerVersion(compiler_version)
def fn(self):
         skip_for_macos_version = (macos_version is None) or (
             (platform.mac_ver()[0] != "") and (_check_expected_version(
                 macos_version[0],
                 macos_version[1],
                 platform.mac_ver()[0])))
         skip_for_dwarf_version = (dwarf_version is None) or (
             _check_expected_version(dwarf_version[0], dwarf_version[1],
                                     self.getDwarfVersion()))
+        skip_for_setting = (setting is None) or (
+            setting in configuration.settings)
 
         # 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"),
                       (debug_info, skip_for_debug_info, "debug info format"),
                       (triple, skip_for_triple, "target triple"),
                       (swig_version, skip_for_swig_version, "swig version"),
                       (py_version, skip_for_py_version, "python version"),
                       (macos_version, skip_for_macos_version, "macOS version"),
                       (remote, skip_for_remote, "platform locality (remote/local)"),
-                      (dwarf_version, skip_for_dwarf_version, "dwarf version")]
+                      (dwarf_version, skip_for_dwarf_version, "dwarf version"),
+                      (setting, skip_for_setting, "setting")]
         reasons = []
         final_skip_result = True
         for this_condition in conditions:
             final_skip_result = final_skip_result and this_condition[1]
             if this_condition[0] is not None and this_condition[1]:
                 reasons.append(this_condition[2])
         reason_str = None
         if final_skip_result:

 # @skipIf(bugnumber, ["linux"], "gcc", ['>=', '4.9'], ['i386']), skip for gcc>=4.9 on linux with i386
 def skipIf(bugnumber=None,
            oslist=None, hostoslist=None,
            compiler=None, compiler_version=None,
            archs=None, triple=None,
            debug_info=None,
            swig_version=None, py_version=None,
            macos_version=None,
-           remote=None, dwarf_version=None):
+           remote=None, dwarf_version=None,
+           setting=None):
     return _decorateTest(DecorateMode.Skip,
                          bugnumber=bugnumber,
                          oslist=oslist, hostoslist=hostoslist,
                          compiler=compiler, compiler_version=compiler_version,
                          archs=archs, triple=triple,
                          debug_info=debug_info,
                          swig_version=swig_version, py_version=py_version,
                          macos_version=macos_version,
-                         remote=remote, dwarf_version=dwarf_version)
+                         remote=remote, dwarf_version=dwarf_version,
+                         setting=setting)
 
 
 def _skip_for_android(reason, api_levels, archs):
     def impl(obj):
         result = lldbplatformutil.match_android_device(
             obj.getArchitecture(), valid_archs=archs, valid_api_levels=api_levels)
         return reason if result else None
     return impl

lldb/test/API/sanity/TestSettingSkipping.py

+"""
+This is a sanity check that verifies that test can be sklipped based on settings.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class SettingSkipSanityTestCase(TestBase):
+
+  mydir = TestBase.compute_mydir(__file__)
+
+  NO_DEBUG_INFO_TESTCASE = True
+
+  @skipIf(setting=('target.prefer-dynamic-value', 'no-dynamic-values'))
+  def testSkip(self):
+    """This setting is on by default"""
+    self.assertTrue(False, "This test should not run!")
+
+  @skipIf(setting=('target.prefer-dynamic-value', 'run-target'))
+  def testNoMatch(self):
+    self.assertTrue(True, "This test should run!")
+
+  @skipIf(setting=('target.i-made-this-one-up', 'true'))
+  def testNotExisting(self):
+    self.assertTrue(True, "This test should run!")
+