From 1a6034dbfc3be629037eb833fd808ad97356db9d Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Fri, 2 Nov 2018 17:43:46 -0400 Subject: [PATCH] go/packages: small fixes For performance, bail out of runNamedQueries before running go env if there's nothing to do. Add t.Helper() calls to packagestest.TestAll. Escape # in packagestest.Export. The testing package adds #NN suffixes to subtests that have redundant names. Log how long gopathwalk.Walk took for name= queries when debug is on. Change-Id: I37cb0ed11cf58e1693e29dea04697e5885ecc62b Reviewed-on: https://go-review.googlesource.com/c/147203 Run-TryBot: Heschi Kreinick Reviewed-by: Ian Cottrell --- go/packages/golist.go | 14 ++++++++++---- go/packages/packagestest/export.go | 10 ++++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/go/packages/golist.go b/go/packages/golist.go index 8adaf05e..2675c882 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -35,9 +35,6 @@ type goTooOldError struct { // the build system package structure. // See driver for more details. func goListDriver(cfg *Config, patterns ...string) (*driverResponse, error) { - if debug { - defer func(start time.Time, patterns []string) { log.Printf("%v for query %v", time.Since(start), patterns) }(time.Now(), patterns) - } // Determine files requested in contains patterns var containFiles []string var packagesNamed []string @@ -180,6 +177,10 @@ func runContainsQueries(cfg *Config, driver driver, addPkg func(*Package), queri var modCacheRegexp = regexp.MustCompile(`(.*)@([^/\\]*)(.*)`) func runNamedQueries(cfg *Config, driver driver, addPkg func(*Package), queries []string) ([]string, error) { + // calling `go env` isn't free; bail out if there's nothing to do. + if len(queries) == 0 { + return nil, nil + } // Determine which directories are relevant to scan. roots, modRoot, err := roots(cfg) if err != nil { @@ -216,7 +217,12 @@ func runNamedQueries(cfg *Config, driver driver, addPkg func(*Package), queries } } } - gopathwalk.Walk(roots, add, gopathwalk.Options{ModulesEnabled: modRoot != ""}) + + startWalk := time.Now() + gopathwalk.Walk(roots, add, gopathwalk.Options{ModulesEnabled: modRoot != "", Debug: debug}) + if debug { + log.Printf("%v for walk", time.Since(startWalk)) + } // Weird special case: the top-level package in a module will be in // whatever directory the user checked the repository out into. It's diff --git a/go/packages/packagestest/export.go b/go/packages/packagestest/export.go index c59f7c00..c85a375a 100644 --- a/go/packages/packagestest/export.go +++ b/go/packages/packagestest/export.go @@ -85,8 +85,12 @@ var All []Exporter // the All global. // Each exporter will be run as a sub-test named after the exporter being used. func TestAll(t *testing.T, f func(*testing.T, Exporter)) { + t.Helper() for _, e := range All { - t.Run(e.Name(), func(t *testing.T) { f(t, e) }) + t.Run(e.Name(), func(t *testing.T) { + t.Helper() + f(t, e) + }) } } @@ -101,7 +105,9 @@ func TestAll(t *testing.T, f func(*testing.T, Exporter)) { // debugging tests. func Export(t *testing.T, exporter Exporter, modules []Module) *Exported { t.Helper() - temp, err := ioutil.TempDir("", strings.Replace(t.Name(), "/", "_", -1)) + dirname := strings.Replace(t.Name(), "/", "_", -1) + dirname = strings.Replace(dirname, "#", "_", -1) // duplicate subtests get a #NNN suffix. + temp, err := ioutil.TempDir("", dirname) if err != nil { t.Fatal(err) }