From 93e9c010f3aaf9e683574991080e85a428d554ee Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Fri, 24 Jul 2020 11:10:20 +0200 Subject: [PATCH] Add more Go leak tests (#7652) * Implement go leak test for promql Signed-off-by: Julien Pivotto * Implement go leak test for Consul SD Signed-off-by: Julien Pivotto * Implement go leak test in discovery manager Signed-off-by: Julien Pivotto --- discovery/consul/consul_test.go | 17 +++++++++++++++-- discovery/manager_test.go | 11 ++++++++--- promql/engine_test.go | 5 +++++ util/testutil/testing.go | 13 +++++++++++++ 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/discovery/consul/consul_test.go b/discovery/consul/consul_test.go index 5be2bd73b0..4b8882ca1d 100644 --- a/discovery/consul/consul_test.go +++ b/discovery/consul/consul_test.go @@ -26,8 +26,13 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/prometheus/discovery/targetgroup" "github.com/prometheus/prometheus/util/testutil" + "go.uber.org/goleak" ) +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} + func TestConfiguredService(t *testing.T) { conf := &SDConfig{ Services: []string{"configuredServiceName"}} @@ -283,10 +288,14 @@ func TestAllServices(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) ch := make(chan []*targetgroup.Group) - go d.Run(ctx, ch) + go func() { + d.Run(ctx, ch) + close(ch) + }() checkOneTarget(t, <-ch) checkOneTarget(t, <-ch) cancel() + <-ch } // Watch only the test service. @@ -319,9 +328,13 @@ func TestAllOptions(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) ch := make(chan []*targetgroup.Group) - go d.Run(ctx, ch) + go func() { + d.Run(ctx, ch) + close(ch) + }() checkOneTarget(t, <-ch) cancel() + <-ch } func TestGetDatacenterShouldReturnError(t *testing.T) { diff --git a/discovery/manager_test.go b/discovery/manager_test.go index 4cdcc6370a..045a071656 100644 --- a/discovery/manager_test.go +++ b/discovery/manager_test.go @@ -25,7 +25,7 @@ import ( "time" "github.com/go-kit/kit/log" - "github.com/prometheus/client_golang/prometheus/testutil" + client_testutil "github.com/prometheus/client_golang/prometheus/testutil" common_config "github.com/prometheus/common/config" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/config" @@ -33,9 +33,14 @@ import ( "github.com/prometheus/prometheus/discovery/consul" "github.com/prometheus/prometheus/discovery/file" "github.com/prometheus/prometheus/discovery/targetgroup" + "github.com/prometheus/prometheus/util/testutil" "gopkg.in/yaml.v2" ) +func TestMain(m *testing.M) { + testutil.TolerantVerifyLeak(m) +} + // TestTargetUpdatesOrder checks that the target updates are received in the expected order. func TestTargetUpdatesOrder(t *testing.T) { @@ -984,7 +989,7 @@ func TestGaugeFailedConfigs(t *testing.T) { discoveryManager.ApplyConfig(c) <-discoveryManager.SyncCh() - failedCount := testutil.ToFloat64(failedConfigs) + failedCount := client_testutil.ToFloat64(failedConfigs) if failedCount != 3 { t.Fatalf("Expected to have 3 failed configs, got: %v", failedCount) } @@ -1004,7 +1009,7 @@ func TestGaugeFailedConfigs(t *testing.T) { discoveryManager.ApplyConfig(c) <-discoveryManager.SyncCh() - failedCount = testutil.ToFloat64(failedConfigs) + failedCount = client_testutil.ToFloat64(failedConfigs) if failedCount != 0 { t.Fatalf("Expected to get no failed config, got: %v", failedCount) } diff --git a/promql/engine_test.go b/promql/engine_test.go index c45f20af46..a437159050 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -28,8 +28,13 @@ import ( "github.com/prometheus/prometheus/promql/parser" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/util/testutil" + "go.uber.org/goleak" ) +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} + func TestQueryConcurrency(t *testing.T) { maxConcurrency := 10 diff --git a/util/testutil/testing.go b/util/testutil/testing.go index f2335034d4..1645f80d5c 100644 --- a/util/testutil/testing.go +++ b/util/testutil/testing.go @@ -25,9 +25,11 @@ package testutil import ( "fmt" "reflect" + "testing" "github.com/davecgh/go-spew/spew" "github.com/pmezard/go-difflib/difflib" + "go.uber.org/goleak" ) // This package is imported by non-test code and therefore cannot import the @@ -154,3 +156,14 @@ func formatMessage(msgAndArgs []interface{}) string { } return "" } + +// TolerantVerifyLeak verifies go leaks but excludes the go routines that are +// launched as side effects of some of our dependencies. +func TolerantVerifyLeak(m *testing.M) { + goleak.VerifyTestMain(m, + // https://github.com/census-instrumentation/opencensus-go/blob/d7677d6af5953e0506ac4c08f349c62b917a443a/stats/view/worker.go#L34 + goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"), + // https://github.com/kubernetes/klog/blob/c85d02d1c76a9ebafa81eb6d35c980734f2c4727/klog.go#L417 + goleak.IgnoreTopFunction("k8s.io/klog/v2.(*loggingT).flushDaemon"), + ) +}