veyron/lib/testutil: Avoid parsing flags in init()
Calling flags.Parse in the init of packages is generally not recommended since
it requires that all flags be defined first, which is impossible to ensure
as the init order of independent packages is not defined.
veyron/lib/testutil had hacks to work around this by introducing dependencies
on all packages that define flags and are used in tests. This was hacky,
fragile and hard to maintain.
In this commit, we change the approach to be an explicit call to flags.Parse
(well, to testutil.Initialize, which calls flags.Parse). You may notice that
this call is made in the init function of packages in _test.go files, which
seems to be contradicting all that was said above.
Yes, it does. But the thinking is that calling flags.Parse in "package main" or
any _test.go files is okay, since in those cases, necessarily all packages that
the main/test depends on are initialized first.
Change-Id: I005e8223ec7e9ed630bdf563df059decbf827e06
diff --git a/lib/modules/core/core_test.go b/lib/modules/core/core_test.go
index a9094bb..843221c 100644
--- a/lib/modules/core/core_test.go
+++ b/lib/modules/core/core_test.go
@@ -16,7 +16,7 @@
"veyron.io/veyron/veyron/lib/modules"
"veyron.io/veyron/veyron/lib/modules/core"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
)
func TestCommands(t *testing.T) {
@@ -30,6 +30,7 @@
}
func init() {
+ testutil.Init()
rt.Init()
}
diff --git a/lib/signals/signals_test.go b/lib/signals/signals_test.go
index 5d9f6a6..e383d39 100644
--- a/lib/signals/signals_test.go
+++ b/lib/signals/signals_test.go
@@ -21,7 +21,7 @@
"veyron.io/veyron/veyron/lib/expect"
"veyron.io/veyron/veyron/lib/modules"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/lib/testutil/security"
"veyron.io/veyron/veyron/profiles"
vflag "veyron.io/veyron/veyron/security/flag"
@@ -34,6 +34,7 @@
}
func init() {
+ testutil.Init()
modules.RegisterChild("handleDefaults", "", handleDefaults)
modules.RegisterChild("handleCustom", "", handleCustom)
modules.RegisterChild("handleCustomWithStop", "", handleCustomWithStop)
diff --git a/lib/testutil/blackbox/subprocess_test.go b/lib/testutil/blackbox/subprocess_test.go
index a35e3da..404eeba 100644
--- a/lib/testutil/blackbox/subprocess_test.go
+++ b/lib/testutil/blackbox/subprocess_test.go
@@ -137,6 +137,7 @@
}
func init() {
+ testutil.Init()
blackbox.CommandTable["sleepChild"] = sleepChild
blackbox.CommandTable["sleepParent"] = sleepParent
}
diff --git a/lib/testutil/init.go b/lib/testutil/init.go
index 07fa6d4..c8aaf5d 100644
--- a/lib/testutil/init.go
+++ b/lib/testutil/init.go
@@ -1,8 +1,9 @@
// Package testutil provides initalization and utility routines for unit tests.
//
-// All tests should import it, even if only for its initialization:
-// import _ "veyron.io/veyron/veyron/lib/testutil"
-//
+// Configures logging, random number generators and other global state.
+// Typical usage in _test.go files:
+// import "veyron.io/veyron/veyron/lib/testutil"
+// func init() { testutil.Init() }
package testutil
import (
@@ -12,26 +13,11 @@
"runtime"
"strconv"
"sync"
- // Need to import all of the packages that could possibly
- // define flags that we care about. In practice, this is the
- // flags defined by the testing package, the logging library
- // and any flags defined by the blackbox package below.
- // TODO(cnicolau,ashankar): This is painful to ensure. Not calling
- // flag.Parse in init() is the right solution?
- _ "testing"
"time"
-
- _ "veyron.io/veyron/veyron/services/mgmt/suidhelper/impl/flag"
-
- // Import blackbox to ensure that it gets to define its flags.
- _ "veyron.io/veyron/veyron/lib/testutil/blackbox"
-
"veyron.io/veyron/veyron2/vlog"
)
-const (
- SeedEnv = "VEYRON_RNG_SEED"
-)
+const SeedEnv = "VEYRON_RNG_SEED"
// Random is a concurrent-access friendly source of randomness.
type Random struct {
@@ -60,11 +46,16 @@
return r.rand.Int63()
}
-var (
- Rand *Random
-)
+var Rand *Random
-func init() {
+// Init sets up state for running tests: Adjusting GOMAXPROCS,
+// configuring the vlog logging library, setting up the random number generator
+// etc.
+//
+// Doing so requires flags to be parse, so this function explicitly parses
+// flags. Thus, it is NOT a good idea to call this from the init() function
+// of any module except "main" or _test.go files.
+func Init() {
if os.Getenv("GOMAXPROCS") == "" {
// Set the number of logical processors to the number of CPUs,
// if GOMAXPROCS is not set in the environment.
@@ -72,6 +63,8 @@
}
// At this point all of the flags that we're going to use for
// tests must be defined.
+ // This will be the case if this is called from the init()
+ // function of a _test.go file.
flag.Parse()
vlog.ConfigureLibraryLoggerFromFlags()
// Initialize pseudo-random number generator.
diff --git a/runtimes/google/ipc/flow_test.go b/runtimes/google/ipc/flow_test.go
index ffc42b9..183930b 100644
--- a/runtimes/google/ipc/flow_test.go
+++ b/runtimes/google/ipc/flow_test.go
@@ -6,7 +6,7 @@
"fmt"
"testing"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/runtimes/google/ipc/stream/sectest"
"veyron.io/veyron/veyron2/ipc"
@@ -15,6 +15,8 @@
"veyron.io/veyron/veyron2/verror"
)
+func init() { testutil.Init() }
+
// newTestFlows returns the two ends of a bidirectional flow. Each end has its
// own bookkeeping, to allow testing of method calls.
func newTestFlows() (*testFlow, *testFlow) {
diff --git a/runtimes/google/ipc/full_test.go b/runtimes/google/ipc/full_test.go
index fb66f19..cbb2f49 100644
--- a/runtimes/google/ipc/full_test.go
+++ b/runtimes/google/ipc/full_test.go
@@ -12,7 +12,7 @@
"time"
"veyron.io/veyron/veyron/lib/netstate"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/profiles"
imanager "veyron.io/veyron/veyron/runtimes/google/ipc/stream/manager"
"veyron.io/veyron/veyron/runtimes/google/ipc/stream/sectest"
@@ -1045,5 +1045,6 @@
}
func init() {
+ testutil.Init()
vom.Register(fakeTimeCaveat(0))
}
diff --git a/runtimes/google/ipc/results_store_test.go b/runtimes/google/ipc/results_store_test.go
index a6875e9..780eff5 100644
--- a/runtimes/google/ipc/results_store_test.go
+++ b/runtimes/google/ipc/results_store_test.go
@@ -8,6 +8,8 @@
"veyron.io/veyron/veyron/lib/testutil"
)
+func init() { testutil.Init() }
+
func randomKeys() []uint64 {
n := (testutil.Rand.Intn(256*10) / 10) + 256
k := make([]uint64, n)
diff --git a/runtimes/google/ipc/stream/manager/manager_test.go b/runtimes/google/ipc/stream/manager/manager_test.go
index 1130e2c..cba98e7 100644
--- a/runtimes/google/ipc/stream/manager/manager_test.go
+++ b/runtimes/google/ipc/stream/manager/manager_test.go
@@ -15,7 +15,7 @@
"veyron.io/veyron/veyron2/security"
"veyron.io/veyron/veyron2/vlog"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/lib/testutil/blackbox"
"veyron.io/veyron/veyron/runtimes/google/ipc/stream/sectest"
"veyron.io/veyron/veyron/runtimes/google/ipc/stream/vc"
@@ -28,9 +28,10 @@
}
func init() {
- // The testutil package's init sets GOMAXPROCS to NumCPU. We want to
- // force GOMAXPROCS to remain at 1, in order to trigger a particular
- // race condition tht occurs when closing the server; also, using 1 cpu
+ testutil.Init()
+ // testutil.Init sets GOMAXPROCS to NumCPU. We want to force
+ // GOMAXPROCS to remain at 1, in order to trigger a particular race
+ // condition tht occurs when closing the server; also, using 1 cpu
// introduces less variance in the behavior of the test.
runtime.GOMAXPROCS(1)
diff --git a/runtimes/google/ipc/stream/proxy/proxy_test.go b/runtimes/google/ipc/stream/proxy/proxy_test.go
index e705c6c..f3c9f33 100644
--- a/runtimes/google/ipc/stream/proxy/proxy_test.go
+++ b/runtimes/google/ipc/stream/proxy/proxy_test.go
@@ -11,13 +11,15 @@
"veyron.io/veyron/veyron2/ipc/stream"
"veyron.io/veyron/veyron2/naming"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/runtimes/google/ipc/stream/manager"
"veyron.io/veyron/veyron/runtimes/google/ipc/stream/proxy"
"veyron.io/veyron/veyron/runtimes/google/ipc/stream/sectest"
"veyron.io/veyron/veyron/runtimes/google/ipc/stream/vc"
)
+func init() { testutil.Init() }
+
func TestProxy(t *testing.T) {
proxy, err := proxy.New(naming.FixedRoutingID(0xbbbbbbbbbbbbbbbb), nil, "tcp", "127.0.0.1:0", "")
if err != nil {
diff --git a/runtimes/google/ipc/stream/vc/vc_test.go b/runtimes/google/ipc/stream/vc/vc_test.go
index a9c4ae4..d20ad5a 100644
--- a/runtimes/google/ipc/stream/vc/vc_test.go
+++ b/runtimes/google/ipc/stream/vc/vc_test.go
@@ -28,6 +28,8 @@
"veyron.io/veyron/veyron2/security"
)
+func init() { testutil.Init() }
+
const (
// Convenience alias to avoid conflicts between the package name "vc" and variables called "vc".
DefaultBytesBufferedPerFlow = vc.DefaultBytesBufferedPerFlow
diff --git a/runtimes/google/ipc/stream/vif/vif_test.go b/runtimes/google/ipc/stream/vif/vif_test.go
index aec55c7..bcaa37a 100644
--- a/runtimes/google/ipc/stream/vif/vif_test.go
+++ b/runtimes/google/ipc/stream/vif/vif_test.go
@@ -25,6 +25,8 @@
"veyron.io/veyron/veyron2/naming"
)
+func init() { testutil.Init() }
+
func newPrincipal(defaultBlessing string) vc.LocalPrincipal {
return vc.LocalPrincipal{sectest.NewPrincipal("defaultBlessing")}
}
diff --git a/runtimes/google/ipc/testutil_test.go b/runtimes/google/ipc/testutil_test.go
index 1f8df64..4144d97 100644
--- a/runtimes/google/ipc/testutil_test.go
+++ b/runtimes/google/ipc/testutil_test.go
@@ -4,12 +4,14 @@
"reflect"
"testing"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron2/security"
"veyron.io/veyron/veyron2/verror"
)
+func init() { testutil.Init() }
+
func makeResultPtrs(ins []interface{}) []interface{} {
outs := make([]interface{}, len(ins))
for ix, in := range ins {
diff --git a/runtimes/google/lib/deque/deque_test.go b/runtimes/google/lib/deque/deque_test.go
index beba945..83e92f1 100644
--- a/runtimes/google/lib/deque/deque_test.go
+++ b/runtimes/google/lib/deque/deque_test.go
@@ -6,6 +6,8 @@
"veyron.io/veyron/veyron/lib/testutil"
)
+func init() { testutil.Init() }
+
func TestBasic(t *testing.T) {
var q T
if q.Size() != 0 {
diff --git a/runtimes/google/lib/functional/rb/rb_set_test.go b/runtimes/google/lib/functional/rb/rb_set_test.go
index 26f8f7b..45eee44 100644
--- a/runtimes/google/lib/functional/rb/rb_set_test.go
+++ b/runtimes/google/lib/functional/rb/rb_set_test.go
@@ -9,6 +9,8 @@
"veyron.io/veyron/veyron/runtimes/google/lib/functional"
)
+func init() { testutil.Init() }
+
const (
// Number of elements to check
kMaxElement = 100
diff --git a/runtimes/google/lib/pcqueue/pcqueue_test.go b/runtimes/google/lib/pcqueue/pcqueue_test.go
index 1439b17..cda519f 100644
--- a/runtimes/google/lib/pcqueue/pcqueue_test.go
+++ b/runtimes/google/lib/pcqueue/pcqueue_test.go
@@ -6,10 +6,12 @@
"testing"
"time"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron2/vlog"
)
+func init() { testutil.Init() }
+
const (
queueSize = 10
elementCount = 100
diff --git a/runtimes/google/lib/sync/semaphore_test.go b/runtimes/google/lib/sync/semaphore_test.go
index 7b71b4d..53a917f 100644
--- a/runtimes/google/lib/sync/semaphore_test.go
+++ b/runtimes/google/lib/sync/semaphore_test.go
@@ -5,9 +5,11 @@
"sync/atomic"
"testing"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
)
+func init() { testutil.Init() }
+
func TestSemaphore(t *testing.T) {
s1 := NewSemaphore()
s2 := NewSemaphore()
diff --git a/runtimes/google/lib/sync/wait_group_test.go b/runtimes/google/lib/sync/wait_group_test.go
index 76e4bb6..dab7f54 100644
--- a/runtimes/google/lib/sync/wait_group_test.go
+++ b/runtimes/google/lib/sync/wait_group_test.go
@@ -5,6 +5,8 @@
"veyron.io/veyron/veyron/lib/testutil"
)
+func init() { testutil.Init() }
+
// TestRandom tests Wait after a random sequence of TryAdd's and Done's that
// leaves the counter at 0.
func TestRandom(t *testing.T) {
diff --git a/runtimes/google/lib/upcqueue/upcqueue_test.go b/runtimes/google/lib/upcqueue/upcqueue_test.go
index adf7b94..cff1393 100644
--- a/runtimes/google/lib/upcqueue/upcqueue_test.go
+++ b/runtimes/google/lib/upcqueue/upcqueue_test.go
@@ -6,11 +6,13 @@
"testing"
"time"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
vsync "veyron.io/veyron/veyron/runtimes/google/lib/sync"
"veyron.io/veyron/veyron2/vlog"
)
+func init() { testutil.Init() }
+
const (
elementCount = 100
writerCount = 10
diff --git a/runtimes/google/naming/namespace/all_test.go b/runtimes/google/naming/namespace/all_test.go
index a346615..8855012 100644
--- a/runtimes/google/naming/namespace/all_test.go
+++ b/runtimes/google/naming/namespace/all_test.go
@@ -8,7 +8,7 @@
"time"
"veyron.io/veyron/veyron/lib/glob"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/runtimes/google/naming/namespace"
service "veyron.io/veyron/veyron/services/mounttable/lib"
@@ -23,6 +23,8 @@
"veyron.io/veyron/veyron2/vlog"
)
+func init() { testutil.Init() }
+
func boom(t *testing.T, f string, v ...interface{}) {
t.Logf(f, v...)
t.Fatal(string(debug.Stack()))
diff --git a/runtimes/google/rt/ipc_test.go b/runtimes/google/rt/ipc_test.go
index 1d30796..c2ccbf7 100644
--- a/runtimes/google/rt/ipc_test.go
+++ b/runtimes/google/rt/ipc_test.go
@@ -11,11 +11,13 @@
"veyron.io/veyron/veyron2/rt"
"veyron.io/veyron/veyron2/security"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/profiles"
vsecurity "veyron.io/veyron/veyron/security"
)
+func init() { testutil.Init() }
+
type testService struct{}
func (testService) EchoBlessings(call ipc.ServerCall) []string {
diff --git a/runtimes/google/rt/mgmt_test.go b/runtimes/google/rt/mgmt_test.go
index fe9e22c..c0446e2 100644
--- a/runtimes/google/rt/mgmt_test.go
+++ b/runtimes/google/rt/mgmt_test.go
@@ -18,7 +18,7 @@
"veyron.io/veyron/veyron/lib/expect"
"veyron.io/veyron/veyron/lib/modules"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/lib/testutil/security"
"veyron.io/veyron/veyron/profiles"
"veyron.io/veyron/veyron/runtimes/google/rt"
@@ -33,6 +33,7 @@
)
func init() {
+ testutil.Init()
modules.RegisterChild(noWaitersCmd, "", noWaiters)
modules.RegisterChild(forceStopCmd, "", forceStop)
modules.RegisterChild(appCmd, "", app)
diff --git a/runtimes/google/rt/rt_test.go b/runtimes/google/rt/rt_test.go
index fd521d9..7620610 100644
--- a/runtimes/google/rt/rt_test.go
+++ b/runtimes/google/rt/rt_test.go
@@ -19,7 +19,7 @@
"veyron.io/veyron/veyron/lib/expect"
"veyron.io/veyron/veyron/lib/modules"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
irt "veyron.io/veyron/veyron/runtimes/google/rt"
vsecurity "veyron.io/veyron/veyron/security"
)
@@ -42,6 +42,7 @@
func (*context) RemoteEndpoint() naming.Endpoint { return nil }
func init() {
+ testutil.Init()
modules.RegisterChild("child", "", child)
}
diff --git a/runtimes/google/testing/concurrency/clock_test.go b/runtimes/google/testing/concurrency/clock_test.go
index 91a52ae..908393f 100644
--- a/runtimes/google/testing/concurrency/clock_test.go
+++ b/runtimes/google/testing/concurrency/clock_test.go
@@ -6,6 +6,8 @@
"veyron.io/veyron/veyron/lib/testutil"
)
+func init() { testutil.Init() }
+
// TestClone checks the clone() method of a clock.
func TestClone(t *testing.T) {
c1 := newClock()
diff --git a/security/serialization/serialization_test.go b/security/serialization/serialization_test.go
index 5a4dcd4..7321182 100644
--- a/security/serialization/serialization_test.go
+++ b/security/serialization/serialization_test.go
@@ -18,6 +18,8 @@
"veyron.io/veyron/veyron2/security"
)
+func init() { testutil.Init() }
+
type bufferCloser struct {
bytes.Buffer
}
diff --git a/services/mgmt/binary/impl/impl_test.go b/services/mgmt/binary/impl/impl_test.go
index e88e539..0f39ea9 100644
--- a/services/mgmt/binary/impl/impl_test.go
+++ b/services/mgmt/binary/impl/impl_test.go
@@ -24,6 +24,7 @@
)
func init() {
+ testutil.Init()
rt.Init()
}
diff --git a/services/mgmt/build/impl/impl_test.go b/services/mgmt/build/impl/impl_test.go
index 3fe0517..14b11d5 100644
--- a/services/mgmt/build/impl/impl_test.go
+++ b/services/mgmt/build/impl/impl_test.go
@@ -12,11 +12,12 @@
"veyron.io/veyron/veyron2/rt"
"veyron.io/veyron/veyron2/services/mgmt/build"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/profiles"
)
func init() {
+ testutil.Init()
rt.Init()
}
diff --git a/services/mgmt/lib/binary/impl_test.go b/services/mgmt/lib/binary/impl_test.go
index 9882cd9..cf8e7c0 100644
--- a/services/mgmt/lib/binary/impl_test.go
+++ b/services/mgmt/lib/binary/impl_test.go
@@ -21,6 +21,7 @@
)
func init() {
+ testutil.Init()
rt.Init()
}
diff --git a/services/mgmt/lib/fs/simplestore.go b/services/mgmt/lib/fs/simplestore.go
index 4fdd45b..d1c82df 100644
--- a/services/mgmt/lib/fs/simplestore.go
+++ b/services/mgmt/lib/fs/simplestore.go
@@ -72,7 +72,7 @@
defer file.Close()
} else {
decoder := gob.NewDecoder(file)
- if err := decoder.Decode(&data); err != nil {
+ if err := decoder.Decode(&data); err != nil {
// Two situations. One is not an error.
fi, err := os.Stat(configuredPersistentFile)
if err != nil {
diff --git a/services/mgmt/lib/fs/simplestore_test.go b/services/mgmt/lib/fs/simplestore_test.go
index fc893d8..b4d102f 100644
--- a/services/mgmt/lib/fs/simplestore_test.go
+++ b/services/mgmt/lib/fs/simplestore_test.go
@@ -448,7 +448,7 @@
}
// Create another memstore that will attempt to deserialize the empty
- // backing file.
+ // backing file.
_, err = NewMemstore(path)
if err != nil {
t.Fatalf("NewMemstore() failed: %v", err)
diff --git a/services/mounttable/lib/mounttable_test.go b/services/mounttable/lib/mounttable_test.go
index 5195126..72fd352 100644
--- a/services/mounttable/lib/mounttable_test.go
+++ b/services/mounttable/lib/mounttable_test.go
@@ -21,7 +21,7 @@
"veyron.io/veyron/veyron2/services/mounttable/types"
"veyron.io/veyron/veyron2/vlog"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/profiles"
)
@@ -469,6 +469,7 @@
}
func init() {
+ testutil.Init()
// Create the runtime for each of the three "processes"
rootRT = rt.Init(options.ForceNewSecurityModel{})
var err error
diff --git a/services/mounttable/lib/neighborhood_test.go b/services/mounttable/lib/neighborhood_test.go
index cb95d4c..db435e9 100644
--- a/services/mounttable/lib/neighborhood_test.go
+++ b/services/mounttable/lib/neighborhood_test.go
@@ -11,10 +11,12 @@
"veyron.io/veyron/veyron2/services/mounttable"
"veyron.io/veyron/veyron2/vlog"
- _ "veyron.io/veyron/veyron/lib/testutil"
+ "veyron.io/veyron/veyron/lib/testutil"
"veyron.io/veyron/veyron/profiles"
)
+func init() { testutil.Init() }
+
func protocolAndAddress(e naming.Endpoint) (string, string, error) {
addr := e.Addr()
if addr == nil {
