testhelper: Move process leak checks into separate package

The function `MustHaveNoChildProcess()` is not a general test helper
that should be used by tests as they see fit, but is instead intended to
be run at the end of tests only. Move it into a separate package and
make it internal to the package to make it less discoverable by callers.

3 files changed, 64 insertions, 68 deletions

diff --git a/internal/testhelper/configure.go b/internal/testhelper/configure.go
index 521cb428b..071f16f56 100644
--- a/internal/testhelper/configure.go
+++ b/internal/testhelper/configure.go
@@ -46,7 +46,7 @@ func Run(m *testing.M, opts ...RunOption) {
 			opt(&cfg)
 		}
 
-		defer MustHaveNoChildProcess()
+		defer mustHaveNoChildProcess()
 
 		cleanup, err := configure()
 		if err != nil {
diff --git a/internal/testhelper/leakage.go b/internal/testhelper/leakage.go
new file mode 100644
index 000000000..f5f56c5f5
--- /dev/null
+++ b/internal/testhelper/leakage.go
@@ -0,0 +1,63 @@
+package testhelper
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+	"syscall"
+	"time"
+
+	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
+)
+
+// 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.
+func mustHaveNoChildProcess() {
+	waitDone := make(chan struct{})
+	go func() {
+		command.WaitAllDone()
+		close(waitDone)
+	}()
+
+	select {
+	case <-waitDone:
+	case <-time.After(2 * time.Second):
+	}
+
+	mustFindNoFinishedChildProcess()
+	mustFindNoRunningChildProcess()
+}
+
+func mustFindNoFinishedChildProcess() {
+	// Wait4(pid int, wstatus *WaitStatus, options int, rusage *Rusage) (wpid int, err error)
+	//
+	// We use pid -1 to wait for any child. We don't care about wstatus or
+	// rusage. Use WNOHANG to return immediately if there is no child waiting
+	// to be reaped.
+	wpid, err := syscall.Wait4(-1, nil, syscall.WNOHANG, nil)
+	if err == nil && wpid > 0 {
+		panic(fmt.Errorf("wait4 found child process %d", wpid))
+	}
+}
+
+func mustFindNoRunningChildProcess() {
+	pgrep := exec.Command("pgrep", "-P", fmt.Sprintf("%d", os.Getpid()))
+	desc := fmt.Sprintf("%q", strings.Join(pgrep.Args, " "))
+
+	out, err := pgrep.Output()
+	if err == nil {
+		pidsComma := strings.Replace(text.ChompBytes(out), "\n", ",", -1)
+		psOut, _ := exec.Command("ps", "-o", "pid,args", "-p", pidsComma).Output()
+		panic(fmt.Errorf("found running child processes %s:\n%s", pidsComma, psOut))
+	}
+
+	if status, ok := command.ExitStatus(err); ok && status == 1 {
+		// Exit status 1 means no processes were found
+		return
+	}
+
+	panic(fmt.Errorf("%s: %w", desc, err))
+}
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 30bd253ac..7a52ce3ee 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -16,8 +16,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strings"
-	"syscall"
 	"testing"
 	"time"
 
@@ -25,12 +23,9 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"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"
 )
 
@@ -165,68 +160,6 @@ 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.
-func MustHaveNoChildProcess() {
-	waitDone := make(chan struct{})
-	go func() {
-		command.WaitAllDone()
-		close(waitDone)
-	}()
-
-	select {
-	case <-waitDone:
-	case <-time.After(2 * time.Second):
-	}
-
-	mustFindNoFinishedChildProcess()
-	mustFindNoRunningChildProcess()
-}
-
-func mustFindNoFinishedChildProcess() {
-	// Wait4(pid int, wstatus *WaitStatus, options int, rusage *Rusage) (wpid int, err error)
-	//
-	// We use pid -1 to wait for any child. We don't care about wstatus or
-	// rusage. Use WNOHANG to return immediately if there is no child waiting
-	// to be reaped.
-	wpid, err := syscall.Wait4(-1, nil, syscall.WNOHANG, nil)
-	if err == nil && wpid > 0 {
-		panic(fmt.Errorf("wait4 found child process %d", wpid))
-	}
-}
-
-func mustFindNoRunningChildProcess() {
-	pgrep := exec.Command("pgrep", "-P", fmt.Sprintf("%d", os.Getpid()))
-	desc := fmt.Sprintf("%q", strings.Join(pgrep.Args, " "))
-
-	out, err := pgrep.Output()
-	if err == nil {
-		pidsComma := strings.Replace(text.ChompBytes(out), "\n", ",", -1)
-		psOut, _ := exec.Command("ps", "-o", "pid,args", "-p", pidsComma).Output()
-		panic(fmt.Errorf("found running child processes %s:\n%s", pidsComma, psOut))
-	}
-
-	if status, ok := command.ExitStatus(err); ok && status == 1 {
-		// Exit status 1 means no processes were found
-		return
-	}
-
-	panic(fmt.Errorf("%s: %w", desc, err))
-}
-
 // ContextOpt returns a new context instance with the new additions to it.
 type ContextOpt func(context.Context) (context.Context, func())