From e85c8fbce016ad764c5cdf6b215b351b0fccb3fa Mon Sep 17 00:00:00 2001 From: Aleem Isiaka <30846935+limistah@users.noreply.github.com> Date: Tue, 2 Jan 2024 22:04:29 +0100 Subject: [PATCH] test: Added goleak to TestMain in cmd/collector/app/server package folder (#5055) ## Which problem is this PR solving? - Part of #5006 ## Description of the changes - Added TestMain to package folders - Added make goleak into unit test ci ## How was this change tested? - please run make goleak ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Aleem Isiaka Signed-off-by: Aleem Isiaka <30846935+limistah@users.noreply.github.com> Co-authored-by: Yuri Shkuro --- cmd/collector/app/server/grpc_test.go | 6 ++++-- cmd/collector/app/server/http_test.go | 20 ++++++++++++++---- cmd/collector/app/server/package_test.go | 27 ++++++++++++++++++++++++ pkg/testutils/leakcheck.go | 6 ++++++ pkg/testutils/leakcheck_test.go | 11 ++++++---- 5 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 cmd/collector/app/server/package_test.go diff --git a/cmd/collector/app/server/grpc_test.go b/cmd/collector/app/server/grpc_test.go index 60d50ea51ca..ac58626577f 100644 --- a/cmd/collector/app/server/grpc_test.go +++ b/cmd/collector/app/server/grpc_test.go @@ -56,7 +56,9 @@ func TestFailServe(t *testing.T) { wg.Add(1) logger := zap.New(core) - serveGRPC(grpc.NewServer(), lis, &GRPCServerParams{ + server := grpc.NewServer() + defer server.Stop() + serveGRPC(server, lis, &GRPCServerParams{ Handler: handler.NewGRPCHandler(logger, &mockSpanProcessor{}, &tenancy.Manager{}), SamplingStore: &mockSamplingStore{}, Logger: logger, @@ -107,10 +109,10 @@ func TestCollectorStartWithTLS(t *testing.T) { ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", }, } - server, err := StartGRPCServer(params) require.NoError(t, err) defer server.Stop() + defer params.TLSConfig.Close() } func TestCollectorReflection(t *testing.T) { diff --git a/cmd/collector/app/server/http_test.go b/cmd/collector/app/server/http_test.go index ede7f063489..20a71289fde 100644 --- a/cmd/collector/app/server/http_test.go +++ b/cmd/collector/app/server/http_test.go @@ -64,26 +64,30 @@ func TestCreateTLSHTTPServerError(t *testing.T) { } _, err := StartHTTPServer(params) require.Error(t, err) + defer params.TLSConfig.Close() } func TestSpanCollectorHTTP(t *testing.T) { + mFact := metricstest.NewFactory(time.Hour) + defer mFact.Backend.Stop() logger, _ := zap.NewDevelopment() params := &HTTPServerParams{ Handler: handler.NewJaegerSpanHandler(logger, &mockSpanProcessor{}), SamplingStore: &mockSamplingStore{}, - MetricsFactory: metricstest.NewFactory(time.Hour), + MetricsFactory: mFact, HealthCheck: healthcheck.New(), Logger: logger, } server := httptest.NewServer(nil) - defer server.Close() serveHTTP(server.Config, server.Listener, params) response, err := http.Post(server.URL, "", nil) require.NoError(t, err) assert.NotNil(t, response) + defer response.Body.Close() + defer server.Close() } func TestSpanCollectorHTTPS(t *testing.T) { @@ -191,15 +195,18 @@ func TestSpanCollectorHTTPS(t *testing.T) { // Cannot reliably use zaptest.NewLogger(t) because it causes race condition // See https://github.com/jaegertracing/jaeger/issues/4497. logger := zap.NewNop() + mFact := metricstest.NewFactory(time.Hour) + defer mFact.Backend.Stop() params := &HTTPServerParams{ HostPort: fmt.Sprintf(":%d", ports.CollectorHTTP), Handler: handler.NewJaegerSpanHandler(logger, &mockSpanProcessor{}), SamplingStore: &mockSamplingStore{}, - MetricsFactory: metricstest.NewFactory(time.Hour), + MetricsFactory: mFact, HealthCheck: healthcheck.New(), Logger: logger, TLSConfig: test.TLS, } + defer params.TLSConfig.Close() server, err := StartHTTPServer(params) require.NoError(t, err) @@ -208,6 +215,7 @@ func TestSpanCollectorHTTPS(t *testing.T) { }() clientTLSCfg, err0 := test.clientTLS.Config(logger) + defer test.clientTLS.Close() require.NoError(t, err0) dialer := &net.Dialer{Timeout: 2 * time.Second} conn, clientError := tls.DialWithDialer(dialer, "tcp", "localhost:"+fmt.Sprintf("%d", ports.CollectorHTTP), clientTLSCfg) @@ -240,6 +248,8 @@ func TestSpanCollectorHTTPS(t *testing.T) { } else { require.NoError(t, requestError) require.NotNil(t, response) + // ensures that the body has been initialized attempting to close + defer response.Body.Close() } }) } @@ -247,11 +257,13 @@ func TestSpanCollectorHTTPS(t *testing.T) { func TestStartHTTPServerParams(t *testing.T) { logger := zap.NewNop() + mFact := metricstest.NewFactory(time.Hour) + defer mFact.Stop() params := &HTTPServerParams{ HostPort: fmt.Sprintf(":%d", ports.CollectorHTTP), Handler: handler.NewJaegerSpanHandler(logger, &mockSpanProcessor{}), SamplingStore: &mockSamplingStore{}, - MetricsFactory: metricstest.NewFactory(time.Hour), + MetricsFactory: mFact, HealthCheck: healthcheck.New(), Logger: logger, IdleTimeout: 5 * time.Minute, diff --git a/cmd/collector/app/server/package_test.go b/cmd/collector/app/server/package_test.go new file mode 100644 index 00000000000..caadfc29f87 --- /dev/null +++ b/cmd/collector/app/server/package_test.go @@ -0,0 +1,27 @@ +// Copyright (c) 2023 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "testing" + + "go.uber.org/goleak" + + "github.com/jaegertracing/jaeger/pkg/testutils" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m, testutils.IgnoreOpenCensusWorkerLeak()) +} diff --git a/pkg/testutils/leakcheck.go b/pkg/testutils/leakcheck.go index b5e25503382..188ba75757f 100644 --- a/pkg/testutils/leakcheck.go +++ b/pkg/testutils/leakcheck.go @@ -25,3 +25,9 @@ import ( func IgnoreGlogFlushDaemonLeak() goleak.Option { return goleak.IgnoreTopFunction("github.com/golang/glog.(*fileSink).flushDaemon") } + +// IgnoreOpenCensusWorkerLeak This prevent catching the leak generated by opencensus defaultWorker.Start which at the time is part of the package's init call. +// See https://github.com/jaegertracing/jaeger/pull/5055#discussion_r1438702168 for more context. +func IgnoreOpenCensusWorkerLeak() goleak.Option { + return goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start") +} diff --git a/pkg/testutils/leakcheck_test.go b/pkg/testutils/leakcheck_test.go index 4fa06e9f8cd..6989a58e4a3 100644 --- a/pkg/testutils/leakcheck_test.go +++ b/pkg/testutils/leakcheck_test.go @@ -16,11 +16,14 @@ package testutils import ( "testing" + + "github.com/stretchr/testify/require" ) func TestIgnoreGlogFlushDaemonLeak(t *testing.T) { - opt := IgnoreGlogFlushDaemonLeak() - if opt == nil { - t.Errorf("IgnoreGlogFlushDaemonLeak() returned nil, want non-nil goleak.Option") - } + require.NotNil(t, IgnoreGlogFlushDaemonLeak()) +} + +func TestIgnoreOpenCensusWorkerLeak(t *testing.T) { + require.NotNil(t, IgnoreOpenCensusWorkerLeak()) }