command: Solve cyclic dependency with testhelper package

The "testhelper" package currently depends on the "command" package,
which is why we cannot use the "testhelper"'s setup functions in the
latter package. The only remaining dependency is the wait group which is
used to count in-flight commands though such that we can assert that
there are no leaking processes after tests have concluded.

Move this wait group into a separate "commandcounter" package to remove
this dependency. While this also allows other packages to modify the
counter due to its interface now being public, this really shouldn't be
much of a problem at all.

internal/command/command.go

@@ -15,6 +15,7 @@ import (
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"github.com/opentracing/opentracing-go"
 	"github.com/sirupsen/logrus"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/command/commandcounter"
 	"gitlab.com/gitlab-org/labkit/tracing"
 )
 
@@ -118,15 +119,6 @@ func (c *Command) Wait() error {
 	return c.waitError
 }
 
-var wg = &sync.WaitGroup{}
-
-// WaitAllDone waits for all commands started by the command package to
-// finish. This can only be called once in the lifecycle of the current
-// Go process.
-func WaitAllDone() {
-	wg.Wait()
-}
-
 type contextWithoutDonePanic string
 
 // New creates a Command from an exec.Cmd. On success, the Command
@@ -225,7 +217,7 @@ func New(ctx context.Context, cmd *exec.Cmd, stdin io.Reader, stdout, stderr io.
 
 	// The goroutine below is responsible for terminating and reaping the
 	// process when ctx is canceled.
-	wg.Add(1)
+	commandcounter.Increment()
 	go func() {
 		<-ctx.Done()
 
@@ -277,14 +269,14 @@ func (c *Command) wait() {
 
 	c.logProcessComplete()
 
-	// This is a bit out-of-place here given that the `wg.Add()` call is in `New()`.
-	// But in `New()`, we have to resort waiting on the context being finished until we
+	// This is a bit out-of-place here given that the `commandcounter.Increment()` call is in
+	// `New()`. But in `New()`, we have to resort waiting on the context being finished until we
 	// would be able to decrement the number of in-flight commands. Given that in some
 	// cases we detach processes from their initial context such that they run in the
-	// background, this would cause us to take longer than necessary to decrease the
-	// wait group counter again. So we instead do it here to accelerate the process,
-	// even though it's less idiomatic.
-	wg.Done()
+	// background, this would cause us to take longer than necessary to decrease the wait group
+	// counter again. So we instead do it here to accelerate the process, even though it's less
+	// idiomatic.
+	commandcounter.Decrement()
 }
 
 // ExitStatus will return the exit-code from an error returned by Wait().

internal/command/commandcounter/counter.go

+package commandcounter
+
+import "sync"
+
+// inFlightCommands tracks the number of in-flight commands. This is hosted outside of the command
+// package to avoid a cyclic dependency with the testhelper package.
+var inFlightCommands = &sync.WaitGroup{}
+
+// Increment will add another process to the command counter. This may only be called by the
+// command package.
+func Increment() {
+	inFlightCommands.Add(1)
+}
+
+// Decrement removes one process from the command counter. This may only be called by the command
+// package.
+func Decrement() {
+	inFlightCommands.Done()
+}
+
+// WaitAllDone waits for all commands started by the command package to finish. This can only be
+// called once in the lifecycle of the current Go process.
+func WaitAllDone() {
+	inFlightCommands.Wait()
+}

internal/testhelper/leakage.go

@@ -8,7 +8,7 @@ import (
 	"syscall"
 	"time"
 
-	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/command/commandcounter"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 )
 
@@ -18,7 +18,7 @@ import (
 func mustHaveNoChildProcess() {
 	waitDone := make(chan struct{})
 	go func() {
-		command.WaitAllDone()
+		commandcounter.WaitAllDone()
 		close(waitDone)
 	}()