Skip to content

Commit

Permalink
go/analysis/analysistest: stricter errors and GOWORK setting
Browse files Browse the repository at this point in the history
This change causes analysistest to report complete failure to
load one or more of the packages named by the load patterns.
(This is indicated by a missing Package.Name.)
Previously, these errors were silently ignored, meaning
that tests were providing less coverage than they seemed.

In particular, the stdversion test was unable to load the
specified modules because there was no GOWORK file to
locate them. (Worse: the user's GOWORK environment was
affecting the test.) So, we now allow tests to specify a
go.work file in the root of the test tree. If present,
we honor it, and, crucially, if absent, we set GOWORK=off.

Also, two other tests in gopls/internal/analysis were
silently doing nothing (!).

Change-Id: I4ee7ae2a636497a64f1e43eb05a4a414ceaa5f4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/582595
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
adonovan committed May 3, 2024
1 parent 4db1697 commit 629a7be
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 5 deletions.
22 changes: 19 additions & 3 deletions go/analysis/analysistest/analysistest.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,16 @@ type Result = checker.TestAnalyzerResult
// loadPackages returns an error if any package had an error, or the pattern
// matched no packages.
func loadPackages(a *analysis.Analyzer, dir string, patterns ...string) ([]*packages.Package, error) {
env := []string{"GOPATH=" + dir, "GO111MODULE=off"} // GOPATH mode
env := []string{"GOPATH=" + dir, "GO111MODULE=off", "GOWORK=off"} // GOPATH mode

// Undocumented module mode. Will be replaced by something better.
if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
env = []string{"GO111MODULE=on", "GOPROXY=off"} // module mode
gowork := filepath.Join(dir, "go.work")
if _, err := os.Stat(gowork); err != nil {
gowork = "off"
}

env = []string{"GO111MODULE=on", "GOPROXY=off", "GOWORK=" + gowork} // module mode
}

// packages.Load loads the real standard library, not a minimal
Expand All @@ -397,12 +402,23 @@ func loadPackages(a *analysis.Analyzer, dir string, patterns ...string) ([]*pack
return nil, err
}

// If any named package couldn't be loaded at all
// (e.g. the Name field is unset), fail fast.
for _, pkg := range pkgs {
if pkg.Name == "" {
return nil, fmt.Errorf("failed to load %q: Errors=%v",
pkg.PkgPath, pkg.Errors)
}
}

// Do NOT print errors if the analyzer will continue running.
// It is incredibly confusing for tests to be printing to stderr
// willy-nilly instead of their test logs, especially when the
// errors are expected and are going to be fixed.
if !a.RunDespiteErrors {
packages.PrintErrors(pkgs)
if packages.PrintErrors(pkgs) > 0 {
return nil, fmt.Errorf("there were package loading errors (and RunDespiteErrors is false)")
}
}

if len(pkgs) == 0 {
Expand Down
7 changes: 7 additions & 0 deletions go/analysis/passes/stdversion/testdata/test.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ See also gopls/internal/test/marker/testdata/diagnostics/stdversion.txt
which runs the same test within the gopls analysis driver, to ensure
coverage of per-file Go version support.

-- go.work --
go 1.21

use .
use sub
use old

-- go.mod --
module example.com

Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/analysis/stubmethods/stubmethods_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ import (

func Test(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, stubmethods.Analyzer, "a")
analysistest.Run(t, testdata, stubmethods.Analyzer, "typeparams")
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

package stubmethods

var _ I = Y{} // want "Implement I"
var _ I = Y{} // want "does not implement I"

type I interface{ F() }

Expand Down

0 comments on commit 629a7be

Please sign in to comment.