llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) <details> <summary>Changes</summary> Instead of directly annotating pexpect-based tests with `@<!-- -->skipIfWindows`, we can tag them with a new `pexpect` category. We still automatically skip windows behavior by adding `pexpect` to the skip category list if the platform is windows, but also allow non-Windows users to skip them by configuring cmake with `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect` As a prerequisite, remove the restriction that `@<!-- -->add_test_categories` can only apply to test cases, and we make the test runner look for categories on both the class and the test method. --- Full diff: https://github.com/llvm/llvm-project/pull/84860.diff 12 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/decorators.py (-4) - (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+20) - (modified) lldb/packages/Python/lldbsuite/test/lldbpexpect.py (+1-1) - (modified) lldb/packages/Python/lldbsuite/test/test_categories.py (+1) - (modified) lldb/packages/Python/lldbsuite/test/test_result.py (+5-5) - (modified) lldb/test/API/benchmarks/expression/TestExpressionCmd.py (+1-4) - (modified) lldb/test/API/benchmarks/expression/TestRepeatedExprs.py (+1-4) - (modified) lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py (+1-4) - (modified) lldb/test/API/benchmarks/startup/TestStartupDelays.py (+1-4) - (modified) lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py (+1-4) - (modified) lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py (+1-4) - (modified) lldb/test/API/terminal/TestSTTYBeforeAndAfter.py (+1-1) ``````````diff diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py index b691f82b90652c..8e13aa6a13882f 100644 --- a/lldb/packages/Python/lldbsuite/test/decorators.py +++ b/lldb/packages/Python/lldbsuite/test/decorators.py @@ -409,10 +409,6 @@ def add_test_categories(cat): cat = test_categories.validate(cat, True) def impl(func): - if isinstance(func, type) and issubclass(func, unittest.TestCase): - raise Exception( - "@add_test_categories can only be used to decorate a test method" - ) try: if hasattr(func, "categories"): cat.extend(func.categories) diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index 291d7bad5c0897..268c02e6b49dde 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -914,6 +914,25 @@ def checkForkVForkSupport(): configuration.skip_categories.append("fork") +def checkPexpectSupport(): + from lldbsuite.test import lldbplatformutil + + platform = lldbplatformutil.getPlatform() + + # llvm.org/pr22274: need a pexpect replacement for windows + if platform in ["windows"]: + if configuration.verbose: + print("pexpect tests will be skipped because of unsupported platform") + configuration.skip_categories.append("pexpect") + elif not configuration.shouldSkipBecauseOfCategories(["pexpect"]): + try: + import pexpect + except: + print( + "Warning: pexpect is not installed, but pexpect tests are not being skipped." + ) + + def run_suite(): # On MacOS X, check to make sure that domain for com.apple.DebugSymbols defaults # does not exist before proceeding to running the test suite. @@ -1013,6 +1032,7 @@ def run_suite(): checkDebugServerSupport() checkObjcSupport() checkForkVForkSupport() + checkPexpectSupport() skipped_categories_list = ", ".join(configuration.skip_categories) print( diff --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py index 9d216d90307473..998a080565b6b3 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py +++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py @@ -10,7 +10,7 @@ @skipIfRemote -@skipIfWindows # llvm.org/pr22274: need a pexpect replacement for windows +@add_test_categories(["pexpect"]) class PExpectTest(TestBase): NO_DEBUG_INFO_TESTCASE = True PROMPT = "(lldb) " diff --git a/lldb/packages/Python/lldbsuite/test/test_categories.py b/lldb/packages/Python/lldbsuite/test/test_categories.py index 3f8de175e29df3..036bda9c957d11 100644 --- a/lldb/packages/Python/lldbsuite/test/test_categories.py +++ b/lldb/packages/Python/lldbsuite/test/test_categories.py @@ -33,6 +33,7 @@ "lldb-server": "Tests related to lldb-server", "lldb-dap": "Tests for the Debug Adaptor Protocol with lldb-dap", "llgs": "Tests for the gdb-server functionality of lldb-server", + "pexpect": "Tests requiring the pexpect library to be available", "objc": "Tests related to the Objective-C programming language support", "pyapi": "Tests related to the Python API", "std-module": "Tests related to importing the std module", diff --git a/lldb/packages/Python/lldbsuite/test/test_result.py b/lldb/packages/Python/lldbsuite/test/test_result.py index 20365f53a67541..cda67ae27d4674 100644 --- a/lldb/packages/Python/lldbsuite/test/test_result.py +++ b/lldb/packages/Python/lldbsuite/test/test_result.py @@ -148,9 +148,11 @@ def getCategoriesForTest(self, test): Gets all the categories for the currently running test method in test case """ test_categories = [] + test_categories.extend(getattr(test, "categories", [])) + test_method = getattr(test, test._testMethodName) - if test_method is not None and hasattr(test_method, "categories"): - test_categories.extend(test_method.categories) + if test_method is not None: + test_categories.extend(getattr(test_method, "categories", [])) test_categories.extend(self._getFileBasedCategories(test)) @@ -158,9 +160,7 @@ def getCategoriesForTest(self, test): def hardMarkAsSkipped(self, test): getattr(test, test._testMethodName).__func__.__unittest_skip__ = True - getattr( - test, test._testMethodName - ).__func__.__unittest_skip_why__ = ( + getattr(test, test._testMethodName).__func__.__unittest_skip_why__ = ( "test case does not fall in any category of interest for this run" ) diff --git a/lldb/test/API/benchmarks/expression/TestExpressionCmd.py b/lldb/test/API/benchmarks/expression/TestExpressionCmd.py index 9b512305d626bd..8261b1b25da9f2 100644 --- a/lldb/test/API/benchmarks/expression/TestExpressionCmd.py +++ b/lldb/test/API/benchmarks/expression/TestExpressionCmd.py @@ -17,10 +17,7 @@ def setUp(self): self.count = 25 @benchmarks_test - @expectedFailureAll( - oslist=["windows"], - bugnumber="llvm.org/pr22274: need a pexpect replacement for windows", - ) + @add_test_categories(["pexpect"]) def test_expr_cmd(self): """Test lldb's expression commands and collect statistics.""" self.build() diff --git a/lldb/test/API/benchmarks/expression/TestRepeatedExprs.py b/lldb/test/API/benchmarks/expression/TestRepeatedExprs.py index 104e69b384237c..acc6b74c17b7e6 100644 --- a/lldb/test/API/benchmarks/expression/TestRepeatedExprs.py +++ b/lldb/test/API/benchmarks/expression/TestRepeatedExprs.py @@ -19,10 +19,7 @@ def setUp(self): self.count = 100 @benchmarks_test - @expectedFailureAll( - oslist=["windows"], - bugnumber="llvm.org/pr22274: need a pexpect replacement for windows", - ) + @add_test_categories(["pexpect"]) def test_compare_lldb_to_gdb(self): """Test repeated expressions with lldb vs. gdb.""" self.build() diff --git a/lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py b/lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py index f3989fc0ff4838..e364fb8ce77821 100644 --- a/lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py +++ b/lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py @@ -17,10 +17,7 @@ def setUp(self): @benchmarks_test @no_debug_info_test - @expectedFailureAll( - oslist=["windows"], - bugnumber="llvm.org/pr22274: need a pexpect replacement for windows", - ) + @add_test_categories(["pexpect"]) def test_startup_delay(self): """Test response time for the 'frame variable' command.""" print() diff --git a/lldb/test/API/benchmarks/startup/TestStartupDelays.py b/lldb/test/API/benchmarks/startup/TestStartupDelays.py index f31a10507ed7ef..faec21e95e5d20 100644 --- a/lldb/test/API/benchmarks/startup/TestStartupDelays.py +++ b/lldb/test/API/benchmarks/startup/TestStartupDelays.py @@ -22,10 +22,7 @@ def setUp(self): @benchmarks_test @no_debug_info_test - @expectedFailureAll( - oslist=["windows"], - bugnumber="llvm.org/pr22274: need a pexpect replacement for windows", - ) + @add_test_categories(["pexpect"]) def test_startup_delay(self): """Test start up delays creating a target, setting a breakpoint, and run to breakpoint stop.""" print() diff --git a/lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py b/lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py index a3264e3f3253bf..d0f9b0d61d17ef 100644 --- a/lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py +++ b/lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py @@ -22,10 +22,7 @@ def setUp(self): @benchmarks_test @no_debug_info_test - @expectedFailureAll( - oslist=["windows"], - bugnumber="llvm.org/pr22274: need a pexpect replacement for windows", - ) + @add_test_categories(["pexpect"]) def test_run_lldb_steppings(self): """Test lldb steppings on a large executable.""" print() diff --git a/lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py b/lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py index 98a2ec9ebf231d..91527cd114534c 100644 --- a/lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py +++ b/lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py @@ -21,10 +21,7 @@ def setUp(self): @benchmarks_test @no_debug_info_test - @expectedFailureAll( - oslist=["windows"], - bugnumber="llvm.org/pr22274: need a pexpect replacement for windows", - ) + @add_test_categories(["pexpect"]) def test_run_lldb_then_gdb(self): """Benchmark turnaround time with lldb vs. gdb.""" print() diff --git a/lldb/test/API/terminal/TestSTTYBeforeAndAfter.py b/lldb/test/API/terminal/TestSTTYBeforeAndAfter.py index e5663c50c736e6..21aca5fc85d5f1 100644 --- a/lldb/test/API/terminal/TestSTTYBeforeAndAfter.py +++ b/lldb/test/API/terminal/TestSTTYBeforeAndAfter.py @@ -19,7 +19,7 @@ def classCleanup(cls): cls.RemoveTempFile("child_send2.txt") cls.RemoveTempFile("child_read2.txt") - @skipIfWindows # llvm.org/pr22274: need a pexpect replacement for windows + @add_test_categories(["pexpect"]) @no_debug_info_test def test_stty_dash_a_before_and_afetr_invoking_lldb_command(self): """Test that 'stty -a' displays the same output before and after running the lldb command.""" `````````` </details> https://github.com/llvm/llvm-project/pull/84860 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits