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

diff --git a/src/cmd/go/doc.go b/src/cmd/go/doc.go
index a76cc530768b8418f9c4e135c38aa2854c3a1e9f..0e59078bb3ee23509fafcfc701db56ba82021e91 100644 (file)
--- a/src/cmd/go/doc.go
+++ b/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,
-           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?
-               count: integer: how many times does this statement execute?
-               atomic: integer: like count, but correct in multithreaded tests;
+               set: bool: does this statement run?
+               count: int: how many times does this statement run?
+               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

diff --git a/src/cmd/go/test.go b/src/cmd/go/test.go
index f20e1efad3230cf93f9ab5d16b290bab9edfcf6f..5a9d8321568b9c15678f920d397c19cd605da96b 100644 (file)
--- a/src/cmd/go/test.go
+++ b/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?
-               count: integer: how many times does this statement execute?
-               atomic: integer: like count, but correct in multithreaded tests;
+               set: bool: does this statement run?
+               count: int: how many times does this statement run?
+               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 != "" {
-               p.coverMode = testCover
+       if 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 {

diff --git a/src/cmd/go/testflag.go b/src/cmd/go/testflag.go
index cd90a2f174c0d294755bf51afc8b8e534fd42d0f..70d40778bbdd7617b6982028aee4e00098ef5c7e 100644 (file)
--- a/src/cmd/go/testflag.go
+++ b/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":
-                       switch value {
-                       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
+               case "covermode":
+                       passToTest = setCoverMode(value, passToTest)
                }
                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]

diff --git a/src/pkg/testing/testing.go b/src/pkg/testing/testing.go
index ef8c77b4976dfa63fbe1938edbd18a0e6d6e78ce..8ef26ebfb394943f0ad758258f541518f2a2256c 100644 (file)
--- a/src/pkg/testing/testing.go
+++ b/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()
        }
 }