cmd/go: allow -coverprofile with multiple packages being tested

It's easy to merge the coverage profiles from the
multiple executed tests, so do that.

Also ensures that at least an empty coverage profile
is always written.

Fixes #6909.
Fixes #18909.

Change-Id: I28b88e1fb0fb773c8f57e956b18904dc388cdd82
Reviewed-on: https://go-review.googlesource.com/76875
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>

src/cmd/go/go_test.go

@@ -2338,7 +2338,6 @@ func checkCoverage(tg *testgoData, data string) {
 	if regexp.MustCompile(`[^0-9]0\.0%`).MatchString(data) {
 		tg.t.Error("some coverage results are 0.0%")
 	}
-	tg.t.Log(data)
 }
 
 func TestCoverageRuns(t *testing.T) {
@@ -2355,6 +2354,7 @@ func TestCoverageRuns(t *testing.T) {
 }
 
 // Check that coverage analysis uses set mode.
+// Also check that coverage profiles merge correctly.
 func TestCoverageUsesSetMode(t *testing.T) {
 	if testing.Short() {
 		t.Skip("don't build libraries for coverage in short mode")
@@ -2362,7 +2362,7 @@ func TestCoverageUsesSetMode(t *testing.T) {
 	tg := testgo(t)
 	defer tg.cleanup()
 	tg.creatingTemp("testdata/cover.out")
-	tg.run("test", "-short", "-cover", "encoding/binary", "-coverprofile=testdata/cover.out")
+	tg.run("test", "-short", "-cover", "encoding/binary", "errors", "-coverprofile=testdata/cover.out")
 	data := tg.getStdout() + tg.getStderr()
 	if out, err := ioutil.ReadFile("testdata/cover.out"); err != nil {
 		t.Error(err)
@@ -2370,6 +2370,15 @@ func TestCoverageUsesSetMode(t *testing.T) {
 		if !bytes.Contains(out, []byte("mode: set")) {
 			t.Error("missing mode: set")
 		}
+		if !bytes.Contains(out, []byte("errors.go")) {
+			t.Error("missing errors.go")
+		}
+		if !bytes.Contains(out, []byte("binary.go")) {
+			t.Error("missing binary.go")
+		}
+		if bytes.Count(out, []byte("mode: set")) != 1 {
+			t.Error("too many mode: set")
+		}
 	}
 	checkCoverage(tg, data)
 }

src/cmd/go/internal/test/cover.go

+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package test
+
+import (
+	"cmd/go/internal/base"
+	"fmt"
+	"io"
+	"os"
+	"sync"
+)
+
+var coverMerge struct {
+	f          *os.File
+	sync.Mutex // for f.Write
+}
+
+// initCoverProfile initializes the test coverage profile.
+// It must be run before any calls to mergeCoverProfile or closeCoverProfile.
+// Using this function clears the profile in case it existed from a previous run,
+// or in case it doesn't exist and the test is going to fail to create it (or not run).
+func initCoverProfile() {
+	if testCoverProfile == "" {
+		return
+	}
+
+	// No mutex - caller's responsibility to call with no racing goroutines.
+	f, err := os.Create(testCoverProfile)
+	if err != nil {
+		base.Fatalf("%v", err)
+	}
+	_, err = fmt.Fprintf(f, "mode: %s\n", testCoverMode)
+	if err != nil {
+		base.Fatalf("%v", err)
+	}
+	coverMerge.f = f
+}
+
+// mergeCoverProfile merges file into the profile stored in testCoverProfile.
+// It prints any errors it encounters to ew.
+func mergeCoverProfile(ew io.Writer, file string) {
+	if coverMerge.f == nil {
+		return
+	}
+	coverMerge.Lock()
+	defer coverMerge.Unlock()
+
+	expect := fmt.Sprintf("mode: %s\n", testCoverMode)
+	buf := make([]byte, len(expect))
+	r, err := os.Open(file)
+	if err != nil {
+		// Test did not create profile, which is OK.
+		return
+	}
+	defer r.Close()
+
+	n, err := io.ReadFull(r, buf)
+	if n == 0 {
+		return
+	}
+	if err != nil || string(buf) != expect {
+		fmt.Fprintf(ew, "error: test wrote malformed coverage profile.\n")
+		return
+	}
+	_, err = io.Copy(coverMerge.f, r)
+	if err != nil {
+		fmt.Fprintf(ew, "error: saving coverage profile: %v\n", err)
+	}
+}
+
+func closeCoverProfile() {
+	if coverMerge.f == nil {
+		return
+	}
+	if err := coverMerge.f.Close(); err != nil {
+		base.Errorf("closing coverage profile: %v", err)
+	}
+}

src/cmd/go/internal/test/test.go

@@ -451,24 +451,25 @@ See the documentation of the testing package for more information.
 }
 
 var (
-	testC          bool            // -c flag
-	testCover      bool            // -cover flag
-	testCoverMode  string          // -covermode flag
-	testCoverPaths []string        // -coverpkg flag
-	testCoverPkgs  []*load.Package // -coverpkg flag
-	testO          string          // -o flag
-	testProfile    bool            // some profiling flag
-	testNeedBinary bool            // profile needs to keep binary around
-	testJSON       bool            // -json flag
-	testV          bool            // -v flag
-	testTimeout    string          // -timeout flag
-	testArgs       []string
-	testBench      bool
-	testList       bool
-	testShowPass   bool   // show passing output
-	testVetList    string // -vet flag
-	pkgArgs        []string
-	pkgs           []*load.Package
+	testC            bool            // -c flag
+	testCover        bool            // -cover flag
+	testCoverMode    string          // -covermode flag
+	testCoverPaths   []string        // -coverpkg flag
+	testCoverPkgs    []*load.Package // -coverpkg flag
+	testCoverProfile string          // -coverprofile flag
+	testO            string          // -o flag
+	testProfile      string          // profiling flag that limits test to one package
+	testNeedBinary   bool            // profile needs to keep binary around
+	testJSON         bool            // -json flag
+	testV            bool            // -v flag
+	testTimeout      string          // -timeout flag
+	testArgs         []string
+	testBench        bool
+	testList         bool
+	testShowPass     bool   // show passing output
+	testVetList      string // -vet flag
+	pkgArgs          []string
+	pkgs             []*load.Package
 
 	testKillTimeout = 10 * time.Minute
 )
@@ -525,9 +526,11 @@ func runTest(cmd *base.Command, args []string) {
 	if testO != "" && len(pkgs) != 1 {
 		base.Fatalf("cannot use -o flag with multiple packages")
 	}
-	if testProfile && len(pkgs) != 1 {
-		base.Fatalf("cannot use test profile flag with multiple packages")
+	if testProfile != "" && len(pkgs) != 1 {
+		base.Fatalf("cannot use %s flag with multiple packages", testProfile)
 	}
+	initCoverProfile()
+	defer closeCoverProfile()
 
 	// If a test timeout was given and is parseable, set our kill timeout
 	// to that timeout plus one minute. This is a backup alarm in case
@@ -1039,6 +1042,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
 			Package:    p,
 			IgnoreFail: true,
 			TryCache:   c.tryCache,
+			Objdir:     testDir,
 		}
 		if len(ptest.GoFiles)+len(ptest.CgoFiles) > 0 {
 			addTestVet(b, ptest, runAction, installAction)
@@ -1220,6 +1224,15 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
 		return nil
 	}
 
+	if testCoverProfile != "" {
+		// Write coverage to temporary profile, for merging later.
+		for i, arg := range args {
+			if strings.HasPrefix(arg, "-test.coverprofile=") {
+				args[i] = "-test.coverprofile=" + a.Objdir + "_cover_.out"
+			}
+		}
+	}
+
 	cmd := exec.Command(args[0], args[1:]...)
 	cmd.Dir = a.Package.Dir
 	cmd.Env = base.EnvForDir(cmd.Dir, cfg.OrigEnv)
@@ -1318,6 +1331,9 @@ func (c *runCache) builderRunTest(b *work.Builder, a *work.Action) error {
 	out := buf.Bytes()
 	a.TestOutput = &buf
 	t := fmt.Sprintf("%.3fs", time.Since(t0).Seconds())
+
+	mergeCoverProfile(cmd.Stdout, a.Objdir+"_cover_.out")
+
 	if err == nil {
 		norun := ""
 		if !testShowPass {

src/cmd/go/internal/test/testflag.go

@@ -156,10 +156,10 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 			case "timeout":
 				testTimeout = value
 			case "blockprofile", "cpuprofile", "memprofile", "mutexprofile":
-				testProfile = true
+				testProfile = "-" + f.Name
 				testNeedBinary = true
 			case "trace":
-				testProfile = true
+				testProfile = "-trace"
 			case "coverpkg":
 				testCover = true
 				if value == "" {
@@ -169,7 +169,7 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 				}
 			case "coverprofile":
 				testCover = true
-				testProfile = true
+				testCoverProfile = value
 			case "covermode":
 				switch value {
 				case "set", "count", "atomic":
@@ -219,7 +219,7 @@ func testFlags(args []string) (packageNames, passToTest []string) {
 	}
 
 	// Tell the test what directory we're running in, so it can write the profiles there.
-	if testProfile && outputDir == "" {
+	if testProfile != "" && outputDir == "" {
 		dir, err := os.Getwd()
 		if err != nil {
 			base.Fatalf("error from os.Getwd: %s", err)