imports: make tests compatible with modules
Tweak the goimports tests to be more module-compatible. This should be
the last big change to the tests; they all pass with the new
implementation.
The primary change is to avoid using packages not provided by the test's
modules. Other modules will be downloaded at test time, which is
nonhermetic and quite slow.
Other miscellanea:
- The appengine grouping tests have to be split out so
they can be GOPATH-only, because modules have to have dots in their
names.
- The tests for .goimportsignore and node_modules are GOPATH-only
because we decided not to include those behaviors in go/packages in
module mode.
- Some vendoring tests are GOPATH-only because vendoring is not a thing
in module mode.
- TestFindImportInLocalGoFiles changes content, because the existing
test was incorrect: bogus.net/bytes was a viable candidate even though
it isn't on disk. I'm not sure why it wasn't flaky.
Change-Id: I35a3aac92d3fb7f70a1a8f027f0b423282420a4d
Reviewed-on: https://go-review.googlesource.com/c/145138
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/imports/fix_test.go b/imports/fix_test.go
index 64f5eb5..7d71ecd 100644
--- a/imports/fix_test.go
+++ b/imports/fix_test.go
@@ -55,11 +55,11 @@
import (
"fmt"
- "appengine"
+ "github.com/golang/snappy"
)
func bar() {
var b bytes.Buffer
-_ = appengine.IsDevServer
+_ = snappy.ErrCorrupt
fmt.Println(b.String())
}
`,
@@ -69,12 +69,12 @@
"bytes"
"fmt"
- "appengine"
+ "github.com/golang/snappy"
)
func bar() {
var b bytes.Buffer
- _ = appengine.IsDevServer
+ _ = snappy.ErrCorrupt
fmt.Println(b.String())
}
`,
@@ -88,12 +88,12 @@
import (
"fmt"
- "appengine"
+ "github.com/golang/snappy"
)
func bar() {
_ = math.NaN
_ = fmt.Sprintf
-_ = appengine.IsDevServer
+_ = snappy.ErrCorrupt
}
`,
out: `package foo
@@ -102,13 +102,13 @@
"fmt"
"math"
- "appengine"
+ "github.com/golang/snappy"
)
func bar() {
_ = math.NaN
_ = fmt.Sprintf
- _ = appengine.IsDevServer
+ _ = snappy.ErrCorrupt
}
`,
},
@@ -352,7 +352,7 @@
func foo () {
_, _ = os.Args, fmt.Println
-_, _ = appengine.Main, datastore.ErrInvalidEntityType
+_, _ = snappy.ErrCorrupt, p.P
}
`,
out: `package foo
@@ -361,13 +361,13 @@
"fmt"
"os"
- "appengine"
- "appengine/datastore"
+ "github.com/golang/snappy"
+ "rsc.io/p"
)
func foo() {
_, _ = os.Args, fmt.Println
- _, _ = appengine.Main, datastore.ErrInvalidEntityType
+ _, _ = snappy.ErrCorrupt, p.P
}
`,
},
@@ -426,7 +426,7 @@
"fmt"
"net"
- "code.google.com/p/snappy-go/snappy"
+ "github.com/golang/snappy"
)
func f() {
@@ -443,7 +443,7 @@
in: `package foo
import (
- "code.google.com/p/snappy-go/snappy"
+ "github.com/golang/snappy"
"fmt"
"net"
)
@@ -460,7 +460,7 @@
"fmt"
"net"
- "code.google.com/p/snappy-go/snappy"
+ "github.com/golang/snappy"
)
func f() {
@@ -501,12 +501,12 @@
"fmt"
"gu"
-"github.com/foo/bar"
+"manypackages.com/packagea"
)
var (
-a = bar.a
-b = gu.a
+a = packagea.A
+b = gu.A
c = fmt.Printf
)
`,
@@ -517,12 +517,12 @@
"gu"
- "github.com/foo/bar"
+ "manypackages.com/packagea"
)
var (
- a = bar.a
- b = gu.a
+ a = packagea.A
+ b = gu.A
c = fmt.Printf
)
`,
@@ -556,14 +556,14 @@
import (
"fmt"
- "github.com/foo/bar"
- "github.com/foo/qux"
+ "manypackages.com/packagea"
+ "manypackages.com/packageb"
)
func main() {
var _ = fmt.Println
- //var _ = bar.A
- var _ = qux.B
+ //var _ = packagea.A
+ var _ = packageb.B
}
`,
out: `package main
@@ -571,13 +571,13 @@
import (
"fmt"
- "github.com/foo/qux"
+ "manypackages.com/packageb"
)
func main() {
var _ = fmt.Println
- //var _ = bar.A
- var _ = qux.B
+ //var _ = packagea.A
+ var _ = packageb.B
}
`,
},
@@ -590,34 +590,34 @@
import (
"fmt"
- renamed_bar "github.com/foo/bar"
+ renamed_packagea "manypackages.com/packagea"
- . "github.com/foo/baz"
+ . "manypackages.com/packageb"
"io"
- _ "github.com/foo/qux"
+ _ "manypackages.com/packagec"
"strings"
)
-var _, _, _, _, _ = fmt.Errorf, io.Copy, strings.Contains, renamed_bar.A, B
+var _, _, _, _, _ = fmt.Errorf, io.Copy, strings.Contains, renamed_packagea.A, B
`,
out: `package main
import (
"fmt"
- renamed_bar "github.com/foo/bar"
+ renamed_packagea "manypackages.com/packagea"
"io"
- . "github.com/foo/baz"
+ . "manypackages.com/packageb"
"strings"
- _ "github.com/foo/qux"
+ _ "manypackages.com/packagec"
)
-var _, _, _, _, _ = fmt.Errorf, io.Copy, strings.Contains, renamed_bar.A, B
+var _, _, _, _, _ = fmt.Errorf, io.Copy, strings.Contains, renamed_packagea.A, B
`,
},
@@ -630,7 +630,7 @@
import (
"fmt" // A
"go/ast" // B
- _ "launchpad.net/gocheck" // C
+ _ "manypackages.com/packagec" // C
)
func main() { _, _ = fmt.Print, ast.Walk }
@@ -641,7 +641,7 @@
"fmt" // A
"go/ast" // B
- _ "launchpad.net/gocheck" // C
+ _ "manypackages.com/packagec" // C
)
func main() { _, _ = fmt.Print, ast.Walk }
@@ -761,7 +761,7 @@
import (
"fmt"
-"golang.org/x/foo"
+"manypackages.com/packagea"
)
func main() {}
@@ -771,7 +771,7 @@
import (
"fmt"
- "golang.org/x/foo"
+ "manypackages.com/packagea"
)
func main() {}
@@ -818,7 +818,7 @@
import (
"time"
- "code.google.com/p/snappy-go/snappy"
+ "github.com/golang/snappy"
"rsc.io/p"
)
@@ -837,7 +837,7 @@
import (
"time"
- "code.google.com/p/snappy-go/snappy"
+ "github.com/golang/snappy"
)
func main() {
@@ -851,7 +851,7 @@
import (
"time"
- "code.google.com/p/snappy-go/snappy"
+ "github.com/golang/snappy"
"rsc.io/p"
)
@@ -863,8 +863,9 @@
`,
},
+ // golang.org/issue/12097
{
- name: "issue #12097",
+ name: "package_statement_insertion_preserves_comments",
in: `// a
// b
// c
@@ -980,7 +981,7 @@
_ "net/http/pprof" // install the pprof http handlers
"strings"
- "github.com/pkg/errors"
+ "manypackages.com/packagea"
)
func main() {
@@ -989,7 +990,7 @@
var (
_ json.Number
_ *http.Request
- _ errors.Frame
+ _ packagea.A
)
}
`,
@@ -1002,7 +1003,7 @@
_ "net/http/pprof" // install the pprof http handlers
"strings"
- "github.com/pkg/errors"
+ "manypackages.com/packagea"
)
func main() {
@@ -1011,7 +1012,7 @@
var (
_ json.Number
_ *http.Request
- _ errors.Frame
+ _ packagea.A
)
}
`,
@@ -1108,7 +1109,7 @@
func TestSimpleCases(t *testing.T) {
defer func(lp string) { LocalPrefix = lp }(LocalPrefix)
- LocalPrefix = "local,github.com/local"
+ LocalPrefix = "local.com,github.com/local"
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
options := &Options{
@@ -1119,28 +1120,39 @@
FormatOnly: tt.formatOnly,
}
testConfig{
- // Skeleton non-stdlib packages for use during testing.
- // Each includes one arbitrary symbol, e.g. the first declaration in the first file.
- // Try not to add more without a good reason.
modules: []packagestest.Module{
{
- Name: "appengine",
- Files: fm{
- "x.go": "package appengine\nfunc Main(){}\n",
- "datastore/x.go": "package datastore\nvar ErrInvalidEntityType error\n",
- },
+ Name: "golang.org/fake",
+ Files: fm{"x.go": tt.in},
},
+ // Skeleton non-stdlib packages for use during testing.
+ // Each includes one arbitrary symbol, e.g. the first declaration in the first file.
+ // Try not to add more without a good reason.
+ // DO NOT USE PACKAGES NOT LISTED HERE -- they will be downloaded!
{
Name: "rsc.io",
Files: fm{"p/x.go": "package p\nfunc P(){}\n"},
},
{
- Name: "code.google.com/p/snappy-go",
- Files: fm{"snappy/x.go": "package snappy\nvar ErrCorrupt error\n"},
+ Name: "github.com/golang/snappy",
+ Files: fm{"x.go": "package snappy\nvar ErrCorrupt error\n"},
},
{
- Name: "golang.org/fake",
- Files: fm{"x.go": tt.in},
+ Name: "manypackages.com",
+ Files: fm{
+ "packagea/x.go": "package packagea\nfunc A(){}\n",
+ "packageb/x.go": "package packageb\nfunc B(){}\n",
+ "packagec/x.go": "package packagec\nfunc C(){}\n",
+ "packaged/x.go": "package packaged\nfunc D(){}\n",
+ },
+ },
+ {
+ Name: "local.com",
+ Files: fm{"foo/x.go": "package foo\nfunc Foo(){}\n"},
+ },
+ {
+ Name: "github.com/local",
+ Files: fm{"bar/x.go": "package bar\nfunc Bar(){}\n"},
},
},
}.processTest(t, "golang.org/fake", "x.go", nil, options, tt.out)
@@ -1148,6 +1160,42 @@
}
}
+func TestAppengine(t *testing.T) {
+ const input = `package p
+
+var _, _, _ = fmt.Printf, appengine.Main, datastore.ErrInvalidEntityType
+`
+
+ const want = `package p
+
+import (
+ "fmt"
+
+ "appengine"
+ "appengine/datastore"
+)
+
+var _, _, _ = fmt.Printf, appengine.Main, datastore.ErrInvalidEntityType
+`
+
+ testConfig{
+ gopathOnly: true, // can't create a module named appengine, so no module tests.
+ modules: []packagestest.Module{
+ {
+ Name: "golang.org/fake",
+ Files: fm{"x.go": input},
+ },
+ {
+ Name: "appengine",
+ Files: fm{
+ "x.go": "package appengine\nfunc Main(){}\n",
+ "datastore/x.go": "package datastore\nvar ErrInvalidEntityType error\n",
+ },
+ },
+ },
+ }.processTest(t, "golang.org/fake", "x.go", nil, nil, want)
+}
+
func TestReadFromFilesystem(t *testing.T) {
tests := []struct {
name string
@@ -1271,6 +1319,7 @@
`
testConfig{
+ gopathOnly: true,
module: packagestest.Module{
Name: "golang.org/fake",
Files: fm{
@@ -1327,6 +1376,7 @@
)
`
testConfig{
+ gopathOnly: true,
module: packagestest.Module{
Name: "mypkg.com/outpkg",
Files: fm{
@@ -1425,8 +1475,9 @@
}
type testConfig struct {
- module packagestest.Module
- modules []packagestest.Module
+ gopathOnly bool
+ module packagestest.Module
+ modules []packagestest.Module
}
// fm is the type for a packagestest.Module's Files, abbreviated for shorter lines.
@@ -1440,72 +1491,68 @@
c.modules = []packagestest.Module{c.module}
}
- exported = packagestest.Export(t, packagestest.GOPATH, c.modules)
- defer exported.Cleanup()
+ for _, exporter := range []packagestest.Exporter{packagestest.GOPATH} {
+ t.Run(exporter.Name(), func(t *testing.T) {
+ t.Helper()
+ exported = packagestest.Export(t, exporter, c.modules)
+ defer exported.Cleanup()
- env := make(map[string]string)
- for _, kv := range exported.Config.Env {
- split := strings.Split(kv, "=")
- k, v := split[0], split[1]
- env[k] = v
+ env := make(map[string]string)
+ for _, kv := range exported.Config.Env {
+ split := strings.Split(kv, "=")
+ k, v := split[0], split[1]
+ env[k] = v
+ }
+
+ goroot := env["GOROOT"]
+ gopath := env["GOPATH"]
+
+ scanOnce = sync.Once{}
+
+ oldGOPATH := build.Default.GOPATH
+ oldGOROOT := build.Default.GOROOT
+ oldCompiler := build.Default.Compiler
+ build.Default.GOROOT = goroot
+ build.Default.GOPATH = gopath
+ build.Default.Compiler = "gc"
+
+ defer func() {
+ build.Default.GOPATH = oldGOPATH
+ build.Default.GOROOT = oldGOROOT
+ build.Default.Compiler = oldCompiler
+ }()
+
+ it := &goimportTest{
+ T: t,
+ gopath: gopath,
+ exported: exported,
+ }
+ fn(it)
+ })
}
-
- goroot := env["GOROOT"]
- gopath := env["GOPATH"]
-
- scanOnce = sync.Once{}
-
- oldGOPATH := build.Default.GOPATH
- oldGOROOT := build.Default.GOROOT
- oldCompiler := build.Default.Compiler
- build.Default.GOPATH = ""
- build.Default.Compiler = "gc"
-
- defer func() {
- build.Default.GOPATH = oldGOPATH
- build.Default.GOROOT = oldGOROOT
- build.Default.Compiler = oldCompiler
- }()
-
- build.Default.GOROOT = goroot
- build.Default.GOPATH = gopath
-
- it := &goimportTest{
- T: t,
- goroot: build.Default.GOROOT,
- gopath: gopath,
- ctx: &build.Default,
- exported: exported,
- }
- fn(it)
}
func (c testConfig) processTest(t *testing.T, module, file string, contents []byte, opts *Options, want string) {
t.Helper()
c.test(t, func(t *goimportTest) {
t.Helper()
- f := t.exported.File(module, file)
- if f == "" {
- t.Fatalf("%v not found in exported files (typo in filename?)", file)
- }
- t.process(f, contents, opts, want)
+ t.process(module, file, contents, opts, want)
})
}
type goimportTest struct {
*testing.T
- ctx *build.Context
- goroot string
gopath string
exported *packagestest.Exported
}
-func (t *goimportTest) process(file string, contents []byte, opts *Options, want string) {
+func (t *goimportTest) process(module, file string, contents []byte, opts *Options, want string) {
t.Helper()
- if !filepath.IsAbs(file) {
- file = filepath.Join(t.gopath, "src", file)
+ f := t.exported.File(module, file)
+ if f == "" {
+ t.Fatalf("%v not found in exported files (typo in filename?)", file)
}
- buf, err := Process(file, contents, opts)
+ buf, err := Process(f, contents, opts)
if err != nil {
t.Fatalf("Process() = %v", err)
}
@@ -1543,12 +1590,14 @@
// to be added into a later group (num=3).
func TestLocalPrefix(t *testing.T) {
tests := []struct {
+ name string
modules []packagestest.Module
localPrefix string
src string
want string
}{
{
+ name: "one_local",
modules: []packagestest.Module{
{
Name: "foo.com",
@@ -1572,6 +1621,7 @@
`,
},
{
+ name: "two_local",
modules: []packagestest.Module{
{
Name: "foo.com",
@@ -1598,6 +1648,7 @@
`,
},
{
+ name: "three_prefixes",
modules: []packagestest.Module{
{
Name: "example.org/pkg",
@@ -1633,12 +1684,18 @@
}
for _, tt := range tests {
- testConfig{
- modules: tt.modules,
- }.test(t, func(t *goimportTest) {
- defer func(s string) { LocalPrefix = s }(LocalPrefix)
- LocalPrefix = tt.localPrefix
- t.process("test/t.go", []byte(tt.src), nil, tt.want)
+ t.Run(tt.name, func(t *testing.T) {
+ testConfig{
+ // The module being processed has to be first so it's the primary module.
+ modules: append([]packagestest.Module{{
+ Name: "test.com",
+ Files: fm{"t.go": tt.src},
+ }}, tt.modules...),
+ }.test(t, func(t *goimportTest) {
+ defer func(s string) { LocalPrefix = s }(LocalPrefix)
+ LocalPrefix = tt.localPrefix
+ t.process("test.com", "t.go", nil, nil, tt.want)
+ })
})
}
}
@@ -1674,6 +1731,7 @@
// never make it that far).
func TestImportPathToNameGoPathParse(t *testing.T) {
testConfig{
+ gopathOnly: true,
module: packagestest.Module{
Name: "example.net/pkg",
Files: fm{
@@ -1708,6 +1766,7 @@
`
testConfig{
+ gopathOnly: true,
module: packagestest.Module{
Name: "foo.com",
Files: fm{
@@ -1734,6 +1793,7 @@
`
testConfig{
+ gopathOnly: true,
module: packagestest.Module{
Name: "foo.com",
Files: fm{
@@ -2035,21 +2095,38 @@
testConfig{
modules: []packagestest.Module{
{
- Name: "bytes.net/bytes",
- Files: fm{"bytes.go": "package bytes\n type Buffer struct {}"}, // Should be selected over standard library
- },
- {
Name: "mycompany.net/tool",
Files: fm{
"io.go": "package main\n import \"bytes.net/bytes\"\n var _ = &bytes.Buffer{}", // Contains package import that will cause stdlib to be ignored
- "err.go": "package main\n import \"bogus.net/bytes\"\n var _ = &bytes.Buffer{}", // Contains import which is not resolved, so it is ignored
"main.go": input,
},
},
+ {
+ Name: "bytes.net/bytes",
+ Files: fm{"bytes.go": "package bytes\n type Buffer struct {}"}, // Should be selected over standard library
+ },
},
}.processTest(t, "mycompany.net/tool", "main.go", nil, nil, want)
}
+func TestInMemoryFile(t *testing.T) {
+ const input = `package main
+ var _ = &bytes.Buffer{}`
+
+ const want = `package main
+
+import "bytes"
+
+var _ = &bytes.Buffer{}
+`
+ testConfig{
+ module: packagestest.Module{
+ Name: "foo.com",
+ Files: fm{"x.go": "package x\n"},
+ },
+ }.processTest(t, "foo.com", "x.go", []byte(input), nil, want)
+}
+
func TestImportNoGoFiles(t *testing.T) {
const input = `package main
var _ = &bytes.Buffer{}`
@@ -2078,12 +2155,11 @@
input := `package testimports
import (
- "fmt"
- "mydomain.mystuff/mypkg"
+ "bytes"
)
const s = fmt.Sprintf("%s", "` + largeString + `")
-const x = mypkg.Sprintf("%s", "my package")
+var _ = bytes.Buffer{}
// end
`
@@ -2091,13 +2167,13 @@
want := `package testimports
import (
+ "bytes"
"fmt"
-
- "mydomain.mystuff/mypkg"
)
const s = fmt.Sprintf("%s", "` + largeString + `")
-const x = mypkg.Sprintf("%s", "my package")
+
+var _ = bytes.Buffer{}
// end
`
