[Lldb-commits] [lldb] [lldb][test] Remove expectedFailureIfFn (PR #81703)

via lldb-commits lldb-commits at lists.llvm.org
Tue Feb 13 20:46:32 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jordan Rupprecht (rupprecht)

<details>
<summary>Changes</summary>

Switching to modern `unittest` in 5b386158aacac4b41126983a5379d36ed413d0ea needs xfail annotations to be known prior to test running. In contrast, skipping can happen at any time, even during test execution.

Thus, `expectedFailureIfFn` inherently doesn't work. Either we eagerly evaluate the function and use `expectedFailureIf` instead, or we use a skip annotation to lazily evaluate the function and potentially skip the test right before it starts.

- For `expectedFailureAndroid`, the intent seems to be that certain tests _should_ work on android, but don't. Thus, xfail is appropriate, to ensure the test is re-enabled once those bugs are ever fixed.
- For the other uses in individual tests, those generally seem to be cases where the test environment doesn't support the setup required by the test, and so it isn't meaningful to run the test at all. For those, a drop-in replacement to `skipTestIfFn` works.


---
Full diff: https://github.com/llvm/llvm-project/pull/81703.diff


4 Files Affected:

- (modified) lldb/packages/Python/lldbsuite/test/decorators.py (+6-33) 
- (modified) lldb/test/API/commands/platform/sdk/TestPlatformSDK.py (+2-2) 
- (modified) lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py (+4-4) 
- (modified) lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py (+4-4) 


``````````diff
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index a5e1fa51cf6e63..86594c2b409e79 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -128,33 +128,6 @@ def expectedFailure_impl(func):
         return expectedFailure_impl
 
 
-def expectedFailureIfFn(expected_fn, bugnumber=None):
-    def expectedFailure_impl(func):
-        if isinstance(func, type) and issubclass(func, unittest.TestCase):
-            raise Exception("Decorator can only be used to decorate a test method")
-
-        @wraps(func)
-        def wrapper(*args, **kwargs):
-            xfail_reason = expected_fn(*args, **kwargs)
-            if xfail_reason is not None:
-                xfail_func = unittest.expectedFailure(func)
-                xfail_func(*args, **kwargs)
-            else:
-                func(*args, **kwargs)
-
-        return wrapper
-
-    # Some decorators can be called both with no arguments (e.g. @expectedFailureWindows)
-    # or with arguments (e.g. @expectedFailureWindows(compilers=['gcc'])).  When called
-    # the first way, the first argument will be the actual function because decorators are
-    # weird like that.  So this is basically a check that says "which syntax was the original
-    # function decorated with?"
-    if callable(bugnumber):
-        return expectedFailure_impl(bugnumber)
-    else:
-        return expectedFailure_impl
-
-
 def skipTestIfFn(expected_fn, bugnumber=None):
     def skipTestIfFn_impl(func):
         if isinstance(func, type) and issubclass(func, unittest.TestCase):
@@ -417,8 +390,8 @@ def skipIf(
     )
 
 
-def _skip_for_android(reason, api_levels, archs):
-    def impl(obj):
+def _skip_fn_for_android(reason, api_levels, archs):
+    def impl():
         result = lldbplatformutil.match_android_device(
             lldbplatformutil.getArchitecture(),
             valid_archs=archs,
@@ -549,8 +522,8 @@ def expectedFailureAndroid(bugnumber=None, api_levels=None, archs=None):
         arch - A sequence of architecture names specifying the architectures
             for which a test is expected to fail. None means all architectures.
     """
-    return expectedFailureIfFn(
-        _skip_for_android("xfailing on android", api_levels, archs), bugnumber
+    return expectedFailureIf(
+        _skip_fn_for_android("xfailing on android", api_levels, archs)(), bugnumber
     )
 
 
@@ -612,7 +585,7 @@ def expectedFlakeyNetBSD(bugnumber=None, compilers=None):
 
 def expectedFlakeyAndroid(bugnumber=None, api_levels=None, archs=None):
     return expectedFlakey(
-        _skip_for_android("flakey on android", api_levels, archs), bugnumber
+        _skip_fn_for_android("flakey on android", api_levels, archs), bugnumber
     )
 
 
@@ -846,7 +819,7 @@ def skipIfTargetAndroid(bugnumber=None, api_levels=None, archs=None):
             for which a test is skipped. None means all architectures.
     """
     return skipTestIfFn(
-        _skip_for_android("skipping for android", api_levels, archs), bugnumber
+        _skip_fn_for_android("skipping for android", api_levels, archs), bugnumber
     )
 
 
diff --git a/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py b/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
index bf79a5bd5537d9..6af5767e26d3e8 100644
--- a/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
+++ b/lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
@@ -39,8 +39,8 @@ def port_not_available(self):
 
     @no_debug_info_test
     @skipUnlessDarwin
-    @expectedFailureIfFn(no_debugserver)
-    @expectedFailureIfFn(port_not_available)
+    @skipTestIfFn(no_debugserver)
+    @skipTestIfFn(port_not_available)
     @skipIfRemote
     def test_macos_sdk(self):
         self.build()
diff --git a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
index ae4f7ea071ed08..5325f0f00affb8 100644
--- a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
+++ b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
@@ -26,7 +26,7 @@ def test_breakpoint(self):
         breakpoint = target.BreakpointCreateByLocation("main.c", 1)
         self.assertTrue(breakpoint.IsHardware())
 
-    @expectedFailureIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
     def test_step_range(self):
         """Test stepping when hardware breakpoints are required."""
         self.build()
@@ -49,7 +49,7 @@ def test_step_range(self):
             "Could not create hardware breakpoint for thread plan" in error.GetCString()
         )
 
-    @expectedFailureIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
     def test_step_out(self):
         """Test stepping out when hardware breakpoints are required."""
         self.build()
@@ -71,7 +71,7 @@ def test_step_out(self):
             "Could not create hardware breakpoint for thread plan" in error.GetCString()
         )
 
-    @expectedFailureIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
     def test_step_over(self):
         """Test stepping over when hardware breakpoints are required."""
         self.build()
@@ -91,7 +91,7 @@ def test_step_over(self):
 
     # Was reported to sometimes pass on certain hardware.
     @skipIf(oslist=["linux"], archs=["arm"])
-    @expectedFailureIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
+    @skipTestIfFn(HardwareBreakpointTestBase.supports_hw_breakpoints)
     def test_step_until(self):
         """Test stepping until when hardware breakpoints are required."""
         self.build()
diff --git a/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py b/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
index 496f9c20c2ce7b..b4e2b39e0d5dbd 100644
--- a/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
+++ b/lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py
@@ -49,15 +49,15 @@ def test_stop_default_platform_async(self):
 
     @skipUnlessDarwin
     @skipIfRemote
-    @expectedFailureIfFn(no_debugserver)
-    @expectedFailureIfFn(port_not_available)
+    @skipTestIfFn(no_debugserver)
+    @skipTestIfFn(port_not_available)
     def test_stop_remote_platform_sync(self):
         self.do_test_stop_at_entry(True, True)
 
     @skipUnlessDarwin
     @skipIfRemote
-    @expectedFailureIfFn(no_debugserver)
-    @expectedFailureIfFn(port_not_available)
+    @skipTestIfFn(no_debugserver)
+    @skipTestIfFn(port_not_available)
     def test_stop_remote_platform_async(self):
         self.do_test_stop_at_entry(False, True)
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/81703


More information about the lldb-commits mailing list