[lldb][test] Remove expectedFailureIfFn (#81703)

Switching to modern `unittest` in
5b38615 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.

Author: rupprecht

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
     )
 
 

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()

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()

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)