Skip to content

Commit

Permalink
cmd/godoc: streamline test subprocesses
Browse files Browse the repository at this point in the history 
- Use testenv.Command to obtain Cmd instances that terminate (with
  useful goroutine dumps!) before the test's timeout, and remove
  arbitrary hard-coded timeouts.

- Execute the test binary itself as cmd/godoc instead of invoking (and
  cleaning up after) 'go build'.

- Use context cancellation to reduce the number of ad-hoc goroutines
  and channels needed by the tests and to provide stronger invariants
  on process cleanup.

For golang/go#50014

Change-Id: I19ae4d10da691db233c79734799ae074ffdf6a03
Reviewed-on: https://go-review.googlesource.com/c/tools/+/377836
Run-TryBot: Bryan Mills <[email protected]>
Reviewed-by: Joedian Reid <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
gopls-CI: kokoro <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
  • Loading branch information
@gopherbot
Bryan C. Mills authored and gopherbot committed Nov 21, 2022
1 parent 932ec22 commit b978661
Showing 1 changed file with 87 additions and 102 deletions.
189 changes: 87 additions & 102 deletions cmd/godoc/godoc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,62 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package main_test
package main

import (
"bytes"
"context"
"fmt"
"go/build"
"io/ioutil"
"net"
"net/http"
"os"
"os/exec"
"path/filepath"
"regexp"
"runtime"
"strings"
"sync"
"testing"
"time"

"golang.org/x/tools/go/packages/packagestest"
"golang.org/x/tools/internal/testenv"
)

// buildGodoc builds the godoc executable.
// It returns its path, and a cleanup function.
//
// TODO(adonovan): opt: do this at most once, and do the cleanup
// exactly once. How though? There's no atexit.
func buildGodoc(t *testing.T) (bin string, cleanup func()) {
t.Helper()

if runtime.GOARCH == "arm" {
t.Skip("skipping test on arm platforms; too slow")
}
if runtime.GOOS == "android" {
t.Skipf("the dependencies are not available on android")
func TestMain(m *testing.M) {
if os.Getenv("GODOC_TEST_IS_GODOC") != "" {
main()
os.Exit(0)
}
testenv.NeedsTool(t, "go")

tmp, err := ioutil.TempDir("", "godoc-regtest-")
if err != nil {
t.Fatal(err)
}
defer func() {
if cleanup == nil { // probably, go build failed.
os.RemoveAll(tmp)
}
}()
// Inform subprocesses that they should run the cmd/godoc main instead of
// running tests. It's a close approximation to building and running the real
// command, and much less complicated and expensive to build and clean up.
os.Setenv("GODOC_TEST_IS_GODOC", "1")

bin = filepath.Join(tmp, "godoc")
if runtime.GOOS == "windows" {
bin += ".exe"
}
cmd := exec.Command("go", "build", "-o", bin)
if err := cmd.Run(); err != nil {
t.Fatalf("Building godoc: %v", err)
os.Exit(m.Run())
}

var exe struct {
path string
err error
once sync.Once
}

func godocPath(t *testing.T) string {
switch runtime.GOOS {
case "js", "ios":
t.Skipf("skipping test that requires exec")
}

return bin, func() { os.RemoveAll(tmp) }
exe.once.Do(func() {
exe.path, exe.err = os.Executable()
})
if exe.err != nil {
t.Fatal(exe.err)
}
return exe.path
}

func serverAddress(t *testing.T) string {
Expand All @@ -74,60 +72,42 @@ func serverAddress(t *testing.T) string {
return ln.Addr().String()
}

func waitForServerReady(t *testing.T, cmd *exec.Cmd, addr string) {
ch := make(chan error, 1)
go func() { ch <- fmt.Errorf("server exited early: %v", cmd.Wait()) }()
go waitForServer(t, ch,
func waitForServerReady(t *testing.T, ctx context.Context, cmd *exec.Cmd, addr string) {
waitForServer(t, ctx,
fmt.Sprintf("http://%v/", addr),
"Go Documentation Server",
15*time.Second,
false)
if err := <-ch; err != nil {
t.Skipf("skipping due to https://go.dev/issue/50014: %v", err)
}
}

func waitForSearchReady(t *testing.T, cmd *exec.Cmd, addr string) {
ch := make(chan error, 1)
go func() { ch <- fmt.Errorf("server exited early: %v", cmd.Wait()) }()
go waitForServer(t, ch,
func waitForSearchReady(t *testing.T, ctx context.Context, cmd *exec.Cmd, addr string) {
waitForServer(t, ctx,
fmt.Sprintf("http://%v/search?q=FALLTHROUGH", addr),
"The list of tokens.",
2*time.Minute,
false)
if err := <-ch; err != nil {
t.Skipf("skipping due to https://go.dev/issue/50014: %v", err)
}
}

func waitUntilScanComplete(t *testing.T, addr string) {
ch := make(chan error)
go waitForServer(t, ch,
func waitUntilScanComplete(t *testing.T, ctx context.Context, addr string) {
waitForServer(t, ctx,
fmt.Sprintf("http://%v/pkg", addr),
"Scan is not yet complete",
2*time.Minute,
// setting reverse as true, which means this waits
// until the string is not returned in the response anymore
true,
)
if err := <-ch; err != nil {
t.Skipf("skipping due to https://go.dev/issue/50014: %v", err)
}
true)
}

const pollInterval = 200 * time.Millisecond
const pollInterval = 50 * time.Millisecond

// waitForServer waits for server to meet the required condition.
// It sends a single error value to ch, unless the test has failed.
// The error value is nil if the required condition was met within
// timeout, or non-nil otherwise.
func waitForServer(t *testing.T, ch chan<- error, url, match string, timeout time.Duration, reverse bool) {
deadline := time.Now().Add(timeout)
for time.Now().Before(deadline) {
time.Sleep(pollInterval)
if t.Failed() {
return
// waitForServer waits for server to meet the required condition,
// failing the test if ctx is canceled before that occurs.
func waitForServer(t *testing.T, ctx context.Context, url, match string, reverse bool) {
start := time.Now()
for {
if ctx.Err() != nil {
t.Helper()
t.Fatalf("server failed to respond in %v", time.Since(start))
}

time.Sleep(pollInterval)
res, err := http.Get(url)
if err != nil {
continue
Expand All @@ -140,11 +120,9 @@ func waitForServer(t *testing.T, ch chan<- error, url, match string, timeout tim
switch {
case !reverse && bytes.Contains(body, []byte(match)),
reverse && !bytes.Contains(body, []byte(match)):
ch <- nil
return
}
}
ch <- fmt.Errorf("server failed to respond in %v", timeout)
}

// hasTag checks whether a given release tag is contained in the current version
Expand All @@ -158,24 +136,18 @@ func hasTag(t string) bool {
return false
}

func killAndWait(cmd *exec.Cmd) {
cmd.Process.Kill()
cmd.Process.Wait()
}

func TestURL(t *testing.T) {
if runtime.GOOS == "plan9" {
t.Skip("skipping on plan9; fails to start up quickly enough")
}
bin, cleanup := buildGodoc(t)
defer cleanup()
bin := godocPath(t)

testcase := func(url string, contents string) func(t *testing.T) {
return func(t *testing.T) {
stdout, stderr := new(bytes.Buffer), new(bytes.Buffer)

args := []string{fmt.Sprintf("-url=%s", url)}
cmd := exec.Command(bin, args...)
cmd := testenv.Command(t, bin, args...)
cmd.Stdout = stdout
cmd.Stderr = stderr
cmd.Args[0] = "godoc"
Expand Down Expand Up @@ -205,8 +177,8 @@ func TestURL(t *testing.T) {

// Basic integration test for godoc HTTP interface.
func TestWeb(t *testing.T) {
bin, cleanup := buildGodoc(t)
defer cleanup()
bin := godocPath(t)

for _, x := range packagestest.All {
t.Run(x.Name(), func(t *testing.T) {
testWeb(t, x, bin, false)
Expand All @@ -217,17 +189,19 @@ func TestWeb(t *testing.T) {
// Basic integration test for godoc HTTP interface.
func TestWebIndex(t *testing.T) {
if testing.Short() {
t.Skip("skipping test in -short mode")
t.Skip("skipping slow test in -short mode")
}
bin, cleanup := buildGodoc(t)
defer cleanup()
bin := godocPath(t)
testWeb(t, packagestest.GOPATH, bin, true)
}

// Basic integration test for godoc HTTP interface.
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")
switch runtime.GOOS {
case "plan9":
t.Skip("skipping on plan9: fails to start up quickly enough")
case "android", "ios":
t.Skip("skipping on mobile: lacks GOROOT/api in test environment")
}

// Write a fake GOROOT/GOPATH with some third party packages.
Expand Down Expand Up @@ -256,23 +230,39 @@ package a; import _ "godoc.test/repo2/a"; const Name = "repo1a"`,
if withIndex {
args = append(args, "-index", "-index_interval=-1s")
}
cmd := exec.Command(bin, args...)
cmd := testenv.Command(t, bin, args...)
cmd.Dir = e.Config.Dir
cmd.Env = e.Config.Env
cmd.Stdout = os.Stderr
cmd.Stderr = os.Stderr
cmdOut := new(strings.Builder)
cmd.Stdout = cmdOut
cmd.Stderr = cmdOut
cmd.Args[0] = "godoc"

if err := cmd.Start(); err != nil {
t.Fatalf("failed to start godoc: %s", err)
}
defer killAndWait(cmd)
ctx, cancel := context.WithCancel(context.Background())
go func() {
err := cmd.Wait()
t.Logf("%v: %v", cmd, err)
cancel()
}()
defer func() {
// Shut down the server cleanly if possible.
if runtime.GOOS == "windows" {
cmd.Process.Kill() // Windows doesn't support os.Interrupt.
} else {
cmd.Process.Signal(os.Interrupt)
}
<-ctx.Done()
t.Logf("server output:\n%s", cmdOut)
}()

if withIndex {
waitForSearchReady(t, cmd, addr)
waitForSearchReady(t, ctx, cmd, addr)
} else {
waitForServerReady(t, cmd, addr)
waitUntilScanComplete(t, addr)
waitForServerReady(t, ctx, cmd, addr)
waitUntilScanComplete(t, ctx, addr)
}

tests := []struct {
Expand Down Expand Up @@ -454,22 +444,17 @@ func TestNoMainModule(t *testing.T) {
if runtime.GOOS == "plan9" {
t.Skip("skipping on plan9; for consistency with other tests that build godoc binary")
}
bin, cleanup := buildGodoc(t)
defer cleanup()
tempDir, err := ioutil.TempDir("", "godoc-test-")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(tempDir)
bin := godocPath(t)
tempDir := t.TempDir()

// Run godoc in an empty directory with module mode explicitly on,
// so that 'go env GOMOD' reports os.DevNull.
cmd := exec.Command(bin, "-url=/")
cmd := testenv.Command(t, bin, "-url=/")
cmd.Dir = tempDir
cmd.Env = append(os.Environ(), "GO111MODULE=on")
var stderr bytes.Buffer
cmd.Stderr = &stderr
err = cmd.Run()
err := cmd.Run()
if err != nil {
t.Fatalf("godoc command failed: %v\nstderr=%q", err, stderr.String())
}
Expand Down

0 comments on commit b978661

Please sign in to comment.