cmd/gb: fix parsing of passToAll test arguments

Fixes #473

Handling of flags that are parsed by both gb and a sub process is tricky.
So that the arguments are not parsed as package parameters by gb test
we move them to the front of the argument list when parsed by gb, but
leave them at the end of the list for the test binary to parse after
invocation.

cmd/gb/alldocs.go

@@ -90,7 +90,7 @@ Usage:
 
         gb env [var ...]
 
-Env prints project environment variables. If one or more variable names is 
+Env prints project environment variables. If one or more variable names is
 given as arguments, env prints the value of each named variable on its own line.
 
 
@@ -130,7 +130,7 @@ Values:
 		The value of runtime.GOROOT for the Go version that built this copy of gb.
 
 info returns 0 if the project is well formed, and non zero otherwise.
-If one or more variable names is given as arguments, info prints the 
+If one or more variable names is given as arguments, info prints the
 value of each named variable on its own line.
 
 

cmd/gb/gb_test.go

@@ -910,13 +910,39 @@ func TestTest(t *testing.T) {
 	gb.mustNotExist(filepath.Join(gb.tempdir, "pkg")) // ensure no pkg directory is created
 }
 
+const issue349 = `package main
+
+import (
+    "flag"
+    "testing"
+)
+
+var name = flag.String("name", "nsf", "what is your name")
+
+func TestX(t *testing.T) {
+    if *name != "jardin" {
+        t.Fatalf("got: '%s', expected: 'jardin'", *name)
+    }
+}
+`
+
 // https://github.com/constabulary/gb/issues/349
 func TestTestGbTestPassesUnknownFlags(t *testing.T) {
 	gb := T{T: t}
 	defer gb.cleanup()
 	gb.tempDir("src")
 	gb.tempDir("src/projectx")
-	gb.tempFile("src/projectx/main_test.go", `package main
+	gb.tempFile("src/projectx/main_test.go", issue349)
+	gb.cd(gb.tempdir)
+	tmpdir := gb.tempDir("tmp")
+	gb.setenv("TMP", tmpdir)
+	gb.run("test", "-name=jardin")
+	gb.grepStdout("^projectx$", "expected projectx") // output from gb test
+	gb.mustBeEmpty(tmpdir)
+	gb.mustNotExist(filepath.Join(gb.tempdir, "pkg")) // ensure no pkg directory is created
+}
+
+const issue473 = `package main
 
 import (
     "flag"
@@ -926,16 +952,77 @@ import (
 var name = flag.String("name", "nsf", "what is your name")
 
 func TestX(t *testing.T) {
-    if *name != "jardin" {
-        t.Fatalf("got: '%s', expected: 'jardin'", *name)
-    }
 }
-`)
+
+func TestY(t *testing.T) {
+}
+`
+
+// https://github.com/constabulary/gb/issues/473
+func TestGbTestIssue473a(t *testing.T) {
+	gb := T{T: t}
+	defer gb.cleanup()
+	gb.tempDir("src")
+	gb.tempDir("src/projectx")
+	gb.tempFile("src/projectx/main_test.go", issue473)
 	gb.cd(gb.tempdir)
 	tmpdir := gb.tempDir("tmp")
 	gb.setenv("TMP", tmpdir)
-	gb.run("test", "-name=jardin")
+	gb.run("test", "-v", "projectx", "-run", "TestX")
+	gb.grepStdout("^projectx$", "expected projectx") // output from gb test
+	gb.grepStdout("TestX", "expected TestX")
+	gb.grepStdoutNot("TestY", "expected TestY")
+	gb.mustBeEmpty(tmpdir)
+	gb.mustNotExist(filepath.Join(gb.tempdir, "pkg")) // ensure no pkg directory is created
+}
+
+func TestGbTestIssue473b(t *testing.T) {
+	gb := T{T: t}
+	defer gb.cleanup()
+	gb.tempDir("src")
+	gb.tempDir("src/projectx")
+	gb.tempFile("src/projectx/main_test.go", issue473)
+	gb.cd(gb.tempdir)
+	tmpdir := gb.tempDir("tmp")
+	gb.setenv("TMP", tmpdir)
+	gb.run("test", "-v", "-run", "TestX", "projectx")
+	gb.grepStdout("^projectx$", "expected projectx") // output from gb test
+	gb.grepStdout("TestX", "expected TestX")
+	gb.grepStdoutNot("TestY", "expected TestY")
+	gb.mustBeEmpty(tmpdir)
+	gb.mustNotExist(filepath.Join(gb.tempdir, "pkg")) // ensure no pkg directory is created
+}
+
+func TestGbTestIssue473c(t *testing.T) {
+	gb := T{T: t}
+	defer gb.cleanup()
+	gb.tempDir("src")
+	gb.tempDir("src/projectx")
+	gb.tempFile("src/projectx/main_test.go", issue473)
+	gb.cd(gb.tempdir)
+	tmpdir := gb.tempDir("tmp")
+	gb.setenv("TMP", tmpdir)
+	gb.run("test", "-v", "projectx")
+	gb.grepStdout("^projectx$", "expected projectx") // output from gb test
+	gb.grepStdout("TestX", "expected TestX")
+	gb.grepStdout("TestY", "expected TestY")
+	gb.mustBeEmpty(tmpdir)
+	gb.mustNotExist(filepath.Join(gb.tempdir, "pkg")) // ensure no pkg directory is created
+}
+
+func TestGbTestIssue473d(t *testing.T) {
+	gb := T{T: t}
+	defer gb.cleanup()
+	gb.tempDir("src")
+	gb.tempDir("src/projectx")
+	gb.tempFile("src/projectx/main_test.go", issue473)
+	gb.cd(gb.tempdir)
+	tmpdir := gb.tempDir("tmp")
+	gb.setenv("TMP", tmpdir)
+	gb.run("test", "projectx", "-v")
 	gb.grepStdout("^projectx$", "expected projectx") // output from gb test
+	gb.grepStdout("TestX", "expected TestX")
+	gb.grepStdout("TestY", "expected TestY")
 	gb.mustBeEmpty(tmpdir)
 	gb.mustNotExist(filepath.Join(gb.tempdir, "pkg")) // ensure no pkg directory is created
 }

cmd/gb/test.go

@@ -85,6 +85,7 @@ Flags:
 	AddFlags: addTestFlags,
 	FlagParse: func(flags *flag.FlagSet, args []string) error {
 		var err error
+		debug.Debugf("%s", args)
 		args, tfs, err = TestFlagsExtraParse(args[2:])
 		debug.Debugf("%s %s", args, tfs)
 		if err != nil {

cmd/gb/testflag.go

@@ -95,16 +95,22 @@ func TestFlagsExtraParse(args []string) (parseArgs []string, extraArgs []string,
 
 	for x := 0; x < argsLen; x++ {
 		nArg := args[x]
+
 		val, ok := testFlagDefn[strings.TrimPrefix(nArg, "-")]
 		if !strings.HasPrefix(nArg, "-") || (ok && !val.passToTest) {
 			err = setArgFound(nArg)
 			if err != nil {
 				return
 			}
-			parseArgs = append(parseArgs, nArg)
 			if ok && val.passToAll {
+				// passToAll arguments, like -v or -cover, are special. They are handled by gb test
+				// and the test sub process. So move them to the front of the parseArgs list but
+				// the back of the extraArgs list.
+				parseArgs = append([]string{nArg}, parseArgs...)
 				extraArgs = append(extraArgs, nArg)
+				continue
 			}
+			parseArgs = append(parseArgs, nArg)
 			continue
 		}
 

cmd/gb/testflag_test.go

@@ -20,6 +20,10 @@ func TestTestFlagsPreParse(t *testing.T) {
 			args:  []string{"-v", "-debug", "package_name"},
 			pargs: []string{"-v", "package_name"},
 			eargs: []string{"-v", "-debug"},
+		}, {
+			args:  []string{"-debug", "package_name", "-v"},
+			pargs: []string{"-v", "package_name"},
+			eargs: []string{"-debug", "-v"},
 		}, {
 			args:  []string{"-q", "-debug", "package_name"},
 			pargs: []string{"package_name"},