lib: update various tests to use v23test
In particular, this CL updates a handful of "v23_test.go"
tests, as well as nearly all of the "v23_internal_test.go"
tests (many of which didn't need v23tests / go generate at
all).
I'd initially suggested doing one massive CL to update all
tests, but my feeling now is that incremental CLs will be
easier to review and will make it easier to tweak
gosh/v23test as needed along the way. Let me know if you
prefer the "one massive CL" approach, though.
Includes a few updates to gosh and v23test:
- Adjust semantics of Shell.Cleanup to allow for multiple
calls, where calls after the first have no effect. This
facilitates Shell wrapping, a la v23test.Shell.
- Update v23test.NewShell to fix edge cases around cleanup,
leveraging the new gosh.Shell.Cleanup semantics.
- Update {,Combined}Output to return strings, since strings
are more convenient and these methods are anyways not
meant for performance/size-sensitive applications.
MultiPart: 1/2
Change-Id: Ib9dfa3d0c7931963e652f1754ff55cc2f2605b52
diff --git a/gosh/.api b/gosh/.api
index 3602790..61d44e5 100644
--- a/gosh/.api
+++ b/gosh/.api
@@ -10,8 +10,8 @@
pkg gosh, func WatchParent()
pkg gosh, method (*Cmd) AwaitReady()
pkg gosh, method (*Cmd) AwaitVars(...string) map[string]string
-pkg gosh, method (*Cmd) CombinedOutput() []byte
-pkg gosh, method (*Cmd) Output() ([]byte, []byte)
+pkg gosh, method (*Cmd) CombinedOutput() string
+pkg gosh, method (*Cmd) Output() (string, string)
pkg gosh, method (*Cmd) Process() *os.Process
pkg gosh, method (*Cmd) Run()
pkg gosh, method (*Cmd) Shutdown(os.Signal)
diff --git a/gosh/cmd.go b/gosh/cmd.go
index 42bfb4d..a3d2a76 100644
--- a/gosh/cmd.go
+++ b/gosh/cmd.go
@@ -116,7 +116,7 @@
// Output calls Start followed by Wait, then returns this command's stdout and
// stderr.
-func (c *Cmd) Output() ([]byte, []byte) {
+func (c *Cmd) Output() (string, string) {
c.sh.Ok()
stdout, stderr, err := c.output()
c.sh.HandleError(err)
@@ -125,7 +125,7 @@
// CombinedOutput calls Start followed by Wait, then returns this command's
// combined stdout and stderr.
-func (c *Cmd) CombinedOutput() []byte {
+func (c *Cmd) CombinedOutput() string {
c.sh.Ok()
res, err := c.combinedOutput()
c.sh.HandleError(err)
@@ -393,12 +393,12 @@
return c.wait()
}
-func (c *Cmd) output() ([]byte, []byte, error) {
+func (c *Cmd) output() (string, string, error) {
var stdout, stderr bytes.Buffer
addWriter(&c.stdoutWriters, &stdout)
addWriter(&c.stderrWriters, &stderr)
err := c.run()
- return stdout.Bytes(), stderr.Bytes(), err
+ return stdout.String(), stderr.String(), err
}
type threadSafeBuffer struct {
@@ -412,18 +412,18 @@
return b.buf.Write(p)
}
-func (b *threadSafeBuffer) Bytes() []byte {
+func (b *threadSafeBuffer) String() string {
b.mu.Lock()
defer b.mu.Unlock()
- return b.buf.Bytes()
+ return b.buf.String()
}
-func (c *Cmd) combinedOutput() ([]byte, error) {
+func (c *Cmd) combinedOutput() (string, error) {
buf := &threadSafeBuffer{}
addWriter(&c.stdoutWriters, buf)
addWriter(&c.stderrWriters, buf)
err := c.run()
- return buf.Bytes(), err
+ return buf.String(), err
}
func (c *Cmd) process() (*os.Process, error) {
diff --git a/gosh/internal/gosh_example/main.go b/gosh/internal/gosh_example/main.go
index 02ed614..51d4754 100644
--- a/gosh/internal/gosh_example/main.go
+++ b/gosh/internal/gosh_example/main.go
@@ -27,7 +27,7 @@
binPath = sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_client")
c = sh.Cmd(binPath, "-addr="+addr)
stdout, _ := c.Output()
- fmt.Print(string(stdout))
+ fmt.Print(stdout)
}
var (
@@ -49,14 +49,14 @@
// Run client.
c = sh.Fn(get, addr)
stdout, _ := c.Output()
- fmt.Print(string(stdout))
+ fmt.Print(stdout)
}
func ExampleShellMain() {
sh := gosh.NewShell(gosh.Opts{})
defer sh.Cleanup()
stdout, _ := sh.Main(lib.HelloWorldMain).Output()
- fmt.Print(string(stdout))
+ fmt.Print(stdout)
}
func main() {
diff --git a/gosh/shell.go b/gosh/shell.go
index faa1cc3..1c0c3ce 100644
--- a/gosh/shell.go
+++ b/gosh/shell.go
@@ -6,7 +6,7 @@
// them, wait for them to exit, capture their output streams, pipe messages
// between them, terminate them (e.g. on SIGINT), and so on.
//
-// Gosh is meant to be used in situations where you might otherwise be tempted
+// Gosh is meant to be used in situations where one might otherwise be tempted
// to write a shell script. (Oh my gosh, no more shell scripts!)
//
// For usage examples, see shell_test.go and internal/gosh_example/main.go.
@@ -188,17 +188,15 @@
// Cleanup cleans up all resources (child processes, temporary files and
// directories) associated with this Shell. It is safe (and recommended) to call
-// Cleanup after a Shell error.
+// Cleanup after a Shell error. It is also safe to call Cleanup multiple times;
+// calls after the first return immediately with no effect.
func (sh *Shell) Cleanup() {
if !sh.calledNewShell {
panic(errDidNotCallNewShell)
}
sh.cleanupMu.Lock()
defer sh.cleanupMu.Unlock()
- if sh.calledCleanup {
- panic(errAlreadyCalledCleanup)
- } else {
- sh.calledCleanup = true
+ if !sh.calledCleanup {
sh.cleanup()
}
}
@@ -259,9 +257,6 @@
if sh.Opts.BinDir == "" {
var err error
if sh.Opts.BinDir, err = sh.makeTempDir(); err != nil {
- // Note: Here and below, we keep sh.calledCleanup false so that clients
- // with a non-fatal Errorf implementation can safely defer sh.Cleanup()
- // before checking sh.Err.
sh.cleanup()
return sh, err
}
@@ -282,11 +277,10 @@
sh.cleanupMu.Lock()
defer sh.cleanupMu.Unlock()
if !sh.calledCleanup {
- sh.calledCleanup = true
sh.cleanup()
}
// Note: We hold cleanupMu during os.Exit(1) so that the main goroutine will
- // not call Shell.ok() or Shell.Cleanup() and panic before we exit.
+ // not call Shell.Ok() and panic before we exit.
os.Exit(1)
})
return sh, nil
@@ -519,6 +513,7 @@
}
func (sh *Shell) cleanup() {
+ sh.calledCleanup = true
// Terminate all children that are still running. Note, newShell() calls
// syscall.Setpgid().
pgid, pid := syscall.Getpgrp(), os.Getpid()
diff --git a/gosh/shell_test.go b/gosh/shell_test.go
index ec13332..c583987 100644
--- a/gosh/shell_test.go
+++ b/gosh/shell_test.go
@@ -6,7 +6,7 @@
// TODO(sadovsky): Add more tests:
// - variadic function registration and invocation
-// - shell cleanup
+// - effects of Shell.Cleanup
// - Cmd.{Wait,Run}
// - Shell.{Args,Wait,Rename,MakeTempFile,MakeTempDir}
// - Opts (including defaulting behavior)
@@ -119,7 +119,7 @@
binPath = sh.BuildGoPkg("v.io/x/lib/gosh/internal/gosh_example_client")
c = sh.Cmd(binPath, "-addr="+addr)
stdout, _ := c.Output()
- eq(t, string(stdout), "Hello, world!\n")
+ eq(t, stdout, "Hello, world!\n")
}
var (
@@ -141,37 +141,38 @@
// Run client.
c = sh.Fn(get, addr)
stdout, _ := c.Output()
- eq(t, string(stdout), "Hello, world!\n")
+ eq(t, stdout, "Hello, world!\n")
}
func TestShellMain(t *testing.T) {
sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
defer sh.Cleanup()
stdout, _ := sh.Main(lib.HelloWorldMain).Output()
- eq(t, string(stdout), "Hello, world!\n")
+ eq(t, stdout, "Hello, world!\n")
}
var write = gosh.Register("write", func(stdout, stderr bool) error {
+ tenMs := 10 * time.Millisecond
if stdout {
- time.Sleep(time.Millisecond)
+ time.Sleep(tenMs)
if _, err := os.Stdout.Write([]byte("A")); err != nil {
return err
}
}
if stderr {
- time.Sleep(time.Millisecond)
+ time.Sleep(tenMs)
if _, err := os.Stderr.Write([]byte("B")); err != nil {
return err
}
}
if stdout {
- time.Sleep(time.Millisecond)
+ time.Sleep(tenMs)
if _, err := os.Stdout.Write([]byte("A")); err != nil {
return err
}
}
if stderr {
- time.Sleep(time.Millisecond)
+ time.Sleep(tenMs)
if _, err := os.Stderr.Write([]byte("B")); err != nil {
return err
}
@@ -194,32 +195,32 @@
// Write to stdout only.
c := sh.Fn(write, true, false)
stdoutPipe, stderrPipe := c.StdoutPipe(), c.StderrPipe()
- eq(t, string(c.CombinedOutput()), "AA")
+ eq(t, c.CombinedOutput(), "AA")
eq(t, toString(stdoutPipe), "AA")
eq(t, toString(stderrPipe), "")
stdout, stderr := sh.Fn(write, true, false).Output()
- eq(t, string(stdout), "AA")
- eq(t, string(stderr), "")
+ eq(t, stdout, "AA")
+ eq(t, stderr, "")
// Write to stderr only.
c = sh.Fn(write, false, true)
stdoutPipe, stderrPipe = c.StdoutPipe(), c.StderrPipe()
- eq(t, string(c.CombinedOutput()), "BB")
+ eq(t, c.CombinedOutput(), "BB")
eq(t, toString(stdoutPipe), "")
eq(t, toString(stderrPipe), "BB")
stdout, stderr = sh.Fn(write, false, true).Output()
- eq(t, string(stdout), "")
- eq(t, string(stderr), "BB")
+ eq(t, stdout, "")
+ eq(t, stderr, "BB")
// Write to both stdout and stderr.
c = sh.Fn(write, true, true)
stdoutPipe, stderrPipe = c.StdoutPipe(), c.StderrPipe()
- eq(t, string(c.CombinedOutput()), "ABAB")
+ eq(t, c.CombinedOutput(), "ABAB")
eq(t, toString(stdoutPipe), "AA")
eq(t, toString(stderrPipe), "BB")
stdout, stderr = sh.Fn(write, true, true).Output()
- eq(t, string(stdout), "AA")
- eq(t, string(stderr), "BB")
+ eq(t, stdout, "AA")
+ eq(t, stderr, "BB")
}
var sleep = gosh.Register("sleep", func(d time.Duration) {
@@ -285,13 +286,6 @@
}()
}
-// Tests that sh.Cleanup succeeds even if sh.Err is not nil.
-func TestCleanupAfterError(t *testing.T) {
- sh := gosh.NewShell(gosh.Opts{Errorf: t.Logf})
- sh.Err = fakeError
- sh.Cleanup()
-}
-
// Tests that sh.Cleanup panics under various conditions.
func TestCleanupPanics(t *testing.T) {
func() { // errDidNotCallNewShell
@@ -299,12 +293,20 @@
defer func() { neq(t, recover(), nil) }()
sh.Cleanup()
}()
- func() { // errAlreadyCalledCleanup
- sh := gosh.NewShell(gosh.Opts{Errorf: t.Logf})
- sh.Cleanup()
- defer func() { neq(t, recover(), nil) }()
- sh.Cleanup()
- }()
+}
+
+// Tests that sh.Cleanup succeeds even if sh.Err is not nil.
+func TestCleanupAfterError(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh.Err = fakeError
+ sh.Cleanup()
+}
+
+// Tests that sh.Cleanup can be called multiple times.
+func TestMultipleCleanup(t *testing.T) {
+ sh := gosh.NewShell(gosh.Opts{Errorf: makeErrorf(t), Logf: t.Logf})
+ sh.Cleanup()
+ sh.Cleanup()
}
func TestMain(m *testing.M) {
diff --git a/vlog/flags_test.go b/vlog/flags_test.go
index fc7d7bb..575504e 100644
--- a/vlog/flags_test.go
+++ b/vlog/flags_test.go
@@ -11,14 +11,11 @@
"path/filepath"
"testing"
+ "v.io/x/lib/gosh"
"v.io/x/lib/vlog"
-
- "v.io/x/ref/test/modules"
)
-//go:generate jiri test generate
-
-var child = modules.Register(func(env *modules.Env, args ...string) error {
+var child = gosh.Register("child", func() error {
tmp := filepath.Join(os.TempDir(), "foo")
flag.Set("log_dir", tmp)
flag.Set("vmodule", "foo=2")
@@ -42,19 +39,14 @@
return fmt.Errorf("max_stack_buf_size unexpectedly set to %v", v)
}
return nil
-}, "child")
+})
func TestFlags(t *testing.T) {
- sh, err := modules.NewShell(nil, nil, testing.Verbose(), t)
- if err != nil {
- t.Fatalf("unexpected error: %s", err)
- }
- defer sh.Cleanup(nil, nil)
- h, err := sh.Start(nil, child)
- if err != nil {
- t.Fatalf("unexpected error: %v", err)
- }
- if err = h.Shutdown(nil, os.Stderr); err != nil {
- t.Errorf("unexpected error: %v", err)
- }
+ sh := gosh.NewShell(gosh.Opts{Errorf: t.Fatalf, Logf: t.Logf})
+ defer sh.Cleanup()
+ sh.Fn(child).Run()
+}
+
+func TestMain(m *testing.M) {
+ os.Exit(gosh.Run(m.Run))
}
diff --git a/vlog/v23_test.go b/vlog/v23_test.go
deleted file mode 100644
index e53a68c..0000000
--- a/vlog/v23_test.go
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright 2015 The Vanadium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// This file was auto-generated via go generate.
-// DO NOT UPDATE MANUALLY
-
-package vlog_test
-
-import (
- "os"
- "testing"
-
- "v.io/x/ref/test/modules"
-)
-
-func TestMain(m *testing.M) {
- modules.DispatchAndExitIfChild()
- os.Exit(m.Run())
-}
