Merge branch 'smh-testhelpers-side-effects' into 'master'

Explicitly load test configuration when needed

See merge request gitlab-org/gitaly!1957

34 files changed, 127 insertions, 29 deletions

diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go
index fd8bbe9ce..30955507f 100644
--- a/cmd/gitaly-hooks/hooks_test.go
+++ b/cmd/gitaly-hooks/hooks_test.go
@@ -19,6 +19,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/cmd/gitaly-ssh/testhelper_test.go b/cmd/gitaly-ssh/testhelper_test.go
index f8dec89e8..afdaeab80 100644
--- a/cmd/gitaly-ssh/testhelper_test.go
+++ b/cmd/gitaly-ssh/testhelper_test.go
@@ -12,6 +12,7 @@ import (
 var gitalySSHPath string
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/cache/cachedb_test.go b/internal/cache/cachedb_test.go
index f556051cf..65ea3ff79 100644
--- a/internal/cache/cachedb_test.go
+++ b/internal/cache/cachedb_test.go
@@ -18,6 +18,11 @@ import (
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestStreamDBNaiveKeyer(t *testing.T) {
 	keyer := cache.LeaseKeyer{}
 
diff --git a/internal/git/catfile/testhelper_test.go b/internal/git/catfile/testhelper_test.go
index 715d770f7..8cbef4ef5 100644
--- a/internal/git/catfile/testhelper_test.go
+++ b/internal/git/catfile/testhelper_test.go
@@ -8,6 +8,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/git/helper_test.go b/internal/git/helper_test.go
index e6ad932bd..ffbf64739 100644
--- a/internal/git/helper_test.go
+++ b/internal/git/helper_test.go
@@ -1,11 +1,18 @@
 package git
 
 import (
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestValidateRevision(t *testing.T) {
 	testCases := []struct {
 		rev string
diff --git a/internal/git/log/commit_test.go b/internal/git/log/commit_test.go
index 0e3a05c69..891443bb1 100644
--- a/internal/git/log/commit_test.go
+++ b/internal/git/log/commit_test.go
@@ -2,6 +2,7 @@ package log
 
 import (
 	"bytes"
+	"os"
 	"testing"
 
 	"github.com/golang/protobuf/ptypes/timestamp"
@@ -12,6 +13,11 @@ import (
 	"google.golang.org/grpc/metadata"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestParseRawCommit(t *testing.T) {
 	info := &catfile.ObjectInfo{
 		Oid:  "a984dfa4dee018c6d5f5f57ffec0d0e22763df16",
diff --git a/internal/git/objectpool/testhelper_test.go b/internal/git/objectpool/testhelper_test.go
index f616fdeae..e60ca14f8 100644
--- a/internal/git/objectpool/testhelper_test.go
+++ b/internal/git/objectpool/testhelper_test.go
@@ -2,12 +2,18 @@ package objectpool
 
 import (
 	"context"
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func NewTestObjectPool(ctx context.Context, t *testing.T, storageName string) (*ObjectPool, func()) {
 	pool, err := NewObjectPool(storageName, testhelper.NewTestObjectPoolName(t))
 	require.NoError(t, err)
diff --git a/internal/git/packfile/packfile_test.go b/internal/git/packfile/packfile_test.go
index 94a053213..d571a668b 100644
--- a/internal/git/packfile/packfile_test.go
+++ b/internal/git/packfile/packfile_test.go
@@ -9,6 +9,11 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestList(t *testing.T) {
 	repoPath0 := "testdata/empty.git"
 	require.NoError(t, os.RemoveAll(repoPath0))
diff --git a/internal/git/remote/remote_test.go b/internal/git/remote/remote_test.go
index c695bd041..30f509bf4 100644
--- a/internal/git/remote/remote_test.go
+++ b/internal/git/remote/remote_test.go
@@ -1,6 +1,7 @@
 package remote
 
 import (
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -8,6 +9,11 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestRemoveRemote(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
diff --git a/internal/git/stats/analyzehttp_test.go b/internal/git/stats/analyzehttp_test.go
index c041db9b1..cb19c735c 100644
--- a/internal/git/stats/analyzehttp_test.go
+++ b/internal/git/stats/analyzehttp_test.go
@@ -3,6 +3,7 @@ package stats
 import (
 	"fmt"
 	"net/http"
+	"os"
 	"path/filepath"
 	"testing"
 	"time"
@@ -11,6 +12,11 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestClone(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
diff --git a/internal/git/stats/git_test.go b/internal/git/stats/git_test.go
index 0909b8400..863644128 100644
--- a/internal/git/stats/git_test.go
+++ b/internal/git/stats/git_test.go
@@ -17,6 +17,11 @@ import (
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestLogObjectInfo(t *testing.T) {
 	repo1, repoPath1, cleanup1 := testhelper.NewTestRepo(t)
 	defer cleanup1()
diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go
index ceb581a1f..f8590ec2c 100644
--- a/internal/git/updateref/updateref_test.go
+++ b/internal/git/updateref/updateref_test.go
@@ -3,6 +3,7 @@ package updateref
 import (
 	"context"
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 
@@ -12,6 +13,11 @@ import (
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func setup(t *testing.T) (context.Context, *gitalypb.Repository, string, func()) {
 	ctx, cancel := testhelper.Context()
 	testRepo, testRepoPath, cleanup := testhelper.NewTestRepo(t)
diff --git a/internal/helper/repo_test.go b/internal/helper/repo_test.go
index c6e9c921b..4683124df 100644
--- a/internal/helper/repo_test.go
+++ b/internal/helper/repo_test.go
@@ -12,6 +12,11 @@ import (
 	"google.golang.org/grpc/codes"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestGetRepoPath(t *testing.T) {
 	defer func(oldStorages []config.Storage) {
 		config.Config.Storages = oldStorages
diff --git a/internal/linguist/linguist_test.go b/internal/linguist/linguist_test.go
index 93c3e8b2b..b825d710c 100644
--- a/internal/linguist/linguist_test.go
+++ b/internal/linguist/linguist_test.go
@@ -1,14 +1,20 @@
 package linguist
 
 import (
+	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
-	_ "gitlab.com/gitlab-org/gitaly/internal/testhelper" // Side effect: set up config.Config
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func TestLoadLanguages(t *testing.T) {
 	colorMap = make(map[string]Language)
 	require.NoError(t, LoadColors(config.Config), "load colors")
diff --git a/internal/praefect/replicator_test.go b/internal/praefect/replicator_test.go
index 3689f868f..07ef6b614 100644
--- a/internal/praefect/replicator_test.go
+++ b/internal/praefect/replicator_test.go
@@ -579,6 +579,7 @@ func newRepositoryClient(t *testing.T, serverSocketPath string) (gitalypb.Reposi
 var RubyServer = &rubyserver.Server{}
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/rubyserver/testhelper_test.go b/internal/rubyserver/testhelper_test.go
index ccb817900..66575e48e 100644
--- a/internal/rubyserver/testhelper_test.go
+++ b/internal/rubyserver/testhelper_test.go
@@ -13,6 +13,7 @@ var (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/blob/testhelper_test.go b/internal/service/blob/testhelper_test.go
index d28ad8eb8..24c4a5b15 100644
--- a/internal/service/blob/testhelper_test.go
+++ b/internal/service/blob/testhelper_test.go
@@ -16,6 +16,7 @@ import (
 var rubyServer = &rubyserver.Server{}
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/cleanup/testhelper_test.go b/internal/service/cleanup/testhelper_test.go
index 112153ab8..7b41fd123 100644
--- a/internal/service/cleanup/testhelper_test.go
+++ b/internal/service/cleanup/testhelper_test.go
@@ -1,6 +1,7 @@
 package cleanup
 
 import (
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -10,6 +11,11 @@ import (
 	"google.golang.org/grpc/reflection"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func runCleanupServiceServer(t *testing.T) (string, func()) {
 	srv := testhelper.NewServer(t, nil, nil)
 
diff --git a/internal/service/commit/testhelper_test.go b/internal/service/commit/testhelper_test.go
index b4621af6a..16d3f1297 100644
--- a/internal/service/commit/testhelper_test.go
+++ b/internal/service/commit/testhelper_test.go
@@ -12,9 +12,8 @@ import (
 	"google.golang.org/grpc/reflection"
 )
 
-var ()
-
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/conflicts/testhelper_test.go b/internal/service/conflicts/testhelper_test.go
index 2b956b0fb..a6d8b808a 100644
--- a/internal/service/conflicts/testhelper_test.go
+++ b/internal/service/conflicts/testhelper_test.go
@@ -17,6 +17,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/diff/testhelper_test.go b/internal/service/diff/testhelper_test.go
index c8bb4219c..d3d32efd2 100644
--- a/internal/service/diff/testhelper_test.go
+++ b/internal/service/diff/testhelper_test.go
@@ -12,6 +12,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/hooks/testhelper_test.go b/internal/service/hooks/testhelper_test.go
index 2d72ba16c..25a59c200 100644
--- a/internal/service/hooks/testhelper_test.go
+++ b/internal/service/hooks/testhelper_test.go
@@ -1,6 +1,7 @@
 package hook
 
 import (
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -12,6 +13,11 @@ import (
 	"google.golang.org/grpc/reflection"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func newHooksClient(t *testing.T, serverSocketPath string) (gitalypb.HookServiceClient, *grpc.ClientConn) {
 	connOpts := []grpc.DialOption{
 		grpc.WithInsecure(),
diff --git a/internal/service/internalgitaly/testhelper_test.go b/internal/service/internalgitaly/testhelper_test.go
index 093de37b9..54ec265e6 100644
--- a/internal/service/internalgitaly/testhelper_test.go
+++ b/internal/service/internalgitaly/testhelper_test.go
@@ -2,6 +2,7 @@ package internalgitaly
 
 import (
 	"net"
+	"os"
 	"testing"
 
 	"gitlab.com/gitlab-org/gitaly/internal/config"
@@ -11,6 +12,11 @@ import (
 	"google.golang.org/grpc/reflection"
 )
 
+func TestMain(m *testing.M) {
+	testhelper.Configure()
+	os.Exit(m.Run())
+}
+
 func runInternalGitalyServer(t *testing.T) (*grpc.Server, string) {
 	serverSocketPath := testhelper.GetTemporaryGitalySocketFileName()
 	grpcServer := testhelper.NewTestGrpcServer(t, nil, nil)
diff --git a/internal/service/namespace/namespace_test.go b/internal/service/namespace/namespace_test.go
index b93c62325..58d68f534 100644
--- a/internal/service/namespace/namespace_test.go
+++ b/internal/service/namespace/namespace_test.go
@@ -16,6 +16,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/objectpool/testhelper_test.go b/internal/service/objectpool/testhelper_test.go
index 448e3d60d..02edfad77 100644
--- a/internal/service/objectpool/testhelper_test.go
+++ b/internal/service/objectpool/testhelper_test.go
@@ -13,6 +13,7 @@ import (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/operations/testhelper_test.go b/internal/service/operations/testhelper_test.go
index 07e23662c..c9a59c580 100644
--- a/internal/service/operations/testhelper_test.go
+++ b/internal/service/operations/testhelper_test.go
@@ -34,6 +34,7 @@ func init() {
 }
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/ref/testhelper_test.go b/internal/service/ref/testhelper_test.go
index 0dd3b9018..cc84e57f8 100644
--- a/internal/service/ref/testhelper_test.go
+++ b/internal/service/ref/testhelper_test.go
@@ -78,6 +78,7 @@ var (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/remote/testhelper_test.go b/internal/service/remote/testhelper_test.go
index 36601f71c..1d07353e7 100644
--- a/internal/service/remote/testhelper_test.go
+++ b/internal/service/remote/testhelper_test.go
@@ -16,6 +16,7 @@ import (
 var RubyServer = &rubyserver.Server{}
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/repository/testhelper_test.go b/internal/service/repository/testhelper_test.go
index cc05745c2..24101410d 100644
--- a/internal/service/repository/testhelper_test.go
+++ b/internal/service/repository/testhelper_test.go
@@ -121,6 +121,7 @@ func assertModTimeAfter(t *testing.T, afterTime time.Time, paths ...string) bool
 }
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/smarthttp/testhelper_test.go b/internal/service/smarthttp/testhelper_test.go
index 587c7bc72..1e867961e 100644
--- a/internal/service/smarthttp/testhelper_test.go
+++ b/internal/service/smarthttp/testhelper_test.go
@@ -17,6 +17,7 @@ const (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/ssh/testhelper_test.go b/internal/service/ssh/testhelper_test.go
index c6af60b20..b9a437039 100644
--- a/internal/service/ssh/testhelper_test.go
+++ b/internal/service/ssh/testhelper_test.go
@@ -27,6 +27,7 @@ var (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/service/wiki/testhelper_test.go b/internal/service/wiki/testhelper_test.go
index efad55063..0024b69dc 100644
--- a/internal/service/wiki/testhelper_test.go
+++ b/internal/service/wiki/testhelper_test.go
@@ -34,6 +34,7 @@ var (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/tempdir/testhelper_test.go b/internal/tempdir/testhelper_test.go
index 74656848a..acb32d1e3 100644
--- a/internal/tempdir/testhelper_test.go
+++ b/internal/tempdir/testhelper_test.go
@@ -8,6 +8,8 @@ import (
 )
 
 func TestMain(m *testing.M) {
+	testhelper.Configure()
+
 	os.Exit(testMain(m))
 }
 
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 8e1825522..1ca55edf6 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -17,6 +17,7 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
+	"sync"
 	"syscall"
 	"time"
 
@@ -47,39 +48,37 @@ const (
 	testGitEnv          = "testdata/git-env"
 )
 
-func init() {
-	if err := configure(); err != nil {
-		log.Fatal(err)
-	}
-}
+var configureOnce sync.Once
 
-func configure() error {
-	config.Config.Storages = []config.Storage{
-		{Name: "default", Path: GitlabTestStoragePath()},
-	}
+// Configure sets up the global test configuration. On failure,
+// terminates the program.
+func Configure() {
+	configureOnce.Do(func() {
+		config.Config.Storages = []config.Storage{
+			{Name: "default", Path: GitlabTestStoragePath()},
+		}
 
-	config.Config.SocketPath = "/bogus"
-	config.Config.GitlabShell.Dir = "/"
+		config.Config.SocketPath = "/bogus"
+		config.Config.GitlabShell.Dir = "/"
 
-	dir, err := ioutil.TempDir("", "internal_socket")
-	if err != nil {
-		return err
-	}
+		dir, err := ioutil.TempDir("", "internal_socket")
+		if err != nil {
+			log.Fatalf("error configuring tests: %v", err)
+		}
 
-	config.Config.InternalSocketDir = dir
+		config.Config.InternalSocketDir = dir
 
-	for _, f := range []func() error{
-		ConfigureRuby,
-		config.Validate,
-	} {
-		if err := f(); err != nil {
-			return err
+		for _, f := range []func() error{
+			ConfigureRuby,
+			config.Validate,
+		} {
+			if err := f(); err != nil {
+				log.Fatalf("error configuring tests: %v", err)
+			}
 		}
-	}
 
-	gitalylog.Configure("", "info")
-
-	return nil
+		gitalylog.Configure("", "info")
+	})
 }
 
 // MustReadFile returns the content of a file or fails at once.