Merge branch 'pks-drop-sha256-build-tag' into 'master'

gittest: Get rid of SHA256 test tags

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6268

Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>

12 files changed, 71 insertions, 50 deletions

diff --git a/Makefile b/Makefile
index 4333c7a41..d24900602 100644
--- a/Makefile
+++ b/Makefile
@@ -345,8 +345,7 @@ endif
 ## Enable testing with the SHA256 object format.
 TEST_WITH_SHA256 ?=
 ifdef TEST_WITH_SHA256
-    SERVER_BUILD_TAGS := ${SERVER_BUILD_TAGS},gitaly_test_sha256
-    GIT2GO_BUILD_TAGS := ${GIT2GO_BUILD_TAGS},gitaly_test_sha256
+export GITALY_TEST_WITH_SHA256 = YesPlease
 endif
 
 ## Enable generating test coverage.
diff --git a/cmd/gitaly-backup/restore_test.go b/cmd/gitaly-backup/restore_test.go
index 386e79d68..7b327f3eb 100644
--- a/cmd/gitaly-backup/restore_test.go
+++ b/cmd/gitaly-backup/restore_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package main
 
 import (
@@ -25,6 +23,8 @@ import (
 )
 
 func TestRestoreSubcommand(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
@@ -99,6 +99,8 @@ func TestRestoreSubcommand(t *testing.T) {
 }
 
 func TestRestoreSubcommand_serverSide(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
diff --git a/cmd/gitaly-git2go/rebase_test.go b/cmd/gitaly-git2go/rebase_test.go
index 6d4a76b99..3a0d6b8f5 100644
--- a/cmd/gitaly-git2go/rebase_test.go
+++ b/cmd/gitaly-git2go/rebase_test.go
@@ -1,4 +1,4 @@
-//go:build static && system_libgit2 && !gitaly_test_sha256
+//go:build static && system_libgit2
 
 package main
 
@@ -20,6 +20,8 @@ import (
 var masterRevision = "1e292f8fedd741b75372e19097c76d327140c312"
 
 func TestRebase_validation(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
@@ -87,6 +89,8 @@ func TestRebase_validation(t *testing.T) {
 }
 
 func TestRebase_rebase(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	testcases := []struct {
 		desc         string
 		branch       string
@@ -248,6 +252,8 @@ func TestRebase_rebase(t *testing.T) {
 }
 
 func TestRebase_skipEmptyCommit(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
diff --git a/cmd/gitaly-git2go/testhelper_test.go b/cmd/gitaly-git2go/testhelper_test.go
index 765b4470e..f798dc417 100644
--- a/cmd/gitaly-git2go/testhelper_test.go
+++ b/cmd/gitaly-git2go/testhelper_test.go
@@ -1,4 +1,4 @@
-//go:build static && system_libgit2 && !gitaly_test_sha256
+//go:build static && system_libgit2
 
 package main
 
diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go
index f437ce6a5..0121f2116 100644
--- a/internal/backup/backup_test.go
+++ b/internal/backup/backup_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package backup_test
 
 import (
@@ -377,6 +375,8 @@ func TestManager_Create_incremental(t *testing.T) {
 }
 
 func TestManager_Restore_latest(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -716,6 +716,8 @@ func TestManager_Restore_latest(t *testing.T) {
 }
 
 func TestManager_Restore_specific(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 
 	const backupID = "abc123"
@@ -917,6 +919,8 @@ func TestManager_Restore_specific(t *testing.T) {
 }
 
 func TestManager_CreateRestore_contextServerInfo(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 
 	cfg := testcfg.Build(t)
@@ -954,6 +958,8 @@ func TestManager_CreateRestore_contextServerInfo(t *testing.T) {
 }
 
 func TestResolveLocator(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 
 	for _, tc := range []struct {
diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go
index 01534d368..c9f48fdf1 100644
--- a/internal/backup/locator_test.go
+++ b/internal/backup/locator_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package backup
 
 import (
@@ -24,11 +22,12 @@ func TestLegacyLocator(t *testing.T) {
 	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
-	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
 		SkipCreationViaService: true,
-		Seed:                   gittest.SeedGitLabTest,
 		RelativePath:           t.Name(),
 	})
+	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch))
+
 	l := LegacyLocator{}
 
 	t.Run("Begin/Commit Full", func(t *testing.T) {
@@ -75,11 +74,11 @@ func TestPointerLocator(t *testing.T) {
 	ctx := testhelper.Context(t)
 	cfg := testcfg.Build(t)
 
-	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
 		SkipCreationViaService: true,
-		Seed:                   gittest.SeedGitLabTest,
 		RelativePath:           t.Name(),
 	})
+	gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch))
 
 	t.Run("Begin/Commit full", func(t *testing.T) {
 		t.Parallel()
diff --git a/internal/backup/server_side_test.go b/internal/backup/server_side_test.go
index 3329c8a78..05f5f9b82 100644
--- a/internal/backup/server_side_test.go
+++ b/internal/backup/server_side_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package backup_test
 
 import (
@@ -23,6 +21,8 @@ import (
 )
 
 func TestServerSideAdapter_Create(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
@@ -131,6 +131,8 @@ func TestServerSideAdapter_Create(t *testing.T) {
 }
 
 func TestServerSideAdapter_Restore(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 	ctx := testhelper.Context(t)
 
diff --git a/internal/git/gittest/object_hash.go b/internal/git/gittest/object_hash.go
new file mode 100644
index 000000000..8ab59d5f8
--- /dev/null
+++ b/internal/git/gittest/object_hash.go
@@ -0,0 +1,39 @@
+package gittest
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
+)
+
+// DefaultObjectHash is the default object hash used for running tests.
+var DefaultObjectHash = func() git.ObjectHash {
+	if _, enabled := os.LookupEnv("GITALY_TEST_WITH_SHA256"); enabled {
+		return git.ObjectHashSHA256
+	}
+
+	return git.ObjectHashSHA1
+}()
+
+// SkipWithSHA256 skips the test in case the default object hash is SHA256.
+func SkipWithSHA256(tb testing.TB) {
+	if DefaultObjectHash.Format == "sha256" {
+		tb.Skip("test is not compatible with SHA256")
+	}
+}
+
+// ObjectHashIsSHA256 returns if the current default object hash is SHA256.
+func ObjectHashIsSHA256() bool {
+	return DefaultObjectHash.EmptyTreeOID == git.ObjectHashSHA256.EmptyTreeOID
+}
+
+// ObjectHashDependent returns the value from the given map that is associated with the default
+// object hash (e.g. "sha1", "sha256"). Fails in case the map doesn't contain the current object
+// hash.
+func ObjectHashDependent[T any](tb testing.TB, valuesByObjectHash map[string]T) T {
+	tb.Helper()
+	require.Contains(tb, valuesByObjectHash, DefaultObjectHash.Format)
+	return valuesByObjectHash[DefaultObjectHash.Format]
+}
diff --git a/internal/git/gittest/objects.go b/internal/git/gittest/objects.go
index 821f58b04..a365dd1e8 100644
--- a/internal/git/gittest/objects.go
+++ b/internal/git/gittest/objects.go
@@ -15,20 +15,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v16/internal/helper/text"
 )
 
-// ObjectHashIsSHA256 returns if the current default object hash is SHA256.
-func ObjectHashIsSHA256() bool {
-	return DefaultObjectHash.EmptyTreeOID == git.ObjectHashSHA256.EmptyTreeOID
-}
-
-// ObjectHashDependent returns the value from the given map that is associated with the default
-// object hash (e.g. "sha1", "sha256"). Fails in case the map doesn't contain the current object
-// hash.
-func ObjectHashDependent[T any](tb testing.TB, valuesByObjectHash map[string]T) T {
-	tb.Helper()
-	require.Contains(tb, valuesByObjectHash, DefaultObjectHash.Format)
-	return valuesByObjectHash[DefaultObjectHash.Format]
-}
-
 // ListObjects returns a list of all object IDs in the repository.
 func ListObjects(tb testing.TB, cfg config.Cfg, repoPath string) []git.ObjectID {
 	tb.Helper()
diff --git a/internal/git/gittest/sha1.go b/internal/git/gittest/sha1.go
deleted file mode 100644
index 76ae7bd3e..000000000
--- a/internal/git/gittest/sha1.go
+++ /dev/null
@@ -1,8 +0,0 @@
-//go:build !gitaly_test_sha256
-
-package gittest
-
-import "gitlab.com/gitlab-org/gitaly/v16/internal/git"
-
-// DefaultObjectHash is the default hash used for running tests.
-var DefaultObjectHash = git.ObjectHashSHA1
diff --git a/internal/git/gittest/sha256.go b/internal/git/gittest/sha256.go
deleted file mode 100644
index f2ac3324c..000000000
--- a/internal/git/gittest/sha256.go
+++ /dev/null
@@ -1,10 +0,0 @@
-//go:build gitaly_test_sha256
-
-package gittest
-
-import (
-	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
-)
-
-// DefaultObjectHash is the default hash used for running tests.
-var DefaultObjectHash = git.ObjectHashSHA256
diff --git a/internal/gitaly/service/repository/restore_repository_test.go b/internal/gitaly/service/repository/restore_repository_test.go
index 2da64b841..5d5fbfbb0 100644
--- a/internal/gitaly/service/repository/restore_repository_test.go
+++ b/internal/gitaly/service/repository/restore_repository_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package repository
 
 import (
@@ -18,6 +16,8 @@ import (
 )
 
 func TestRestoreRepository(t *testing.T) {
+	gittest.SkipWithSHA256(t)
+
 	t.Parallel()
 	ctx := testhelper.Context(t)