Repository navigation
testscript: add waitmatch command - #311
Conversation
rogpeppe
left a comment
rogpeppe
left a comment
There was a problem hiding this comment.
LGTM modulo a few suggestions and comments.
| '&command&' to print a line to standard output, or to standard error with | ||
| -stderr, which matches the regular expression pattern. This is useful to |
There was a problem hiding this comment.
The distance between "print a line" and "which matches" reads awkwardly to me.
| '&command&' to print a line to standard output, or to standard error with | |
| -stderr, which matches the regular expression pattern. This is useful to | |
| '&command&' to print a line matching the regular expression pattern. | |
| It matches against the command's standard output unless -stderr is specified. | |
| This is useful to |
There was a problem hiding this comment.
Also, I wonder if it should be possible to specify the timeout explicitly: some commands might legitimately take a long time to run.
There was a problem hiding this comment.
Applied, thanks.
As for a timeout: waitmatch does not impose one of its own. It waits until the command exits or the test deadline is reached, so commands which take a long time already work. An explicit timeout would only let a script fail sooner than the test deadline, which we could add later without breaking anything if someone needs it.
|
|
||
| // next returns the output written so far, | ||
| // along with a channel which is closed when more output is written. | ||
| func (o *backgroundOutput) next() (string, <-chan struct{}) { |
There was a problem hiding this comment.
It seems a bit odd to me that this returns the entire output (allocating the entire buffer each time) when the command this is intended for only wants to see newly added bytes.
I wonder whether it might work out more nicely to have something like:
// reader returns a reader that produces the text written to o.
// If the context is cancelled, the final read will return ctx.Err().
func (o *backgroundOutput) reader(ctx context.Context) io.Reader
//
then we can just use `bufio.Scanner` to implement the `waitmatch` command,
and that'll work with \r\n terminated lines too, and probably be a decent amount simpler (although perhaps we'll trade off too much complexity here...)
There was a problem hiding this comment.
Good idea, done: waitmatch is now a bufio.Scanner over a reader of the buffer, which is quite a bit simpler. For what it is worth, next did not allocate, as strings.Builder.String does not copy the buffer.
| break | ||
| } | ||
| scanned += len(line) + 1 | ||
| if m := re.FindStringSubmatch(line); m != nil { |
There was a problem hiding this comment.
ISTM that we should be catering for \r\n line termination here, and that as written, if a command produces \r\n as a line endinng, we probably won't match it against $.
There was a problem hiding this comment.
Good catch, confirmed. Fixed by the switch to bufio.Scanner, which drops a trailing \r, and I added a CRLF test case.
It waits for a background command to print a line matching a regular expression, optionally setting variables to its subexpressions. This lets a script wait for a server to be ready, for example one which listens on a port chosen by the kernel and prints its address. To read the output of a running command, background commands now write to a buffer which is safe for concurrent use and signals new writes. waitmatch fails if the command exits without printing a matching line, or if the test times out first.
signalcatcher now prints a line once it is ready to catch a signal, rather than creating a file which the test had to poll for, so the waitfile test command is no longer needed.
See rogpeppe/go-internal#311. Signed-off-by: Daniel Martí <mvdan@mvdan.cc> Change-Id: I76ecaa62e52a3cb06d910e628437274704edcaea GitOrigin-RevId: 890c324e1754ed1567d58771d1f79ece910f5dda
(see commit messages - please do not squash)