testhelper: Introduce new function to check for leaking Goroutines

In some packages, we have started to assert that we do not leak
Goroutines via the goleak package. One exception we need to always have
present is for the opencensus package given that it starts a Goroutine
in its `init()` function which we have no way to stop.

Provide a central function `MustHaveNoGoroutines()` in the testhelper
package such that we do not have to duplicate this exception whenever we
use the goleak package.

2 files changed, 23 insertions, 8 deletions

diff --git a/internal/praefect/grpc-proxy/proxy/handler_ext_test.go b/internal/praefect/grpc-proxy/proxy/handler_ext_test.go
index d33fa458d..0aaea9cfe 100644
--- a/internal/praefect/grpc-proxy/proxy/handler_ext_test.go
+++ b/internal/praefect/grpc-proxy/proxy/handler_ext_test.go
@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"os"
 	"path/filepath"
 	"testing"
 	"time"
@@ -30,7 +31,6 @@ import (
 	pb "gitlab.com/gitlab-org/gitaly/v14/internal/praefect/grpc-proxy/testdata"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testassert"
-	"go.uber.org/goleak"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	grpc_metadata "google.golang.org/grpc/metadata"
@@ -49,11 +49,18 @@ const (
 )
 
 func TestMain(m *testing.M) {
-	defer testhelper.MustHaveNoChildProcess()
+	os.Exit(testMain(m))
+}
+
+func testMain(m *testing.M) int {
+	defer func() {
+		testhelper.MustHaveNoChildProcess()
+		testhelper.MustHaveNoGoroutines()
+	}()
 	cleanup := testhelper.Configure()
 	defer cleanup()
 
-	goleak.VerifyTestMain(m, goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"))
+	return m.Run()
 }
 
 // asserting service is implemented on the server side and serves as a handler for stuff
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 3d87fd898..30bd253ac 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -1,6 +1,5 @@
 package testhelper
 
-//nolint: gci
 import (
 	"context"
 	"crypto/ecdsa"
@@ -31,11 +30,8 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
+	"go.uber.org/goleak"
 	"google.golang.org/grpc/metadata"
-
-	// The goleak import only exists such that this test-only dependency is properly being
-	// attributed in our NOTICE file.
-	_ "go.uber.org/goleak"
 )
 
 const (
@@ -169,6 +165,18 @@ func GetLocalhostListener(t testing.TB) (net.Listener, string) {
 	return l, addr
 }
 
+// MustHaveNoGoroutines panics if it finds any Goroutines running.
+func MustHaveNoGoroutines() {
+	if err := goleak.Find(
+		// opencensus has a "defaultWorker" which is started by the package's
+		// `init()` function. There is no way to stop this worker, so it will leak
+		// whenever we import the package.
+		goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"),
+	); err != nil {
+		panic(fmt.Errorf("goroutines running: %w", err))
+	}
+}
+
 // MustHaveNoChildProcess panics if it finds a running or finished child
 // process. It waits for 2 seconds for processes to be cleaned up by other
 // goroutines.