cmd/go: fix processing of flags for test binaries.
The usage message says:
test [-c] [-i] [build and test flags] [packages] [flags for test binary]
but this was not what was implemented. Instead, after packages are named,
flag processing continues, which makes it impossible, for example, to pass
to the binary a flag with the same name as a test flag. This was triggered
by the -v flag in glog.
Consider this test:
package pkg
... imports ...
var v = flag.Int("v", 0, "v flag")
func TestFoo(t *testing.T) {
if *v != 7 { log.Fatal(*v) }
}
Attempting to run this test with go test pkg -v=7 would give a usage
message. This change allows it. In fact it allows
go test -v pkg -v=7
The solution is to implement the usage message. One compatibility
issue is that flags after the package name are no longer processed
as test flags, so this no longer works:
go test foo -cover
One must write
go test -cover foo
I do not think this is onerous but it must be called out in the
release notes.
Fixes #12177.
Change-Id: Ib9267884b47a6b0c183efa888ec78333272113aa
Reviewed-on: https://go-review.googlesource.com/14826
Reviewed-by: Ian Lance Taylor <iant@golang.org>
diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index abd6308..ab78fe9 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -1698,7 +1698,7 @@
tg := testgo(t)
defer tg.cleanup()
tg.creatingTemp("testdata/cover.out")
- tg.run("test", "-short", "-cover", "encoding/binary", "-coverprofile=testdata/cover.out")
+ tg.run("test", "-short", "-coverprofile=testdata/cover.out", "encoding/binary")
data := tg.getStdout() + tg.getStderr()
if out, err := ioutil.ReadFile("testdata/cover.out"); err != nil {
t.Error(err)
@@ -1721,7 +1721,7 @@
tg := testgo(t)
defer tg.cleanup()
tg.creatingTemp("testdata/cover.out")
- tg.run("test", "-short", "-race", "-cover", "encoding/binary", "-coverprofile=testdata/cover.out")
+ tg.run("test", "-short", "-race", "-coverprofile=testdata/cover.out", "encoding/binary")
data := tg.getStdout() + tg.getStderr()
if out, err := ioutil.ReadFile("testdata/cover.out"); err != nil {
t.Error(err)
@@ -1744,7 +1744,7 @@
tg := testgo(t)
defer tg.cleanup()
tg.creatingTemp("testdata/cover.out")
- tg.run("test", "-short", "-race", "-cover", "encoding/binary", "-covermode=count", "-coverprofile=testdata/cover.out")
+ tg.run("test", "-short", "-race", "-covermode=count", "-coverprofile=testdata/cover.out", "encoding/binary")
data := tg.getStdout() + tg.getStderr()
if out, err := ioutil.ReadFile("testdata/cover.out"); err != nil {
t.Error(err)
@@ -1940,6 +1940,12 @@
tg.run("test", "testdata/standalone_test.go")
}
+func TestGoTestFlagsAfterPackage(t *testing.T) {
+ tg := testgo(t)
+ defer tg.cleanup()
+ tg.run("test", "-v", "testdata/flag_test.go", "-v=7") // Two distinct -v flags.
+}
+
func TestGoTestXtestonlyWorks(t *testing.T) {
tg := testgo(t)
defer tg.cleanup()
diff --git a/src/cmd/go/testdata/flag_test.go b/src/cmd/go/testdata/flag_test.go
new file mode 100644
index 0000000..ddf613d
--- /dev/null
+++ b/src/cmd/go/testdata/flag_test.go
@@ -0,0 +1,16 @@
+package flag_test
+
+import (
+ "flag"
+ "log"
+ "testing"
+)
+
+var v = flag.Int("v", 0, "v flag")
+
+// Run this as go test pkg -v=7
+func TestVFlagIsSet(t *testing.T) {
+ if *v != 7 {
+ log.Fatal("v flag not set")
+ }
+}
diff --git a/src/cmd/go/testflag.go b/src/cmd/go/testflag.go
index 1f3e3d3..32a84b6 100644
--- a/src/cmd/go/testflag.go
+++ b/src/cmd/go/testflag.go
@@ -80,40 +80,29 @@
// Unfortunately for us, we need to do our own flag processing because go test
// grabs some flags but otherwise its command line is just a holding place for
// pkg.test's arguments.
-// We allow known flags both before and after the package name list,
-// to allow both
-// go test fmt -custom-flag-for-fmt-test
-// go test -x math
+// The usage is:
+// go test [test flags] [packages] [flags for test binary]
+// Thus we process test flags (adding -test. to each) until we find a non-flag,
+// which introduces the optional list of packages. We collect the package paths
+// until we find another -flag, and pass that and the rest of the command line
+// to the test binary untouched.
+// For backwards compatibility with a poor design, if while processing test
+// flags we see an unrecognized flag, we accept it as an argument to the binary.
+// For this to work in general, one must say -foo=xxx not -foo xxx or else
+// xxx will be taken to be a package path. As said, the design is poor.
func testFlags(args []string) (packageNames, passToTest []string) {
- inPkg := false
outputDir := ""
- for i := 0; i < len(args); i++ {
+ // Flags.
+ var i int
+ for i = 0; i < len(args); i++ {
if !strings.HasPrefix(args[i], "-") {
- if !inPkg && packageNames == nil {
- // First package name we've seen.
- inPkg = true
- }
- if inPkg {
- packageNames = append(packageNames, args[i])
- continue
- }
- }
-
- if inPkg {
- // Found an argument beginning with "-"; end of package list.
- inPkg = false
+ break // Start of packages.
}
f, value, extraWord := testFlag(args, i)
if f == nil {
- // This is a flag we do not know; we must assume
- // that any args we see after this might be flag
- // arguments, not package names.
- inPkg = false
- if packageNames == nil {
- // make non-nil: we have seen the empty package list
- packageNames = []string{}
- }
+ // This is a flag we do not know. Pass it to the test but keep
+ // processing flags.
passToTest = append(passToTest, args[i])
continue
}
@@ -174,6 +163,15 @@
passToTest = append(passToTest, "-test."+f.name+"="+value)
}
}
+ // Package names.
+ for ; i < len(args); i++ {
+ if strings.HasPrefix(args[i], "-") {
+ break // Start of trailing arguments.
+ }
+ packageNames = append(packageNames, args[i])
+ }
+ // Trailing arguments.
+ passToTest = append(passToTest, args[i:]...)
if testCoverMode == "" {
testCoverMode = "set"
