Browse Source

tests: update + enable check for leaked goroutines

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
Jonathan Boulle 10 years ago
parent
commit
e1fe7350a2
1 changed files with 26 additions and 19 deletions
  1. 26 19
      integration/main_test.go

+ 26 - 19
integration/z_last_test.go → integration/main_test.go

@@ -5,7 +5,9 @@
 package integration
 
 import (
+	"fmt"
 	"net/http"
+	"os"
 	"runtime"
 	"sort"
 	"strings"
@@ -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
@@ -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()
 
@@ -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) {
@@ -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++ {