- Notifications
You must be signed in to change notification settings - Fork928
test: Improve TestSSH/ForwardGPG stability on macOS via pty.ReadRune#5739
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Writing to stdin for `coder ssh` too early could result in the inputbeing discarded. To work around this we add a new `ptytest` methodcalled `ReadRune` that lets us read one character of output. This willindicate the command is ready to accept input.It could be one character of the prompt, or of the loading messagewaiting for connection to be established.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The rest of the methods in ptytest should probably be updated to take a context too
@@ -475,6 +475,10 @@ Expire-Date: 0 | |||
// real error from being printed. | |||
t.Cleanup(cancel) | |||
// Wait for the prompt or any output really to indicate the command has | |||
// started and accepting input on stdin. | |||
_ = pty.ReadRune(ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If the goal is to wait for output why not make a methodWaitForOutput
instead or something, which has similar behavior but doesn't advance the reader (e.g. using Peek instead of Read)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That's not a bad suggestion, I think both methods can be useful though. I'll add Peek and use that instead. 👍🏻
Agreed. I was in the process of rewriting this and ReadLine to use a common method. I'll put it on my TODO to do a more complete refactor later. |
Writing to stdin for
coder ssh
too early could result in the inputbeing discarded. To work around this we add a new
ptytest
methodcalled
ReadRune
that lets us read one character of output. This willindicate the command is ready to accept input.
It could be one character of the prompt, or of the loading message
waiting for connection to be established.