Merge pull request #478 from constabulary/fixedbugs/473

cmd/gb: fix parsing of passToAll test arguments

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"},