[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
[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/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)
@@ -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/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
[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
[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)
@@ -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."
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."
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)
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
[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/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
[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)
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)
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":
[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
