https://github.com/rupprecht updated https://github.com/llvm/llvm-project/pull/84860
>From b5e04eb49c89c8c654305c6ba5db5e2f237bccec Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht <ruppre...@google.com> Date: Mon, 11 Mar 2024 11:23:59 -0700 Subject: [PATCH 1/5] [lldb][test] Add `pexpect` category for tests that `import pexpect` - Add pexpect category - Replace skipIfWindows with this - Make categories apply to test classes too --- .../Python/lldbsuite/test/decorators.py | 4 ---- lldb/packages/Python/lldbsuite/test/dotest.py | 20 +++++++++++++++++++ .../Python/lldbsuite/test/lldbpexpect.py | 2 +- .../Python/lldbsuite/test/test_categories.py | 1 + .../Python/lldbsuite/test/test_result.py | 10 +++++----- .../expression/TestExpressionCmd.py | 5 +---- .../expression/TestRepeatedExprs.py | 5 +---- .../TestFrameVariableResponse.py | 5 +---- .../benchmarks/startup/TestStartupDelays.py | 5 +---- .../benchmarks/stepping/TestSteppingSpeed.py | 5 +---- .../TestCompileRunToBreakpointTurnaround.py | 5 +---- .../API/terminal/TestSTTYBeforeAndAfter.py | 2 +- 12 files changed, 34 insertions(+), 35 deletions(-) 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.""" >From 1ee6748aa93fb38e7fc81ed8ac61817c3e90b293 Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht <ruppre...@google.com> Date: Mon, 11 Mar 2024 18:21:13 -0700 Subject: [PATCH 2/5] Fix formatting (different versions of darker?) --- lldb/packages/Python/lldbsuite/test/test_result.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/packages/Python/lldbsuite/test/test_result.py b/lldb/packages/Python/lldbsuite/test/test_result.py index cda67ae27d4674..2d574b343b4134 100644 --- a/lldb/packages/Python/lldbsuite/test/test_result.py +++ b/lldb/packages/Python/lldbsuite/test/test_result.py @@ -160,7 +160,9 @@ 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" ) >From 7f2ec70a35d1f5426afcf354f9b16b0052e81df2 Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht <ruppre...@google.com> Date: Tue, 12 Mar 2024 07:01:53 -0700 Subject: [PATCH 3/5] Promote pexpect warning to error --- lldb/packages/Python/lldbsuite/test/dotest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index 268c02e6b49dde..a94afbd55e773d 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -928,8 +928,8 @@ def checkPexpectSupport(): try: import pexpect except: - print( - "Warning: pexpect is not installed, but pexpect tests are not being skipped." + raise Exception( + "pexpect is not installed, but pexpect tests are not being skipped." ) >From bcfc5e8f2f18e8eb8f5a96e18c649f3cb2e248c0 Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht <ruppre...@google.com> Date: Tue, 12 Mar 2024 17:57:58 -0700 Subject: [PATCH 4/5] More specific cmake message --- lldb/packages/Python/lldbsuite/test/dotest.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index a94afbd55e773d..a3e60c892fe9b8 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -929,7 +929,10 @@ def checkPexpectSupport(): import pexpect except: raise Exception( - "pexpect is not installed, but pexpect tests are not being skipped." + "Tests in the 'pexpect' category will run, but pexpect is not installed. " + "Either install it (via pip or via a distro package), " + "or skip them by configuring cmake with " + "-DLLDB_TEST_USER_ARGS=--skip-category=pexpect" ) >From 03e13152eac8416c31414b4f469e281c40b80deb Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht <ruppre...@google.com> Date: Tue, 12 Mar 2024 18:22:28 -0700 Subject: [PATCH 5/5] Drop early check for import pexpect --- lldb/packages/Python/lldbsuite/test/dotest.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index a3e60c892fe9b8..8c29145ecc5272 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -924,16 +924,6 @@ def checkPexpectSupport(): 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: - raise Exception( - "Tests in the 'pexpect' category will run, but pexpect is not installed. " - "Either install it (via pip or via a distro package), " - "or skip them by configuring cmake with " - "-DLLDB_TEST_USER_ARGS=--skip-category=pexpect" - ) def run_suite(): _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits