-
Notifications
You must be signed in to change notification settings - Fork 13
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
session: add expect_eager
method
#35
Conversation
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.
Hi @tzemanovic
Thank you for opening the PR, and reporting the issue.
I consider that rexpect
version may be faster because it uses an additional thread.
expectrl
doesn't do it, we try to read from a output when necessary.
I guess the approach with a thread may make some sense.
We could create some util function for this I guess. What do you think?
Maybe Session::check
help you?
} else if !data.is_empty() { | ||
let data_len = data.len(); | ||
self.stream.consume_from_buffer(data_len); | ||
continue; |
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.
Is this what you wan't?
I mean potentially a mach will never be found because we drop data here.
Here's an example.
The output Hello World
.
We do expect_eager("Hello World")
First read_available
reads only Hello
,
Second read_available
reads World
.
Because you drop the buffer the match will never found even though it was there.
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.
ah, good point! I guess we should never drain the buffer for this approach
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.
I was thinking, maybe we could still drain the unmatched buffer parts up to line breaks that would keep the buffer from growing until the match is found, but only if the Needle itself doesn't contain a line break
hi @zhiburt, thanks for checking and sorry about the delay! I might have misunderstood, but from what I saw in expectrl, the slowdown was caused by reading 1 byte at time and trying to match every time, whereas rexpect reads whatever is available at once |
I think it is true. As far as I remember the main reason is. expectrl/src/session/sync_session.rs Lines 89 to 93 in 4025cce
Could you try to call PS: Maybe we could configure a read logic, whether do ready by byte or not. what do you think? |
Unfortunately, I'm not sure if I follow the reasoning for reading byte-by-byte:
|
I'll take a look at it.
Imagine string "123" is ready to be read,
I don't remember clearly the case. But as I do it's more not about EOF (comment is outdated probably). On linux we can read something even when the process is gone. From what I remember we actually should not be allowed to do it on Linux either but we do. So imagine you do So it's like a mechanism to read everything possible whenever it's possible. Maybe wrong in my comments here. The point is this comment about EOF is probably outdated. ref: #19 |
You can check out the new approach to Change your expectrl = { git = "https://github.com/zhiburt/expectrl/", branch = "configuration-of-expect-logic" } |
Thank you, this branch works fine for us too! (Just noticed couple things in 8040332 -
I think when the regex is |
fixed 4c43fe1
It is; and you're right. |
If you don't mind I'll close this PR? Thank you for your contribution. |
Thank you, you too! |
hi, we're trying to migrate from rexpect to this crate in anoma/anoma#1095. It was pretty smooth, thank you for this crate! We just hit couple road-blocks - one of them is that the lazy approach in
Session::expect
is too slow for processes with many lines of output. We get around it by adding thisexpect_eager
variation.