Merge "v.io/jiir/runutil: more cleanup"
diff --git a/runutil/sequence.go b/runutil/sequence.go
index 0873c4a..f01d1ea 100644
--- a/runutil/sequence.go
+++ b/runutil/sequence.go
@@ -24,21 +24,20 @@
// methods and the result of that first error is returned by the
// Done method or any of the other 'terminating methods' (see below).
//
-// Unless directed to specific stdout and stderr io.Writers using Capture(), the
-// stdout and stderr output from the command is discarded, except in verbose
-// mode or upon error: when in verbose mode (set either via NewSequence or an
-// Opt) or when the command fails, all the command's output (stdout and stderr)
-// is written to the stdout io.Writer configured either via NewSequence or an
-// Opt. In addition, in verbose mode, command execution logging is written to
-// the stdout and stderr io.Writers configured via NewSequence.
+// Unless directed to specific stdout and stderr io.Writers using Capture(),
+// the stdout and stderr output from the command is discarded, unless an error
+// is encountered, in which case the output from the command that failed (both
+// stdout and stderr) is written to the stderr io.Writer specified via
+// NewSequence. In addition, in verbose mode, command execution logging
+// is written to the stdout an stderr io.Writers configured via NewSequence.
//
// Modifier methods are provided that influence the behaviour of the
-// next invocation of the Run method to set timeouts (Timeout), to
-// capture output (Capture), an set options (Opts).
+// next invocation of the Run method to set timeouts (Timed), to
+// capture output (Capture), input (Read) and to set the environment (Env).
// For example, the following will result in a timeout error.
//
-// err := s.Timed(time.Second).Run("sleep","10").Done()
-// err := s.Timed(time.Second).Last("sleep","10")
+// err := s.Timeout(time.Second).Run("sleep","10").Done()
+// err := s.Timeout(time.Second).Last("sleep","10")
//
// A sequence of commands must be terminated with a call to a 'terminating'
// method. The simplest are the Done or Last methods used in the examples above,
@@ -47,7 +46,7 @@
// would be:
//
// o.Stdout, _ = os.Create("foo")
-// data, err := s.Opts(o).Run("echo","b").ReadFile("foo")
+// data, err := s.Capture(o, nil).Run("echo","b").ReadFile("foo")
// // data == "b"
//
// Note that terminating functions, even those that take an action, may
@@ -55,26 +54,38 @@
//
// In addtion to Run which will always run a command as a subprocess,
// the Call method will invoke a function. Note that Capture and Timeout
-// do not affect such calls, but Opts can be used to control logging.
+// do not affect such calls.
type Sequence struct {
- r *Run
- err error
- caller string
- stdout, stderr io.Writer
- opts *Opts
- dirs []string
- timeout time.Duration
+ r *Run
+ err error
+ caller string
+ stdout, stderr io.Writer
+ stdin io.Reader
+ reading bool
+ env map[string]string
+ opts *Opts
+ defaultStdin io.Reader
+ defaultStdout, defaultStderr io.Writer
+ dirs []string
+ timeout time.Duration
}
// NewSequence creates an instance of Sequence with default values for its
// environment, stdin, stderr, stdout and other supported options.
func NewSequence(env map[string]string, stdin io.Reader, stdout, stderr io.Writer, color, dryRun, verbose bool) *Sequence {
- return &Sequence{r: NewRun(env, stdin, stdout, stderr, color, dryRun, verbose)}
+ return &Sequence{
+ r: NewRun(env, stdin, stdout, stderr, color, dryRun, verbose),
+ defaultStdin: stdin,
+ defaultStdout: stdout,
+ defaultStderr: stderr,
+ }
}
// Capture arranges for the next call to Run or Last to write its stdout and
// stderr output to the supplied io.Writers. This will be cleared and not used
-// for any calls to Run or Last beyond the next one.
+// for any calls to Run or Last beyond the next one. Specifying nil for
+// a writer will result in using the the corresponding io.Writer supplied
+// to NewSequence. ioutil.Discard should be used to discard output.
func (s *Sequence) Capture(stdout, stderr io.Writer) *Sequence {
if s.err != nil {
return s
@@ -83,20 +94,43 @@
return s
}
-// Opts arranges for the next call to Run or Last to use the supplied options.
-// This will be cleared and not used for any calls to Run or Last beyond the
-// next one.
-func (s *Sequence) Opts(opts Opts) *Sequence {
+// Read arranges for the next call to Run or Last to read from the supplied
+// io.Reader. This will be cleared and not used for any calls to Run or Last
+// beyond the next one. Specifying nil will result in reading from os.DevNull.
+func (s *Sequence) Read(stdin io.Reader) *Sequence {
if s.err != nil {
return s
}
- s.opts = &opts
+ s.reading = true
+ s.stdin = stdin
return s
}
+// Env arranges for the next call to Run, Call or Last to use the supplied
+// environment variables. This will be cleared and not used for any calls
+// to Run, Call or Last beyond the next one.
+func (s *Sequence) Env(env map[string]string) *Sequence {
+ if s.err != nil {
+ return s
+ }
+ s.env = env
+ return s
+}
+
+// internal getOpts that doesn't override stdin, stdout, stderr
+func (s *Sequence) getOpts() Opts {
+ var opts Opts
+ if s.opts != nil {
+ opts = *s.opts
+ } else {
+ opts = s.r.Opts()
+ }
+ return opts
+}
+
// Timeout arranges for the next call to Run or Last to be subject to the
// specified timeout. The timeout will be cleared and not used any calls to Run
-// or Last beyond the next one.
+// or Last beyond the next one. It has no effect for calls to Call.
func (s *Sequence) Timeout(timeout time.Duration) *Sequence {
if s.err != nil {
return s
@@ -105,11 +139,8 @@
return s
}
-func (s *Sequence) GetOpts() Opts {
- if s.opts != nil {
- return *s.opts
- }
- return s.r.Opts()
+func (s *Sequence) setOpts(opts Opts) {
+ s.opts = &opts
}
func (s *Sequence) Error() error {
@@ -119,10 +150,6 @@
return s.err
}
-func (s *Sequence) setOpts(opts Opts) {
- s.opts = &opts
-}
-
func fmtError(depth int, err error, detail string) string {
_, file, line, _ := runtime.Caller(depth + 1)
return fmt.Sprintf("%s:%d: %s", filepath.Base(file), line, detail)
@@ -137,7 +164,8 @@
}
func (s *Sequence) reset() {
- s.stdout, s.stderr, s.opts = nil, nil, nil
+ s.stdin, s.stdout, s.stderr, s.env, s.opts = nil, nil, nil, nil, nil
+ s.reading = false
s.timeout = 0
}
@@ -158,52 +186,72 @@
return nil
}
+func useIfNotNil(a, b io.Writer) io.Writer {
+ if a != nil {
+ return a
+ }
+ return b
+}
+
+func writeOutput(from string, to io.Writer) {
+ if fi, err := os.Open(from); err == nil {
+ io.Copy(to, fi)
+ fi.Close()
+ }
+ if wd, err := os.Getwd(); err == nil {
+ fmt.Fprintf(to, "Current Directory: %v\n", wd)
+ }
+}
+
func (s *Sequence) initAndDefer() func() {
if s.stdout == nil && s.stderr == nil {
- f, err := ioutil.TempFile("", "seq")
+ fout, err := ioutil.TempFile("", "seq")
if err != nil {
return func() {}
}
- opts := s.GetOpts()
- stdout := opts.Stdout
- if stdout == nil {
- return func() {}
+ opts := s.getOpts()
+ opts.Stdout = fout
+ opts.Stderr = fout
+ opts.Env = s.env
+ if s.reading {
+ opts.Stdin = s.stdin
}
- opts.Stdout = f
- opts.Stderr = f
s.setOpts(opts)
return func() {
- filename := f.Name()
- f.Close()
- if opts.Verbose || s.err != nil {
- // TODO(cnicolaou): probably best to stream this out rather
- // than buffer the whole file into memory.
- if data, err := ioutil.ReadFile(filename); err == nil {
- fmt.Fprint(stdout, string(data))
- if wd, err := os.Getwd(); err == nil {
- fmt.Fprintf(stdout, "Current Directory: %v\n", wd)
- }
- }
+ filename := fout.Name()
+ fout.Close()
+ defer func() { os.Remove(filename); s.opts = nil }()
+ if s.err != nil {
+ writeOutput(filename, useIfNotNil(s.defaultStderr, os.Stderr))
}
- os.Remove(filename)
- s.opts = nil
+ if opts.Verbose && s.defaultStderr != s.defaultStdout {
+ writeOutput(filename, useIfNotNil(s.defaultStdout, os.Stdout))
+ }
}
}
- opts := s.GetOpts()
+ opts := s.getOpts()
rStdin, wStdin := io.Pipe()
rStderr, wStderr := io.Pipe()
opts.Stdout = wStdin
opts.Stderr = wStderr
- s.setOpts(opts)
+ opts.Env = s.env
+ if s.reading {
+ opts.Stdin = s.stdin
+ }
var stdinCh, stderrCh chan error
if s.stdout != nil {
stdinCh = make(chan error)
go copy(s.stdout, rStdin, stdinCh)
+ } else {
+ opts.Stdout = s.defaultStdout
}
if s.stderr != nil {
stderrCh = make(chan error)
go copy(s.stderr, rStderr, stderrCh)
+ } else {
+ opts.Stderr = s.defaultStderr
}
+ s.setOpts(opts)
return func() {
if err := s.done(wStdin, wStderr, stdinCh, stderrCh); err != nil && s.err == nil {
s.err = err
@@ -276,7 +324,7 @@
return s
}
defer s.initAndDefer()()
- s.setError(s.r.command(s.timeout, s.GetOpts(), path, args...), fmt.Sprintf("Run(%q%s)", path, fmtStringArgs(args...)))
+ s.setError(s.r.command(s.timeout, s.getOpts(), path, args...), fmt.Sprintf("Run(%q%s)", path, fmtStringArgs(args...)))
return s
}
@@ -288,7 +336,7 @@
return s.Done()
}
defer s.initAndDefer()()
- s.setError(s.r.command(s.timeout, s.GetOpts(), path, args...), fmt.Sprintf("Last(%q%s)", path, fmtStringArgs(args...)))
+ s.setError(s.r.command(s.timeout, s.getOpts(), path, args...), fmt.Sprintf("Last(%q%s)", path, fmtStringArgs(args...)))
return s.Done()
}
@@ -299,7 +347,7 @@
return s
}
defer s.initAndDefer()()
- s.setError(s.r.FunctionWithOpts(s.GetOpts(), fn, format, args...), fmt.Sprintf("Call(%s,%s%s)", fn, format, fmtArgs(args)))
+ s.setError(s.r.FunctionWithOpts(s.getOpts(), fn, format, args...), fmt.Sprintf("Call(%s,%s%s)", fn, format, fmtArgs(args)))
return s
}
@@ -381,7 +429,7 @@
if s.err != nil {
return s
}
- s.r.OutputWithOpts(s.GetOpts(), output)
+ s.r.OutputWithOpts(s.getOpts(), output)
return s
}
diff --git a/runutil/sequence_test.go b/runutil/sequence_test.go
index 62df6f0..279e967 100644
--- a/runutil/sequence_test.go
+++ b/runutil/sequence_test.go
@@ -52,6 +52,7 @@
if err != nil {
t.Fatal(err)
}
+ dir := "Current Directory: " + cwd
// Case 1: we only specify stdout/stderr at constructor time.
//
@@ -66,95 +67,42 @@
seq.Run("bash", "-c", "echo a; echo b >&2").
Timeout(time.Microsecond).
Run("sleep", "10000")
- want := "Current Directory: " + cwd + "\n"
+ want := ""
if verbose {
want = `[hh:mm:ss.xx] >> bash -c "echo a; echo b >&2"
[hh:mm:ss.xx] >> OK
a
b
-Current Directory: ` + cwd + `
+` + dir + `
[hh:mm:ss.xx] >> sleep 10000
[hh:mm:ss.xx] >> TIMED OUT
-Current Directory: ` + cwd + `
+` + dir + `
`
}
if got := sanitizeTimestamps(cnstrStdout.String()); want != got {
t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
}
- if got, want := cnstrStderr.String(), "Waiting for command to exit: [\"sleep\" \"10000\"]\n"; want != got {
+ if got, want := cnstrStderr.String(), "Waiting for command to exit: [\"sleep\" \"10000\"]\n"+dir+"\n"; want != got {
t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
}
}
// Case 2: we specify stdout/stderr at constructor time, and also via
- // Opts. The verbose setting from opts takes precedence and controls
- // the output that goes both to constructor stdout and to opts stdout.
- //
- // Verbose mode: The command execution logging goes to constructor
- // stdout, command execution errors go to constructor stderr, and the
- // actual command output goes to opts stdout. Nothing goes to opts
- // stderr.
- //
- // Non-Verbose mode: No stdout output; execution error messages to
- // constructor stderr.
- for _, verbose := range []bool{false, true} {
- var cnstrStdout, cnstrStderr, optsStdout, optsStderr bytes.Buffer
- cstrVerbose := false // irellevant, the opts verbose flag takes precedence.
- seq := runutil.NewSequence(nil, os.Stdin, &cnstrStdout, &cnstrStderr, false, false, cstrVerbose)
- opts := runutil.Opts{Stdout: &optsStdout, Stderr: &optsStderr, Verbose: verbose}
- seq.Opts(opts).
- Run("bash", "-c", "echo a; echo b >&2").
- Opts(opts).
- Timeout(time.Microsecond).
- Run("sleep", "10000")
- want := ""
- if verbose {
- want = `[hh:mm:ss.xx] >> bash -c "echo a; echo b >&2"
-[hh:mm:ss.xx] >> OK
-[hh:mm:ss.xx] >> sleep 10000
-[hh:mm:ss.xx] >> TIMED OUT
-`
- }
- if got := sanitizeTimestamps(cnstrStdout.String()); want != got {
- t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
- }
- if got, want := cnstrStderr.String(), "Waiting for command to exit: [\"sleep\" \"10000\"]\n"; want != got {
- t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
- }
- want = "Current Directory: " + cwd + "\n"
- if verbose {
- want = "a\nb\nCurrent Directory: " + cwd + "\nCurrent Directory: " + cwd + "\n"
- }
- if got := optsStdout.String(); want != got {
- t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
- }
- if got, want := optsStderr.String(), ""; want != got {
- t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
- }
- }
-
- // Case 3: we specify stdout/stderr at constructor time, also via Opts,
- // and also via Capture. The verbose setting from opts takes
- // precedence.
+ // Capture.
//
// Verbose mode: The command execution log goes to constructor stdout,
// command execution errors go to constructor stderr, and the
// stdout/stderr output from the command goes to capture stdout/stderr
- // respectively. Nothing goes to opts stdout/stderr.
+ // respectively.
//
// Non-Verbose mode: The stdout/stderr output from the command goes to
// capture stdout/stderr respectively. No command execution log, but
- // the command execution errors go to constructor stderr. Nothing goes
- // to opts stdout/stderr.
+ // the command execution errors go to constructor stderr.
for _, verbose := range []bool{false, true} {
- var cnstrStdout, cnstrStderr, optsStdout, optsStderr, captureStdout, captureStderr bytes.Buffer
- cstrVerbose := false // irellevant, the opts verbose flag takes precedence.
- seq := runutil.NewSequence(nil, os.Stdin, &cnstrStdout, &cnstrStderr, false, false, cstrVerbose)
- opts := runutil.Opts{Stdout: &optsStdout, Stderr: &optsStderr, Verbose: verbose}
- seq.Opts(opts).
- Capture(&captureStdout, &captureStderr).
+ var cnstrStdout, cnstrStderr, captureStdout, captureStderr bytes.Buffer
+ seq := runutil.NewSequence(nil, os.Stdin, &cnstrStdout, &cnstrStderr, false, false, verbose)
+ seq.Capture(&captureStdout, &captureStderr).
Run("bash", "-c", "echo a; echo b >&2").
- Opts(opts).
Timeout(time.Microsecond).
Run("sleep", "10000")
want := ""
@@ -163,18 +111,13 @@
[hh:mm:ss.xx] >> OK
[hh:mm:ss.xx] >> sleep 10000
[hh:mm:ss.xx] >> TIMED OUT
+` + dir + `
`
}
if got := sanitizeTimestamps(cnstrStdout.String()); want != got {
t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
}
- if got, want := cnstrStderr.String(), "Waiting for command to exit: [\"sleep\" \"10000\"]\n"; want != got {
- t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
- }
- if got, want := optsStdout.String(), "Current Directory: "+cwd+"\n"; want != got {
- t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
- }
- if got, want := optsStderr.String(), ""; want != got {
+ if got, want := cnstrStderr.String(), "Waiting for command to exit: [\"sleep\" \"10000\"]\n"+dir+"\n"; want != got {
t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
}
if got, want := captureStdout.String(), "a\n"; want != got {
@@ -184,6 +127,27 @@
t.Errorf("verbose: %t, got %v, want %v", verbose, got, want)
}
}
+
+ // Case 3: we specify stdout/stderr at constructor and use nil
+ // with Capture to verify that the constructor values are used.
+ var cnstrStdout, cnstrStderr, captureStdout, captureStderr bytes.Buffer
+ seq := runutil.NewSequence(nil, os.Stdin, &cnstrStdout, &cnstrStderr, false, false, false)
+ err = seq.
+ Capture(&captureStdout, nil).Run("bash", "-c", "echo a; echo b >&2").
+ Capture(nil, &captureStderr).Last("bash", "-c", "echo c; echo d >&2")
+
+ if got, want := cnstrStdout.String(), "c\n"; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := cnstrStderr.String(), "b\n"; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := captureStdout.String(), "a\n"; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
+ if got, want := captureStderr.String(), "d\n"; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
}
func TestSequence(t *testing.T) {
@@ -203,14 +167,13 @@
t.Errorf("got %v, want %v", got, want)
}
out.Reset()
- opts := seq.GetOpts()
- opts.Env = map[string]string{
+ env := map[string]string{
"MYTEST": "hi",
"MYTEST2": "there",
}
err = seq.
- Capture(&out, nil).Opts(opts).Run("sh", "-c", "echo $MYTEST").
- Opts(opts).Capture(&out, nil).Run("sh", "-c", "echo $MYTEST2").
+ Capture(&out, nil).Env(env).Run("sh", "-c", "echo $MYTEST").
+ Env(env).Capture(&out, nil).Run("sh", "-c", "echo $MYTEST2").
Done()
if err != nil {
t.Fatal(err)
@@ -239,20 +202,17 @@
if got, want := rmLineNumbers(err.Error()), "sequence_test.go:-: Run(\"sleep\", \"10\"): command timed out"; got != want {
t.Errorf("got %v, want %v", got, want)
}
-
}
// Test that modifiers don't get applied beyond the first invocation of Run.
func TestSequenceModifiers(t *testing.T) {
seq := runutil.NewSequence(nil, os.Stdin, os.Stdout, os.Stderr, false, false, true)
var out bytes.Buffer
-
- opts := seq.GetOpts()
- opts.Env = map[string]string{
+ env := map[string]string{
"MYTEST": "hi",
}
err := seq.
- Capture(&out, nil).Opts(opts).Run("sh", "-c", "echo $MYTEST").
+ Capture(&out, nil).Env(env).Run("sh", "-c", "echo $MYTEST").
Capture(&out, nil).Last("sh", "-c", "echo $MYTEST")
if err != nil {
t.Fatal(err)
@@ -272,27 +232,39 @@
if got, want := out.String(), "hello\n"; got != want {
t.Errorf("got %v, want %v", got, want)
}
+ out.Reset()
+
+ in := bytes.Buffer{}
+ in.WriteString("Hello\n")
+ in.WriteString("World\n")
+
+ if err := seq.Read(&in).Capture(&out, nil).Last("sh", "-c", "read x; echo $x; read y; echo $y"); err != nil {
+ t.Fatal(err)
+ }
+ if got, want := out.String(), "Hello\nWorld\n"; got != want {
+ t.Errorf("got %v, want %v", got, want)
+ }
}
func TestSequenceOutputOnError(t *testing.T) {
var out bytes.Buffer
// Only the output from the command that generates an error is written
- // to out when not in verbose mode.
- seq := runutil.NewSequence(nil, os.Stdin, &out, os.Stderr, false, false, false)
+ // to stderr (i.e. out) when not in verbose mode.
+ seq := runutil.NewSequence(nil, os.Stdin, os.Stdout, &out, false, false, false)
err := seq.Run("sh", "-c", "echo not me").
Run("sh", "-c", "echo ooh; echo ah; echo me; exit 1").
Last("sh", "-c", "echo not me either")
if err == nil {
t.Errorf("expected an error")
}
- if got, want := out.String(), "oh\nah"; !strings.Contains(got, want) {
+ if got, want := out.String(), "oh\nah\nme\n"; !strings.Contains(got, want) {
t.Errorf("got %v doesn't contain %v", got, want)
}
- if got, notWant := out.String(), "not me either"; strings.Contains(got, notWant) {
+ if got, notWant := out.String(), "not me"; strings.Contains(got, notWant) {
t.Errorf("got %v contains %v", got, notWant)
}
-
out.Reset()
+
err = seq.Run("sh", "-c", "echo hard to not include me").
Run("sh", "-c", "echo ooh; echo ah; echo me").
Last("sh", "-c", "echo not me either")
@@ -303,19 +275,7 @@
t.Logf(out.String())
t.Errorf("got %v, want %v", got, want)
}
-
out.Reset()
- // All output is written to out when in verbose mode.
- seq = runutil.NewSequence(nil, os.Stdin, &out, os.Stderr, false, false, true)
- err = seq.Run("sh", "-c", "echo AA").
- Run("sh", "-c", "echo BB; exit 1").
- Last("sh", "-c", "echo CC")
- if got, want := strings.Count(out.String(), "\n"), 8; got != want {
- t.Errorf("got %v, want %v", got, want)
- }
- if got, want := strings.Count(out.String(), "AA"), 2; got != want {
- t.Errorf("got %v, want %v", got, want)
- }
}
type timestamped struct {
@@ -333,16 +293,16 @@
seq := runutil.NewSequence(nil, os.Stdin, os.Stdout, os.Stderr, false, false, true)
ts := ×tamped{}
err := seq.
- Capture(ts, nil).Run("sh", "-c", `
+ Capture(ts, nil).Last("sh", "-c", `
for i in $(seq 1 5); do
echo $i
sleep 1
- done`).Done()
+ done`)
if err != nil {
t.Fatal(err)
}
if got, want := len(ts.data), 5; got != want {
- t.Errorf("got %v, want %v", got, want)
+ t.Fatalf("got %v, want %v", got, want)
}
prev := ts.times[0]
for _, nth := range ts.times[1:] {
@@ -353,7 +313,7 @@
}
}
-func TestSequenceTermination(t *testing.T) {
+func TestSequenceTerminatingMethod(t *testing.T) {
seq := runutil.NewSequence(nil, os.Stdin, os.Stdout, os.Stderr, false, false, true)
filename := "./test-file"
fi, err := os.Create(filename)
@@ -361,17 +321,11 @@
t.Fatal(err)
}
defer os.Remove(filename)
- opts := seq.GetOpts()
- opts.Stdout = fi
- data, err := seq.Opts(opts).Run("echo", "aha").ReadFile(filename)
+ data, err := seq.Capture(fi, nil).Run("echo", "aha").ReadFile(filename)
if err != nil {
t.Fatal(err)
}
- cwd, err := os.Getwd()
- if err != nil {
- t.Fatal(err)
- }
- if got, want := string(data), "aha\nCurrent Directory: "+cwd+"\n"; got != want {
+ if got, want := string(data), "aha\n"; got != want {
t.Errorf("got %v, want %v", got, want)
}
}
