AlecAivazis / survey

A golang library for building interactive and accessible prompts with full support for windows and posix terminals.
MIT License
4.08k stars 350 forks source link

Issue interfacing with Pexpect on Windows #202

Open th3coop opened 5 years ago

th3coop commented 5 years ago

Hey @AlecAivazis, thanks for this awesome Library. We're making great use of it here at ActiveState.

I recently ran into issues with our integration tests which we're running from Python for reasons. The issue I encountered was that the ReadRune function is never able to retrieve any input from the terminal since it's actually expecting a terminal to be there (please correct me if I'm wrong about that assumption) and there isn't one in our context.

I eventually swapped the Posix ReadRune in place of the Windows one and it appears to be working as expected. My question is if there were specific cases that the Windows version of ReadRune were accounting for and what are they? It seems like the Windows version might be unnecessary overhead for your library.

I'll be happy to create a PR if it turns out I'm right.

Thanks again for you work!

AlecAivazis commented 5 years ago

Hey @th3coop! Thanks for the kind words.

If I'm understanding your question, you tested some prompts in a windows environment and things worked as expected? Strange. Which environments and prompts did you test? There are A LOT of nuances between the different windows shells so I can't remember all of the specific tests that the platform specific code accounts for.

th3coop commented 5 years ago

@AlecAivazis, thanks for getting back to me so quick! To clarify, these tests are integration tests in our CLI tool, not the survey modules tests.

you tested some prompts in a windows environment and things worked as expected?

No, they did not work as expected.

The specific failing use case is Windows platform, CMD terminal, using the survey.Input prompt.

The specific use case we've hit is when we prompt a "user" for input. We're using survey.AskOne with an survey.Input struct. Our "user" in this case is the pexpect library in Python. When our CLI prompts the user for input by calling survey.Askone($survey.Input{...}) the Pexpect code writes the input to stdin. This text never makes it through the Survey code. Our test would then hang/fail due to an expect next step not occurring.

I was able to track the issue to the windows implementation of the ReadRune function where it calls readConsoleInput which fails with an Incorrect Function error (Some Windows error). I mentioned above how I got around the issue by using the posix version of ReadRune.

My main inquiry here is why the special Windows implementation of the ReadRune function? So far the Posix method of reading from stdin works as expected. Perhaps there could be a slight simplification of your code base unless there is a "gotcha" with Windows terminal input I'm not aware of yet (possibilities very possible).

Hope I'm being clear (wish I could be more succinct)!

AlecAivazis commented 5 years ago

I don't remember all of the gotchas that we ran into that justified supporting a windows-specific implementation of RuneReader. I just remember that @coryb and I tried everything we could think of before we decided it was the only option. I wish I could be more helpful.

That being said, I'm curious to see if we can figure out why the windows RuneReader isn't behaving as you expect - can you post the full error? Afaik, you shouldn't be encountering an error

th3coop commented 5 years ago

@AlecAivazis, for the ridiculous delay. This can be closed. This breaks input from arrow keys. Not sure how I missed that but apologies for wasting your time! Thanks again for the great library!

th3coop commented 5 years ago

Oops, not close. Just ignore my suggested fix of using the Linux version of the ReadRune. I had the following open in a browers since June third:

I wish I could be more helpful.

It may not seem like it but this is helpful. It gives me a path forward.

can you post the full error? Afaik, you shouldn't be encountering an error

Unfortunately there are no further error details to share. At the line I shared in my last comment, the only thing returned is Incorrect Function which seems to be a generic error msg returned by the Windows api.

I've finally got some time to devote to getting a repro script that will hopefully be easy to use to show the issue.

th3coop commented 5 years ago

Ok, I made a repo with an MVP for the issue. As is often the case, in doing so I found that my original theory of what triggers the error was wrong, the error is still the same though. Originally I thought it was input being sent to the survey lib from Pexpect that was causing the issue but it turns out it's the line that's reading to see if the expected output from the child process is what we...expect :D

I hope this is helpful and isn't overkill: https://github.com/th3coop/test-survey

Let me know if you need clarification or any thoughts.

AlecAivazis commented 5 years ago

Thanks for creating the example repo!

Unfortunately I am very busy for the next few months so I don't think I will be able to dig into this but if you start poking around and have questions, you can always ask here or in the gophers slack channel 😄 Maybe @coryb or @MarkusFreitag have some time and can take a look?

th3coop commented 5 years ago

@AlecAivazis understood. When we get a chance to dig in to it again we'll report back.

steventangx commented 5 years ago

FYI, we're seeing running into this issue on the Posix ReadRune function as well.

Python 3.7.2
Darwin Kernel Version 18.2.0
Pexpect 4.7.0

Like OP described, pexpect just times out because Survey never receives the stdin.

Naatan commented 5 years ago

Hi guys, I took over for @th3coop on this task and wanted to report my findings.

The issue is that when using cmd.exe the shell expects a "windows console program". Survey correctly emulates this but it makes the fatal mistake of assuming that windows == cmd.exe. If you were to use Survey with msys or WSL it will fail just as it did for us in our integration tests.

What should happen is the Survey package should check if we are in a windows console program, and if not then use the "unix runereader" instead. You could check if we are in a console program by checking the result of https://github.com/AlecAivazis/survey/blob/master/terminal/runereader_windows.go#L65 - if this returns a "Invalid function" error it means you are not in a console program.

We managed to find a workaround in using https://github.com/iamacarpet/go-winpty, but it's by no means ideal. I will likely be spending some time forking the Survey package and implementing the suggestion I gave above. I will open a PR if I end up doing this.

gbraad commented 5 years ago

Not sure, but this might be related to #148 ?

Naatan commented 5 years ago

Yep, that seems to be the same issue.

gbraad commented 5 years ago

See https://github.com/mintty/mintty/issues/906