gittest: Convert `WriteTag()` to optionally get config

The `WriteTag()` helper function currently always requires a
`WriteTagOpts` pointer to be passed, even if one wants to have defaults
in place. Convert it to use an optional struct via varargs. While at it,
this also renames the strutc from `WriteTagOpts` to `WriteTagConfig`.

9 files changed, 53 insertions, 52 deletions

diff --git a/internal/git/catfile/tag_test.go b/internal/git/catfile/tag_test.go
index 86b88fe09..4452653ab 100644
--- a/internal/git/catfile/tag_test.go
+++ b/internal/git/catfile/tag_test.go
@@ -53,7 +53,7 @@ func TestGetTag(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.tagName, func(t *testing.T) {
-			tagID := gittest.WriteTag(t, cfg, testRepoPath, testCase.tagName, testCase.rev, &gittest.WriteTagOpts{Message: testCase.message})
+			tagID := gittest.WriteTag(t, cfg, testRepoPath, testCase.tagName, testCase.rev, gittest.WriteTagConfig{Message: testCase.message})
 
 			tag, err := GetTag(ctx, c, git.Revision(tagID), testCase.tagName, testCase.trim, true)
 			require.NoError(t, err)
diff --git a/internal/git/gittest/tag.go b/internal/git/gittest/tag.go
index 9e10f0600..773c616ae 100644
--- a/internal/git/gittest/tag.go
+++ b/internal/git/gittest/tag.go
@@ -5,36 +5,35 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 )
 
-// WriteTagOpts holds extra options for WriteTag.
-type WriteTagOpts struct {
+// WriteTagConfig holds extra options for WriteTag.
+type WriteTagConfig struct {
+	// Message is the message of an annotated tag. If left empty, then a lightweight tag will
+	// be created.
 	Message string
-	Force   bool
+	// Force indicates whether existing tags with the same name shall be overwritten.
+	Force bool
 }
 
 // WriteTag writes a new tag into the repository. This function either returns the tag ID in case
 // an annotated tag was created, or otherwise the target object ID when a lightweight tag was
-// created.
-func WriteTag(t testing.TB, cfg config.Cfg, repoPath, tagName, targetID string, opts *WriteTagOpts) string {
-	var message string
-	force := false
-
-	if opts != nil {
-		if opts.Message != "" {
-			message = opts.Message
-		}
-		force = opts.Force
+// created. Takes either no WriteTagConfig, in which case the default values will be used, or
+// exactly one.
+func WriteTag(t testing.TB, cfg config.Cfg, repoPath, tagName, targetID string, optionalConfig ...WriteTagConfig) string {
+	require.Less(t, len(optionalConfig), 2, "only a single config may be passed")
+
+	var config WriteTagConfig
+	if len(optionalConfig) == 1 {
+		config = optionalConfig[0]
 	}
 
 	committerName := "Scrooge McDuck"
 	committerEmail := "scrooge@mcduck.com"
 
-	// message can be very large, passing it directly in args would blow things up!
-	stdin := bytes.NewBufferString(message)
-
 	args := []string{
 		"-C", repoPath,
 		"-c", fmt.Sprintf("user.name=%s", committerName),
@@ -42,11 +41,13 @@ func WriteTag(t testing.TB, cfg config.Cfg, repoPath, tagName, targetID string,
 		"tag",
 	}
 
-	if force {
+	if config.Force {
 		args = append(args, "-f")
 	}
 
-	if message != "" {
+	// The message can be very large, passing it directly in args would blow things up.
+	stdin := bytes.NewBufferString(config.Message)
+	if config.Message != "" {
 		args = append(args, "-F", "-")
 	}
 	args = append(args, tagName, targetID)
diff --git a/internal/git/objectpool/fetch_test.go b/internal/git/objectpool/fetch_test.go
index 7cf1a2779..d200ab06e 100644
--- a/internal/git/objectpool/fetch_test.go
+++ b/internal/git/objectpool/fetch_test.go
@@ -51,7 +51,7 @@ func TestFetchFromOriginDangling(t *testing.T) {
 
 	// A tag with random hex characters in its name should be unique.
 	newTagName := "tag-" + nonce
-	newTag := gittest.WriteTag(t, pool.cfg, pool.FullPath(), newTagName, existingCommit, &gittest.WriteTagOpts{
+	newTag := gittest.WriteTag(t, pool.cfg, pool.FullPath(), newTagName, existingCommit, gittest.WriteTagConfig{
 		Message: "msg",
 	})
 
diff --git a/internal/gitaly/service/ref/find_all_tags_test.go b/internal/gitaly/service/ref/find_all_tags_test.go
index 0db31545f..80ccdc5c5 100644
--- a/internal/gitaly/service/ref/find_all_tags_test.go
+++ b/internal/gitaly/service/ref/find_all_tags_test.go
@@ -53,26 +53,26 @@ func TestFindAllTags_successful(t *testing.T) {
 	bigCommit, err := repo.ReadCommit(ctx, git.Revision(bigCommitID))
 	require.NoError(t, err)
 
-	annotatedTagID := gittest.WriteTag(t, cfg, repoPath, "v1.2.0", blobID, &gittest.WriteTagOpts{Message: "Blob tag"})
+	annotatedTagID := gittest.WriteTag(t, cfg, repoPath, "v1.2.0", blobID, gittest.WriteTagConfig{Message: "Blob tag"})
 
-	gittest.WriteTag(t, cfg, repoPath, "v1.3.0", commitID, nil)
-	gittest.WriteTag(t, cfg, repoPath, "v1.4.0", blobID, nil)
+	gittest.WriteTag(t, cfg, repoPath, "v1.3.0", commitID)
+	gittest.WriteTag(t, cfg, repoPath, "v1.4.0", blobID)
 
 	// To test recursive resolving to a commit
-	gittest.WriteTag(t, cfg, repoPath, "v1.5.0", "v1.3.0", nil)
+	gittest.WriteTag(t, cfg, repoPath, "v1.5.0", "v1.3.0")
 
 	// A tag to commit with a big message
-	gittest.WriteTag(t, cfg, repoPath, "v1.6.0", bigCommitID.String(), nil)
+	gittest.WriteTag(t, cfg, repoPath, "v1.6.0", bigCommitID.String())
 
 	// A tag with a big message
 	bigMessage := strings.Repeat("a", 11*1024)
-	bigMessageTag1ID := gittest.WriteTag(t, cfg, repoPath, "v1.7.0", commitID, &gittest.WriteTagOpts{Message: bigMessage})
+	bigMessageTag1ID := gittest.WriteTag(t, cfg, repoPath, "v1.7.0", commitID, gittest.WriteTagConfig{Message: bigMessage})
 
 	// A tag with a commit id as its name
-	commitTagID := gittest.WriteTag(t, cfg, repoPath, commitID, commitID, &gittest.WriteTagOpts{Message: "commit tag with a commit sha as the name"})
+	commitTagID := gittest.WriteTag(t, cfg, repoPath, commitID, commitID, gittest.WriteTagConfig{Message: "commit tag with a commit sha as the name"})
 
 	// a tag of a tag
-	tagOfTagID := gittest.WriteTag(t, cfg, repoPath, "tag-of-tag", commitTagID, &gittest.WriteTagOpts{Message: "tag of a tag"})
+	tagOfTagID := gittest.WriteTag(t, cfg, repoPath, "tag-of-tag", commitTagID, gittest.WriteTagConfig{Message: "tag of a tag"})
 
 	rpcRequest := &gitalypb.FindAllTagsRequest{Repository: repoProto}
 
@@ -232,7 +232,7 @@ func TestFindAllTags_simpleNestedTags(t *testing.T) {
 		gittest.WithParents(),
 	)
 
-	tagID := gittest.WriteTag(t, cfg, repoPath, "my/nested/tag", commitID.String(), nil)
+	tagID := gittest.WriteTag(t, cfg, repoPath, "my/nested/tag", commitID.String())
 
 	stream, err := client.FindAllTags(ctx, &gitalypb.FindAllTagsRequest{Repository: repoProto})
 	require.NoError(t, err)
@@ -411,7 +411,7 @@ func TestFindAllTags_nestedTags(t *testing.T) {
 			for depth := 0; depth < tc.depth; depth++ {
 				tagName := fmt.Sprintf("tag-depth-%d", depth)
 				tagMessage := fmt.Sprintf("a commit %d deep", depth)
-				tagID = gittest.WriteTag(t, cfg, repoPath, tagName, tagID, &gittest.WriteTagOpts{Message: tagMessage})
+				tagID = gittest.WriteTag(t, cfg, repoPath, tagName, tagID, gittest.WriteTagConfig{Message: tagMessage})
 
 				expectedTag := &gitalypb.Tag{
 					Name:        []byte(tagName),
diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go
index 005dcaf34..1d09c3fca 100644
--- a/internal/gitaly/service/ref/refs_test.go
+++ b/internal/gitaly/service/ref/refs_test.go
@@ -891,26 +891,26 @@ func TestSuccessfulFindTagRequest(t *testing.T) {
 	bigCommit, err := repo.ReadCommit(ctx, git.Revision(bigCommitID))
 	require.NoError(t, err)
 
-	annotatedTagID := gittest.WriteTag(t, cfg, repoPath, "v1.2.0", blobID, &gittest.WriteTagOpts{Message: "Blob tag"})
+	annotatedTagID := gittest.WriteTag(t, cfg, repoPath, "v1.2.0", blobID, gittest.WriteTagConfig{Message: "Blob tag"})
 
-	gittest.WriteTag(t, cfg, repoPath, "v1.3.0", commitID, nil)
-	gittest.WriteTag(t, cfg, repoPath, "v1.4.0", blobID, nil)
+	gittest.WriteTag(t, cfg, repoPath, "v1.3.0", commitID)
+	gittest.WriteTag(t, cfg, repoPath, "v1.4.0", blobID)
 
 	// To test recursive resolving to a commit
-	gittest.WriteTag(t, cfg, repoPath, "v1.5.0", "v1.3.0", nil)
+	gittest.WriteTag(t, cfg, repoPath, "v1.5.0", "v1.3.0")
 
 	// A tag to commit with a big message
-	gittest.WriteTag(t, cfg, repoPath, "v1.6.0", bigCommitID.String(), nil)
+	gittest.WriteTag(t, cfg, repoPath, "v1.6.0", bigCommitID.String())
 
 	// A tag with a big message
 	bigMessage := strings.Repeat("a", 11*1024)
-	bigMessageTag1ID := gittest.WriteTag(t, cfg, repoPath, "v1.7.0", commitID, &gittest.WriteTagOpts{Message: bigMessage})
+	bigMessageTag1ID := gittest.WriteTag(t, cfg, repoPath, "v1.7.0", commitID, gittest.WriteTagConfig{Message: bigMessage})
 
 	// A tag with a commit id as its name
-	commitTagID := gittest.WriteTag(t, cfg, repoPath, commitID, commitID, &gittest.WriteTagOpts{Message: "commit tag with a commit sha as the name"})
+	commitTagID := gittest.WriteTag(t, cfg, repoPath, commitID, commitID, gittest.WriteTagConfig{Message: "commit tag with a commit sha as the name"})
 
 	// a tag of a tag
-	tagOfTagID := gittest.WriteTag(t, cfg, repoPath, "tag-of-tag", commitTagID, &gittest.WriteTagOpts{Message: "tag of a tag"})
+	tagOfTagID := gittest.WriteTag(t, cfg, repoPath, "tag-of-tag", commitTagID, gittest.WriteTagConfig{Message: "tag of a tag"})
 
 	expectedTags := []*gitalypb.Tag{
 		{
@@ -1093,7 +1093,7 @@ func TestFindTagNestedTag(t *testing.T) {
 			for depth := 0; depth < tc.depth; depth++ {
 				tagName = fmt.Sprintf("tag-depth-%d", depth)
 				tagMessage = fmt.Sprintf("a commit %d deep", depth)
-				tagID = gittest.WriteTag(t, cfg, repoPath, tagName, tagID, &gittest.WriteTagOpts{Message: tagMessage})
+				tagID = gittest.WriteTag(t, cfg, repoPath, tagName, tagID, gittest.WriteTagConfig{Message: tagMessage})
 			}
 			expectedTag := &gitalypb.Tag{
 				Name:        []byte(tagName),
diff --git a/internal/gitaly/service/ref/tag_messages_test.go b/internal/gitaly/service/ref/tag_messages_test.go
index 64d100081..c4870b6b3 100644
--- a/internal/gitaly/service/ref/tag_messages_test.go
+++ b/internal/gitaly/service/ref/tag_messages_test.go
@@ -23,8 +23,8 @@ func TestSuccessfulGetTagMessagesRequest(t *testing.T) {
 	message1 := strings.Repeat("a", helper.MaxCommitOrTagMessageSize*2)
 	message2 := strings.Repeat("b", helper.MaxCommitOrTagMessageSize)
 
-	tag1ID := gittest.WriteTag(t, cfg, repoPath, "big-tag-1", "master", &gittest.WriteTagOpts{Message: message1})
-	tag2ID := gittest.WriteTag(t, cfg, repoPath, "big-tag-2", "master~", &gittest.WriteTagOpts{Message: message2})
+	tag1ID := gittest.WriteTag(t, cfg, repoPath, "big-tag-1", "master", gittest.WriteTagConfig{Message: message1})
+	tag2ID := gittest.WriteTag(t, cfg, repoPath, "big-tag-2", "master~", gittest.WriteTagConfig{Message: message2})
 
 	request := &gitalypb.GetTagMessagesRequest{
 		Repository: repo,
diff --git a/internal/gitaly/service/ref/tag_signatures_test.go b/internal/gitaly/service/ref/tag_signatures_test.go
index 9cbfc99b7..0bc98ef36 100644
--- a/internal/gitaly/service/ref/tag_signatures_test.go
+++ b/internal/gitaly/service/ref/tag_signatures_test.go
@@ -24,16 +24,16 @@ func TestGetTagSignatures(t *testing.T) {
 
 	message1 := strings.Repeat("a", helper.MaxCommitOrTagMessageSize) + "\n"
 	signature1 := string(testhelper.MustReadFile(t, "testdata/tag-1e292f8fedd741b75372e19097c76d327140c312-signature"))
-	tag1ID := gittest.WriteTag(t, cfg, repoPath, "big-tag-1", "master", &gittest.WriteTagOpts{Message: message1 + signature1})
+	tag1ID := gittest.WriteTag(t, cfg, repoPath, "big-tag-1", "master", gittest.WriteTagConfig{Message: message1 + signature1})
 	content1 := "object 1e292f8fedd741b75372e19097c76d327140c312\ntype commit\ntag big-tag-1\ntagger Scrooge McDuck <scrooge@mcduck.com> 1572776879 +0100\n\n" + message1
 
 	message2 := strings.Repeat("b", helper.MaxCommitOrTagMessageSize) + "\n"
 	signature2 := string(testhelper.MustReadFile(t, "testdata/tag-7975be0116940bf2ad4321f79d02a55c5f7779aa-signature"))
-	tag2ID := gittest.WriteTag(t, cfg, repoPath, "big-tag-2", "master~", &gittest.WriteTagOpts{Message: message2 + signature2})
+	tag2ID := gittest.WriteTag(t, cfg, repoPath, "big-tag-2", "master~", gittest.WriteTagConfig{Message: message2 + signature2})
 	content2 := "object 7975be0116940bf2ad4321f79d02a55c5f7779aa\ntype commit\ntag big-tag-2\ntagger Scrooge McDuck <scrooge@mcduck.com> 1572776879 +0100\n\n" + message2
 
 	message3 := "tag message\n"
-	tag3ID := gittest.WriteTag(t, cfg, repoPath, "tag-3", "master~~", &gittest.WriteTagOpts{Message: message3})
+	tag3ID := gittest.WriteTag(t, cfg, repoPath, "tag-3", "master~~", gittest.WriteTagConfig{Message: message3})
 	content3 := "object 60ecb67744cb56576c30214ff52294f8ce2def98\ntype commit\ntag tag-3\ntagger Scrooge McDuck <scrooge@mcduck.com> 1572776879 +0100\n\n" + message3
 
 	for _, tc := range []struct {
diff --git a/internal/gitaly/service/remote/update_remote_mirror_test.go b/internal/gitaly/service/remote/update_remote_mirror_test.go
index a48412e8a..e56171609 100644
--- a/internal/gitaly/service/remote/update_remote_mirror_test.go
+++ b/internal/gitaly/service/remote/update_remote_mirror_test.go
@@ -641,9 +641,9 @@ func TestSuccessfulUpdateRemoteMirrorRequest(t *testing.T) {
 	testRepo, testRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
 	_, mirrorPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
 
-	gittest.WriteTag(t, cfg, mirrorPath, "v0.0.1", "master", nil) // I needed another tag for the tests
-	gittest.WriteTag(t, cfg, testRepoPath, "new-tag", "60ecb67744cb56576c30214ff52294f8ce2def98", nil)
-	gittest.WriteTag(t, cfg, testRepoPath, "v1.0.0", "0b4bc9a49b562e85de7cc9e834518ea6828729b9", &gittest.WriteTagOpts{
+	gittest.WriteTag(t, cfg, mirrorPath, "v0.0.1", "master") // I needed another tag for the tests
+	gittest.WriteTag(t, cfg, testRepoPath, "new-tag", "60ecb67744cb56576c30214ff52294f8ce2def98")
+	gittest.WriteTag(t, cfg, testRepoPath, "v1.0.0", "0b4bc9a49b562e85de7cc9e834518ea6828729b9", gittest.WriteTagConfig{
 		Message: "Overriding tag", Force: true,
 	})
 
@@ -758,9 +758,9 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithWildcards(t *testing.T) {
 		{"tag", "--delete", "v1.1.0"},         // v1.1.0 is ambiguous, maps to a branch and a tag in gitlab-test repository
 	}
 
-	gittest.WriteTag(t, cfg, testRepoPath, "new-tag", "60ecb67744cb56576c30214ff52294f8ce2def98", nil) // Add tag
+	gittest.WriteTag(t, cfg, testRepoPath, "new-tag", "60ecb67744cb56576c30214ff52294f8ce2def98") // Add tag
 	gittest.WriteTag(t, cfg, testRepoPath, "v1.0.0", "0b4bc9a49b562e85de7cc9e834518ea6828729b9",
-		&gittest.WriteTagOpts{Message: "Overriding tag", Force: true}) // Update tag
+		gittest.WriteTagConfig{Message: "Overriding tag", Force: true}) // Update tag
 
 	for _, args := range setupCommands {
 		gitArgs := []string{"-C", testRepoPath}
@@ -770,7 +770,7 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithWildcards(t *testing.T) {
 
 	// Workaround for https://gitlab.com/gitlab-org/gitaly/issues/1439
 	// Create a tag on the remote to ensure it gets deleted later
-	gittest.WriteTag(t, cfg, mirrorPath, "v1.2.0", "master", nil)
+	gittest.WriteTag(t, cfg, mirrorPath, "v1.2.0", "master")
 
 	newTagOid := string(gittest.Exec(t, cfg, "-C", testRepoPath, "rev-parse", "v1.0.0"))
 	newTagOid = strings.TrimSpace(newTagOid)
@@ -874,7 +874,7 @@ func TestSuccessfulUpdateRemoteMirrorRequestWithKeepDivergentRefs(t *testing.T)
 	testRepo, testRepoPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
 	_, mirrorPath := gittest.CloneRepo(t, cfg, cfg.Storages[0])
 
-	gittest.WriteTag(t, cfg, mirrorPath, "v2.0.0", "master", nil)
+	gittest.WriteTag(t, cfg, mirrorPath, "v2.0.0", "master")
 
 	setupCommands := [][]string{
 		// Preconditions
diff --git a/internal/gitaly/service/repository/fetch_remote_test.go b/internal/gitaly/service/repository/fetch_remote_test.go
index 523a89a13..5fac9e4a5 100644
--- a/internal/gitaly/service/repository/fetch_remote_test.go
+++ b/internal/gitaly/service/repository/fetch_remote_test.go
@@ -57,7 +57,7 @@ func TestFetchRemoteSuccess(t *testing.T) {
 	cloneRepo := copyRepo(t, cfg, repo, repoPath)
 
 	// Ensure there's a new tag to fetch
-	gittest.WriteTag(t, cfg, repoPath, "testtag", "master", nil)
+	gittest.WriteTag(t, cfg, repoPath, "testtag", "master")
 
 	req := &gitalypb.FetchRemoteRequest{Repository: cloneRepo, RemoteParams: &gitalypb.Remote{
 		Url: repoPath,