Merge branch 'pks-find-commits-sha256' into 'master'

commit: Convert FindCommits tests to support SHA256

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

Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>

1 files changed, 570 insertions, 786 deletions

diff --git a/internal/gitaly/service/commit/find_commits_test.go b/internal/gitaly/service/commit/find_commits_test.go
index 8cbef97cf..0f68c6a87 100644
--- a/internal/gitaly/service/commit/find_commits_test.go
+++ b/internal/gitaly/service/commit/find_commits_test.go
@@ -1,5 +1,3 @@
-//go:build !gitaly_test_sha256
-
 package commit
 
 import (
@@ -8,447 +6,645 @@ import (
 	"io"
 	"path/filepath"
 	"testing"
+	"time"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
+	"golang.org/x/text/encoding/charmap"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
-func TestFindCommitsFields(t *testing.T) {
+func TestFindCommits(t *testing.T) {
 	t.Parallel()
-	windows1251Message := testhelper.MustReadFile(t, "testdata/commit-c809470461118b7bcab850f6e9a7ca97ac42f8ea-message.txt")
 
 	ctx := testhelper.Context(t)
-	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
+	cfg, client := setupCommitService(t, ctx)
+
+	writeCommit := func(t *testing.T, repoProto *gitalypb.Repository, opts ...gittest.WriteCommitOption) (git.ObjectID, *gitalypb.GitCommit) {
+		t.Helper()
+
+		repo := localrepo.NewTestRepo(t, cfg, repoProto)
+		repoPath, err := repo.Path()
+		require.NoError(t, err)
+
+		commitID := gittest.WriteCommit(t, cfg, repoPath, opts...)
+		commitProto, err := repo.ReadCommit(ctx, commitID.Revision())
+		require.NoError(t, err)
+
+		return commitID, commitProto
+	}
+
+	type setupData struct {
+		request         *gitalypb.FindCommitsRequest
+		expectedErr     error
+		expectedCommits []*gitalypb.GitCommit
+	}
 
-	testCases := []struct {
-		id       string
-		trailers bool
-		commit   *gitalypb.GitCommit
+	for _, tc := range []struct {
+		desc  string
+		setup func(t *testing.T) setupData
 	}{
 		{
-			id:     "b83d6e391c22777fca1ed3012fce84f633d7fed0",
-			commit: testhelper.GitLabTestCommit("b83d6e391c22777fca1ed3012fce84f633d7fed0"),
+			desc: "unset repository",
+			setup: func(t *testing.T) setupData {
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: nil,
+					},
+					expectedErr: structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
+				}
+			},
 		},
 		{
-			id: "c809470461118b7bcab850f6e9a7ca97ac42f8ea",
-			commit: &gitalypb.GitCommit{
-				Id:      "c809470461118b7bcab850f6e9a7ca97ac42f8ea",
-				Subject: windows1251Message[:len(windows1251Message)-1],
-				Body:    windows1251Message,
-				Author: &gitalypb.CommitAuthor{
-					Name:     []byte("Jacob Vosmaer"),
-					Email:    []byte("jacob@gitlab.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 1512132977},
-					Timezone: []byte("+0100"),
-				},
-				Committer: &gitalypb.CommitAuthor{
-					Name:     []byte("Jacob Vosmaer"),
-					Email:    []byte("jacob@gitlab.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 1512132977},
-					Timezone: []byte("+0100"),
-				},
-				ParentIds: []string{"e63f41fe459e62e1228fcef60d7189127aeba95a"},
-				BodySize:  49,
-				TreeId:    "86ec18bfe87ad42a782fdabd8310f9b7ac750f51",
-				Encoding:  "windows-1251",
+			desc: "paths with empty string",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Paths:      [][]byte{[]byte("")},
+					},
+					expectedErr: status.Error(codes.InvalidArgument, "path is empty string"),
+				}
 			},
 		},
 		{
-			id:     "0999bb770f8dc92ab5581cc0b474b3e31a96bf5c",
-			commit: testhelper.GitLabTestCommit("0999bb770f8dc92ab5581cc0b474b3e31a96bf5c"),
+			desc: "invalid revision",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte("--output=/meow"),
+					},
+					expectedErr: status.Error(codes.InvalidArgument, "revision can't start with '-'"),
+				}
+			},
 		},
+
 		{
-			id:     "77e835ef0856f33c4f0982f84d10bdb0567fe440",
-			commit: testhelper.GitLabTestCommit("77e835ef0856f33c4f0982f84d10bdb0567fe440"),
+			desc: "plain commit",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+				commitID, commit := writeCommit(t, repo)
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Limit:      1,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commit},
+				}
+			},
 		},
 		{
-			id:     "189a6c924013fc3fe40d6f1ec1dc20214183bc97",
-			commit: testhelper.GitLabTestCommit("189a6c924013fc3fe40d6f1ec1dc20214183bc97"),
+			desc: "encoded commit message",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				windows1250, err := charmap.Windows1250.NewEncoder().String("üöä")
+				require.NoError(t, err)
+
+				commitID, commit := writeCommit(t, repo, gittest.WithMessage(windows1250), gittest.WithEncoding("windows-1250"))
+				commit.Body = []byte(windows1250)
+				commit.Encoding = "windows-1250"
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Limit:      1,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commit},
+				}
+			},
 		},
 		{
-			id:       "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
-			trailers: false,
-			commit: &gitalypb.GitCommit{
-				Id:      "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
-				Subject: []byte("Add submodule from gitlab.com"),
-				Body:    []byte("Add submodule from gitlab.com\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"),
-				Author: &gitalypb.CommitAuthor{
-					Name:     []byte("Dmitriy Zaporozhets"),
-					Email:    []byte("dmitriy.zaporozhets@gmail.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 1393491698},
-					Timezone: []byte("+0200"),
-				},
-				Committer: &gitalypb.CommitAuthor{
-					Name:     []byte("Dmitriy Zaporozhets"),
-					Email:    []byte("dmitriy.zaporozhets@gmail.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 1393491698},
-					Timezone: []byte("+0200"),
-				},
-				ParentIds:     []string{"570e7b2abdd848b95f2f578043fc23bd6f6fd24d"},
-				BodySize:      98,
-				SignatureType: gitalypb.SignatureType_PGP,
-				TreeId:        "a6973545d42361b28bfba5ced3b75dba5848b955",
+			desc: "without commit trailers",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				commitID, commit := writeCommit(t, repo, gittest.WithMessage("message\n\nSigned-off-by: me\n"))
+				commit.Trailers = nil
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Trailers:   false,
+						Limit:      1,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commit},
+				}
 			},
 		},
 		{
-			id:       "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
-			trailers: true,
-			commit: &gitalypb.GitCommit{
-				Id:      "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
-				Subject: []byte("Add submodule from gitlab.com"),
-				Body:    []byte("Add submodule from gitlab.com\n\nSigned-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>\n"),
-				Author: &gitalypb.CommitAuthor{
-					Name:     []byte("Dmitriy Zaporozhets"),
-					Email:    []byte("dmitriy.zaporozhets@gmail.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 1393491698},
-					Timezone: []byte("+0200"),
-				},
-				Committer: &gitalypb.CommitAuthor{
-					Name:     []byte("Dmitriy Zaporozhets"),
-					Email:    []byte("dmitriy.zaporozhets@gmail.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 1393491698},
-					Timezone: []byte("+0200"),
-				},
-				ParentIds:     []string{"570e7b2abdd848b95f2f578043fc23bd6f6fd24d"},
-				BodySize:      98,
-				SignatureType: gitalypb.SignatureType_PGP,
-				TreeId:        "a6973545d42361b28bfba5ced3b75dba5848b955",
-				Trailers: []*gitalypb.CommitTrailer{
-					{
-						Key:   []byte("Signed-off-by"),
-						Value: []byte("Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>"),
+			desc: "with commit trailers",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				commitID, commit := writeCommit(t, repo, gittest.WithMessage("message\n\nSigned-off-by: me\n"))
+				commit.Trailers = []*gitalypb.CommitTrailer{
+					{Key: []byte("Signed-off-by"), Value: []byte("me")},
+				}
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Trailers:   true,
+						Limit:      1,
 					},
-				},
+					expectedCommits: []*gitalypb.GitCommit{commit},
+				}
 			},
 		},
 		{
-			id: "c1c67abbaf91f624347bb3ae96eabe3a1b742478",
-			commit: &gitalypb.GitCommit{
-				Id:      "c1c67abbaf91f624347bb3ae96eabe3a1b742478",
-				Subject: []byte("Add file with a _flattable_ path"),
-				Body:    []byte("Add file with a _flattable_ path\n\n\n(cherry picked from commit ce369011c189f62c815f5971d096b26759bab0d1)"),
-				Author: &gitalypb.CommitAuthor{
-					Name:     []byte("Alejandro Rodríguez"),
-					Email:    []byte("alejorro70@gmail.com"),
-					Date:     &timestamppb.Timestamp{Seconds: 1504382739},
-					Timezone: []byte("+0000"),
-				},
-				Committer: &gitalypb.CommitAuthor{
-					Name:     []byte("Drew Blessing"),
-					Email:    []byte("drew@blessing.io"),
-					Date:     &timestamppb.Timestamp{Seconds: 1540823671},
-					Timezone: []byte("+0000"),
-				},
-				ParentIds: []string{"7975be0116940bf2ad4321f79d02a55c5f7779aa"},
-				BodySize:  103,
-				TreeId:    "07f8147e8e73aab6c935c296e8cdc5194dee729b",
+			desc: "by author",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				parentID, parent := writeCommit(t, repo, gittest.WithAuthorName("Some author"))
+				childID, _ := writeCommit(t, repo, gittest.WithParents(parentID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(childID),
+						Author:     []byte("Some author"),
+						Limit:      9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{parent},
+				}
 			},
 		},
-	}
+		{
+			desc: "limit by count",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
 
-	for _, tc := range testCases {
-		t.Run(tc.id, func(t *testing.T) {
-			request := &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte(tc.id),
-				Trailers:   tc.trailers,
-				Limit:      1,
-			}
-			stream, err := client.FindCommits(ctx, request)
-			require.NoError(t, err)
+				commitAID, _ := writeCommit(t, repo)
+				commitBID, commitB := writeCommit(t, repo, gittest.WithParents(commitAID))
+				commitCID, commitC := writeCommit(t, repo, gittest.WithParents(commitBID))
 
-			resp, err := stream.Recv()
-			require.NoError(t, err)
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitCID),
+						Limit:      2,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commitC, commitB},
+				}
+			},
+		},
+		{
+			desc: "default limit returns no commits",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+				commitID, _ := writeCommit(t, repo)
 
-			require.Equal(t, 1, len(resp.Commits), "expected exactly one commit in the first message")
-			firstCommit := resp.Commits[0]
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Limit:      0,
+					},
+				}
+			},
+		},
+		{
+			desc: "limit by path",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
 
-			testhelper.ProtoEqual(t, tc.commit, firstCommit)
+				commitAID, commitA := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "a", Mode: "100644", Content: "a"},
+				))
+				commitBID, _ := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "a", Mode: "100644", Content: "a"},
+					gittest.TreeEntry{Path: "b", Mode: "100644", Content: "b"},
+				), gittest.WithParents(commitAID))
 
-			_, err = stream.Recv()
-			require.Equal(t, io.EOF, err, "there should be no further messages in the stream")
-		})
-	}
-}
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitBID),
+						Paths: [][]byte{
+							[]byte("a"),
+						},
+						Limit: 9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commitA},
+				}
+			},
+		},
+		{
+			desc: "limit by path with wildcard",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
 
-func TestSuccessfulFindCommitsRequest(t *testing.T) {
-	t.Parallel()
+				commitAID, commitA := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "a-foo", Mode: "100644", Content: "a"},
+				))
+				commitBID, _ := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "a-foo", Mode: "100644", Content: "a"},
+					gittest.TreeEntry{Path: "b-foo", Mode: "100644", Content: "b"},
+				), gittest.WithParents(commitAID))
 
-	ctx := testhelper.Context(t)
-	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
-
-	testCases := []struct {
-		desc    string
-		request *gitalypb.FindCommitsRequest
-		// Use 'ids' if you know the exact commits id's that should be returned
-		ids []string
-		// Use minCommits if you don't know the exact commit id's
-		minCommits int
-	}{
-		{
-			desc: "commit by author",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("0031876facac3f2b2702a0e53a26e89939a42209"),
-				Author:     []byte("Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>"),
-				Limit:      20,
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitBID),
+						Paths: [][]byte{
+							[]byte("a-*"),
+						},
+						Limit: 9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commitA},
+				}
 			},
-			ids: []string{"1a0b36b3cdad1d2ee32457c102a8c0b7056fa863"},
 		},
 		{
-			desc: "only revision, limit commits",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("0031876facac3f2b2702a0e53a26e89939a42209"),
-				Limit:      3,
-			},
-			ids: []string{
-				"0031876facac3f2b2702a0e53a26e89939a42209",
-				"bf6e164cac2dc32b1f391ca4290badcbe4ffc5fb",
-				"48ca272b947f49eee601639d743784a176574a09",
+			desc: "limit by path with non-existent literal pathspec",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				commitAID, _ := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "a-foo", Mode: "100644", Content: "a"},
+				))
+				commitBID, _ := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "a-foo", Mode: "100644", Content: "a"},
+					gittest.TreeEntry{Path: "b-foo", Mode: "100644", Content: "b"},
+				), gittest.WithParents(commitAID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitBID),
+						Paths: [][]byte{
+							[]byte("a-*"),
+						},
+						Limit:         9000,
+						GlobalOptions: &gitalypb.GlobalOptions{LiteralPathspecs: true},
+					},
+					expectedCommits: nil,
+				}
 			},
 		},
 		{
-			desc: "revision, default commit limit",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("0031876facac3f2b2702a0e53a26e89939a42209"),
+			desc: "empty revision uses default branch",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				_, commit := writeCommit(t, repo, gittest.WithBranch(git.DefaultBranch))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Limit:      9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commit},
+				}
 			},
 		},
 		{
-			desc: "revision, default commit limit, bypassing rugged walk",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:  repo,
-				Revision:    []byte("0031876facac3f2b2702a0e53a26e89939a42209"),
-				DisableWalk: true,
+			desc: "limit by date",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				oldDate := time.Date(2000, 1, 1, 1, 1, 1, 0, time.UTC)
+				medDate := time.Date(2010, 1, 1, 1, 1, 1, 0, time.UTC)
+				newDate := time.Date(2020, 1, 1, 1, 1, 1, 0, time.UTC)
+
+				oldCommitID, _ := writeCommit(t, repo, gittest.WithCommitterDate(oldDate))
+				medCommitID, medCommit := writeCommit(t, repo, gittest.WithCommitterDate(medDate), gittest.WithParents(oldCommitID))
+				newCommitID, _ := writeCommit(t, repo, gittest.WithCommitterDate(newDate), gittest.WithParents(medCommitID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(newCommitID),
+						Before:     timestamppb.New(newDate.Add(-1 * time.Second)),
+						After:      timestamppb.New(oldDate.Add(+1 * time.Second)),
+						Limit:      9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{medCommit},
+				}
 			},
 		},
 		{
-			desc: "revision and paths",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("0031876facac3f2b2702a0e53a26e89939a42209"),
-				Paths:      [][]byte{[]byte("LICENSE")},
-				Limit:      10,
+			desc: "skip merges",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				mergeBaseID, mergeBase := writeCommit(t, repo)
+				leftCommitID, leftCommit := writeCommit(t, repo, gittest.WithMessage("left"), gittest.WithParents(mergeBaseID))
+				rightCommitID, rightCommit := writeCommit(t, repo, gittest.WithMessage("right"), gittest.WithParents(mergeBaseID))
+				mergeCommitID, _ := writeCommit(t, repo, gittest.WithParents(leftCommitID, rightCommitID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(mergeCommitID),
+						SkipMerges: true,
+						Limit:      9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{leftCommit, rightCommit, mergeBase},
+				}
 			},
-			ids: []string{"1a0b36b3cdad1d2ee32457c102a8c0b7056fa863"},
 		},
 		{
-			desc: "revision and wildcard pathspec",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("0031876facac3f2b2702a0e53a26e89939a42209"),
-				Paths:      [][]byte{[]byte("LICEN*")},
-				Limit:      10,
+			desc: "follow renames",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				commitAID, commitA := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "rename-me", Mode: "100644", Content: "something\n"},
+				))
+				commitBID, commitB := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "renamed", Mode: "100644", Content: "something\n"},
+				), gittest.WithParents(commitAID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitBID),
+						Paths: [][]byte{
+							[]byte("renamed"),
+						},
+						Follow: true,
+						Limit:  9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commitB, commitA},
+				}
 			},
-			ids: []string{"1a0b36b3cdad1d2ee32457c102a8c0b7056fa863"},
 		},
 		{
-			desc: "revision and non-existent literal pathspec",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:    repo,
-				Revision:      []byte("0031876facac3f2b2702a0e53a26e89939a42209"),
-				Paths:         [][]byte{[]byte("LICEN*")},
-				Limit:         10,
-				GlobalOptions: &gitalypb.GlobalOptions{LiteralPathspecs: true},
+			desc: "all references",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				_, commitA := writeCommit(t, repo, gittest.WithMessage("a"), gittest.WithBranch("a"))
+				_, commitB := writeCommit(t, repo, gittest.WithMessage("b"), gittest.WithBranch("b"))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						All:        true,
+						Limit:      9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commitB, commitA},
+				}
 			},
-			ids: []string{},
 		},
 		{
-			desc: "empty revision",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Limit:      35,
+			desc: "first parents",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				mergeBaseID, mergeBase := writeCommit(t, repo)
+				leftCommitID, leftCommit := writeCommit(t, repo, gittest.WithMessage("left"), gittest.WithParents(mergeBaseID))
+				rightCommitID, _ := writeCommit(t, repo, gittest.WithMessage("right"), gittest.WithParents(mergeBaseID))
+				mergeCommitID, mergeCommit := writeCommit(t, repo, gittest.WithParents(leftCommitID, rightCommitID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository:  repo,
+						Revision:    []byte(mergeCommitID),
+						FirstParent: true,
+						Limit:       9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{mergeCommit, leftCommit, mergeBase},
+				}
 			},
-			minCommits: 35,
 		},
 		{
-			desc: "before and after",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Before:     &timestamppb.Timestamp{Seconds: 1483225200},
-				After:      &timestamppb.Timestamp{Seconds: 1472680800},
-				Limit:      10,
-			},
-			ids: []string{
-				"b83d6e391c22777fca1ed3012fce84f633d7fed0",
-				"498214de67004b1da3d820901307bed2a68a8ef6",
+			desc: "order by none",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				mergeBaseID, mergeBase := writeCommit(t, repo)
+				leftCommitID, leftCommit := writeCommit(t, repo, gittest.WithParents(mergeBaseID), gittest.WithCommitterDate(time.Date(2020, 1, 1, 1, 1, 1, 0, time.UTC)))
+				rightCommitID, rightCommit := writeCommit(t, repo, gittest.WithParents(mergeBaseID), gittest.WithCommitterDate(time.Date(2000, 1, 1, 1, 1, 1, 0, time.UTC)))
+				mergeCommitID, mergeCommit := writeCommit(t, repo, gittest.WithParents(leftCommitID, rightCommitID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(mergeCommitID),
+						Order:      gitalypb.FindCommitsRequest_NONE,
+						Limit:      9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{mergeCommit, leftCommit, mergeBase, rightCommit},
+				}
 			},
 		},
 		{
-			desc: "no merges",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"),
-				SkipMerges: true,
-				Limit:      10,
-			},
-			ids: []string{
-				"4a24d82dbca5c11c61556f3b35ca472b7463187e",
-				"498214de67004b1da3d820901307bed2a68a8ef6",
-				"38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e",
-				"c347ca2e140aa667b968e51ed0ffe055501fe4f4",
-				"d59c60028b053793cecfb4022de34602e1a9218e",
-				"a5391128b0ef5d21df5dd23d98557f4ef12fae20",
-				"54fcc214b94e78d7a41a9a8fe6d87a5e59500e51",
-				"048721d90c449b244b7b4c53a9186b04330174ec",
-				"5f923865dde3436854e9ceb9cdb7815618d4e849",
-				"2ea1f3dec713d940208fb5ce4a38765ecb5d3f73",
+			desc: "order by topo",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				mergeBaseID, mergeBase := writeCommit(t, repo)
+				leftCommitID, leftCommit := writeCommit(t, repo, gittest.WithParents(mergeBaseID), gittest.WithCommitterDate(time.Date(2020, 1, 1, 1, 1, 1, 0, time.UTC)))
+				rightCommitID, rightCommit := writeCommit(t, repo, gittest.WithParents(mergeBaseID), gittest.WithCommitterDate(time.Date(2000, 1, 1, 1, 1, 1, 0, time.UTC)))
+				mergeCommitID, mergeCommit := writeCommit(t, repo, gittest.WithParents(leftCommitID, rightCommitID))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(mergeCommitID),
+						Order:      gitalypb.FindCommitsRequest_TOPO,
+						Limit:      9000,
+					},
+					// Note how the right commit is sorted immediately after the merge commit now
+					// and compare this with the preceding test where it is sorted towards the end.
+					expectedCommits: []*gitalypb.GitCommit{mergeCommit, rightCommit, leftCommit, mergeBase},
+				}
 			},
 		},
 		{
-			desc: "following renames",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("94bb47ca1297b7b3731ff2a36923640991e9236f"),
-				Paths:      [][]byte{[]byte("CHANGELOG.md")},
-				Follow:     true,
-				Limit:      10,
-			},
-			ids: []string{
-				"94bb47ca1297b7b3731ff2a36923640991e9236f",
-				"5f923865dde3436854e9ceb9cdb7815618d4e849",
-				"913c66a37b4a45b9769037c55c2d238bd0942d2e",
+			desc: "ambiguous reference",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				// The second commit is created with the first commit's object ID as branch name, thus
+				// creating ambiguity.
+				commitAID, commitA := writeCommit(t, repo, gittest.WithMessage("a"))
+				writeCommit(t, repo, gittest.WithMessage("b"), gittest.WithBranch(commitAID.String()))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitAID),
+						Limit:      9000,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commitA},
+				}
 			},
 		},
 		{
-			desc: "all refs",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				All:        true,
-				Limit:      90,
+			desc: "exceeding offset",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				commitID, _ := writeCommit(t, repo, gittest.WithMessage("a"))
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Limit:      9000,
+						Offset:     9000,
+					},
+					expectedCommits: nil,
+				}
 			},
-			minCommits: 90,
 		},
 		{
-			desc: "first parents",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:  repo,
-				Revision:    []byte("e63f41fe459e62e1228fcef60d7189127aeba95a"),
-				FirstParent: true,
-				Limit:       10,
-			},
-			ids: []string{
-				"e63f41fe459e62e1228fcef60d7189127aeba95a",
-				"b83d6e391c22777fca1ed3012fce84f633d7fed0",
-				"1b12f15a11fc6e62177bef08f47bc7b5ce50b141",
-				"6907208d755b60ebeacb2e9dfea74c92c3449a1f",
-				"281d3a76f31c812dbf48abce82ccf6860adedd81",
-				"54fcc214b94e78d7a41a9a8fe6d87a5e59500e51",
-				"be93687618e4b132087f430a4d8fc3a609c9b77c",
-				"5f923865dde3436854e9ceb9cdb7815618d4e849",
-				"d2d430676773caa88cdaf7c55944073b2fd5561a",
-				"59e29889be61e6e0e5e223bfa9ac2721d31605b8",
+			desc: "referenced by tags and branches",
+			setup: func(t *testing.T) setupData {
+				repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+
+				commitID, commit := writeCommit(t, repo, gittest.WithBranch("branch"))
+				gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision())
+
+				commit.ReferencedBy = [][]byte{
+					[]byte("refs/tags/v1.0.0"),
+					[]byte("refs/heads/branch"),
+				}
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Limit:      9000,
+						IncludeReferencedBy: [][]byte{
+							[]byte("refs/heads/"),
+							[]byte("refs/tags"),
+						},
+					},
+					expectedCommits: []*gitalypb.GitCommit{commit},
+				}
 			},
 		},
 		{
-			// Ordering by none implies that commits appear in
-			// chronological order:
-			//
-			// git log --graph -n 6 --pretty=format:"%h" --date-order 0031876
-			// *   0031876
-			// |\
-			// * | bf6e164
-			// | * 48ca272
-			// * | 9d526f8
-			// | * 335bc94
-			// |/
-			// * 1039376
-			desc: "ordered by none",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("0031876"),
-				Order:      gitalypb.FindCommitsRequest_NONE,
-				Limit:      6,
-			},
-			ids: []string{
-				"0031876facac3f2b2702a0e53a26e89939a42209",
-				"bf6e164cac2dc32b1f391ca4290badcbe4ffc5fb",
-				"48ca272b947f49eee601639d743784a176574a09",
-				"9d526f87b82e2b2fd231ca44c95508e5e85624ca",
-				"335bc94d5b7369b10251e612158da2e4a4aaa2a5",
-				"1039376155a0d507eba0ea95c29f8f5b983ea34b",
+			desc: "referenced by tags only",
+			setup: func(t *testing.T) setupData {
+				repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
+
+				commitID, commit := writeCommit(t, repo, gittest.WithBranch("branch"))
+				gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision())
+				commit.ReferencedBy = [][]byte{
+					[]byte("refs/tags/v1.0.0"),
+				}
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Limit:      9000,
+						IncludeReferencedBy: [][]byte{
+							[]byte("refs/tags"),
+						},
+					},
+					expectedCommits: []*gitalypb.GitCommit{commit},
+				}
 			},
 		},
 		{
-			// When ordering by topology, all commit children will
-			// be shown before parents:
-			//
-			// git log --graph -n 6 --pretty=format:"%h" --topo-order 0031876
-			// *   0031876
-			// |\
-			// | * 48ca272
-			// | * 335bc94
-			// * | bf6e164
-			// * | 9d526f8
-			// |/
-			// * 1039376
-			desc: "ordered by topo",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Revision:   []byte("0031876"),
-				Order:      gitalypb.FindCommitsRequest_TOPO,
-				Limit:      6,
-			},
-			ids: []string{
-				"0031876facac3f2b2702a0e53a26e89939a42209",
-				"48ca272b947f49eee601639d743784a176574a09",
-				"335bc94d5b7369b10251e612158da2e4a4aaa2a5",
-				"bf6e164cac2dc32b1f391ca4290badcbe4ffc5fb",
-				"9d526f87b82e2b2fd231ca44c95508e5e85624ca",
-				"1039376155a0d507eba0ea95c29f8f5b983ea34b",
+			desc: "referenced by HEAD",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
+
+				commitID, commit := writeCommit(t, repo, gittest.WithBranch(git.DefaultBranch))
+				commit.ReferencedBy = [][]byte{
+					[]byte("HEAD"),
+				}
+
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository: repo,
+						Revision:   []byte(commitID),
+						Limit:      9000,
+						IncludeReferencedBy: [][]byte{
+							[]byte("HEAD"),
+						},
+					},
+					expectedCommits: []*gitalypb.GitCommit{commit},
+				}
 			},
 		},
-	}
+		{
+			desc: "with short stats",
+			setup: func(t *testing.T) setupData {
+				repo, _ := gittest.CreateRepository(t, ctx, cfg)
 
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			stream, err := client.FindCommits(ctx, tc.request)
-			require.NoError(t, err)
+				commitAID, commitA := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "deleted", Mode: "100644", Content: "to be deleted\n"},
+					gittest.TreeEntry{Path: "modified", Mode: "100644", Content: "to be modified\n"},
+				))
+				commitA.ShortStats = &gitalypb.CommitStatInfo{
+					Additions: 2, ChangedFiles: 2,
+				}
 
-			var ids []string
-			for err == nil {
-				var resp *gitalypb.FindCommitsResponse
-				resp, err = stream.Recv()
-				for _, c := range resp.GetCommits() {
-					ids = append(ids, c.Id)
+				commitBID, commitB := writeCommit(t, repo, gittest.WithTreeEntries(
+					gittest.TreeEntry{Path: "created", Mode: "100644", Content: "created\n"},
+					gittest.TreeEntry{Path: "modified", Mode: "100644", Content: "modified\n"},
+				), gittest.WithParents(commitAID))
+				commitB.ShortStats = &gitalypb.CommitStatInfo{
+					Additions: 2, Deletions: 2, ChangedFiles: 3,
 				}
-			}
-			require.Equal(t, io.EOF, err)
 
-			if tc.minCommits > 0 {
-				require.True(t, len(ids) >= tc.minCommits, "expected at least %d commits, got %d", tc.minCommits, len(ids))
-				return
-			}
+				return setupData{
+					request: &gitalypb.FindCommitsRequest{
+						Repository:       repo,
+						Revision:         []byte(commitBID),
+						Limit:            9000,
+						IncludeShortstat: true,
+					},
+					expectedCommits: []*gitalypb.GitCommit{commitB, commitA},
+				}
+			},
+		},
+	} {
+		tc := tc
 
-			require.Equal(t, len(tc.ids), len(ids))
-			for i, id := range tc.ids {
-				require.Equal(t, id, ids[i])
-			}
+		t.Run(tc.desc, func(t *testing.T) {
+			t.Parallel()
+
+			setup := tc.setup(t)
+
+			commits, err := getCommits(t, ctx, client, setup.request)
+			testhelper.RequireGrpcError(t, setup.expectedErr, err)
+			testhelper.ProtoEqual(t, setup.expectedCommits, commits)
 		})
 	}
 }
 
-func TestSuccessfulFindCommitsRequestWithAltGitObjectDirs(t *testing.T) {
+func TestFindCommits_quarantine(t *testing.T) {
 	t.Parallel()
 
 	ctx := testhelper.Context(t)
-	cfg, repo, repoPath, client := setupCommitServiceWithRepo(t, ctx)
+	cfg, client := setupCommitService(t, ctx)
 
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
 	altObjectsDir := "./alt-objects"
 	commitID := gittest.WriteCommit(t, cfg, repoPath,
 		gittest.WithAlternateObjectDirectory(filepath.Join(repoPath, altObjectsDir)),
 	)
 
-	testCases := []struct {
+	for _, tc := range []struct {
 		desc          string
 		altDirs       []string
 		expectedCount int
@@ -463,485 +659,73 @@ func TestSuccessfulFindCommitsRequestWithAltGitObjectDirs(t *testing.T) {
 			altDirs:       []string{},
 			expectedCount: 0,
 		},
-	}
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			repo.GitAlternateObjectDirectories = tc.altDirs
 
-	for _, testCase := range testCases {
-		t.Run(testCase.desc, func(t *testing.T) {
-			repo.GitAlternateObjectDirectories = testCase.altDirs
-			request := &gitalypb.FindCommitsRequest{
+			commits, err := getCommits(t, ctx, client, &gitalypb.FindCommitsRequest{
 				Repository: repo,
 				Revision:   []byte(commitID.String()),
 				Limit:      1,
-			}
-
-			c, err := client.FindCommits(ctx, request)
+			})
 			require.NoError(t, err)
-
-			receivedCommits := getAllCommits(t, func() (gitCommitsGetter, error) { return c.Recv() })
-
-			require.Equal(t, testCase.expectedCount, len(receivedCommits), "number of commits received")
+			require.Len(t, commits, tc.expectedCount)
 		})
 	}
 }
 
-func TestSuccessfulFindCommitsRequestWithAmbiguousRef(t *testing.T) {
+func TestFindCommits_followWithOffset(t *testing.T) {
 	t.Parallel()
 
 	ctx := testhelper.Context(t)
-	cfg, repo, repoPath, client := setupCommitServiceWithRepo(t, ctx)
+	cfg, client := setupCommitService(t, ctx)
 
-	// These are arbitrary SHAs in the repository. The important part is
-	// that we create a branch using one of them with a different SHA so
-	// that Git detects an ambiguous reference.
-	branchName := "1e292f8fedd741b75372e19097c76d327140c312"
-	commitSha := "6907208d755b60ebeacb2e9dfea74c92c3449a1f"
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
-	gittest.Exec(t, cfg, "-C", repoPath, "branch", branchName, commitSha)
+	var commitID git.ObjectID
+	for i := 0; i < 10; i++ {
+		var parents []git.ObjectID
+		if commitID != "" {
+			parents = []git.ObjectID{commitID}
+		}
 
-	request := &gitalypb.FindCommitsRequest{
-		Repository: repo,
-		Revision:   []byte(branchName),
-		Limit:      1,
+		commitID = gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(parents...), gittest.WithTreeEntries(
+			gittest.TreeEntry{Path: fmt.Sprintf("%d", i), Mode: "100644", Content: "content that is being moved around\n"},
+		))
 	}
 
-	c, err := client.FindCommits(ctx, request)
-	require.NoError(t, err)
-
-	receivedCommits := getAllCommits(t, func() (gitCommitsGetter, error) { return c.Recv() })
-
-	require.Equal(t, 1, len(receivedCommits), "number of commits received")
-}
-
-func TestFailureFindCommitsRequest(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
-
-	testCases := []struct {
-		desc        string
-		request     *gitalypb.FindCommitsRequest
-		expectedErr error
-	}{
-		{
-			desc: "no repository provided",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: nil,
-				Revision:   []byte("HEAD"),
-			},
-			expectedErr: structerr.NewInvalidArgument("%w", storage.ErrRepositoryNotSet),
-		},
-		{
-			desc: "empty path string",
-			request: &gitalypb.FindCommitsRequest{
-				Repository: repo,
-				Paths:      [][]byte{[]byte("")},
-			},
-			expectedErr: status.Error(codes.InvalidArgument, "path is empty string"),
-		},
-		{
-			desc: "invalid revision",
-			request: &gitalypb.FindCommitsRequest{
+	for offset := 0; offset < 10; offset++ {
+		t.Run(fmt.Sprintf("testing with offset %d", offset), func(t *testing.T) {
+			commits, err := getCommits(t, ctx, client, &gitalypb.FindCommitsRequest{
 				Repository: repo,
-				Revision:   []byte("--output=/meow"),
-				Limit:      1,
-			},
-			expectedErr: status.Error(codes.InvalidArgument, "revision can't start with '-'"),
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			stream, err := client.FindCommits(ctx, tc.request)
+				Revision:   []byte(commitID),
+				Follow:     true,
+				Paths:      [][]byte{[]byte("9")},
+				Offset:     int32(offset),
+				Limit:      9000,
+			})
 			require.NoError(t, err)
-
-			for err == nil {
-				_, err = stream.Recv()
-			}
-
-			testhelper.RequireGrpcError(t, tc.expectedErr, err)
-		})
-	}
-}
-
-func TestFindCommitsRequestWithFollowAndOffset(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
-
-	request := &gitalypb.FindCommitsRequest{
-		Repository: repo,
-		Follow:     true,
-		Paths:      [][]byte{[]byte("CHANGELOG")},
-		Limit:      100,
-	}
-	allCommits := getCommits(t, ctx, request, client)
-	totalCommits := len(allCommits)
-
-	for offset := 0; offset < totalCommits; offset++ {
-		t.Run(fmt.Sprintf("testing with offset %d", offset), func(t *testing.T) {
-			ctx := testhelper.Context(t)
-			request.Offset = int32(offset)
-			request.Limit = int32(totalCommits)
-			commits := getCommits(t, ctx, request, client)
-			assert.Len(t, commits, totalCommits-offset)
-			assert.Equal(t, allCommits[offset:], commits)
+			require.Len(t, commits, 10-offset)
 		})
 	}
 }
 
-func TestFindCommitsWithExceedingOffset(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
-
-	stream, err := client.FindCommits(ctx, &gitalypb.FindCommitsRequest{
-		Repository: repo,
-		Follow:     true,
-		Paths:      [][]byte{[]byte("CHANGELOG")},
-		Offset:     9000,
-	})
-	require.NoError(t, err)
-
-	response, err := stream.Recv()
-	require.Nil(t, response)
-	require.EqualError(t, err, "EOF")
-}
-
-func getCommits(t *testing.T, ctx context.Context, request *gitalypb.FindCommitsRequest, client gitalypb.CommitServiceClient) []*gitalypb.GitCommit {
+func getCommits(t *testing.T, ctx context.Context, client gitalypb.CommitServiceClient, request *gitalypb.FindCommitsRequest) ([]*gitalypb.GitCommit, error) {
 	t.Helper()
 
 	stream, err := client.FindCommits(ctx, request)
 	require.NoError(t, err)
 
-	var commits []*gitalypb.GitCommit
-	for err == nil {
-		var resp *gitalypb.FindCommitsResponse
-		resp, err = stream.Recv()
-		commits = append(commits, resp.GetCommits()...)
-	}
-
-	require.Equal(t, io.EOF, err)
-	return commits
-}
-
-func TestFindCommits_withReferencedBy(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-	cfg, repo, repoPath, client := setupCommitServiceWithRepo(t, ctx)
-
-	type commitInfo struct {
-		id         string
-		shortStats *gitalypb.CommitStatInfo
-		refs       [][]byte
-	}
-
-	gittest.WriteTag(t, cfg, repoPath, "a-tag", "570e7b2abdd848b95f2f578043fc23bd6f6fd24d")
-	gittest.WriteTag(t, cfg, repoPath, "another-tag", "570e7b2abdd848b95f2f578043fc23bd6f6fd24d")
-
-	testCases := []struct {
-		desc        string
-		request     *gitalypb.FindCommitsRequest
-		commitStats []commitInfo
-	}{
-		{
-			desc: "with stats and trailers",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:          repo,
-				Revision:            []byte("4cd80ccab63c82b4bad16faa5193fbd2aa06df40"),
-				Limit:               3,
-				SkipMerges:          true,
-				IncludeShortstat:    true,
-				Trailers:            true,
-				IncludeReferencedBy: [][]byte{[]byte("refs/tags"), []byte("refs/heads")},
-			},
-			commitStats: []commitInfo{
-				{
-					id: "4cd80ccab63c82b4bad16faa5193fbd2aa06df40",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    0,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-					refs: nil,
-				},
-				{
-					id: "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    4,
-						Deletions:    0,
-						ChangedFiles: 2,
-					},
-					refs: [][]byte{[]byte("refs/tags/v1.1.0"), []byte("refs/heads/improve/awesome")},
-				},
-				{
-					id: "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    11,
-						Deletions:    6,
-						ChangedFiles: 2,
-					},
-					refs: [][]byte{[]byte("refs/tags/another-tag"), []byte("refs/tags/a-tag")},
-				},
-			},
-		},
-		{
-			desc: "without stats or trailers",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:          repo,
-				Revision:            []byte("4cd80ccab63c82b4bad16faa5193fbd2aa06df40"),
-				Limit:               3,
-				IncludeShortstat:    false,
-				SkipMerges:          true,
-				Trailers:            false,
-				IncludeReferencedBy: [][]byte{[]byte("refs/tags"), []byte("refs/heads/")},
-			},
-			commitStats: []commitInfo{
-				{
-					id:         "4cd80ccab63c82b4bad16faa5193fbd2aa06df40",
-					shortStats: nil,
-					refs:       nil,
-				},
-				{
-					id:         "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
-					shortStats: nil,
-					refs:       [][]byte{[]byte("refs/tags/v1.1.0"), []byte("refs/heads/improve/awesome")},
-				},
-				{
-					id:         "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
-					shortStats: nil,
-					refs:       [][]byte{[]byte("refs/tags/another-tag"), []byte("refs/tags/a-tag")},
-				},
-			},
-		},
-		{
-			desc: "only tags",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:          repo,
-				Revision:            []byte("5937ac0a7beb003549fc5fd26fc247adbce4a52e"),
-				Limit:               1,
-				IncludeShortstat:    false,
-				SkipMerges:          true,
-				Trailers:            false,
-				IncludeReferencedBy: [][]byte{[]byte("refs/tags")},
-			},
-			commitStats: []commitInfo{
-				{
-					id:         "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
-					shortStats: nil,
-					refs:       [][]byte{[]byte("refs/tags/v1.1.0")},
-				},
-			},
-		},
-		{
-			desc: "with HEAD",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:          repo,
-				Revision:            []byte("refs/heads/master"),
-				Limit:               1,
-				IncludeReferencedBy: [][]byte{[]byte("HEAD")},
-			},
-			commitStats: []commitInfo{
-				{
-					id:   "1e292f8fedd741b75372e19097c76d327140c312",
-					refs: [][]byte{[]byte("HEAD")},
-				},
-			},
-		},
-		{
-			desc: "with HEAD pointing to another branch in the set",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:          repo,
-				Revision:            []byte("refs/heads/master"),
-				Limit:               1,
-				IncludeReferencedBy: [][]byte{[]byte("refs/heads"), []byte("HEAD")},
-			},
-			commitStats: []commitInfo{
-				{
-					id:   "1e292f8fedd741b75372e19097c76d327140c312",
-					refs: [][]byte{[]byte("HEAD"), []byte("refs/heads/master")},
-				},
-			},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			commits := getCommits(t, ctx, tc.request, client)
-			assert.Equal(t, len(tc.commitStats), len(commits))
-
-			for i, commit := range commits {
-				assert.Equal(t, tc.commitStats[i].id, commit.Id)
-				assert.Equal(t, tc.commitStats[i].shortStats, commit.ShortStats)
-				assert.Equal(t, tc.commitStats[i].refs, commit.ReferencedBy)
-			}
-		})
-	}
-}
-
-func TestFindCommits_withStats(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-	_, repo, _, client := setupCommitServiceWithRepo(t, ctx)
-
-	type commitInfo struct {
-		id         string
-		shortStats *gitalypb.CommitStatInfo
-	}
-
-	testCases := []struct {
-		desc        string
-		request     *gitalypb.FindCommitsRequest
-		commitStats []commitInfo
-	}{
-		{
-			desc: "no merges",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:       repo,
-				Limit:            10,
-				SkipMerges:       true,
-				IncludeShortstat: true,
-				Trailers:         true,
-			},
-			commitStats: []commitInfo{
-				{
-					id: "c1c67abbaf91f624347bb3ae96eabe3a1b742478",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    0,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id: "c84ff944ff4529a70788a5e9003c2b7feae29047",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    0,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id: "55bc176024cfa3baaceb71db584c7e5df900ea65",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    1,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id: "4a24d82dbca5c11c61556f3b35ca472b7463187e",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    12,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id: "498214de67004b1da3d820901307bed2a68a8ef6",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    1,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id: "38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    1,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id: "c347ca2e140aa667b968e51ed0ffe055501fe4f4",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    6,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id: "d59c60028b053793cecfb4022de34602e1a9218e",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    0,
-						Deletions:    6,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id: "a5391128b0ef5d21df5dd23d98557f4ef12fae20",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    6,
-						Deletions:    0,
-						ChangedFiles: 2,
-					},
-				},
-				{
-					id: "54fcc214b94e78d7a41a9a8fe6d87a5e59500e51",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    1,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-			},
-		},
-		{
-			desc: "with merges",
-			request: &gitalypb.FindCommitsRequest{
-				Repository:       repo,
-				Limit:            5,
-				IncludeShortstat: true,
-				Trailers:         true,
-			},
-			commitStats: []commitInfo{
-				{
-					id:         "1e292f8fedd741b75372e19097c76d327140c312",
-					shortStats: nil,
-				},
-				{
-					id: "c1c67abbaf91f624347bb3ae96eabe3a1b742478",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    0,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id:         "7975be0116940bf2ad4321f79d02a55c5f7779aa",
-					shortStats: nil,
-				},
-				{
-					id: "c84ff944ff4529a70788a5e9003c2b7feae29047",
-					shortStats: &gitalypb.CommitStatInfo{
-						Additions:    0,
-						Deletions:    0,
-						ChangedFiles: 1,
-					},
-				},
-				{
-					id:         "60ecb67744cb56576c30214ff52294f8ce2def98",
-					shortStats: nil,
-				},
-			},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			commits := getCommits(t, ctx, tc.request, client)
-			assert.Equal(t, len(tc.commitStats), len(commits))
+	return testhelper.ReceiveAndFold(stream.Recv, func(
+		result []*gitalypb.GitCommit,
+		response *gitalypb.FindCommitsResponse,
+	) []*gitalypb.GitCommit {
+		if response == nil {
+			return result
+		}
 
-			for i, commit := range commits {
-				assert.Equal(t, tc.commitStats[i].id, commit.Id)
-				assert.Equal(t, tc.commitStats[i].shortStats, commit.ShortStats)
-			}
-		})
-	}
+		return append(result, response.GetCommits()...)
+	})
 }
 
 func BenchmarkCommitStats(b *testing.B) {