From 7da961ddcf1a7ebeb61c0e69e9a165c83cb551e6 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Mon, 23 Sep 2019 15:43:37 -0400 Subject: [PATCH] cmd/godoc: convert tests to packagestest, cover third party packages This change converts cmd/godoc tests to use the packagestest package in its basic integration tests. For now, those tests continue to run in GOPATH mode only. When module support is added to cmd/godoc, then the same tests will be made to run in module mode too. Previously, the basic integration test covered godoc functionality on Go packages in GOROOT only. This change also adds some third party packages to increase test coverage. This is easy to do with the packagestest API. Updates golang/go#33655 Change-Id: If3fce913140b81ed9340556d6bb4b963f5f98813 Reviewed-on: https://go-review.googlesource.com/c/tools/+/196981 Run-TryBot: Dmitri Shuralyov TryBot-Result: Gobot Gobot Reviewed-by: Andrew Bonventre --- cmd/godoc/godoc_test.go | 128 ++++++++++++++++++++++++++-------------- 1 file changed, 84 insertions(+), 44 deletions(-) diff --git a/cmd/godoc/godoc_test.go b/cmd/godoc/godoc_test.go index 8d8216c1b3..dff079d8aa 100644 --- a/cmd/godoc/godoc_test.go +++ b/cmd/godoc/godoc_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "golang.org/x/tools/go/packages/packagestest" "golang.org/x/tools/internal/testenv" ) @@ -181,12 +182,12 @@ func TestURL(t *testing.T) { cmd.Stderr = stderr cmd.Args[0] = "godoc" - // Set GOPATH variable to non-existing path + // Set GOPATH variable to non-existing absolute path // and GOPROXY=off to disable module fetches. // We cannot just unset GOPATH variable because godoc would default it to ~/go. // (We don't want the indexer looking at the local workspace during tests.) cmd.Env = append(os.Environ(), - "GOPATH=does_not_exist", + "GOPATH=/does_not_exist", "GOPROXY=off", "GO111MODULE=off") @@ -206,7 +207,10 @@ func TestURL(t *testing.T) { // Basic integration test for godoc HTTP interface. func TestWeb(t *testing.T) { - testWeb(t, false) + bin, cleanup := buildGodoc(t) + defer cleanup() + testWeb(t, packagestest.GOPATH, bin, false) + // TODO(golang.org/issue/33655): Add support for module mode, then enable its test coverage. } // Basic integration test for godoc HTTP interface. @@ -214,35 +218,50 @@ func TestWebIndex(t *testing.T) { if testing.Short() { t.Skip("skipping test in -short mode") } - testWeb(t, true) + bin, cleanup := buildGodoc(t) + defer cleanup() + testWeb(t, packagestest.GOPATH, bin, true) } // Basic integration test for godoc HTTP interface. -func testWeb(t *testing.T, withIndex bool) { +func testWeb(t *testing.T, x packagestest.Exporter, bin string, withIndex bool) { if runtime.GOOS == "plan9" { t.Skip("skipping on plan9; fails to start up quickly enough") } - bin, cleanup := buildGodoc(t) - defer cleanup() + + // Write a fake GOROOT/GOPATH with some third party packages. + e := packagestest.Export(t, x, []packagestest.Module{ + { + Name: "godoc.test/repo1", + Files: map[string]interface{}{ + "a/a.go": `// Package a is a package in godoc.test/repo1. +package a; import _ "godoc.test/repo2/a"; const Name = "repo1a"`, + "b/b.go": `package b; const Name = "repo1b"`, + }, + }, + { + Name: "godoc.test/repo2", + Files: map[string]interface{}{ + "a/a.go": `package a; const Name = "repo2a"`, + "b/b.go": `package b; const Name = "repo2b"`, + }, + }, + }) + defer e.Cleanup() + + // Start the server. addr := serverAddress(t) args := []string{fmt.Sprintf("-http=%s", addr)} if withIndex { args = append(args, "-index", "-index_interval=-1s") } cmd := exec.Command(bin, args...) + cmd.Dir = e.Config.Dir + cmd.Env = e.Config.Env cmd.Stdout = os.Stderr cmd.Stderr = os.Stderr cmd.Args[0] = "godoc" - // Set GOPATH variable to non-existing path - // and GOPROXY=off to disable module fetches. - // We cannot just unset GOPATH variable because godoc would default it to ~/go. - // (We don't want the indexer looking at the local workspace during tests.) - cmd.Env = append(os.Environ(), - "GOPATH=does_not_exist", - "GOPROXY=off", - "GO111MODULE=off") - if err := cmd.Start(); err != nil { t.Fatalf("failed to start godoc: %s", err) } @@ -284,6 +303,8 @@ func testWeb(t *testing.T, withIndex bool) { contains: []string{ "Standard library", "Package fmt implements formatted I/O", + "Third party", + "Package a is a package in godoc.test/repo1.", }, notContains: []string{ "internal/syscall", @@ -359,6 +380,16 @@ func testWeb(t *testing.T, withIndex bool) { }, releaseTag: "go1.11", }, + + // Third party packages. + { + path: "/pkg/godoc.test/repo1/a", + contains: []string{`const Name = "repo1a"`}, + }, + { + path: "/pkg/godoc.test/repo2/b", + contains: []string{`const Name = "repo2b"`}, + }, } for _, test := range tests { if test.needIndex && !withIndex { @@ -412,49 +443,58 @@ func testWeb(t *testing.T, withIndex bool) { // Basic integration test for godoc -analysis=type (via HTTP interface). func TestTypeAnalysis(t *testing.T) { + bin, cleanup := buildGodoc(t) + defer cleanup() + testTypeAnalysis(t, packagestest.GOPATH, bin) + // TODO(golang.org/issue/34473): Add support for type, pointer + // analysis in module mode, then enable its test coverage here. +} +func testTypeAnalysis(t *testing.T, x packagestest.Exporter, bin string) { if runtime.GOOS == "plan9" { t.Skip("skipping test on plan9 (issue #11974)") // see comment re: Plan 9 below } // Write a fake GOROOT/GOPATH. - tmpdir, err := ioutil.TempDir("", "godoc-analysis") - if err != nil { - t.Fatalf("ioutil.TempDir failed: %s", err) - } - defer os.RemoveAll(tmpdir) - for _, f := range []struct{ file, content string }{ - {"goroot/src/lib/lib.go", ` + // TODO(golang.org/issue/34473): This test uses import paths without a dot in first + // path element. This is not viable in module mode; import paths will need to change. + e := packagestest.Export(t, x, []packagestest.Module{ + { + Name: "app", + Files: map[string]interface{}{ + "main.go": ` +package main +import "lib" +func main() { print(lib.V) } +`, + }, + }, + { + Name: "lib", + Files: map[string]interface{}{ + "lib.go": ` package lib type T struct{} const C = 3 var V T func (T) F() int { return C } -`}, - {"gopath/src/app/main.go", ` -package main -import "lib" -func main() { print(lib.V) } -`}, - } { - file := filepath.Join(tmpdir, f.file) - if err := os.MkdirAll(filepath.Dir(file), 0755); err != nil { - t.Fatalf("MkdirAll(%s) failed: %s", filepath.Dir(file), err) - } - if err := ioutil.WriteFile(file, []byte(f.content), 0644); err != nil { - t.Fatal(err) - } +`, + }, + }, + }) + goroot := filepath.Join(e.Temp(), "goroot") + if err := os.Mkdir(goroot, 0755); err != nil { + t.Fatalf("os.Mkdir(%q) failed: %v", goroot, err) } + defer e.Cleanup() // Start the server. - bin, cleanup := buildGodoc(t) - defer cleanup() addr := serverAddress(t) cmd := exec.Command(bin, fmt.Sprintf("-http=%s", addr), "-analysis=type") - cmd.Env = os.Environ() - cmd.Env = append(cmd.Env, fmt.Sprintf("GOROOT=%s", filepath.Join(tmpdir, "goroot"))) - cmd.Env = append(cmd.Env, fmt.Sprintf("GOPATH=%s", filepath.Join(tmpdir, "gopath"))) - cmd.Env = append(cmd.Env, "GO111MODULE=off") - cmd.Env = append(cmd.Env, "GOPROXY=off") + cmd.Dir = e.Config.Dir + // Point to an empty GOROOT directory to speed things up + // by not doing type analysis for the entire real GOROOT. + // TODO(golang.org/issue/34473): This test optimization may not be viable in module mode. + cmd.Env = append(e.Config.Env, fmt.Sprintf("GOROOT=%s", goroot)) cmd.Stdout = os.Stderr stderr, err := cmd.StderrPipe() if err != nil {