close_issue_1
/
tools
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Heschi Kreinick 2018-10-26 14:54:05 -04:00
parent 00c5fa5868
commit a0a13e073c
1 changed files with 203 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

266
imports/fix_test.go
View File

@ -55,11 +55,11 @@ func bar() {
import ( import (
"fmt" "fmt"
"appengine" "github.com/golang/snappy"
) )
func bar() { func bar() {
var b bytes.Buffer var b bytes.Buffer
_ = appengine.IsDevServer _ = snappy.ErrCorrupt
fmt.Println(b.String()) fmt.Println(b.String())
} }
`, `,
@ -69,12 +69,12 @@ import (
"bytes" "bytes"
"fmt" "fmt"
"appengine" "github.com/golang/snappy"
) )
func bar() { func bar() {
var b bytes.Buffer var b bytes.Buffer
_ = appengine.IsDevServer _ = snappy.ErrCorrupt
fmt.Println(b.String()) fmt.Println(b.String())
} }
`, `,
@ -88,12 +88,12 @@ func bar() {
import ( import (
"fmt" "fmt"
"appengine" "github.com/golang/snappy"
) )
func bar() { func bar() {
_ = math.NaN _ = math.NaN
_ = fmt.Sprintf _ = fmt.Sprintf
_ = appengine.IsDevServer _ = snappy.ErrCorrupt
} }
`, `,
out: `package foo out: `package foo
@ -102,13 +102,13 @@ import (
"fmt" "fmt"
"math" "math"
"appengine" "github.com/golang/snappy"
) )
func bar() { func bar() {
_ = math.NaN _ = math.NaN
_ = fmt.Sprintf _ = fmt.Sprintf
_ = appengine.IsDevServer _ = snappy.ErrCorrupt
} }
`, `,
}, },
@ -352,7 +352,7 @@ import (
func foo () { func foo () {
_, _ = os.Args, fmt.Println _, _ = os.Args, fmt.Println
_, _ = appengine.Main, datastore.ErrInvalidEntityType _, _ = snappy.ErrCorrupt, p.P
} }
`, `,
out: `package foo out: `package foo
@ -361,13 +361,13 @@ import (
"fmt" "fmt"
"os" "os"
"appengine" "github.com/golang/snappy"
"appengine/datastore" "rsc.io/p"
) )
func foo() { func foo() {
_, _ = os.Args, fmt.Println _, _ = os.Args, fmt.Println
_, _ = appengine.Main, datastore.ErrInvalidEntityType _, _ = snappy.ErrCorrupt, p.P
} }
`, `,
}, },
@ -426,7 +426,7 @@ import (
"fmt" "fmt"
"net" "net"
"code.google.com/p/snappy-go/snappy" "github.com/golang/snappy"
) )
func f() { func f() {
@ -443,7 +443,7 @@ func f() {
in: `package foo in: `package foo
import ( import (
"code.google.com/p/snappy-go/snappy" "github.com/golang/snappy"
"fmt" "fmt"
"net" "net"
) )
@ -460,7 +460,7 @@ import (
"fmt" "fmt"
"net" "net"
"code.google.com/p/snappy-go/snappy" "github.com/golang/snappy"
) )
func f() { func f() {
@ -501,12 +501,12 @@ import (
"fmt" "fmt"
"gu" "gu"
"github.com/foo/bar" "manypackages.com/packagea"
) )
var ( var (
a = bar.a a = packagea.A
b = gu.a b = gu.A
c = fmt.Printf c = fmt.Printf
) )
`, `,
@ -517,12 +517,12 @@ import (
"gu" "gu"
"github.com/foo/bar" "manypackages.com/packagea"
) )
var ( var (
a = bar.a a = packagea.A
b = gu.a b = gu.A
c = fmt.Printf c = fmt.Printf
) )
`, `,
@ -556,14 +556,14 @@ func notmain() { fmt.Println("Hello, world") }`,
import ( import (
"fmt" "fmt"
"github.com/foo/bar" "manypackages.com/packagea"
"github.com/foo/qux" "manypackages.com/packageb"
) )
func main() { func main() {
var _ = fmt.Println var _ = fmt.Println
//var _ = bar.A //var _ = packagea.A
var _ = qux.B var _ = packageb.B
} }
`, `,
out: `package main out: `package main
@ -571,13 +571,13 @@ func main() {
import ( import (
"fmt" "fmt"
"github.com/foo/qux" "manypackages.com/packageb"
) )
func main() { func main() {
var _ = fmt.Println var _ = fmt.Println
//var _ = bar.A //var _ = packagea.A
var _ = qux.B var _ = packageb.B
} }
`, `,
}, },
@ -590,34 +590,34 @@ func main() {
import ( import (
"fmt" "fmt"
renamed_bar "github.com/foo/bar" renamed_packagea "manypackages.com/packagea"
. "github.com/foo/baz" . "manypackages.com/packageb"
"io" "io"
_ "github.com/foo/qux" _ "manypackages.com/packagec"
"strings" "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 out: `package main
import ( import (
"fmt" "fmt"
renamed_bar "github.com/foo/bar" renamed_packagea "manypackages.com/packagea"
"io" "io"
. "github.com/foo/baz" . "manypackages.com/packageb"
"strings" "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 @@ var _, _, _, _, _ = fmt.Errorf, io.Copy, strings.Contains, renamed_bar.A, B
import ( import (
"fmt" // A "fmt" // A
"go/ast" // B "go/ast" // B
_ "launchpad.net/gocheck" // C _ "manypackages.com/packagec" // C
) )
func main() { _, _ = fmt.Print, ast.Walk } func main() { _, _ = fmt.Print, ast.Walk }
@ -641,7 +641,7 @@ import (
"fmt" // A "fmt" // A
"go/ast" // B "go/ast" // B
_ "launchpad.net/gocheck" // C _ "manypackages.com/packagec" // C
) )
func main() { _, _ = fmt.Print, ast.Walk } func main() { _, _ = fmt.Print, ast.Walk }
@ -761,7 +761,7 @@ func main() { fmt.Println() }
import ( import (
"fmt" "fmt"
"golang.org/x/foo" "manypackages.com/packagea"
) )
func main() {} func main() {}
@ -771,7 +771,7 @@ func main() {}
import ( import (
"fmt" "fmt"
"golang.org/x/foo" "manypackages.com/packagea"
) )
func main() {} func main() {}
@ -818,7 +818,7 @@ func main() {
import ( import (
"time" "time"
"code.google.com/p/snappy-go/snappy" "github.com/golang/snappy"
"rsc.io/p" "rsc.io/p"
) )
@ -837,7 +837,7 @@ func main() {
import ( import (
"time" "time"
"code.google.com/p/snappy-go/snappy" "github.com/golang/snappy"
) )
func main() { func main() {
@ -851,7 +851,7 @@ func main() {
import ( import (
"time" "time"
"code.google.com/p/snappy-go/snappy" "github.com/golang/snappy"
"rsc.io/p" "rsc.io/p"
) )
@ -863,8 +863,9 @@ func main() {
`, `,
}, },
// golang.org/issue/12097
{ {
name: "issue #12097", name: "package_statement_insertion_preserves_comments",
in: `// a in: `// a
// b // b
// c // c
@ -980,7 +981,7 @@ import (
_ "net/http/pprof" // install the pprof http handlers _ "net/http/pprof" // install the pprof http handlers
"strings" "strings"
"github.com/pkg/errors" "manypackages.com/packagea"
) )
func main() { func main() {
@ -989,7 +990,7 @@ func main() {
var ( var (
_ json.Number _ json.Number
_ *http.Request _ *http.Request
_ errors.Frame _ packagea.A
) )
} }
`, `,
@ -1002,7 +1003,7 @@ import (
_ "net/http/pprof" // install the pprof http handlers _ "net/http/pprof" // install the pprof http handlers
"strings" "strings"
"github.com/pkg/errors" "manypackages.com/packagea"
) )
func main() { func main() {
@ -1011,7 +1012,7 @@ func main() {
var ( var (
_ json.Number _ json.Number
_ *http.Request _ *http.Request
_ errors.Frame _ packagea.A
) )
} }
`, `,
@ -1108,7 +1109,7 @@ var _, _ = rand.Read, rand.NewZipf
func TestSimpleCases(t *testing.T) { func TestSimpleCases(t *testing.T) {
defer func(lp string) { LocalPrefix = lp }(LocalPrefix) defer func(lp string) { LocalPrefix = lp }(LocalPrefix)
LocalPrefix = "local,github.com/local" LocalPrefix = "local.com,github.com/local"
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
options := &Options{ options := &Options{
@ -1119,10 +1120,71 @@ func TestSimpleCases(t *testing.T) {
FormatOnly: tt.formatOnly, FormatOnly: tt.formatOnly,
} }
testConfig{ testConfig{
modules: []packagestest.Module{
{
Name: "golang.org/fake",
Files: fm{"x.go": tt.in},
},
// Skeleton non-stdlib packages for use during testing. // Skeleton non-stdlib packages for use during testing.
// Each includes one arbitrary symbol, e.g. the first declaration in the first file. // Each includes one arbitrary symbol, e.g. the first declaration in the first file.
// Try not to add more without a good reason. // 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: "github.com/golang/snappy",
Files: fm{"x.go": "package snappy\nvar ErrCorrupt error\n"},
},
{
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)
})
}
}
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{ modules: []packagestest.Module{
{
Name: "golang.org/fake",
Files: fm{"x.go": input},
},
{ {
Name: "appengine", Name: "appengine",
Files: fm{ Files: fm{
@ -1130,22 +1192,8 @@ func TestSimpleCases(t *testing.T) {
"datastore/x.go": "package datastore\nvar ErrInvalidEntityType error\n", "datastore/x.go": "package datastore\nvar ErrInvalidEntityType error\n",
}, },
}, },
{
Name: "rsc.io",
Files: fm{"p/x.go": "package p\nfunc P(){}\n"},
}, },
{ }.processTest(t, "golang.org/fake", "x.go", nil, nil, want)
Name: "code.google.com/p/snappy-go",
Files: fm{"snappy/x.go": "package snappy\nvar ErrCorrupt error\n"},
},
{
Name: "golang.org/fake",
Files: fm{"x.go": tt.in},
},
},
}.processTest(t, "golang.org/fake", "x.go", nil, options, tt.out)
})
}
} }
func TestReadFromFilesystem(t *testing.T) { func TestReadFromFilesystem(t *testing.T) {
@ -1271,6 +1319,7 @@ var (
` `
testConfig{ testConfig{
gopathOnly: true,
module: packagestest.Module{ module: packagestest.Module{
Name: "golang.org/fake", Name: "golang.org/fake",
Files: fm{ Files: fm{
@ -1327,6 +1376,7 @@ var (
) )
` `
testConfig{ testConfig{
gopathOnly: true,
module: packagestest.Module{ module: packagestest.Module{
Name: "mypkg.com/outpkg", Name: "mypkg.com/outpkg",
Files: fm{ Files: fm{
@ -1425,6 +1475,7 @@ func TestFindStdlib(t *testing.T) {
} }
type testConfig struct { type testConfig struct {
gopathOnly bool
module packagestest.Module module packagestest.Module
modules []packagestest.Module modules []packagestest.Module
} }
@ -1440,7 +1491,10 @@ func (c testConfig) test(t *testing.T, fn func(*goimportTest)) {
c.modules = []packagestest.Module{c.module} c.modules = []packagestest.Module{c.module}
} }
exported = packagestest.Export(t, packagestest.GOPATH, c.modules) 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() defer exported.Cleanup()
env := make(map[string]string) env := make(map[string]string)
@ -1458,7 +1512,8 @@ func (c testConfig) test(t *testing.T, fn func(*goimportTest)) {
oldGOPATH := build.Default.GOPATH oldGOPATH := build.Default.GOPATH
oldGOROOT := build.Default.GOROOT oldGOROOT := build.Default.GOROOT
oldCompiler := build.Default.Compiler oldCompiler := build.Default.Compiler
build.Default.GOPATH = "" build.Default.GOROOT = goroot
build.Default.GOPATH = gopath
build.Default.Compiler = "gc" build.Default.Compiler = "gc"
defer func() { defer func() {
@ -1467,45 +1522,37 @@ func (c testConfig) test(t *testing.T, fn func(*goimportTest)) {
build.Default.Compiler = oldCompiler build.Default.Compiler = oldCompiler
}() }()
build.Default.GOROOT = goroot
build.Default.GOPATH = gopath
it := &goimportTest{ it := &goimportTest{
T: t, T: t,
goroot: build.Default.GOROOT,
gopath: gopath, gopath: gopath,
ctx: &build.Default,
exported: exported, exported: exported,
} }
fn(it) fn(it)
})
}
} }
func (c testConfig) processTest(t *testing.T, module, file string, contents []byte, opts *Options, want string) { func (c testConfig) processTest(t *testing.T, module, file string, contents []byte, opts *Options, want string) {
t.Helper() t.Helper()
c.test(t, func(t *goimportTest) { c.test(t, func(t *goimportTest) {
t.Helper() t.Helper()
f := t.exported.File(module, file) t.process(module, file, contents, opts, want)
if f == "" {
t.Fatalf("%v not found in exported files (typo in filename?)", file)
}
t.process(f, contents, opts, want)
}) })
} }
type goimportTest struct { type goimportTest struct {
*testing.T *testing.T
ctx *build.Context
goroot string
gopath string gopath string
exported *packagestest.Exported 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() t.Helper()
if !filepath.IsAbs(file) { f := t.exported.File(module, file)
file = filepath.Join(t.gopath, "src", 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 { if err != nil {
t.Fatalf("Process() = %v", err) t.Fatalf("Process() = %v", err)
} }
@ -1543,12 +1590,14 @@ const Y = bar.X
// to be added into a later group (num=3). // to be added into a later group (num=3).
func TestLocalPrefix(t *testing.T) { func TestLocalPrefix(t *testing.T) {
tests := []struct { tests := []struct {
name string
modules []packagestest.Module modules []packagestest.Module
localPrefix string localPrefix string
src string src string
want string want string
}{ }{
{ {
name: "one_local",
modules: []packagestest.Module{ modules: []packagestest.Module{
{ {
Name: "foo.com", Name: "foo.com",
@ -1572,6 +1621,7 @@ const _ = runtime.GOOS
`, `,
}, },
{ {
name: "two_local",
modules: []packagestest.Module{ modules: []packagestest.Module{
{ {
Name: "foo.com", Name: "foo.com",
@ -1598,6 +1648,7 @@ const _ = runtime.GOOS
`, `,
}, },
{ {
name: "three_prefixes",
modules: []packagestest.Module{ modules: []packagestest.Module{
{ {
Name: "example.org/pkg", Name: "example.org/pkg",
@ -1633,12 +1684,18 @@ const _ = runtime.GOOS
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
testConfig{ testConfig{
modules: tt.modules, // 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) { }.test(t, func(t *goimportTest) {
defer func(s string) { LocalPrefix = s }(LocalPrefix) defer func(s string) { LocalPrefix = s }(LocalPrefix)
LocalPrefix = tt.localPrefix LocalPrefix = tt.localPrefix
t.process("test/t.go", []byte(tt.src), nil, tt.want) t.process("test.com", "t.go", nil, nil, tt.want)
})
}) })
} }
} }
@ -1674,6 +1731,7 @@ const Y = foo.X
// never make it that far). // never make it that far).
func TestImportPathToNameGoPathParse(t *testing.T) { func TestImportPathToNameGoPathParse(t *testing.T) {
testConfig{ testConfig{
gopathOnly: true,
module: packagestest.Module{ module: packagestest.Module{
Name: "example.net/pkg", Name: "example.net/pkg",
Files: fm{ Files: fm{
@ -1708,6 +1766,7 @@ const _ = pkg.X
` `
testConfig{ testConfig{
gopathOnly: true,
module: packagestest.Module{ module: packagestest.Module{
Name: "foo.com", Name: "foo.com",
Files: fm{ Files: fm{
@ -1734,6 +1793,7 @@ const _ = pkg.X
` `
testConfig{ testConfig{
gopathOnly: true,
module: packagestest.Module{ module: packagestest.Module{
Name: "foo.com", Name: "foo.com",
Files: fm{ Files: fm{
@ -2034,22 +2094,39 @@ var _ = &bytes.Buffer{}
` `
testConfig{ testConfig{
modules: []packagestest.Module{ 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", Name: "mycompany.net/tool",
Files: fm{ 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 "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, "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) }.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) { func TestImportNoGoFiles(t *testing.T) {
const input = `package main const input = `package main
var _ = &bytes.Buffer{}` var _ = &bytes.Buffer{}`
@ -2078,12 +2155,11 @@ func TestProcessLargeToken(t *testing.T) {
input := `package testimports input := `package testimports
import ( import (
"fmt" "bytes"
"mydomain.mystuff/mypkg"
) )
const s = fmt.Sprintf("%s", "` + largeString + `") const s = fmt.Sprintf("%s", "` + largeString + `")
const x = mypkg.Sprintf("%s", "my package") var _ = bytes.Buffer{}
// end // end
` `
@ -2091,13 +2167,13 @@ const x = mypkg.Sprintf("%s", "my package")
want := `package testimports want := `package testimports
import ( import (
"bytes"
"fmt" "fmt"
"mydomain.mystuff/mypkg"
) )
const s = fmt.Sprintf("%s", "` + largeString + `") const s = fmt.Sprintf("%s", "` + largeString + `")
const x = mypkg.Sprintf("%s", "my package")
var _ = bytes.Buffer{}
// end // end
` `