[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
https://github.com/rupprecht created https://github.com/llvm/llvm-project/pull/84860 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. >From b5e04eb49c89c8c654305c6ba5db5e2f237bccec Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht Date: Mon, 11 Mar 2024 11:23:59 -0700 Subject: [PATCH] [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 Objectiv
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) Changes 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": "T
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
github-actions[bot] wrote: :warning: Python code formatter, darker found issues in your code. :warning: You can test this locally with the following command: ``bash darker --check --diff -r 2a3f27cce8983e5d6871b9ebb8f5e9dd91884f0c...b5e04eb49c89c8c654305c6ba5db5e2f237bccec lldb/packages/Python/lldbsuite/test/decorators.py lldb/packages/Python/lldbsuite/test/dotest.py lldb/packages/Python/lldbsuite/test/lldbpexpect.py lldb/packages/Python/lldbsuite/test/test_categories.py lldb/packages/Python/lldbsuite/test/test_result.py lldb/test/API/benchmarks/expression/TestExpressionCmd.py lldb/test/API/benchmarks/expression/TestRepeatedExprs.py lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py lldb/test/API/benchmarks/startup/TestStartupDelays.py lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py lldb/test/API/terminal/TestSTTYBeforeAndAfter.py `` View the diff from darker here. ``diff --- packages/Python/lldbsuite/test/test_result.py 2024-03-11 18:23:59.00 + +++ packages/Python/lldbsuite/test/test_result.py 2024-03-12 01:18:26.696261 + @@ -158,11 +158,13 @@ return test_categories 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" ) def checkExclusion(self, exclusion_list, name): if exclusion_list: `` 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
@@ -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." rupprecht wrote: Note: instead of just printing a warning, we could make this also add `pexpect` to `skip_categories` as above. However, that would lead to a false notion that `pexpect` tests are running and passing. Someone who is running these tests may not be aware that `pexpect` needs to be installed, or maybe they are aware but a system change made them go away. IMHO, it is safer to let the tests fail and make the user acknowledge that `pexpect` tests should be skipped. 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
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 Date: Mon, 11 Mar 2024 11:23:59 -0700 Subject: [PATCH 1/2] [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 meth
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
@@ -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." DavidSpickett wrote: Would it be even safer to error here? 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
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 Date: Mon, 11 Mar 2024 11:23:59 -0700 Subject: [PATCH 1/3] [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 meth
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
@@ -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." rupprecht wrote: For a pexpect test, it would fail either way, but raising an error here would be more clear about the failure. For a non-pexpect test, this will unnecessarily fail. `ninja check-lldb-api` will have ~1k test failures instead of ~30 failures. Updated to `raise` instead here. 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
@@ -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." rupprecht wrote: `checkPexpectSupport()` will be called for every test, just like the other check* methods above. This `try` block here will only run if a non-Windows platform is configured to run `pexpect` tests, either because: * The test is running w/ various `--skip-category=foo --skip-category=bar`, and `pexpect` is not one of the skipped categories, or * The test is running w/ `--category=pexpect` (this is unusual) I don't think there's a way to only run this check if the test has a certain category. We don't know what the category is, because the test file isn't loaded, and loading it would mean potentially evaluating the `import pexpect` statement that will fail. The whole `elif` block is just to provide a better message if `pexpect` is not installed. If I remove it, then running a `pexpect` test will fail like so: ``` $ bin/lldb-dotest -p TestSTTYBeforeAndAfter.py ... lldb version 19.0.0git (https://github.com/llvm/llvm-project.git revision b5e04eb49c89c8c654305c6ba5db5e2f237bccec) clang revision b5e04eb49c89c8c654305c6ba5db5e2f237bccec llvm revision b5e04eb49c89c8c654305c6ba5db5e2f237bccec Skipping the following test categories: ['dsym', 'gmodules', 'debugserver', 'objc'] FAIL: LLDB (/home/rupprecht/src/llvm-build/dev/bin/clang-x86_64) :: test_stty_dash_a_before_and_afetr_invoking_lldb_command (TestSTTYBeforeAndAfter.TestSTTYBeforeAndAfter.test_stty_dash_a_before_and_afetr_invoking_lldb_command) == ERROR: test_stty_dash_a_before_and_afetr_invoking_lldb_command (TestSTTYBeforeAndAfter.TestSTTYBeforeAndAfter.test_stty_dash_a_before_and_afetr_invoking_lldb_command) Test that 'stty -a' displays the same output before and after running the lldb command. -- Traceback (most recent call last): File "/home/rupprecht/src/llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py", line 446, in wrapper return func(self, *args, **kwargs) ^^^ File "/home/rupprecht/src/llvm-project/lldb/test/API/terminal/TestSTTYBeforeAndAfter.py", line 26, in test_stty_dash_a_before_and_afetr_invoking_lldb_command import pexpect ModuleNotFoundError: No module named 'pexpect' Config=x86_64-/home/rupprecht/src/llvm-build/dev/bin/clang -- Ran 1 test in 0.011s FAILED (errors=1) ``` I suppose the error raised by the `elif` block, as written, doesn't provide much benefit on top of that -- it's clear that the issue is trying to run a pexpect test w/o pexpect being installed. So maybe I should replace it with something more actionable, e.g. to suggest configuring cmake w/ `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect`. Or just remove it altogether? 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
@@ -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." DavidSpickett wrote: I understand now. Yeah adding something actionable would be a good improvement, tell them to install pexpect or to skip the category. 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
@@ -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." JDevlieghere wrote: So what would happens without this code? Wouldn't we try to import pexpect anyway in the relevant test? In other words, do we even need this? It seems that printing a warning for non-pexpect either adds noise to the already verbose test output or will be invisible if the test passes. If the error message is poor enough that we would want to print something more actionable, could we do it from the `PExpectTest` base class? 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
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 Date: Mon, 11 Mar 2024 11:23:59 -0700 Subject: [PATCH 1/4] [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 meth
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
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 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 meth
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
@@ -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." rupprecht wrote: Proposed some more specific wording in https://github.com/llvm/llvm-project/pull/84860/commits/bcfc5e8f2f18e8eb8f5a96e18c649f3cb2e248c0 But this `elif` block is the least thing that I care about actually landing :) I think it's helpful, but if this is contentious, it's not a big deal to drop it and just have a potentially worse error message. So I dropped it altogether in https://github.com/llvm/llvm-project/pull/84860/commits/03e13152eac8416c31414b4f469e281c40b80deb. I can always add it back if needed, either in this PR or as a followup commit. > So what would happens without this code? Wouldn't we try to import pexpect > anyway in the relevant test? In other words, do we even need this? Yep, see the snippet above ([link](#discussion_r1521684820)) for what it'd look like if we just remove it. Having a specific check+error text here is not essential, but might help some developer if they aren't aware of how to skip this category. > It seems that printing a warning for non-pexpect either adds noise to the > already verbose test output or will be invisible if the test passes. Agreed about spamminess; that was changed in https://github.com/llvm/llvm-project/pull/84860/commits/7f2ec70a35d1f5426afcf354f9b16b0052e81df2 to make this a hard error. > If the error message is poor enough that we would want to print something > more actionable, could we do it from the PExpectTest base class? That would not work for tests that use pexpect outside of `PExpectTest`, which are: - lldb/test/API/terminal/TestSTTYBeforeAndAfter.py - lldb/test/API/macosx/nslog/TestDarwinNSLogOutput.py - lldb/test/API/benchmarks/expression/TestRepeatedExprs.py (and many other benchmark tests) So if the user just runs `ninja check-lldb`, they'd get an actionable message from PExpectTest-based tests, but a potentially confusing one from TestSTTYBeforeAndAfter. 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
https://github.com/JDevlieghere approved this pull request. +1 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
https://github.com/mysterymath approved this pull request. Thanks! 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
[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)
https://github.com/rupprecht closed 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