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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Andrew Bonventre <[email protected]>

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,43 +207,61 @@ 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.
 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 <span id="Name">Name</span> = &#34;repo1a&#34;`},
+		},
+		{
+			path:     "/pkg/godoc.test/repo2/b",
+			contains: []string{`const <span id="Name">Name</span> = &#34;repo2b&#34;`},
+		},
 	}
 	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 {