From 283558e42b88a6afa39da6ad4ae87558dc053776 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 9 Nov 2017 21:23:08 -0500 Subject: [PATCH] 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 Reviewed-by: David Crawshaw --- src/cmd/go/go_test.go | 13 ++++- src/cmd/go/internal/test/cover.go | 80 ++++++++++++++++++++++++++++ src/cmd/go/internal/test/test.go | 56 ++++++++++++------- src/cmd/go/internal/test/testflag.go | 8 +-- 4 files changed, 131 insertions(+), 26 deletions(-) create mode 100644 src/cmd/go/internal/test/cover.go diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 02f7c2713d..84fcac25ed 100644 --- a/src/cmd/go/go_test.go +++ b/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) } diff --git a/src/cmd/go/internal/test/cover.go b/src/cmd/go/internal/test/cover.go new file mode 100644 index 0000000000..2a2c563a76 --- /dev/null +++ b/src/cmd/go/internal/test/cover.go @@ -0,0 +1,80 @@ +// 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) + } +} diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index c88f68291d..0ead178b9a 100644 --- a/src/cmd/go/internal/test/test.go +++ b/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 { diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go index cdf43a7249..661b4d8f1d 100644 --- a/src/cmd/go/internal/test/testflag.go +++ b/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) -- 2.48.1