Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: add goleak to check goroutine leak in tests #5010

Merged
merged 13 commits into from
Dec 18, 2023
9 changes: 7 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ ALL_SRC = $(shell find . -name '*.go' \
-type f | \
sort)

# ALL_PKGS is used with 'nocover'
# ALL_PKGS is used with 'nocover' and 'goleak'
ALL_PKGS = $(shell echo $(dir $(ALL_SRC)) | tr ' ' '\n' | sort -u)

UNAME := $(shell uname -m)
Expand Down Expand Up @@ -162,6 +162,11 @@ nocover:
@echo Verifying that all packages have test files to count in coverage
@scripts/check-test-files.sh $(ALL_PKGS)

.PHONY: goleak
goleak:
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
@echo Verifying that all packages with tests have goleak in their TestMain
@scripts/check-goleak-files.sh $(ALL_PKGS)

.PHONY: fmt
fmt:
./scripts/import-order-cleanup.sh inplace
Expand All @@ -172,7 +177,7 @@ fmt:
./scripts/updateLicenses.sh

.PHONY: lint
lint:
lint: goleak
golangci-lint -v run
./scripts/updateLicenses.sh > $(FMT_LOG)
./scripts/import-order-cleanup.sh stdout > $(IMPORT_LOG)
Expand Down
5 changes: 5 additions & 0 deletions cmd/agent/app/configmanager/grpc/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"

Expand Down Expand Up @@ -81,3 +82,7 @@ func initializeGRPCTestServer(t *testing.T, beforeServe func(server *grpc.Server
}()
return server, lis.Addr()
}

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

"github.com/stretchr/testify/require"
"go.uber.org/goleak"
)

// TestTBufferedReadTransport tests the TBufferedReadTransport
Expand Down Expand Up @@ -69,3 +70,7 @@ func TestTBufferedReadTransportEmptyFunctions(t *testing.T) {
isOpen := trans.IsOpen()
require.True(t, isOpen)
}

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
5 changes: 5 additions & 0 deletions cmd/agent/app/httpserver/srv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"go.uber.org/goleak"
"go.uber.org/zap"
)

func TestHTTPServer(t *testing.T) {
s := NewHTTPServer(":1", nil, nil, zap.NewNop())
assert.NotNil(t, s)
}

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
5 changes: 5 additions & 0 deletions cmd/agent/app/processors/thrift_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/apache/thrift/lib/go/thrift"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"go.uber.org/zap/zaptest"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -243,3 +244,7 @@ func assertCollectorReceivedData(
{Name: "thrift.udp.server.packets.processed", Value: 1},
}...)
}

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
5 changes: 5 additions & 0 deletions cmd/agent/app/servers/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"go.uber.org/goleak"
)

func TestReadBuf_EOF(t *testing.T) {
Expand All @@ -37,3 +38,7 @@ func TestReadBuf_Read(t *testing.T) {
assert.Equal(t, 5, n)
assert.Equal(t, "hello", string(r))
}

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
5 changes: 5 additions & 0 deletions cmd/agent/app/servers/thriftudp/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
)

var localListenAddr = &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)}
Expand Down Expand Up @@ -239,3 +240,7 @@ func TestCreateClient(t *testing.T) {
_, err := createClient(nil, nil)
assert.EqualError(t, err, "dial udp: missing address")
}

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ require (
go.opentelemetry.io/otel/trace v1.21.0
go.uber.org/atomic v1.11.0
go.uber.org/automaxprocs v1.5.3
go.uber.org/goleak v1.3.0
go.uber.org/zap v1.26.0
golang.org/x/net v0.19.0
golang.org/x/sys v0.15.0
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,7 @@ go.uber.org/automaxprocs v1.5.3 h1:kWazyxZUrS3Gs4qUpbwo5kEIMGe/DAvi5Z4tl2NW4j8=
go.uber.org/automaxprocs v1.5.3/go.mod h1:eRbA25aqJrxAbsLO0xy5jVwPt7FQnRgjW+efnwa1WM0=
go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A=
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
Expand Down
5 changes: 5 additions & 0 deletions model/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"go.uber.org/goleak"

"github.com/jaegertracing/jaeger/model"
)
Expand Down Expand Up @@ -66,3 +67,7 @@ func TestTraceNormalizeTimestamps(t *testing.T) {
assert.Equal(t, span.StartTime, tt1.UTC())
assert.Equal(t, span.Logs[0].Timestamp, tt2.UTC())
}

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
31 changes: 31 additions & 0 deletions scripts/check-goleak-files.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/bash

set -euo pipefail

bad_pkgs=0

# shellcheck disable=SC2048
for dir in $*; do
if [[ -f "${dir}/.nocover" ]]; then
continue
fi
testFiles=$(find "${dir}" -maxdepth 1 -name '*_test.go')
good=0
for test in ${testFiles}; do
if grep -q "TestMain" "${test}" | grep -q "goleak.VerifyTestMain" "${test}"; then
good=1
break
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
fi
done
if ((good == 0)); then
echo "🔴 Error(check-goleak): no goleak check in package ${dir}"
((bad_pkgs+=1))
fi
done

if ((bad_pkgs > 0)); then
echo "Error(check-goleak): no goleak check in ${bad_pkgs} package(s)."
echo "See https://github.com/jaegertracing/jaeger/pull/5010/files for example of adding the checks."
echo "In the future this will be a fatal error in the CI."
exit 0 # TODO change to 1 in the future
fi
Loading