Revert "cmd/go: fix processing of flags for test binaries."

This broke a number of common "go test" invocations.
Will fix the original concern differently.

This reverts commit 6acb4d944dafa13a6c80faffc4e7ecc47d2bcdbc.

Fixes #13583.

Change-Id: If582b81061df28173c698bed1d7d8283b0713cae
Reviewed-on: https://go-review.googlesource.com/17773
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index b07b7460548ab8ca9364a27db577406bc23125b4..735c935b575f044be325d44b61b70f52618ee98d 100644 (file)
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -1741,7 +1741,7 @@ func TestCoverageUsesSetMode(t *testing.T) {
        tg := testgo(t)
        defer tg.cleanup()
        tg.creatingTemp("testdata/cover.out")
-       tg.run("test", "-short", "-coverprofile=testdata/cover.out", "encoding/binary")
+       tg.run("test", "-short", "-cover", "encoding/binary", "-coverprofile=testdata/cover.out")
        data := tg.getStdout() + tg.getStderr()
        if out, err := ioutil.ReadFile("testdata/cover.out"); err != nil {
                t.Error(err)
@@ -1764,7 +1764,7 @@ func TestCoverageUsesAtomicModeForRace(t *testing.T) {
        tg := testgo(t)
        defer tg.cleanup()
        tg.creatingTemp("testdata/cover.out")
-       tg.run("test", "-short", "-race", "-coverprofile=testdata/cover.out", "encoding/binary")
+       tg.run("test", "-short", "-race", "-cover", "encoding/binary", "-coverprofile=testdata/cover.out")
        data := tg.getStdout() + tg.getStderr()
        if out, err := ioutil.ReadFile("testdata/cover.out"); err != nil {
                t.Error(err)
@@ -1787,7 +1787,7 @@ func TestCoverageUsesActualSettingToOverrideEvenForRace(t *testing.T) {
        tg := testgo(t)
        defer tg.cleanup()
        tg.creatingTemp("testdata/cover.out")
-       tg.run("test", "-short", "-race", "-covermode=count", "-coverprofile=testdata/cover.out", "encoding/binary")
+       tg.run("test", "-short", "-race", "-cover", "encoding/binary", "-covermode=count", "-coverprofile=testdata/cover.out")
        data := tg.getStdout() + tg.getStderr()
        if out, err := ioutil.ReadFile("testdata/cover.out"); err != nil {
                t.Error(err)
@@ -1983,12 +1983,6 @@ func TestGoTestFooTestWorks(t *testing.T) {
        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/testflag.go b/src/cmd/go/testflag.go
index 32a84b698f7cdc2e023d6111255bdd3dc1c2c157..1f3e3d316af5a8794e8746e6617d9a943a5fc3a3 100644 (file)
--- a/src/cmd/go/testflag.go
+++ b/src/cmd/go/testflag.go
@@ -80,29 +80,40 @@ func init() {
 // 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.
-// 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.
+// 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
 func testFlags(args []string) (packageNames, passToTest []string) {
+       inPkg := false
        outputDir := ""
-       // Flags.
-       var i int
-       for i = 0; i < len(args); i++ {
+       for i := 0; i < len(args); i++ {
                if !strings.HasPrefix(args[i], "-") {
-                       break // Start of packages.
+                       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
                }
 
                f, value, extraWord := testFlag(args, i)
                if f == nil {
-                       // This is a flag we do not know. Pass it to the test but keep
-                       // processing flags.
+                       // 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{}
+                       }
                        passToTest = append(passToTest, args[i])
                        continue
                }
@@ -163,15 +174,6 @@ func testFlags(args []string) (packageNames, passToTest []string) {
                        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"