From 830618e44dcf076ae2b3c8136b86414c402d4efe Mon Sep 17 00:00:00 2001 From: Piotr Tabor Date: Fri, 7 Aug 2020 09:49:33 +0200 Subject: [PATCH] ./integration: Fix flakes due to .setupLogging race. The source of problem was the fact that multiple tests were creating their clusters (and some of them were setting global grpclog). If the test was running after some other test that created HttpServer (so accessed grpclog), this was reported as race. Tested with: go test ./clientv3/. -v "--run=(Example).*" --count=2 go test ./clientv3/. -v "--run=(Test).*" --count=2 go test ./integration/embed/. -v "--run=(Test).*" --count=2 --- clientv3/example_test.go | 23 ++++++++--------------- clientv3/main_test.go | 21 +++++++++++++++------ integration/{ => embed}/embed_test.go | 15 +++++++++++++-- pkg/testutil/leak.go | 3 ++- 4 files changed, 38 insertions(+), 24 deletions(-) rename integration/{ => embed}/embed_test.go (92%) diff --git a/clientv3/example_test.go b/clientv3/example_test.go index 0c6cb9a21..d3aacb80f 100644 --- a/clientv3/example_test.go +++ b/clientv3/example_test.go @@ -16,25 +16,12 @@ package clientv3_test import ( "context" - "log" - "os" - "time" - "go.etcd.io/etcd/v3/clientv3" "go.etcd.io/etcd/v3/pkg/transport" - - "google.golang.org/grpc/grpclog" + "log" ) -var ( - dialTimeout = 5 * time.Second - requestTimeout = 10 * time.Second - endpoints = []string{"localhost:2379", "localhost:22379", "localhost:32379"} -) - -func Example() { - clientv3.SetLogger(grpclog.NewLoggerV2(os.Stderr, os.Stderr, os.Stderr)) - +func ExampleConfig_insecure() { cli, err := clientv3.New(clientv3.Config{ Endpoints: endpoints, DialTimeout: dialTimeout, @@ -48,6 +35,10 @@ func Example() { if err != nil { log.Fatal(err) } + + // Without the line below the test is not being executed + + // Output: } func ExampleConfig_withTLS() { @@ -74,4 +65,6 @@ func ExampleConfig_withTLS() { if err != nil { log.Fatal(err) } + // Without the line below the test is not being executed + // Output: } diff --git a/clientv3/main_test.go b/clientv3/main_test.go index 5b0f39232..d91473c8c 100644 --- a/clientv3/main_test.go +++ b/clientv3/main_test.go @@ -16,14 +16,20 @@ package clientv3_test import ( "fmt" + "go.etcd.io/etcd/v3/clientv3" + "go.etcd.io/etcd/v3/integration" + "go.etcd.io/etcd/v3/pkg/testutil" + "google.golang.org/grpc/grpclog" "os" - "regexp" "strings" "testing" "time" +) - "go.etcd.io/etcd/v3/integration" - "go.etcd.io/etcd/v3/pkg/testutil" +var ( + dialTimeout = 5 * time.Second + requestTimeout = 10 * time.Second + endpoints = []string{"localhost:2379", "localhost:22379", "localhost:32379"} ) // TestMain sets up an etcd cluster if running the examples. @@ -32,8 +38,7 @@ func TestMain(m *testing.M) { for _, arg := range os.Args { if strings.HasPrefix(arg, "-test.run=") { exp := strings.Split(arg, "=")[1] - match, err := regexp.MatchString(exp, "Example") - useCluster = (err == nil && match) || strings.Contains(exp, "Example") + useCluster = strings.Contains(exp, "Example") hasRunArg = true break } @@ -46,6 +51,11 @@ func TestMain(m *testing.M) { var v int if useCluster { + // Redirecting outputs to Stderr, such that they not interleave with examples outputs. + // Setting it once and before running any of the test such that it not data-races + // between HTTP servers running in different tests. + clientv3.SetLogger(grpclog.NewLoggerV2(os.Stderr, os.Stderr, os.Stderr)) + cfg := integration.ClusterConfig{Size: 3} clus := integration.NewClusterV3(nil, &cfg) endpoints = make([]string, 3) @@ -61,7 +71,6 @@ func TestMain(m *testing.M) { } else { v = m.Run() } - if v == 0 && testutil.CheckLeakedGoroutine() { os.Exit(1) } diff --git a/integration/embed_test.go b/integration/embed/embed_test.go similarity index 92% rename from integration/embed_test.go rename to integration/embed/embed_test.go index 3ca096113..06723852f 100644 --- a/integration/embed_test.go +++ b/integration/embed/embed_test.go @@ -14,13 +14,15 @@ // +build !cluster_proxy -// TODO: fix race conditions with setupLogging +// Keep the test in a separate package from other tests such that +// .setupLogging method does not race with other (previously running) servers (grpclog is global). -package integration +package embed_test import ( "context" "fmt" + "go.etcd.io/etcd/c/v3/pkg/transport" "net/url" "os" "path/filepath" @@ -32,6 +34,15 @@ import ( "go.etcd.io/etcd/v3/embed" ) +var ( + testTLSInfo = transport.TLSInfo{ + KeyFile: "../fixtures/server.key.insecure", + CertFile: "../fixtures/server.crt", + TrustedCAFile: "../fixtures/ca.crt", + ClientCertAuth: true, + } +) + func TestEmbedEtcd(t *testing.T) { tests := []struct { cfg embed.Config diff --git a/pkg/testutil/leak.go b/pkg/testutil/leak.go index 74a0ebbe5..34a5a7306 100644 --- a/pkg/testutil/leak.go +++ b/pkg/testutil/leak.go @@ -55,7 +55,7 @@ func CheckLeakedGoroutine() bool { stackCount[normalized]++ } - fmt.Fprintf(os.Stderr, "Too many goroutines running after all test(s).\n") + fmt.Fprintf(os.Stderr, "Unexpected goroutines running after all test(s).\n") for stack, count := range stackCount { fmt.Fprintf(os.Stderr, "%d instances of:\n%s\n", count, stack) } @@ -63,6 +63,7 @@ func CheckLeakedGoroutine() bool { } // CheckAfterTest returns an error if AfterTest would fail with an error. +// Waits for go-routines shutdown for 'd'. func CheckAfterTest(d time.Duration) error { http.DefaultTransport.(*http.Transport).CloseIdleConnections() if testing.Short() {