================
@@ -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
