[clang-tools-extra] [lldb] [llvm] [clang] [lldb][test] Apply @expectedFailureAll/@skipIf early for debug_info tests (PR #73067)

Jordan Rupprecht via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 19 07:59:51 PST 2024


https://github.com/rupprecht updated https://github.com/llvm/llvm-project/pull/73067

>From 22bfc5878f1f96b3138a03eea4dc856948185c89 Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht <rupprecht at google.com>
Date: Tue, 21 Nov 2023 17:28:30 -0800
Subject: [PATCH 1/2] [lldb][test] Apply @expectedFailureAll/@skipIf early for
 debug_info tests

The @expectedFailureAll and @skipIf decorators will mark the test case as xfail/skip if _all_ conditions passed in match, including debug_info.
* If debug_info is not one of the matching conditions, we can immediately evaluate the check and decide if it should be decorated.
* If debug_info *is* present as a match condition, we need to defer whether or not to decorate until when the `LLDBTestCaseFactory` metaclass expands the test case into its potential variants. This is still early enough that the standard `unittest` framework will recognize the test as xfail/skip by the time the test actually runs.

TestDecorators exhibits the edge cases more thoroughly. With the exception of `@expectedFailureIf` (added by this commit), all those test cases pass prior to this commit.

This is a followup to 212a60ec37322f853e91e171b305479b1abff2f2.
---
 .../Python/lldbsuite/test/decorators.py       |  53 ++++++++-
 .../Python/lldbsuite/test/lldbtest.py         |  20 ++++
 lldb/test/API/test_utils/TestDecorators.py    | 110 +++++++++++++++++-
 3 files changed, 177 insertions(+), 6 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index bb06a5ee20f2532..2398892b9e14814 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -117,6 +117,21 @@ def expectedFailure(func):
     return unittest2.expectedFailure(func)
 
 
+def expectedFailureIf(condition, 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")
+
+        if condition:
+            return expectedFailure(func)
+        return func
+
+    if callable(bugnumber):
+        return expectedFailure_impl(bugnumber)
+    else:
+        return expectedFailure_impl
+
+
 def expectedFailureIfFn(expected_fn, bugnumber=None):
     def expectedFailure_impl(func):
         if isinstance(func, type) and issubclass(func, unittest2.TestCase):
@@ -178,6 +193,34 @@ def wrapper(*args, **kwargs):
         return skipTestIfFn_impl
 
 
+def _xfailForDebugInfo(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")
+
+        func.__xfail_for_debug_info_cat_fn__ = expected_fn
+        return func
+
+    if callable(bugnumber):
+        return expectedFailure_impl(bugnumber)
+    else:
+        return expectedFailure_impl
+
+
+def _skipForDebugInfo(expected_fn, bugnumber=None):
+    def skipImpl(func):
+        if isinstance(func, type) and issubclass(func, unittest2.TestCase):
+            raise Exception("Decorator can only be used to decorate a test method")
+
+        func.__skip_for_debug_info_cat_fn__ = expected_fn
+        return func
+
+    if callable(bugnumber):
+        return skipImpl(bugnumber)
+    else:
+        return skipImpl
+
+
 def _decorateTest(
     mode,
     bugnumber=None,
@@ -195,7 +238,7 @@ def _decorateTest(
     dwarf_version=None,
     setting=None,
 ):
-    def fn(self):
+    def fn(actual_debug_info=None):
         skip_for_os = _match_decorator_property(
             lldbplatform.translate(oslist), lldbplatformutil.getPlatform()
         )
@@ -208,7 +251,7 @@ def fn(self):
         skip_for_arch = _match_decorator_property(
             archs, lldbplatformutil.getArchitecture()
         )
-        skip_for_debug_info = _match_decorator_property(debug_info, self.getDebugInfo())
+        skip_for_debug_info = _match_decorator_property(debug_info, actual_debug_info)
         skip_for_triple = _match_decorator_property(
             triple, lldb.selected_platform.GetTriple()
         )
@@ -283,9 +326,13 @@ def fn(self):
         return reason_str
 
     if mode == DecorateMode.Skip:
+        if debug_info:
+            return _skipForDebugInfo(fn, bugnumber)
         return skipTestIfFn(fn, bugnumber)
     elif mode == DecorateMode.Xfail:
-        return expectedFailureIfFn(fn, bugnumber)
+        if debug_info:
+            return _xfailForDebugInfo(fn, bugnumber)
+        return expectedFailureIf(fn(), bugnumber)
     else:
         return None
 
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index dc4e322c675dc9d..872866655093d21 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1667,6 +1667,11 @@ def __new__(cls, name, bases, attrs):
         if original_testcase.NO_DEBUG_INFO_TESTCASE:
             return original_testcase
 
+        # Default implementation for skip/xfail reason based on the debug category,
+        # where "None" means to run the test as usual.
+        def no_reason(_):
+            return None
+
         newattrs = {}
         for attrname, attrvalue in attrs.items():
             if attrname.startswith("test") and not getattr(
@@ -1688,6 +1693,12 @@ def __new__(cls, name, bases, attrs):
                         if can_replicate
                     ]
 
+                xfail_for_debug_info_cat_fn = getattr(
+                    attrvalue, "__xfail_for_debug_info_cat_fn__", no_reason
+                )
+                skip_for_debug_info_cat_fn = getattr(
+                    attrvalue, "__skip_for_debug_info_cat_fn__", no_reason
+                )
                 for cat in categories:
 
                     @decorators.add_test_categories([cat])
@@ -1698,6 +1709,15 @@ def test_method(self, attrvalue=attrvalue):
                     method_name = attrname + "_" + cat
                     test_method.__name__ = method_name
                     test_method.debug_info = cat
+
+                    xfail_reason = xfail_for_debug_info_cat_fn(cat)
+                    if xfail_reason:
+                        test_method = unittest2.expectedFailure(xfail_reason)(test_method)
+
+                    skip_reason = skip_for_debug_info_cat_fn(cat)
+                    if skip_reason:
+                        test_method = unittest2.skip(skip_reason)(test_method)
+
                     newattrs[method_name] = test_method
 
             else:
diff --git a/lldb/test/API/test_utils/TestDecorators.py b/lldb/test/API/test_utils/TestDecorators.py
index 97d144b6d44412b..eb09db69de34975 100644
--- a/lldb/test/API/test_utils/TestDecorators.py
+++ b/lldb/test/API/test_utils/TestDecorators.py
@@ -1,11 +1,115 @@
-from lldbsuite.test.lldbtest import Base
+import re
+
+from lldbsuite.test.lldbtest import TestBase
 from lldbsuite.test.decorators import *
 
 
-class TestDecorators(Base):
+def expectedFailureDwarf(bugnumber=None):
+    return expectedFailureAll(bugnumber, debug_info="dwarf")
+
+
+class TestDecoratorsNoDebugInfoClass(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
     @expectedFailureAll(debug_info="dwarf")
-    def test_decorator_skip_no_debug_info(self):
+    def test_decorator_xfail(self):
         """Test that specifying a debug info category works for a NO_DEBUG_INFO_TESTCASE"""
+
+    @expectedFailureDwarf
+    def test_decorator_xfail_bare_decorator(self):
+        """Same as test_decorator_xfail, but with a custom decorator w/ a bare syntax"""
+
+    @expectedFailureDwarf()
+    def test_decorator_xfail_decorator_empty_args(self):
+        """Same as test_decorator_xfail, but with a custom decorator w/ no args"""
+
+    @add_test_categories(["dwarf"])
+    def test_add_test_categories(self):
+        # Note: the "dwarf" test category is ignored, because we don't generate any debug info test variants
+        self.assertIsNone(self.getDebugInfo())
+
+    @expectedFailureAll
+    def test_xfail_regexp(self):
+        """Test that expectedFailureAll can be empty (but please just use expectedFailure)"""
+        self.fail()
+
+    @expectedFailureAll(compiler=re.compile(".*"))
+    def test_xfail_regexp(self):
+        """Test that xfail can take a regex as a matcher"""
+        self.fail()
+
+    @expectedFailureAll(compiler=no_match(re.compile(".*")))
+    def test_xfail_no_match(self):
+        """Test that xfail can take a no_match matcher"""
+        pass
+
+    @expectedFailureIf(condition=True)
+    def test_xfail_condition_true(self):
+        self.fail()
+
+    @expectedFailureIf(condition=False)
+    def test_xfail_condition_false(self):
+        pass
+
+
+class TestDecorators(TestBase):
+    @expectedFailureAll(debug_info="dwarf")
+    def test_decorator_xfail(self):
+        """Test that expectedFailureAll fails for the debug_info variant"""
+        if self.getDebugInfo() == "dwarf":
+            self.fail()
+
+    @skipIf(debug_info="dwarf")
+    def test_decorator_skip(self):
+        """Test that skipIf skips the debug_info variant"""
+        self.assertNotEqual(self.getDebugInfo(), "dwarf")
+
+    @expectedFailureDwarf
+    def test_decorator_xfail2(self):
+        """Same as test_decorator_xfail, but with a custom decorator w/ a bare syntax"""
+        if self.getDebugInfo() == "dwarf":
+            self.fail()
+
+    @expectedFailureDwarf()
+    def test_decorator_xfail3(self):
+        """Same as test_decorator_xfail, but with a custom decorator w/ no args"""
+        if self.getDebugInfo() == "dwarf":
+            self.fail()
+
+    @add_test_categories(["dwarf"])
+    def test_add_test_categories(self):
+        """Test that add_test_categories limits the kinds of debug info test variants"""
+        self.assertEqual(self.getDebugInfo(), "dwarf")
+
+    @expectedFailureAll(compiler="fake", debug_info="dwarf")
+    def test_decorator_xfail_all(self):
+        """Test that expectedFailureAll requires all conditions to match to be xfail"""
+
+    @skipIf(compiler="fake", debug_info="dwarf")
+    def test_decorator_skip2(self):
+        """Test that expectedFailureAll fails for the debug_info variant"""
+        # Note: the following assertion would fail, if this were not skipped:
+        # self.assertNotEqual(self.getDebugInfo(), "dwarf")
+
+    @expectedFailureAll
+    def test_xfail_regexp(self):
+        """Test that xfail can be empty"""
+        self.fail()
+
+    @expectedFailureAll(compiler=re.compile(".*"))
+    def test_xfail_regexp(self):
+        """Test that xfail can take a regex as a matcher"""
+        self.fail()
+
+    @expectedFailureAll(compiler=no_match(re.compile(".*")))
+    def test_xfail_no_match(self):
+        """Test that xfail can take a no_match matcher"""
+        pass
+
+    @expectedFailureIf(condition=True)
+    def test_xfail_condition_true(self):
+        self.fail()
+
+    @expectedFailureIf(condition=False)
+    def test_xfail_condition_false(self):
         pass

>From 6eb245033df8925032975bbcf28dd7b35e34f6e5 Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht <rupprecht at google.com>
Date: Tue, 21 Nov 2023 17:54:41 -0800
Subject: [PATCH 2/2] missed some py formatting

---
 lldb/packages/Python/lldbsuite/test/lldbtest.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 872866655093d21..3abc713398490e7 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1712,7 +1712,9 @@ def test_method(self, attrvalue=attrvalue):
 
                     xfail_reason = xfail_for_debug_info_cat_fn(cat)
                     if xfail_reason:
-                        test_method = unittest2.expectedFailure(xfail_reason)(test_method)
+                        test_method = unittest2.expectedFailure(xfail_reason)(
+                            test_method
+                        )
 
                     skip_reason = skip_for_debug_info_cat_fn(cat)
                     if skip_reason:



More information about the cfe-commits mailing list