From 771af7e25c19513441fa290c83076e1af8e876c0 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 14 Jan 2015 16:53:34 -0500 Subject: [PATCH] go/buildutil: FakeContext: create build.Context of fake file tree, for testing This refactoring of the tests of go/loader and refactor/rename made it possible to write some loader tests I wanted, but the new tests reveal bugs so they're commented out for now. I will fix them in a follow-up. Change-Id: Iae3a20681a0a0791076debd4b82bb5ed74b0c577 Reviewed-on: https://go-review.googlesource.com/2825 Reviewed-by: Robert Griesemer --- go/buildutil/fakecontext.go | 108 +++++++++++++++++++++++++++++++++ go/loader/loader_test.go | 91 +++++++++++++++++---------- refactor/rename/rename_test.go | 89 ++++----------------------- 3 files changed, 178 insertions(+), 110 deletions(-) create mode 100644 go/buildutil/fakecontext.go diff --git a/go/buildutil/fakecontext.go b/go/buildutil/fakecontext.go new file mode 100644 index 0000000000..79c5a1bfd9 --- /dev/null +++ b/go/buildutil/fakecontext.go @@ -0,0 +1,108 @@ +package buildutil + +import ( + "fmt" + "go/build" + "io" + "io/ioutil" + "os" + "path" + "path/filepath" + "sort" + "strings" + "time" +) + +// FakeContext returns a build.Context for the fake file tree specified +// by pkgs, which maps package import paths to a mapping from file base +// names to contents. +// +// The fake Context has a GOROOT of "/go" and no GOPATH, and overrides +// the necessary file access methods to read from memory instead of the +// real file system. +// +// Unlike a real file tree, the fake one has only two levels---packages +// and files---so ReadDir("/go/src/") returns all packages under +// /go/src/ including, for instance, "math" and "math/big". +// ReadDir("/go/src/math/big") would return all the files in the +// "math/big" package. +// +func FakeContext(pkgs map[string]map[string]string) *build.Context { + clean := func(filename string) string { + f := path.Clean(filepath.ToSlash(filename)) + // Removing "/go/src" while respecting segment + // boundaries has this unfortunate corner case: + if f == "/go/src" { + return "" + } + return strings.TrimPrefix(f, "/go/src/") + } + + ctxt := build.Default // copy + ctxt.GOROOT = "/go" + ctxt.GOPATH = "" + ctxt.IsDir = func(dir string) bool { + dir = clean(dir) + if dir == "" { + return true // needed by (*build.Context).SrcDirs + } + return pkgs[dir] != nil + } + ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) { + dir = clean(dir) + var fis []os.FileInfo + if dir == "" { + // enumerate packages + for importPath := range pkgs { + fis = append(fis, fakeDirInfo(importPath)) + } + } else { + // enumerate files of package + for basename := range pkgs[dir] { + fis = append(fis, fakeFileInfo(basename)) + } + } + sort.Sort(byName(fis)) + return fis, nil + } + ctxt.OpenFile = func(filename string) (io.ReadCloser, error) { + filename = clean(filename) + dir, base := filepath.Split(filename) + content, ok := pkgs[filepath.Clean(dir)][base] + if !ok { + return nil, fmt.Errorf("file not found: %s", filename) + } + return ioutil.NopCloser(strings.NewReader(content)), nil + } + ctxt.IsAbsPath = func(path string) bool { + path = filepath.ToSlash(path) + // Don't rely on the default (filepath.Path) since on + // Windows, it reports virtual paths as non-absolute. + return strings.HasPrefix(path, "/") + } + return &ctxt +} + +type byName []os.FileInfo + +func (s byName) Len() int { return len(s) } +func (s byName) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s byName) Less(i, j int) bool { return s[i].Name() < s[j].Name() } + +type fakeFileInfo string + +func (fi fakeFileInfo) Name() string { return string(fi) } +func (fakeFileInfo) Sys() interface{} { return nil } +func (fakeFileInfo) ModTime() time.Time { return time.Time{} } +func (fakeFileInfo) IsDir() bool { return false } +func (fakeFileInfo) Size() int64 { return 0 } +func (fakeFileInfo) Mode() os.FileMode { return 0644 } + +type fakeDirInfo string + +func (fd fakeDirInfo) Name() string { return string(fd) } +func (fakeDirInfo) Sys() interface{} { return nil } +func (fakeDirInfo) ModTime() time.Time { return time.Time{} } +func (fakeDirInfo) IsDir() bool { return true } +func (fakeDirInfo) Size() int64 { return 0 } +func (fakeDirInfo) Mode() os.FileMode { return 0755 } diff --git a/go/loader/loader_test.go b/go/loader/loader_test.go index eb47e6e6dd..5cc3bdbcbf 100644 --- a/go/loader/loader_test.go +++ b/go/loader/loader_test.go @@ -5,17 +5,14 @@ package loader_test import ( - "bytes" "fmt" "go/build" - "io" - "io/ioutil" - "os" "sort" "strings" + "sync" "testing" - "time" + "golang.org/x/tools/go/buildutil" "golang.org/x/tools/go/loader" ) @@ -116,29 +113,13 @@ func TestLoadFromArgsSource(t *testing.T) { } } -type fakeFileInfo struct{} - -func (fakeFileInfo) Name() string { return "x.go" } -func (fakeFileInfo) Sys() interface{} { return nil } -func (fakeFileInfo) ModTime() time.Time { return time.Time{} } -func (fakeFileInfo) IsDir() bool { return false } -func (fakeFileInfo) Size() int64 { return 0 } -func (fakeFileInfo) Mode() os.FileMode { return 0644 } - -var justXgo = [1]os.FileInfo{fakeFileInfo{}} // ["x.go"] - +// Simplifying wrapper around buildutil.FakeContext for single-file packages. func fakeContext(pkgs map[string]string) *build.Context { - ctxt := build.Default // copy - ctxt.GOROOT = "/go" - ctxt.GOPATH = "" - ctxt.IsDir = func(path string) bool { return true } - ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) { return justXgo[:], nil } - ctxt.OpenFile = func(path string) (io.ReadCloser, error) { - path = path[len("/go/src/"):] - importPath := path[:strings.IndexByte(path, '/')] - return ioutil.NopCloser(bytes.NewBufferString(pkgs[importPath])), nil + pkgs2 := make(map[string]map[string]string) + for path, content := range pkgs { + pkgs2[path] = map[string]string{"x.go": content} } - return &ctxt + return buildutil.FakeContext(pkgs2) } func TestTransitivelyErrorFreeFlag(t *testing.T) { @@ -222,9 +203,12 @@ func TestErrorReporting(t *testing.T) { SourceImports: true, Build: fakeContext(pkgs), } + var mu sync.Mutex var allErrors []error conf.TypeChecker.Error = func(err error) { + mu.Lock() allErrors = append(allErrors, err) + mu.Unlock() } conf.Import("a") @@ -261,10 +245,12 @@ func TestErrorReporting(t *testing.T) { func TestCycles(t *testing.T) { for _, test := range []struct { + descr string ctxt *build.Context wantErr string }{ { + "self-cycle", fakeContext(map[string]string{ "main": `package main; import _ "selfcycle"`, "selfcycle": `package selfcycle; import _ "selfcycle"`, @@ -272,6 +258,7 @@ func TestCycles(t *testing.T) { `import cycle: selfcycle -> selfcycle`, }, { + "three-package cycle", fakeContext(map[string]string{ "main": `package main; import _ "a"`, "a": `package a; import _ "b"`, @@ -280,35 +267,73 @@ func TestCycles(t *testing.T) { }), `import cycle: c -> a -> b -> c`, }, + { + "self-cycle in dependency of test file", + buildutil.FakeContext(map[string]map[string]string{ + "main": { + "main.go": `package main`, + "main_test.go": `package main; import _ "a"`, + }, + "a": { + "a.go": `package a; import _ "a"`, + }, + }), + `import cycle: a -> a`, + }, + // TODO(adonovan): fix: these fail + // { + // "two-package cycle in dependency of test file", + // buildutil.FakeContext(map[string]map[string]string{ + // "main": { + // "main.go": `package main`, + // "main_test.go": `package main; import _ "a"`, + // }, + // "a": { + // "a.go": `package a; import _ "main"`, + // }, + // }), + // `import cycle: main -> a -> main`, + // }, + // { + // "self-cycle in augmented package", + // buildutil.FakeContext(map[string]map[string]string{ + // "main": { + // "main.go": `package main`, + // "main_test.go": `package main; import _ "main"`, + // }, + // }), + // `import cycle: main -> main`, + // }, } { conf := loader.Config{ AllowErrors: true, SourceImports: true, Build: test.ctxt, } + var mu sync.Mutex var allErrors []error conf.TypeChecker.Error = func(err error) { + mu.Lock() allErrors = append(allErrors, err) + mu.Unlock() } - conf.Import("main") + conf.ImportWithTests("main") prog, err := conf.Load() if err != nil { - t.Errorf("Load failed: %s", err) + t.Errorf("%s: Load failed: %s", test.descr, err) } if prog == nil { - t.Fatalf("Load returned nil *Program") + t.Fatalf("%s: Load returned nil *Program", test.descr) } if !hasError(allErrors, test.wantErr) { - t.Errorf("Load() errors = %q, want %q", allErrors, test.wantErr) + t.Errorf("%s: Load() errors = %q, want %q", + test.descr, allErrors, test.wantErr) } } // TODO(adonovan): // - Test that in a legal test cycle, none of the symbols // defined by augmentation are visible via import. - // - Test when augmentation discovers a wholly new cycle among the deps. - // - // These tests require that fakeContext let us control the filenames. } diff --git a/refactor/rename/rename_test.go b/refactor/rename/rename_test.go index 50f2a1a055..1d282a1aec 100644 --- a/refactor/rename/rename_test.go +++ b/refactor/rename/rename_test.go @@ -11,15 +11,12 @@ import ( "go/build" "go/format" "go/token" - "io" - "io/ioutil" - "os" "path/filepath" "regexp" - "strconv" "strings" "testing" - "time" + + "golang.org/x/tools/go/buildutil" ) // TODO(adonovan): test reported source positions, somehow. @@ -1022,84 +1019,22 @@ var _ = I(C(0)).(J) // --------------------------------------------------------------------- -// Plundered/adapted from go/loader/loader_test.go - -// TODO(adonovan): make this into a nice testing utility within go/buildutil. - -// pkgs maps the import path of a fake package to a list of its file contents; -// file names are synthesized, e.g. %d.go. +// Simplifying wrapper around buildutil.FakeContext for packages whose +// filenames are sequentially numbered (%d.go). pkgs maps a package +// import path to its list of file contents. func fakeContext(pkgs map[string][]string) *build.Context { - ctxt := build.Default // copy - ctxt.GOROOT = "/go" - ctxt.GOPATH = "" - ctxt.IsDir = func(path string) bool { - path = filepath.ToSlash(path) - if path == "/go/src" { - return true // needed by (*build.Context).SrcDirs + pkgs2 := make(map[string]map[string]string) + for path, files := range pkgs { + filemap := make(map[string]string) + for i, contents := range files { + filemap[fmt.Sprintf("%d.go", i)] = contents } - if p := strings.TrimPrefix(path, "/go/src/"); p == path { - return false - } else { - path = p - } - _, ok := pkgs[path] - return ok + pkgs2[path] = filemap } - ctxt.ReadDir = func(dir string) ([]os.FileInfo, error) { - dir = filepath.ToSlash(dir) - dir = dir[len("/go/src/"):] - var fis []os.FileInfo - if dir == "" { - // Assumes keys of pkgs are single-segment. - for p := range pkgs { - fis = append(fis, fakeDirInfo(p)) - } - } else { - for i := range pkgs[dir] { - fis = append(fis, fakeFileInfo(i)) - } - } - return fis, nil - } - ctxt.OpenFile = func(path string) (io.ReadCloser, error) { - path = filepath.ToSlash(path) - path = path[len("/go/src/"):] - dir, base := filepath.Split(path) - dir = filepath.Clean(dir) - index, _ := strconv.Atoi(strings.TrimSuffix(base, ".go")) - if _, ok := pkgs[dir]; !ok || index >= len(pkgs[dir]) { - return nil, fmt.Errorf("file does not exist") - } - return ioutil.NopCloser(bytes.NewBufferString(pkgs[dir][index])), nil - } - ctxt.IsAbsPath = func(path string) bool { - path = filepath.ToSlash(path) - // Don't rely on the default (filepath.Path) since on - // Windows, it reports our virtual paths as non-absolute. - return strings.HasPrefix(path, "/") - } - return &ctxt + return buildutil.FakeContext(pkgs2) } // helper for single-file main packages with no imports. func main(content string) *build.Context { return fakeContext(map[string][]string{"main": {content}}) } - -type fakeFileInfo int - -func (fi fakeFileInfo) Name() string { return fmt.Sprintf("%d.go", fi) } -func (fakeFileInfo) Sys() interface{} { return nil } -func (fakeFileInfo) ModTime() time.Time { return time.Time{} } -func (fakeFileInfo) IsDir() bool { return false } -func (fakeFileInfo) Size() int64 { return 0 } -func (fakeFileInfo) Mode() os.FileMode { return 0644 } - -type fakeDirInfo string - -func (fd fakeDirInfo) Name() string { return string(fd) } -func (fakeDirInfo) Sys() interface{} { return nil } -func (fakeDirInfo) ModTime() time.Time { return time.Time{} } -func (fakeDirInfo) IsDir() bool { return true } -func (fakeDirInfo) Size() int64 { return 0 } -func (fakeDirInfo) Mode() os.FileMode { return 0755 }