Disable storage breaking tests with WAL enabled

Some tests are deleting a storage directory after initializing. This
causes issues with WAL enabled as the database's state is deleted
suddenly. Disable these tests with WAL. This is more of a detail to
be tested in the storage implementation than the RPC anyway.

4 files changed, 73 insertions, 21 deletions

diff --git a/internal/gitaly/config/locator_test.go b/internal/gitaly/config/locator_test.go
index dc78fe493..fb6761eb9 100644
--- a/internal/gitaly/config/locator_test.go
+++ b/internal/gitaly/config/locator_test.go
@@ -34,19 +34,22 @@ func TestConfigLocator_GetRepoPath(t *testing.T) {
 		repo.RelativePath = strings.TrimPrefix(repoPath, cfg.Storages[0].Path)
 	}
 
-	// The storage name still present in the storages list, but not on the disk.
-	require.NoError(t, os.RemoveAll(cfg.Storages[1].Path))
+	if !testhelper.IsWALEnabled() {
+		// The storage name still present in the storages list, but not on the disk.
+		require.NoError(t, os.RemoveAll(cfg.Storages[1].Path))
+	}
 
 	// The repository path exists on the disk, but it is not a git repository.
 	const notRepositoryFolder = "not-a-git-repo"
 	require.NoError(t, os.MkdirAll(filepath.Join(cfg.Storages[0].Path, notRepositoryFolder), perm.SharedDir))
 
 	for _, tc := range []struct {
-		desc    string
-		repo    *gitalypb.Repository
-		opts    []storage.GetRepoPathOption
-		expPath string
-		expErr  error
+		desc        string
+		repo        *gitalypb.Repository
+		opts        []storage.GetRepoPathOption
+		expPath     string
+		expErr      error
+		skipWithWAL string
 	}{
 		{
 			desc:   "storage is empty",
@@ -62,6 +65,11 @@ func TestConfigLocator_GetRepoPath(t *testing.T) {
 			desc:   "storage doesn't exist on disk",
 			repo:   &gitalypb.Repository{StorageName: cfg.Storages[1].Name, RelativePath: repo.RelativePath},
 			expErr: structerr.NewNotFound("storage does not exist").WithMetadata("storage_path", cfg.Storages[1].Path),
+			skipWithWAL: `
+The test is testing a broken storage by deleting the storage after initializing it.
+This causes problems with WAL as the disk state expected to be present by the database
+and the transaction manager suddenly don't exist. Skip the test here with WAL and rely
+on the storage implementation to handle broken storage on initialization.`,
 		},
 		{
 			desc:   "relative path is empty",
@@ -103,6 +111,10 @@ func TestConfigLocator_GetRepoPath(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
+			if tc.skipWithWAL != "" {
+				testhelper.SkipWithWAL(t, tc.skipWithWAL)
+			}
+
 			path, err := locator.GetRepoPath(tc.repo, tc.opts...)
 			require.Equal(t, tc.expPath, path)
 			require.Equal(t, tc.expErr, err)
diff --git a/internal/gitaly/service/repository/repository_exists_test.go b/internal/gitaly/service/repository/repository_exists_test.go
index 41470617f..61336701f 100644
--- a/internal/gitaly/service/repository/repository_exists_test.go
+++ b/internal/gitaly/service/repository/repository_exists_test.go
@@ -23,7 +23,9 @@ func TestRepositoryExists(t *testing.T) {
 	client, socketPath := runRepositoryService(t, cfg)
 	cfg.SocketPath = socketPath
 
-	require.NoError(t, os.RemoveAll(cfg.Storages[2].Path), "third storage needs to be invalid")
+	if !testhelper.IsWALEnabled() {
+		require.NoError(t, os.RemoveAll(cfg.Storages[2].Path), "third storage needs to be invalid")
+	}
 
 	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{})
 
@@ -32,6 +34,7 @@ func TestRepositoryExists(t *testing.T) {
 		request     *gitalypb.RepositoryExistsRequest
 		expectedErr error
 		exists      bool
+		skipWithWAL string
 	}{
 		{
 			desc: "repository nil",
@@ -117,11 +120,20 @@ func TestRepositoryExists(t *testing.T) {
 					"storage_path", cfg.Storages[2].Path,
 				)
 			}(),
+			skipWithWAL: `
+The test is testing a broken storage by deleting the storage after initializing it.
+This causes problems with WAL as the disk state expected to be present by the database
+and the transaction manager suddenly don't exist. Skip the test here with WAL and rely
+on the storage implementation to handle broken storage on initialization.`,
 		},
 	}
 
 	for _, tc := range queries {
 		t.Run(tc.desc, func(t *testing.T) {
+			if tc.skipWithWAL != "" {
+				testhelper.SkipWithWAL(t, tc.skipWithWAL)
+			}
+
 			response, err := client.RepositoryExists(ctx, tc.request)
 			testhelper.RequireGrpcError(t, tc.expectedErr, err)
 			if err != nil {
diff --git a/internal/gitaly/service/server/disk_stats_test.go b/internal/gitaly/service/server/disk_stats_test.go
index a6f5bf083..9814cf412 100644
--- a/internal/gitaly/service/server/disk_stats_test.go
+++ b/internal/gitaly/service/server/disk_stats_test.go
@@ -13,10 +13,22 @@ import (
 )
 
 func TestStorageDiskStatistics(t *testing.T) {
-	cfg := testcfg.Build(t, testcfg.WithStorages("default", "broken"))
+	storageOpt := testcfg.WithStorages("default")
+	if !testhelper.IsWALEnabled() {
+		// The test is testing a broken storage by deleting the storage after initializing it.
+		// This causes problems with WAL as the disk state expected to be present by the database
+		// and the transaction manager suddenly don't exist. Skip the test here with WAL and rely
+		// on the storage implementation to handle broken storage on initialization.
+		storageOpt = testcfg.WithStorages("default", "broken")
+	}
+
+	cfg := testcfg.Build(t, storageOpt)
 
 	addr := runServer(t, cfg)
-	require.NoError(t, os.RemoveAll(cfg.Storages[1].Path), "second storage needs to be invalid")
+
+	if !testhelper.IsWALEnabled() {
+		require.NoError(t, os.RemoveAll(cfg.Storages[1].Path), "second storage needs to be invalid")
+	}
 
 	client := newServerClient(t, addr)
 	ctx := testhelper.Context(t)
@@ -25,7 +37,7 @@ func TestStorageDiskStatistics(t *testing.T) {
 	require.NoError(t, err)
 
 	expectedStorages := len(cfg.Storages)
-	if testhelper.IsPraefectEnabled() {
+	if testhelper.IsPraefectEnabled() && !testhelper.IsWALEnabled() {
 		// Praefect does not virtualize StorageDiskStatistics correctly. It proxies the call to each Gitaly
 		// and returns the results of all of their storages. However, not all storages on a Gitaly node are
 		// necessarily part of a virtual storage. Likewise, Praefect should not expose the individual storages
@@ -47,11 +59,13 @@ func TestStorageDiskStatistics(t *testing.T) {
 	approxEqual(t, c.GetStorageStatuses()[0].Used, used)
 	require.Equal(t, cfg.Storages[0].Name, c.GetStorageStatuses()[0].StorageName)
 
-	require.Equal(t, int64(0), c.GetStorageStatuses()[1].Available)
-	require.Equal(t, int64(0), c.GetStorageStatuses()[1].Used)
-	require.Equal(t, cfg.Storages[1].Name, c.GetStorageStatuses()[1].StorageName)
+	if !testhelper.IsWALEnabled() {
+		require.Equal(t, int64(0), c.GetStorageStatuses()[1].Available)
+		require.Equal(t, int64(0), c.GetStorageStatuses()[1].Used)
+		require.Equal(t, cfg.Storages[1].Name, c.GetStorageStatuses()[1].StorageName)
+	}
 
-	if testhelper.IsPraefectEnabled() {
+	if testhelper.IsPraefectEnabled() && !testhelper.IsWALEnabled() {
 		// This is incorrect behavior caused by the bug explained above.
 		approxEqual(t, c.GetStorageStatuses()[2].Available, avail)
 		approxEqual(t, c.GetStorageStatuses()[2].Used, used)
diff --git a/internal/gitaly/service/server/info_test.go b/internal/gitaly/service/server/info_test.go
index b13666b38..6d3179ebc 100644
--- a/internal/gitaly/service/server/info_test.go
+++ b/internal/gitaly/service/server/info_test.go
@@ -22,10 +22,22 @@ import (
 )
 
 func TestGitalyServerInfo(t *testing.T) {
-	cfg := testcfg.Build(t, testcfg.WithStorages("default", "broken"))
+	storageOpt := testcfg.WithStorages("default")
+	if !testhelper.IsWALEnabled() {
+		// The test is testing a broken storage by deleting the storage after initializing it.
+		// This causes problems with WAL as the disk state expected to be present by the database
+		// and the transaction manager suddenly don't exist. Skip the test here with WAL and rely
+		// on the storage implementation to handle broken storage on initialization.
+		storageOpt = testcfg.WithStorages("default", "broken")
+	}
+
+	cfg := testcfg.Build(t, storageOpt)
 
 	addr := runServer(t, cfg, testserver.WithDisablePraefect())
-	require.NoError(t, os.RemoveAll(cfg.Storages[1].Path), "second storage needs to be invalid")
+
+	if !testhelper.IsWALEnabled() {
+		require.NoError(t, os.RemoveAll(cfg.Storages[1].Path), "second storage needs to be invalid")
+	}
 
 	client := newServerClient(t, addr)
 	ctx := testhelper.Context(t)
@@ -49,10 +61,12 @@ func TestGitalyServerInfo(t *testing.T) {
 	require.NotEmpty(t, c.GetStorageStatuses()[0].FsType)
 	require.Equal(t, uint32(1), c.GetStorageStatuses()[0].ReplicationFactor)
 
-	require.False(t, c.GetStorageStatuses()[1].Readable)
-	require.False(t, c.GetStorageStatuses()[1].Writeable)
-	require.Equal(t, metadata.GitalyFilesystemID, c.GetStorageStatuses()[0].FilesystemId)
-	require.Equal(t, uint32(1), c.GetStorageStatuses()[1].ReplicationFactor)
+	if !testhelper.IsWALEnabled() {
+		require.False(t, c.GetStorageStatuses()[1].Readable)
+		require.False(t, c.GetStorageStatuses()[1].Writeable)
+		require.Equal(t, metadata.GitalyFilesystemID, c.GetStorageStatuses()[0].FilesystemId)
+		require.Equal(t, uint32(1), c.GetStorageStatuses()[1].ReplicationFactor)
+	}
 }
 
 func runServer(t *testing.T, cfg config.Cfg, opts ...testserver.GitalyServerOpt) string {