Skip to content

Commit

Permalink
tests: update + enable check for leaked goroutines
Browse files Browse the repository at this point in the history
Go 1.4 landed a new testing.M type [1][1] which allows for start-up and
shutdown hooks when running tests. The standard library now uses this
for checking for leaked goroutines in net/http [2][2].

This patch essentially re-ports the updated code from the net/http test
(we were using an older version of it) - in detail:
- updates the test to use `TestMain` instead of relying on
  `TestGoroutinesRunning` to be implicitly run after all other tests
- adds a few new goroutines to the list of exceptions (the test itself,
  as well as the golang/glog package and pkg/log.MergeLogger, both of
  which spin off goroutines to handle log flushing/merging respectively)
- removes a couple of TODOs in the test for extra goroutines that's run
  after individual tests (one of these re-enables the http package's
  `.readLoop` and the other was an out-of-date TODO)
- re-enables the test

[1]: https://golang.org/pkg/testing/#M
[2]: https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/main_test.go#L18
  • Loading branch information
jonboulle committed Dec 19, 2015
1 parent 2e736b5 commit c32a74f
Showing 1 changed file with 26 additions and 19 deletions.
45 changes: 26 additions & 19 deletions integration/z_last_test.go → integration/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
package integration

import (
"fmt"
"net/http"
"os"
"runtime"
"sort"
"strings"
Expand All @@ -26,6 +28,9 @@ func interestingGoroutines() (gs []string) {
strings.Contains(stack, "created by testing.RunTests") ||
strings.Contains(stack, "testing.Main(") ||
strings.Contains(stack, "runtime.goexit") ||
strings.Contains(stack, "github.com/coreos/etcd/integration.interestingGoroutines") ||
strings.Contains(stack, "github.com/coreos/etcd/pkg/logutil.(*MergeLogger).outputLoop") ||
strings.Contains(stack, "github.com/golang/glog.(*loggingT).flushDaemon") ||
strings.Contains(stack, "created by runtime.gc") ||
strings.Contains(stack, "runtime.MHeap_Scavenger") {
continue
Expand All @@ -37,11 +42,18 @@ func interestingGoroutines() (gs []string) {
}

// Verify the other tests didn't leave any goroutines running.
// This is in a file named z_last_test.go so it sorts at the end.
func TestGoroutinesRunning(t *testing.T) {
t.Skip("TODO: etcdserver.Sender may still dial closed remote endpoint and need some time to timeout.")
func TestMain(m *testing.M) {
v := m.Run()
if v == 0 && goroutineLeaked() {
os.Exit(1)
}
os.Exit(v)
}

func goroutineLeaked() bool {
if testing.Short() {
t.Skip("not counting goroutines for leakage in -short mode")
// not counting goroutines for leakage in -short mode
return false
}
gs := interestingGoroutines()

Expand All @@ -52,13 +64,14 @@ func TestGoroutinesRunning(t *testing.T) {
n++
}

t.Logf("num goroutines = %d", n)
if n > 0 {
t.Error("Too many goroutines.")
for stack, count := range stackCount {
t.Logf("%d instances of:\n%s", count, stack)
}
if n == 0 {
return false
}
fmt.Fprintf(os.Stderr, "Too many goroutines running after integration test(s).\n")
for stack, count := range stackCount {
fmt.Fprintf(os.Stderr, "%d instances of:\n%s\n", count, stack)
}
return true
}

func afterTest(t *testing.T) {
Expand All @@ -68,18 +81,12 @@ func afterTest(t *testing.T) {
}
var bad string
badSubstring := map[string]string{
// TODO: there might exist a bug in http package, which will leave
// readLoop without writeLoop after close all idle connections.
// comment this line until we have time to dig into it.
// ").readLoop(": "a Transport",
").readLoop(": "a Transport",
").writeLoop(": "a Transport",
"created by net/http/httptest.(*Server).Start": "an httptest.Server",
"timeoutHandler": "a TimeoutHandler",
// TODO: dial goroutines leaks even if the request is cancelled.
// It needs to wait dial timeout to recycle the goroutine.
// comment this line until we have time to dig into it.
"net.(*netFD).connect(": "a timing out dial",
").noteClientGone(": "a closenotifier sender",
"net.(*netFD).connect(": "a timing out dial",
").noteClientGone(": "a closenotifier sender",
}
var stacks string
for i := 0; i < 6; i++ {
Expand Down

0 comments on commit c32a74f

Please sign in to comment.