cmd/go: simplify flags for coverage

The single flag -cover provides the default simplest behavior.
The other flags, -covermode and -coverprofile, provide more
control. The three flags interconnect to work well.

R=rsc, adg
CC=golang-dev
https://golang.org/cl/10364044

src/cmd/go/doc.go

@@ -738,17 +738,25 @@ control the execution of any test:
 	    if -test.blockprofile is set without this flag, all blocking events
 	    are recorded, equivalent to -test.blockprofilerate=1.
 
-	-cover set,count,atomic
+	-cover
+	    Enable basic coverage analysis; shorthand for -covermode=set.
 	    TODO: This feature is not yet fully implemented.
-	    TODO: Must run with -v to see output.
+
-	    TODO: Need control over output format,
+	-covermode set,count,atomic
-	    Set the mode for coverage analysis for the package[s] being tested.
+	    Set the mode for coverage analysis for the package[s]
-	    The default is to do none.
+	    being tested. The default is to do none, but if -cover or
+	    -coverprofile is specified, coverage is enabled in "set"
+	    mode unless this flag is also specified.
 	    The values:
-		set: boolean: does this statement execute?
+		set: bool: does this statement run?
-		count: integer: how many times does this statement execute?
+		count: int: how many times does this statement run?
-		atomic: integer: like count, but correct in multithreaded tests;
+		atomic: int: count, but correct in multithreaded tests;
 			significantly more expensive.
+	    Sets -v. TODO: This will change.
+
+	-coverprofile cover.out
+	    Write a coverage profile to the specified file after all tests
+	    have passed.
 
 	-cpu 1,2,4
 	    Specify a list of GOMAXPROCS values for which the tests or

src/cmd/go/test.go

@@ -124,14 +124,19 @@ control the execution of any test:
 	    if -test.blockprofile is set without this flag, all blocking events
 	    are recorded, equivalent to -test.blockprofilerate=1.
 
-	-cover set,count,atomic
+	-cover
+	    Enable basic coverage analysis; shorthand for -covermode=set.
 	    TODO: This feature is not yet fully implemented.
-	    Set the mode for coverage analysis for the package[s] being tested.
+
-	    The default is to do none.
+	-covermode set,count,atomic
+	    Set the mode for coverage analysis for the package[s]
+	    being tested. The default is to do none, but if -cover or
+	    -coverprofile is specified, coverage is enabled in "set"
+	    mode unless this flag is also specified.
 	    The values:
-		set: boolean: does this statement execute?
+		set: bool: does this statement run?
-		count: integer: how many times does this statement execute?
+		count: int: how many times does this statement run?
-		atomic: integer: like count, but correct in multithreaded tests;
+		atomic: int: count, but correct in multithreaded tests;
 			significantly more expensive.
 	    Sets -v. TODO: This will change.
 
@@ -254,7 +259,8 @@ See the documentation of the testing package for more information.
 
 var (
 	testC            bool     // -c flag
-	testCover        string   // -cover flag
+	testCover        bool     // -cover flag
+	testCoverMode    string   // -covermode flag
 	testProfile      bool     // some profiling flag
 	testI            bool     // -i flag
 	testV            bool     // -v flag
@@ -494,7 +500,7 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
 		// - that is, any code imported by the external test that in turn
 		// imports p - needs to be rebuilt too. For now, just report
 		// that coverage is unavailable.
-		if testCover != "" && contains(p1.Deps, p.ImportPath) {
+		if testCover && contains(p1.Deps, p.ImportPath) {
 			return nil, nil, nil, fmt.Errorf("coverage analysis cannot handle package (%s_test imports %s imports %s)", p.Name, path, p.ImportPath)
 		}
 	}
@@ -535,8 +541,8 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
 		return nil, nil, nil, err
 	}
 
-	if testCover != "" {
+	if testCover {
-		p.coverMode = testCover
+		p.coverMode = testCoverMode
 		p.coverVars = declareCoverVars(p.ImportPath, p.GoFiles...)
 	}
 
@@ -545,7 +551,7 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
 	}
 
 	// Test package.
-	if len(p.TestGoFiles) > 0 || testCover != "" {
+	if len(p.TestGoFiles) > 0 || testCover {
 		ptest = new(Package)
 		*ptest = *p
 		ptest.GoFiles = nil
@@ -871,7 +877,7 @@ type testFuncs struct {
 }
 
 func (t *testFuncs) CoverEnabled() bool {
-	return testCover != ""
+	return testCover
 }
 
 type testFunc struct {

src/cmd/go/testflag.go

@@ -27,7 +27,8 @@ var usageMessage = `Usage of go test:
   -bench="": passes -test.bench to test
   -benchmem=false: print memory allocation statistics for benchmarks
   -benchtime=1s: passes -test.benchtime to test
-  -cover="": passes -test.cover to test
+  -cover=false: basic coverage; equivalent to -covermode=set
+  -covermode="": passes -test.covermode to test
   -coverprofile="": passes -test.coverprofile to test
   -cpu="": passes -test.cpu to test
   -cpuprofile="": passes -test.cpuprofile to test
@@ -65,6 +66,7 @@ var testFlagDefn = []*testFlagSpec{
 	{name: "c", boolVar: &testC},
 	{name: "file", multiOK: true},
 	{name: "i", boolVar: &testI},
+	{name: "cover", boolVar: &testCover},
 
 	// build flags.
 	{name: "a", boolVar: &buildA},
@@ -83,7 +85,7 @@ var testFlagDefn = []*testFlagSpec{
 	{name: "bench", passToTest: true},
 	{name: "benchmem", boolVar: new(bool), passToTest: true},
 	{name: "benchtime", passToTest: true},
-	{name: "cover", passToTest: true},
+	{name: "covermode"}, // Passed to test by special arrangement.
 	{name: "coverprofile", passToTest: true},
 	{name: "cpu", passToTest: true},
 	{name: "cpuprofile", passToTest: true},
@@ -144,7 +146,7 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 		var err error
 		switch f.name {
 		// bool flags.
-		case "a", "c", "i", "n", "x", "v", "work", "race":
+		case "a", "c", "i", "n", "x", "v", "race", "cover", "work":
 			setBoolFlag(f.boolVar, value)
 		case "p":
 			setIntFlag(&buildP, value)
@@ -174,19 +176,15 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 			testBench = true
 		case "timeout":
 			testTimeout = value
-		case "blockprofile", "coverprofile", "cpuprofile", "memprofile":
+		case "blockprofile", "cpuprofile", "memprofile":
+			testProfile = true
+		case "coverprofile":
+			passToTest = setCoverMode("set", passToTest)
 			testProfile = true
 		case "outputdir":
 			outputDir = value
-		case "cover":
+		case "covermode":
-			switch value {
+			passToTest = setCoverMode(value, passToTest)
-			case "set", "count", "atomic":
-				testCover = value
-			default:
-				fatalf("invalid flag argument for -cover: %q", value)
-			}
-			// Guarantee we see the coverage statistics. Doesn't turn -v on generally; tricky. TODO?
-			testV = true
 		}
 		if extraWord {
 			i++
@@ -195,6 +193,10 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 			passToTest = append(passToTest, "-test."+f.name+"="+value)
 		}
 	}
+	// -cover is shorthand for -covermode=set.
+	if testCover && testCoverMode == "" {
+		passToTest = setCoverMode("set", passToTest)
+	}
 	// Tell the test what directory we're running in, so it can write the profiles there.
 	if testProfile && outputDir == "" {
 		dir, err := os.Getwd()
@@ -206,6 +208,24 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 	return
 }
 
+// setCoverMode sets the cover mode if not already specified; it captures the default behavior and
+// canonicalizes the coverage flags to pass to the test binary.
+func setCoverMode(mode string, passToTest []string) []string {
+	if testCoverMode != "" {
+		return passToTest
+	}
+	switch mode {
+	case "set", "count", "atomic":
+		testCoverMode = mode
+	default:
+		fatalf("invalid flag argument for -cover: %q", mode)
+	}
+	testCover = true
+	// Guarantee we see the coverage statistics. Doesn't turn -v on generally; tricky. TODO?
+	testV = true
+	return append(passToTest, "-test.covermode", "set")
+}
+
 // testFlag sees if argument i is a known flag and returns its definition, value, and whether it consumed an extra word.
 func testFlag(args []string, i int) (f *testFlagSpec, value string, extra bool) {
 	arg := args[i]

src/pkg/testing/testing.go

@@ -122,7 +122,7 @@ var (
 
 	// Report as tests are run; default is silent for success.
 	chatty           = flag.Bool("test.v", false, "verbose: print additional output")
-	cover            = flag.String("test.cover", "", "cover mode: set, count, atomic; default is none")
+	coverMode        = flag.String("test.covermode", "", "cover mode: set, count, atomic; default is none")
 	coverProfile     = flag.String("test.coverprofile", "", "write a coveraage profile to the named file after execution")
 	match            = flag.String("test.run", "", "regular expression to select tests and examples to run")
 	memProfile       = flag.String("test.memprofile", "", "write a memory profile to the named file after execution")
@@ -520,7 +520,7 @@ func after() {
 		}
 		f.Close()
 	}
-	if *cover != "" {
+	if *coverMode != "" {
 		coverReport()
 	}
 }