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/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.
