Switching coverage to a better mechanism

go/private/library.bzl

@@ -22,10 +22,12 @@ def emit_library_actions(ctx, srcs, deps, cgo_object, library, want_coverage, im
   direct = depset(golibs)
   gc_goopts = tuple(ctx.attr.gc_goopts)
   cgo_deps = depset()
+  cover_vars = ()
   if library:
     golib = library[GoLibrary]
     cgolib = library[CgoLibrary]
     srcs = golib.transformed + srcs
+    cover_vars += golib.cover_vars
     direct += golib.direct
     dep_runfiles += [library.data_runfiles]
     gc_goopts += golib.gc_goopts
@@ -70,7 +72,8 @@ def emit_library_actions(ctx, srcs, deps, cgo_object, library, want_coverage, im
 
   go_srcs = source.go
   if want_coverage:
-    go_srcs = _emit_go_cover_action(ctx, out_object, go_srcs)
+    go_srcs, cvars = _emit_go_cover_action(ctx, go_toolchain, go_srcs)
+    cover_vars += cvars
 
   emit_go_compile_action(ctx,
       sources = go_srcs,
@@ -114,6 +117,7 @@ def emit_library_actions(ctx, srcs, deps, cgo_object, library, want_coverage, im
           cgo_deps = cgo_deps, # The direct cgo dependancies of this library
           gc_goopts = gc_goopts, # The options this library was compiled with
           runfiles = runfiles, # The runfiles needed for things including this library
+          cover_vars = cover_vars, # The cover variables for this library
       ),
       CgoLibrary(
           object = cgo_object,
@@ -248,7 +252,7 @@ def emit_go_pack_action(ctx, out_lib, objects):
       env = go_toolchain.env,
   )
 
-def _emit_go_cover_action(ctx, out_object, sources):
+def _emit_go_cover_action(ctx, go_toolchain, sources):
   """Construct the command line for test coverage instrument.
 
   Args:
@@ -260,10 +264,9 @@ def _emit_go_cover_action(ctx, out_object, sources):
   Returns:
     A list of Go source code files which might be coverage instrumented.
   """
-  go_toolchain = get_go_toolchain(ctx)
   outputs = []
   # TODO(linuxerwang): make the mode configurable.
-  count = 0
+  cover_vars = []
 
   for src in sources:
     if (not src.basename.endswith(".go") or 
@@ -272,8 +275,9 @@ def _emit_go_cover_action(ctx, out_object, sources):
       outputs += [src]
       continue
 
-    cover_var = "GoCover_%d" % count
-    out = ctx.new_file(out_object, out_object.basename + '_' + src.basename[:-3] + '_' + cover_var + '.cover.go')
+    cover_var = "Cover_" + src.basename[:-3].replace("-", "_").replace(".", "_")
+    cover_vars += ["{}={}".format(cover_var,src.short_path)]
+    out = ctx.new_file(cover_var + '.cover.go')
     outputs += [out]
     ctx.action(
         inputs = [src] + go_toolchain.tools,
@@ -283,6 +287,5 @@ def _emit_go_cover_action(ctx, out_object, sources):
         arguments = ["tool", "cover", "--mode=set", "-var=%s" % cover_var, "-o", out.path, src.path],
         env = go_toolchain.env,
     )
-    count += 1
 
-  return outputs
+  return outputs, tuple(cover_vars)

go/private/test.bzl

@@ -41,21 +41,30 @@ def _go_test_impl(ctx):
   else:
     run_dir = pkg_dir(ctx.label.workspace_root, ctx.label.package)
 
-  go_srcs = list(split_srcs(golib.transformed).go)
+  go_srcs = list(split_srcs(golib.srcs).go)
   main_go = ctx.new_file(ctx.label.name + "_main_test.go")
+  arguments = [
+      '--package',
+      golib.importpath,
+      '--rundir',
+      run_dir,
+      '--output',
+      main_go.path,
+  ]
+  cover_vars = []
+  covered_libs = []
+  for golib in depset([golib]) + golib.transitive:
+    if golib.cover_vars:
+      covered_libs += [golib]
+      for var in golib.cover_vars:
+        arguments += ["-cover", "{}={}".format(var, golib.importpath)]
+
   ctx.action(
       inputs = go_srcs,
       outputs = [main_go],
       mnemonic = "GoTestGenTest",
       executable = go_toolchain.test_generator,
-      arguments = [
-          '--package',
-          golib.importpath,
-          '--rundir',
-          run_dir,
-          '--output',
-          main_go.path,
-      ] + [src.path for src in go_srcs],
+      arguments = arguments + [src.path for src in go_srcs],
       env = dict(go_toolchain.env, RUNDIR=ctx.label.package)
   )
 
@@ -66,7 +75,7 @@ def _go_test_impl(ctx):
       library = None,
       want_coverage = False,
       importpath = ctx.label.name + "~testmain~",
-      golibs = [golib],
+      golibs = [golib] + covered_libs,
   )
 
   mode = ""

go/tools/builders/BUILD

@@ -48,6 +48,7 @@ go_tool_binary(
     name = "generate_test_main",
     srcs = [
         "filter.go",
+        "flags.go",
         "generate_test_main.go",
     ],
     visibility = ["//visibility:public"],

go/tools/builders/generate_test_main.go

@@ -27,30 +27,21 @@ import (
 	"log"
 	"os"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"strconv"
 	"strings"
 	"text/template"
 )
 
-var (
-	coverFileRegexp = regexp.MustCompile(".+_(?P<cover_var>GoCover_\\d+)\\.cover\\.go")
-
-	testCover      bool     // -cover flag
-	testCoverMode  string   // -covermode flag
-	testCoverPaths []string // -coverpkg flag
-)
-
-// CoverVar holds the name of the generated coverage variables targeting
-// the named file.
-type CoverVar struct {
-	File string // local file name
-	Var  string // name of count struct
+type CoverFile struct {
+	File string
+	Var  string
 }
 
-type coverInfo struct {
-	Vars map[string]*CoverVar
+type CoverPackage struct {
+	Name   string
+	Import string
+	Files  []CoverFile
 }
 
 // Cases holds template data.
@@ -62,29 +53,7 @@ type Cases struct {
 	HasTestMain      bool
 	Version17        bool
 	Version18OrNewer bool
-	Cover            []coverInfo
-}
-
-func (c *Cases) CoverMode() string {
-	if testCoverMode == "" {
-		return "set"
-	}
-	return testCoverMode
-}
-
-func (c *Cases) 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 (c *Cases) Covered() string {
-	if testCoverPaths == nil {
-		return ""
-	}
-	return " in " + strings.Join(testCoverPaths, ", ")
+	Cover            []*CoverPackage
 }
 
 var codeTpl = `
@@ -93,6 +62,7 @@ import (
 	"flag"
 	"log"
 	"os"
+	"fmt"
 {{if .Version17}}
 	"regexp"
 {{end}}
@@ -107,11 +77,8 @@ import (
 	undertest "{{.Package}}"
 {{end}}
 
-{{if .CoverEnabled}}
-	{{$pkg := .Package}}
-	{{range $i, $p := .Cover}}
-		_cover{{$i}} {{$pkg | printf "%q"}}
-	{{end}}
+{{range $p := .Cover}}
+	{{$p.Name}} {{$p.Import | printf "%q"}}
 {{end}}
 )
 
@@ -127,31 +94,33 @@ var benchmarks = []testing.InternalBenchmark{
 {{end}}
 }
 
-{{if .CoverEnabled}}
-
-// Only updated by init functions, so no need for atomicity.
-var (
-	coverCounters = make(map[string][]uint32)
-	coverBlocks = make(map[string][]testing.CoverBlock)
-)
-
-func init() {
-	{{range $i, $p := .Cover}}
-	{{range $file, $cover := $p.Vars}}
-	coverRegisterFile({{printf "%q" $cover.File}}, _cover{{$i}}.{{$cover.Var}}.Count[:], _cover{{$i}}.{{$cover.Var}}.Pos[:], _cover{{$i}}.{{$cover.Var}}.NumStmt[:])
-	{{end}}
-	{{end}}
+func coverRegisterAll() testing.Cover {
+	coverage := testing.Cover{
+		Mode: "set",
+		CoveredPackages: "",
+		Counters: map[string][]uint32{},
+		Blocks: map[string][]testing.CoverBlock{},
+	}
+{{range $p := .Cover}}
+	//{{$p.Import}}
+{{range $v := $p.Files}}
+	{{$var := printf "%s.%s" $p.Name $v.Var}}
+	coverRegisterFile(&coverage, {{$v.File | printf "%q"}}, {{$var}}.Count[:], {{$var}}.Pos[:], {{$var}}.NumStmt[:])
+{{end}}
+{{end}}
+	return coverage
 }
 
-func coverRegisterFile(fileName string, counter []uint32, pos []uint32, numStmts []uint16) {
+func coverRegisterFile(coverage *testing.Cover, fileName string, counter []uint32, pos []uint32, numStmts []uint16) {
 	if 3*len(counter) != len(pos) || len(counter) != len(numStmts) {
 		panic("coverage: mismatched sizes")
 	}
-	if coverCounters[fileName] != nil {
+	if coverage.Counters[fileName] != nil {
 		// Already registered.
+		fmt.Printf("Already covered %s\n", fileName)
 		return
 	}
-	coverCounters[fileName] = counter
+	coverage.Counters[fileName] = counter
 	block := make([]testing.CoverBlock, len(counter))
 	for i := range counter {
 		block[i] = testing.CoverBlock{
@@ -162,9 +131,8 @@ func coverRegisterFile(fileName string, counter []uint32, pos []uint32, numStmts
 			Stmts: numStmts[i],
 		}
 	}
-	coverBlocks[fileName] = block
+	coverage.Blocks[fileName] = block
 }
-{{end}}
 
 func main() {
 	// Check if we're being run by Bazel and change directories if so.
@@ -181,14 +149,10 @@ func main() {
 		}
 	}
 
-{{if .CoverEnabled}}
-	testing.RegisterCover(testing.Cover{
-		Mode: {{printf "%q" .CoverMode}},
-		Counters: coverCounters,
-		Blocks: coverBlocks,
-		CoveredPackages: {{printf "%q" .Covered}},
-	})
-{{end}}
+	coverage := coverRegisterAll()
+	if len(coverage.Counters) > 0 {
+		testing.RegisterCover(coverage)
+	}
 
 {{if .Version18OrNewer}}
 	m := testing.MainStart(testdeps.TestDeps{}, tests, benchmarks, nil)
@@ -210,11 +174,13 @@ func main() {
 
 func run(args []string) error {
 	// Prepare our flags
+	cover := multiFlag{}
 	flags := flag.NewFlagSet("generate_test_main", flag.ExitOnError)
 	pkg := flags.String("package", "", "package from which to import test methods.")
 	runDir := flags.String("rundir", ".", "Path to directory where tests should run.")
 	out := flags.String("output", "", "output file to write. Defaults to stdout.")
 	tags := flags.String("tags", "", "Only pass through files that match these tags.")
+	flags.Var(&cover, "cover", "Information about a coverage variable")
 	if err := flags.Parse(args); err != nil {
 		return err
 	}
@@ -240,24 +206,34 @@ func run(args []string) error {
 		defer outFile.Close()
 	}
 
-	ci := coverInfo{
-		Vars: map[string]*CoverVar{},
-	}
 	cases := Cases{
 		Package: *pkg,
 		RunDir:  filepath.FromSlash(*runDir),
-		Cover:   []coverInfo{ci},
 	}
-	testFileSet := token.NewFileSet()
-	for _, f := range filenames {
-		coverVar := extractCoverVar(f)
-		if coverVar != "" {
-			ci.Vars[f] = &CoverVar{
-				File: f,
-				Var:  coverVar,
+	covered := map[string]*CoverPackage{}
+	for _, c := range cover {
+		bits := strings.SplitN(c, "=", 3)
+		if len(bits) != 3 {
+			return fmt.Errorf("Invalid cover variable arg, expected var=file=package got %s", c)
+		}
+		importPath := bits[2]
+		pkg, found := covered[importPath]
+		if !found {
+			pkg = &CoverPackage{
+				Name:   fmt.Sprintf("covered%d", len(covered)),
+				Import: importPath,
 			}
+			covered[importPath] = pkg
+			cases.Cover = append(cases.Cover, pkg)
 		}
+		pkg.Files = append(pkg.Files, CoverFile{
+			File: bits[1],
+			Var:  bits[0],
+		})
+	}
 
+	testFileSet := token.NewFileSet()
+	for _, f := range filenames {
 		parse, err := parser.ParseFile(testFileSet, f, nil, parser.ParseComments)
 		if err != nil {
 			return fmt.Errorf("ParseFile(%q): %v", f, err)
@@ -321,8 +297,6 @@ func run(args []string) error {
 		}
 	}
 
-	testCover = len(ci.Vars) > 0
-
 	goVersion, err := parseVersion(runtime.Version())
 	if err != nil {
 		return err
@@ -371,14 +345,6 @@ func (x version) Less(y version) bool {
 	return len(x) < len(y)
 }
 
-func extractCoverVar(fn string) string {
-	res := coverFileRegexp.FindStringSubmatch(fn)
-	if len(res) > 1 {
-		return res[1]
-	}
-	return ""
-}
-
 func main() {
 	if err := run(os.Args[1:]); err != nil {
 		log.Fatal(err)