featureflag: Remove SimplifyFindLocalBranchesResponse

Remove the featureflag SimplifyFindLocalBranchesResponse. This was made
default true in 15.5 and now we want to remove the code entirely in
15.6.

Remove test helper functions which are no longer required.

4 files changed, 72 insertions, 258 deletions

diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go
index 4825363fe..afd88b05e 100644
--- a/internal/gitaly/service/ref/refs_test.go
+++ b/internal/gitaly/service/ref/refs_test.go
@@ -5,7 +5,6 @@ package ref
 import (
 	"bufio"
 	"bytes"
-	"context"
 	"fmt"
 	"io"
 	"strings"
@@ -17,7 +16,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -255,76 +253,38 @@ func TestInvalidRepoFindDefaultBranchNameRequest(t *testing.T) {
 
 func TestSuccessfulFindLocalBranches(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.SimplifyFindLocalBranchesResponse).Run(t, testSuccessfulFindLocalBranches)
-}
+	ctx := testhelper.Context(t)
 
-func testSuccessfulFindLocalBranches(t *testing.T, ctx context.Context) {
 	_, repo, _, client := setupRefService(t, ctx)
 
 	rpcRequest := &gitalypb.FindLocalBranchesRequest{Repository: repo}
 	c, err := client.FindLocalBranches(ctx, rpcRequest)
 	require.NoError(t, err)
 
-	if featureflag.SimplifyFindLocalBranchesResponse.IsEnabled(ctx) {
-		var branches []*gitalypb.Branch
-		for {
-			r, err := c.Recv()
-			if err == io.EOF {
-				break
-			}
-			require.NoError(t, err)
-			branches = append(branches, r.GetLocalBranches()...)
-		}
-
-		for name, target := range localBranches {
-			localBranch := &gitalypb.Branch{
-				Name:         []byte(name),
-				TargetCommit: target,
-			}
-
-			assertContainsBranch(t, branches, localBranch)
+	var branches []*gitalypb.Branch
+	for {
+		r, err := c.Recv()
+		if err == io.EOF {
+			break
 		}
+		require.NoError(t, err)
+		branches = append(branches, r.GetLocalBranches()...)
+	}
 
-	} else {
-		var branches []*gitalypb.FindLocalBranchResponse
-		for {
-			r, err := c.Recv()
-			if err == io.EOF {
-				break
-			}
-			require.NoError(t, err)
-			branches = append(branches, r.GetBranches()...)
+	for name, target := range localBranches {
+		localBranch := &gitalypb.Branch{
+			Name:         []byte(name),
+			TargetCommit: target,
 		}
 
-		for name, target := range localBranches {
-			localBranch := &gitalypb.FindLocalBranchResponse{
-				Name:          []byte(name),
-				CommitId:      target.Id,
-				CommitSubject: target.Subject,
-				CommitAuthor: &gitalypb.FindLocalBranchCommitAuthor{
-					Name:  target.Author.Name,
-					Email: target.Author.Email,
-					Date:  target.Author.Date,
-				},
-				CommitCommitter: &gitalypb.FindLocalBranchCommitAuthor{
-					Name:  target.Committer.Name,
-					Email: target.Committer.Email,
-					Date:  target.Committer.Date,
-				},
-				Commit: target,
-			}
-
-			assertContainsLocalBranch(t, branches, localBranch)
-		}
+		assertContainsBranch(t, branches, localBranch)
 	}
 }
 
 func TestFindLocalBranchesHugeCommitter(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.SimplifyFindLocalBranchesResponse).Run(t, testFindLocalBranchesHugeCommitter)
-}
+	ctx := testhelper.Context(t)
 
-func testFindLocalBranchesHugeCommitter(t *testing.T, ctx context.Context) {
 	cfg, repo, repoPath, client := setupRefService(t, ctx)
 
 	gittest.WriteCommit(t, cfg, repoPath,
@@ -348,10 +308,8 @@ func testFindLocalBranchesHugeCommitter(t *testing.T, ctx context.Context) {
 
 func TestFindLocalBranchesPagination(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.SimplifyFindLocalBranchesResponse).Run(t, testFindLocalBranchesPagination)
-}
+	ctx := testhelper.Context(t)
 
-func testFindLocalBranchesPagination(t *testing.T, ctx context.Context) {
 	_, repo, _, client := setupRefService(t, ctx)
 
 	limit := 1
@@ -368,63 +326,29 @@ func testFindLocalBranchesPagination(t *testing.T, ctx context.Context) {
 	expectedBranch := "refs/heads/improve/awesome"
 	target := localBranches[expectedBranch]
 
-	if featureflag.SimplifyFindLocalBranchesResponse.IsEnabled(ctx) {
-		var branches []*gitalypb.Branch
-		for {
-			r, err := c.Recv()
-			if err == io.EOF {
-				break
-			}
-			require.NoError(t, err)
-			branches = append(branches, r.GetLocalBranches()...)
-		}
-
-		require.Len(t, branches, limit)
-
-		branch := &gitalypb.Branch{
-			Name:         []byte(expectedBranch),
-			TargetCommit: target,
-		}
-		assertContainsBranch(t, branches, branch)
-	} else {
-		var branches []*gitalypb.FindLocalBranchResponse
-		for {
-			r, err := c.Recv()
-			if err == io.EOF {
-				break
-			}
-			require.NoError(t, err)
-			branches = append(branches, r.GetBranches()...)
+	var branches []*gitalypb.Branch
+	for {
+		r, err := c.Recv()
+		if err == io.EOF {
+			break
 		}
+		require.NoError(t, err)
+		branches = append(branches, r.GetLocalBranches()...)
+	}
 
-		require.Len(t, branches, limit)
+	require.Len(t, branches, limit)
 
-		branch := &gitalypb.FindLocalBranchResponse{
-			Name:          []byte(expectedBranch),
-			CommitId:      target.Id,
-			CommitSubject: target.Subject,
-			CommitAuthor: &gitalypb.FindLocalBranchCommitAuthor{
-				Name:  target.Author.Name,
-				Email: target.Author.Email,
-				Date:  target.Author.Date,
-			},
-			CommitCommitter: &gitalypb.FindLocalBranchCommitAuthor{
-				Name:  target.Committer.Name,
-				Email: target.Committer.Email,
-				Date:  target.Committer.Date,
-			},
-			Commit: target,
-		}
-		assertContainsLocalBranch(t, branches, branch)
+	branch := &gitalypb.Branch{
+		Name:         []byte(expectedBranch),
+		TargetCommit: target,
 	}
+	assertContainsBranch(t, branches, branch)
 }
 
 func TestFindLocalBranchesPaginationSequence(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.SimplifyFindLocalBranchesResponse).Run(t, testFindLocalBranchesPaginationSequence)
-}
+	ctx := testhelper.Context(t)
 
-func testFindLocalBranchesPaginationSequence(t *testing.T, ctx context.Context) {
 	_, repo, _, client := setupRefService(t, ctx)
 
 	limit := 2
@@ -437,85 +361,46 @@ func testFindLocalBranchesPaginationSequence(t *testing.T, ctx context.Context)
 	c, err := client.FindLocalBranches(ctx, firstRPCRequest)
 	require.NoError(t, err)
 
-	if featureflag.SimplifyFindLocalBranchesResponse.IsEnabled(ctx) {
-		var firstResponseBranches []*gitalypb.Branch
-		for {
-			r, err := c.Recv()
-			if err == io.EOF {
-				break
-			}
-			require.NoError(t, err)
-			firstResponseBranches = append(firstResponseBranches, r.GetLocalBranches()...)
-		}
-
-		require.Len(t, firstResponseBranches, limit)
-
-		secondRPCRequest := &gitalypb.FindLocalBranchesRequest{
-			Repository: repo,
-			PaginationParams: &gitalypb.PaginationParameter{
-				Limit:     1,
-				PageToken: string(firstResponseBranches[0].Name),
-			},
+	var firstResponseBranches []*gitalypb.Branch
+	for {
+		r, err := c.Recv()
+		if err == io.EOF {
+			break
 		}
-		c, err = client.FindLocalBranches(ctx, secondRPCRequest)
 		require.NoError(t, err)
+		firstResponseBranches = append(firstResponseBranches, r.GetLocalBranches()...)
+	}
 
-		var secondResponseBranches []*gitalypb.Branch
-		for {
-			r, err := c.Recv()
-			if err == io.EOF {
-				break
-			}
-			require.NoError(t, err)
-			secondResponseBranches = append(secondResponseBranches, r.GetLocalBranches()...)
-		}
-
-		require.Len(t, secondResponseBranches, 1)
-		require.Equal(t, firstResponseBranches[1], secondResponseBranches[0])
-	} else {
-		var firstResponseBranches []*gitalypb.FindLocalBranchResponse
-		for {
-			r, err := c.Recv()
-			if err == io.EOF {
-				break
-			}
-			require.NoError(t, err)
-			firstResponseBranches = append(firstResponseBranches, r.GetBranches()...)
-		}
+	require.Len(t, firstResponseBranches, limit)
 
-		require.Len(t, firstResponseBranches, limit)
+	secondRPCRequest := &gitalypb.FindLocalBranchesRequest{
+		Repository: repo,
+		PaginationParams: &gitalypb.PaginationParameter{
+			Limit:     1,
+			PageToken: string(firstResponseBranches[0].Name),
+		},
+	}
+	c, err = client.FindLocalBranches(ctx, secondRPCRequest)
+	require.NoError(t, err)
 
-		secondRPCRequest := &gitalypb.FindLocalBranchesRequest{
-			Repository: repo,
-			PaginationParams: &gitalypb.PaginationParameter{
-				Limit:     1,
-				PageToken: string(firstResponseBranches[0].Name),
-			},
+	var secondResponseBranches []*gitalypb.Branch
+	for {
+		r, err := c.Recv()
+		if err == io.EOF {
+			break
 		}
-		c, err = client.FindLocalBranches(ctx, secondRPCRequest)
 		require.NoError(t, err)
-
-		var secondResponseBranches []*gitalypb.FindLocalBranchResponse
-		for {
-			r, err := c.Recv()
-			if err == io.EOF {
-				break
-			}
-			require.NoError(t, err)
-			secondResponseBranches = append(secondResponseBranches, r.GetBranches()...)
-		}
-
-		require.Len(t, secondResponseBranches, 1)
-		require.Equal(t, firstResponseBranches[1], secondResponseBranches[0])
+		secondResponseBranches = append(secondResponseBranches, r.GetLocalBranches()...)
 	}
+
+	require.Len(t, secondResponseBranches, 1)
+	require.Equal(t, firstResponseBranches[1], secondResponseBranches[0])
 }
 
 func TestFindLocalBranchesPaginationWithIncorrectToken(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.SimplifyFindLocalBranchesResponse).Run(t, testFindLocalBranchesPaginationWithIncorrectToken)
-}
+	ctx := testhelper.Context(t)
 
-func testFindLocalBranchesPaginationWithIncorrectToken(t *testing.T, ctx context.Context) {
 	_, repo, _, client := setupRefService(t, ctx)
 
 	limit := 1
@@ -554,10 +439,8 @@ func isOrderedSubset(subset, set []string) bool {
 
 func TestFindLocalBranchesSort(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.SimplifyFindLocalBranchesResponse).Run(t, testFindLocalBranchesSort)
-}
+	ctx := testhelper.Context(t)
 
-func testFindLocalBranchesSort(t *testing.T, ctx context.Context) {
 	testCases := []struct {
 		desc          string
 		relativeOrder []string
@@ -597,16 +480,9 @@ func testFindLocalBranchesSort(t *testing.T, ctx context.Context) {
 				}
 				require.NoError(t, err)
 
-				if featureflag.SimplifyFindLocalBranchesResponse.IsEnabled(ctx) {
-					for _, branch := range r.GetLocalBranches() {
-						branches = append(branches, string(branch.Name))
-					}
-				} else {
-					for _, branch := range r.GetBranches() {
-						branches = append(branches, string(branch.Name))
-					}
+				for _, branch := range r.GetLocalBranches() {
+					branches = append(branches, string(branch.Name))
 				}
-
 			}
 
 			if !isOrderedSubset(testCase.relativeOrder, branches) {
@@ -618,10 +494,8 @@ func testFindLocalBranchesSort(t *testing.T, ctx context.Context) {
 
 func TestEmptyFindLocalBranchesRequest(t *testing.T) {
 	t.Parallel()
-	testhelper.NewFeatureSets(featureflag.SimplifyFindLocalBranchesResponse).Run(t, testEmptyFindLocalBranchesRequest)
-}
+	ctx := testhelper.Context(t)
 
-func testEmptyFindLocalBranchesRequest(t *testing.T, ctx context.Context) {
 	_, client := setupRefServiceWithoutRepo(t)
 
 	rpcRequest := &gitalypb.FindLocalBranchesRequest{}
diff --git a/internal/gitaly/service/ref/testhelper_test.go b/internal/gitaly/service/ref/testhelper_test.go
index 2f66789cf..dda2730ba 100644
--- a/internal/gitaly/service/ref/testhelper_test.go
+++ b/internal/gitaly/service/ref/testhelper_test.go
@@ -92,35 +92,6 @@ func newRefServiceClient(tb testing.TB, serverSocketPath string) (gitalypb.RefSe
 	return gitalypb.NewRefServiceClient(conn), conn
 }
 
-func assertContainsLocalBranch(t *testing.T, branches []*gitalypb.FindLocalBranchResponse, branch *gitalypb.FindLocalBranchResponse) {
-	t.Helper()
-
-	for _, b := range branches {
-		if bytes.Equal(branch.Name, b.Name) {
-			if !findLocalBranchResponsesEqual(branch, b) {
-				t.Errorf("Expected branch\n%v\ngot\n%v", branch, b)
-			}
-
-			testhelper.ProtoEqual(t, branch.Commit, b.Commit)
-			return // Found the branch and it matches. Success!
-		}
-	}
-	t.Errorf("Expected to find branch %q in local branches", branch.Name)
-}
-
-func findLocalBranchCommitAuthorsEqual(a *gitalypb.FindLocalBranchCommitAuthor, b *gitalypb.FindLocalBranchCommitAuthor) bool {
-	return bytes.Equal(a.Name, b.Name) &&
-		bytes.Equal(a.Email, b.Email) &&
-		a.Date.Seconds == b.Date.Seconds
-}
-
-func findLocalBranchResponsesEqual(a *gitalypb.FindLocalBranchResponse, b *gitalypb.FindLocalBranchResponse) bool {
-	return a.CommitId == b.CommitId &&
-		bytes.Equal(a.CommitSubject, b.CommitSubject) &&
-		findLocalBranchCommitAuthorsEqual(a.CommitAuthor, b.CommitAuthor) &&
-		findLocalBranchCommitAuthorsEqual(a.CommitCommitter, b.CommitCommitter)
-}
-
 func assertContainsAllBranchesResponseBranch(t *testing.T, branches []*gitalypb.FindAllBranchesResponse_Branch, branch *gitalypb.FindAllBranchesResponse_Branch) {
 	t.Helper()
 
diff --git a/internal/gitaly/service/ref/util.go b/internal/gitaly/service/ref/util.go
index 309a5ad77..ebeb48a68 100644
--- a/internal/gitaly/service/ref/util.go
+++ b/internal/gitaly/service/ref/util.go
@@ -8,7 +8,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/lines"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 )
 
@@ -88,44 +87,24 @@ func newFindLocalBranchesWriter(stream gitalypb.RefService_FindLocalBranchesServ
 		ctx := stream.Context()
 		var response *gitalypb.FindLocalBranchesResponse
 
-		if featureflag.SimplifyFindLocalBranchesResponse.IsEnabled(ctx) {
-			var branches []*gitalypb.Branch
-
-			for _, ref := range refs {
-				elements, err := parseRef(ref)
-				if err != nil {
-					return err
-				}
-
-				branch, err := buildBranch(ctx, objectReader, elements)
-				if err != nil {
-					return err
-				}
+		var branches []*gitalypb.Branch
 
-				branches = append(branches, branch)
+		for _, ref := range refs {
+			elements, err := parseRef(ref)
+			if err != nil {
+				return err
 			}
 
-			response = &gitalypb.FindLocalBranchesResponse{LocalBranches: branches}
-		} else {
-			var branches []*gitalypb.FindLocalBranchResponse
-
-			for _, ref := range refs {
-				elements, err := parseRef(ref)
-				if err != nil {
-					return err
-				}
-
-				target, err := catfile.GetCommit(ctx, objectReader, git.Revision(elements[1]))
-				if err != nil {
-					return err
-				}
-
-				branches = append(branches, buildLocalBranch(elements[0], target))
+			branch, err := buildBranch(ctx, objectReader, elements)
+			if err != nil {
+				return err
 			}
 
-			response = &gitalypb.FindLocalBranchesResponse{Branches: branches}
+			branches = append(branches, branch)
 		}
 
+		response = &gitalypb.FindLocalBranchesResponse{LocalBranches: branches}
+
 		return stream.Send(response)
 	}
 }
diff --git a/internal/metadata/featureflag/ff_simplify_find_local_branches_response.go b/internal/metadata/featureflag/ff_simplify_find_local_branches_response.go
deleted file mode 100644
index 04dfc9f3b..000000000
--- a/internal/metadata/featureflag/ff_simplify_find_local_branches_response.go
+++ /dev/null
@@ -1,10 +0,0 @@
-package featureflag
-
-// SimplifyFindLocalBranchesResponse enables the simplification of FindLocalBranchesRespnose
-// by adding the generic Branch type in the response
-var SimplifyFindLocalBranchesResponse = NewFeatureFlag(
-	"simplify_find_local_branches_response",
-	"v15.4.0",
-	"https://gitlab.com/gitlab-org/gitaly/-/issues/1294",
-	false,
-)