Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find a suitable pexpect module for Windows #22648

Open
llvmbot opened this issue Jan 20, 2015 · 6 comments
Open

Find a suitable pexpect module for Windows #22648

llvmbot opened this issue Jan 20, 2015 · 6 comments
Labels
bugzilla Issues migrated from bugzilla lldb

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 20, 2015

Bugzilla Link 22274
Version unspecified
OS Windows NT
Blocks #24826
Reporter LLVM Bugzilla Contributor
CC @vadimcn

Extended Description

For example, maybe this will work. https://gist.github.com/anthonyeden/8488763

If this doesn't work, maybe we can find another, or write our own.

Until then, many tests are XFAILed on Windows.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 24, 2016

New pyexpect version is working on Windows as well.
https://pexpect.readthedocs.org/en/stable/overview.html#pexpect-on-windows

I have tested it on Python 3.5.1 and it was working well (expect including more advanced regexp with capture groups, sendline, timeouts).

@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 24, 2016

This looks very promising. One thing that concerns me is this:

pexpect.spawn and pexpect.run are not available on Windows, as they rely on Unix pseudoterminals (ptys). Cross platform code must not use these.

The best possible scenario is that we're not using either of these methods. I'm 99% sure we are using pexpect.spawn though, so I don't think we will be this lucky.

The second best scenario is that all uses of pexpect.spawn can be ported to some other mechanism that pexpect provides. It looks like that method would be pexpect.popen_spawn.PopenSpawn. I don't know what the difference is between pexpect.spawn and pexpect.popen_spawn.PopenSpawn or what features we're missing out on.

We will also have to decide whether we want to support multiple pexpect versions. If 4.0 is the minimum required version for Windows, it might be worth asking other platform owners if they are ok with requiring pexpect 4.0 for their platforms as well. If it is ok, this could simplify some of the logic in the test suite and we won't have to worry about someone using one version of pexpect breaking someone that uses a different version.

I CC'ed the OSX platform owner and the Android platform owner to see if they have any concerns about bumping the required pexpect version.

I'm looking forward to one day seeing the pexpect tests working on Windows.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 25, 2016

I believe it makes sense to bump up pexpect version for every platform to 4.0.
Multiple versions of pexpect may introduce additional complexity since theoretically we may get skewed results per platform due differences between pexpect versions?

@vadimcn
Copy link
Contributor

vadimcn commented Sep 17, 2018

It looks like LLDB carries its own version of pexpect (under third_party/Python/module/pexpect-2.4). So replacing it with 4.0 shouldn't be a problem?

What is a problem, though, is that LLDB is one of those programs that alter behavior depending on whether stdio is attached to a tty. I tried replacing pexpect.spawn with pexpect.popen_spawn.PopenSpawn (after upgrading pexpect), but that doesn't work because LLDB won't output the "(lldb)" prompt when stdout is a pipe.
One way to fix this is to give LLDB some sort of --force-interactive flag (and probably add File::SetIsInterative() method). Another - to modify all tests to not expect the prompt, though it might make them more fragile...
Thoughts?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Nov 26, 2021

mentioned in issue #24826

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
DavidSpickett added a commit that referenced this issue Mar 1, 2024
…T_TEST_REQUIREMENTS

See #22648 for why we don't use it on
Windows. Any pexpect tests are skipped there.
@DavidSpickett
Copy link
Collaborator

I tried switching to PopenSpawn for all the tests that don't need interaction or to set terminal width. By the time you do that, you get 1 maybe 2 new tests running on Windows (once you port the example source too). So I don't think pursuing that direction right now is worth it (this issue itself is still valid though, ideally we'd have parity across platforms).

Of the tests marked with llvm.org/pr22274, most are benchmarks and then there were 4 other tests I had to mark as interactive (and therefore not going to run on Windows at all).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla lldb
Projects
None yet
Development

No branches or pull requests

3 participants