From: Rob Pike Date: Tue, 9 Jul 2013 23:52:36 +0000 (+1000) Subject: cmd/go: fix a couple of bugs in coverage tooling X-Git-Tag: go1.2rc2~1098 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=6d86c14efab9bdd9d071ac081fa6f8ea62f956c9;p=gostls13.git cmd/go: fix a couple of bugs in coverage tooling Merging a couple of CLs into one, since they collided in my client and I'm lazy. 1) Fix up output in "go test -cover" case. We need to tell the testing package the name of the package being tested and the name of the package being covered. It can then sort out the report. 2) Filter out the _test.go files from coverage processing. We want to measure what the tests cover, not what's covered in the tests, The coverage for encoding/gob goes from 82.2% to 88.4%. There may be a cleaner way to do this - suggestions welcome - but ça suffit. Fixes #5810. R=rsc CC=golang-dev https://golang.org/cl/10868047 --- diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go index 123eb662e5..dd015eeac6 100644 --- a/src/cmd/go/build.go +++ b/src/cmd/go/build.go @@ -796,8 +796,8 @@ func (b *builder) build(a *action) (err error) { for _, file := range a.p.GoFiles { sourceFile := filepath.Join(a.p.Dir, file) cover := a.p.coverVars[file] - if cover == nil { - // Not covering this file + if cover == nil || isTestFile(file) { + // Not covering this file. gofiles = append(gofiles, file) continue } diff --git a/src/cmd/go/test.go b/src/cmd/go/test.go index d30b96bf0e..57b764586a 100644 --- a/src/cmd/go/test.go +++ b/src/cmd/go/test.go @@ -808,11 +808,21 @@ func recompileForTest(pmain, preal, ptest *Package, testDir string) { var coverIndex = 0 +// isTestFile reports whether the source file is a set of tests and should therefore +// be excluded from coverage analysis. +func isTestFile(file string) bool { + // We don't cover tests, only the code they test. + return strings.HasSuffix(file, "_test.go") +} + // declareCoverVars attaches the required cover variables names // to the files, to be used when annotating the files. func declareCoverVars(importPath string, files ...string) map[string]*CoverVar { coverVars := make(map[string]*CoverVar) for _, file := range files { + if isTestFile(file) { + continue + } coverVars[file] = &CoverVar{ File: filepath.Join(importPath, file), Var: fmt.Sprintf("GoCover_%d", coverIndex), @@ -902,11 +912,7 @@ func (b *builder) runTest(a *action) error { if testShowPass { a.testOutput.Write(out) } - coverWhere := "" - if testCoverPaths != nil { - coverWhere = " in " + strings.Join(testCoverPaths, ", ") - } - fmt.Fprintf(a.testOutput, "ok \t%s\t%s%s%s\n", a.p.ImportPath, t, coveragePercentage(out), coverWhere) + fmt.Fprintf(a.testOutput, "ok \t%s\t%s%s\n", a.p.ImportPath, t, coveragePercentage(out)) return nil } @@ -931,10 +937,12 @@ func coveragePercentage(out []byte) string { // The string looks like // test coverage for encoding/binary: 79.9% of statements // Extract the piece from the percentage to the end of the line. - re := regexp.MustCompile(`test coverage for [^ ]+: (.*)\n`) + re := regexp.MustCompile(`coverage for [^ ]+: (.*)\n`) matches := re.FindSubmatch(out) if matches == nil { - return "(missing coverage statistics)" + // Probably running "go test -cover" not "go test -cover fmt". + // The coverage output will appear in the output directly. + return "" } return fmt.Sprintf("\tcoverage: %s", matches[1]) } @@ -1036,6 +1044,22 @@ func (t *testFuncs) CoverEnabled() bool { return testCover } +// Covered returns a string describing which packages are being tested for coverage. +// If the covered package is the same as the tested package, it returns the empty string. +// Otherwise it is a comma-separated human-readable list of packages beginning with +// " in", ready for use in the coverage message. +func (t *testFuncs) Covered() string { + if testCoverPaths == nil { + return "" + } + return " in " + strings.Join(testCoverPaths, ", ") +} + +// Tested returns the name of the package being tested. +func (t *testFuncs) Tested() string { + return t.Package.Name +} + type testFunc struct { Package string // imported package name (_test or _xtest) Name string // function name @@ -1157,7 +1181,8 @@ func coverRegisterFile(fileName string, counter []uint32, pos []uint32, numStmts panic("coverage: mismatched sizes") } if coverCounters[fileName] != nil { - panic("coverage: duplicate counter array for " + fileName) + // Already registered. + return } coverCounters[fileName] = counter block := make([]testing.CoverBlock, len(counter)) @@ -1176,6 +1201,7 @@ func coverRegisterFile(fileName string, counter []uint32, pos []uint32, numStmts func main() { {{if .CoverEnabled}} + testing.CoveredPackage({{printf "%q" .Tested}}, {{printf "%q" .Covered}}) testing.RegisterCover(coverCounters, coverBlocks) {{end}} testing.Main(matchString, tests, benchmarks, examples) diff --git a/src/pkg/testing/cover.go b/src/pkg/testing/cover.go index 4136becbfc..4f66a0b792 100644 --- a/src/pkg/testing/cover.go +++ b/src/pkg/testing/cover.go @@ -27,6 +27,11 @@ var ( coverBlocks map[string][]CoverBlock ) +var ( + testedPackage string // The package being tested. + coveredPackage string // List of the package[s] being covered, if distinct from the tested package. +) + // RegisterCover records the coverage data accumulators for the tests. // NOTE: This struct is internal to the testing infrastructure and may change. // It is not covered (yet) by the Go 1 compatibility guidelines. @@ -35,6 +40,14 @@ func RegisterCover(c map[string][]uint32, b map[string][]CoverBlock) { coverBlocks = b } +// CoveredPackage records the names of the packages being tested and covered. +// NOTE: This function is internal to the testing infrastructure and may change. +// It is not covered (yet) by the Go 1 compatibility guidelines. +func CoveredPackage(tested, covered string) { + testedPackage = tested + coveredPackage = covered +} + // mustBeNil checks the error and, if present, reports it and exits. func mustBeNil(err error) { if err != nil { @@ -55,16 +68,7 @@ func coverReport() { } var active, total int64 - packageName := "" for name, counts := range coverCounters { - if packageName == "" { - // Package name ends at last slash. - for i, c := range name { - if c == '/' { - packageName = name[:i] - } - } - } blocks := coverBlocks[name] for i, count := range counts { stmts := int64(blocks[i].Stmts) @@ -85,8 +89,5 @@ func coverReport() { if total == 0 { total = 1 } - if packageName == "" { - packageName = "package" - } - fmt.Printf("test coverage for %s: %.1f%% of statements\n", packageName, 100*float64(active)/float64(total)) + fmt.Printf("coverage for %s: %.1f%% of statements%s\n", testedPackage, 100*float64(active)/float64(total), coveredPackage) }