From a40281112cb38ada79bfe32bb150982abf57d0a9 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 14 Dec 2015 15:09:26 +0000 Subject: [PATCH] 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 --- src/cmd/go/go_test.go | 12 +++------- src/cmd/go/testflag.go | 52 ++++++++++++++++++++++-------------------- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index b07b746054..735c935b57 100644 --- 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 32a84b698f..1f3e3d316a 100644 --- 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" -- 2.50.0