Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

Merge changes from upstream #5

Merged

Conversation

krnowak
Copy link

@krnowak krnowak commented Mar 31, 2016

Looks like Stefan's work was merged into upstream finally.

Anthony Romano and others added 16 commits February 16, 2016 15:19
This function returns the output of the child, which might be of interest for
the caller.
- New function that returns the output of the child, which might be of interest for
the caller. Also offers to timeout after a specified time.
- Introduces tests for regex related functions.
The expectRegexOutput uses the regexp.FindReaderSubmatchIndex function
to do the matching. As its docs say, it may read more characters than
it is necessary from the stream. This may be problematic if the regex
we use can match chunks of the stream that come right after
another. This is because the function might match the first chunk and
read into the middle of the second chunk, so the subsequent call to
the function will match the third chunk, not the second. For example:

data: "one two three"
regexp: "\w{3,}"

The first call to the function may return "one" as a match and "one
tw" as an output. We can see that the output has the beginning of the
second word which comes after the matched first word. This part is not
available anymore in the stream, thus the following call to the
function will return "three" as a match (instead of "two") and "o
three" as an output.

To fix it, we put the excess characters coming after the whole match
back into the stream, so the subsequent calls to the function may
start their matching from the place when last whole match ended.
…f instead of echo due to it not working on OSX any more
Expect was passing the full chunk buffer to PutBack instead of the valid data subslice.
if err != nil {
t.Fatal(err)
}
if s != "foo\r" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it expect "\r" if the command prints "\n"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My earlier experiments shown that lines read from pty always end in \r\n.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above was a bit too short - they end in \r\n and ReadLine reads pty until it finds \n, so it returns a line ending with \r.

@alban
Copy link

alban commented Mar 31, 2016

Do you have an opinion how to best keep this in sync with upstream? It will help Debian packaging if rkt uses a release of gexpect instead of a commit hash. But how do we manage versions numbers from ThomasRooney/gexpect and from coreos/gexpect? I don't know if Debian has packaged ThomasRooney/gexpect already.

/cc @steveej @onlyjob

Let's see if there is any regression in rkt: rkt/rkt#2346

@alban
Copy link

alban commented Mar 31, 2016

I see that the name of the Debian package is namespaced with "github-coreos":
https://packages.debian.org/sid/golang-github-coreos-gexpect-dev

@krnowak
Copy link
Author

krnowak commented Mar 31, 2016

Upstream is not very active, so sometimes we have to wait really long until the PR gets addressed/merged, so for time being I would prefer to stick to our fork. Also, upstream didn't do any releases so far.

@krnowak
Copy link
Author

krnowak commented Mar 31, 2016

@alban
Copy link

alban commented Mar 31, 2016

👍

LGTM

@jonboulle jonboulle merged commit ca42424 into coreos:master Mar 31, 2016
@alban
Copy link

alban commented Mar 31, 2016

@onlyjob
Copy link

onlyjob commented Mar 31, 2016

Thanks for keeping me in loop guys. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants