Skip to content

Commit b752317

Browse files
committed
internal/analysisinternal: disable AddImport test without go command
The AddImport test uses the default importer, which calls go list. This fails in environments that can't call the go command, like js/wasm. Add a predicate to testenv that asserts the need for the default importer, and call it from TestAddImport. A subtlety: although this bug manifested itself only for the dot-import cases, in fact all the test cases failed type checking on js/wasm for this reason. But a successful type-check is not a precondition for the test (see the new comment in TestAddImport). What caused the particular test case to fail was a bad diff resulting from how the edit was applied in the presence of that failure. Fixes golang/go#71645. Change-Id: Ib04afad108c323999bb67f329cf8d9cf329fead1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/648580 Reviewed-by: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent d98774e commit b752317

File tree

2 files changed

+16
-0
lines changed

2 files changed

+16
-0
lines changed

internal/analysisinternal/addimport_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@ import (
1818
"github.com/google/go-cmp/cmp"
1919
"golang.org/x/tools/go/analysis"
2020
"golang.org/x/tools/internal/analysisinternal"
21+
"golang.org/x/tools/internal/testenv"
2122
)
2223

2324
func TestAddImport(t *testing.T) {
25+
testenv.NeedsDefaultImporter(t)
26+
2427
descr := func(s string) string {
2528
if _, _, line, ok := runtime.Caller(1); ok {
2629
return fmt.Sprintf("L%d %s", line, s)
@@ -270,6 +273,9 @@ func _(io.Reader) {
270273
Implicits: make(map[ast.Node]types.Object),
271274
}
272275
conf := &types.Config{
276+
// We don't want to fail if there is an error during type checking:
277+
// the error may be because we're missing an import, and adding imports
278+
// is the whole point of AddImport.
273279
Error: func(err error) { t.Log(err) },
274280
Importer: importer.Default(),
275281
}

internal/testenv/testenv.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,16 @@ func NeedsGoBuild(t testing.TB) {
278278
NeedsTool(t, "go")
279279
}
280280

281+
// NeedsDefaultImporter skips t if the test uses the default importer,
282+
// returned by [go/importer.Default].
283+
func NeedsDefaultImporter(t testing.TB) {
284+
t.Helper()
285+
// The default importer may call `go list`
286+
// (in src/internal/exportdata/exportdata.go:lookupGorootExport),
287+
// so check for the go tool.
288+
NeedsTool(t, "go")
289+
}
290+
281291
// ExitIfSmallMachine emits a helpful diagnostic and calls os.Exit(0) if the
282292
// current machine is a builder known to have scarce resources.
283293
//

0 commit comments

Comments
 (0)