diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-03-01 15:24:07 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2023-03-07 09:35:33 +0300 |
commit | a77dab53579a40e0732bd881c041fc4bcb5eca57 (patch) | |
tree | b46b00b5af5bc167b8d7b7c44a70711736d37dce /internal | |
parent | 2b2e7546b308197ddeb577d20567312c29b6984a (diff) |
blob: Convert ListAllLFSPointers tests to be table-driven
Convert tests for the ListAllLFSPointers RPC to be table-driven.
Diffstat (limited to 'internal')
-rw-r--r-- | internal/gitaly/service/blob/lfs_pointers_test.go | 278 |
1 files changed, 155 insertions, 123 deletions
diff --git a/internal/gitaly/service/blob/lfs_pointers_test.go b/internal/gitaly/service/blob/lfs_pointers_test.go index 44f3fca04..de351061b 100644 --- a/internal/gitaly/service/blob/lfs_pointers_test.go +++ b/internal/gitaly/service/blob/lfs_pointers_test.go @@ -6,8 +6,6 @@ import ( "bytes" "context" "io" - "os" - "path/filepath" "sort" "strings" "testing" @@ -17,8 +15,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" + "gitlab.com/gitlab-org/gitaly/v15/internal/git/quarantine" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper/perm" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" @@ -183,131 +181,174 @@ func TestListLFSPointers(t *testing.T) { } func TestListAllLFSPointers(t *testing.T) { - ctx := testhelper.Context(t) - - receivePointers := func(t *testing.T, stream gitalypb.BlobService_ListAllLFSPointersClient) []*gitalypb.LFSPointer { - t.Helper() + t.Parallel() - var pointers []*gitalypb.LFSPointer - for { - resp, err := stream.Recv() - if err == io.EOF { - break - } - require.Nil(t, err) - pointers = append(pointers, resp.GetLfsPointers()...) - } - return pointers - } + ctx := testhelper.Context(t) + cfg, client := setupWithoutRepo(t, ctx) lfsPointerContents := `version https://git-lfs.github.com/spec/v1 oid sha256:1111111111111111111111111111111111111111111111111111111111111111 size 12345` - t.Run("normal repository", func(t *testing.T) { - _, repo, _, client := setupWithLFS(t, ctx) - stream, err := client.ListAllLFSPointers(ctx, &gitalypb.ListAllLFSPointersRequest{ - Repository: repo, - }) - require.NoError(t, err) - lfsPointersEqual(t, []*gitalypb.LFSPointer{ - lfsPointers[lfsPointer1], - lfsPointers[lfsPointer2], - lfsPointers[lfsPointer3], - lfsPointers[lfsPointer4], - lfsPointers[lfsPointer5], - lfsPointers[lfsPointer6], - }, receivePointers(t, stream)) - }) + type setupData struct { + repo *gitalypb.Repository + expectedErr error + expectedPointers []*gitalypb.LFSPointer + } - t.Run("dangling LFS pointer", func(t *testing.T) { - cfg, repo, repoPath, client := setupWithLFS(t, ctx) + for _, tc := range []struct { + desc string + setup func(t *testing.T) setupData + }{ + { + desc: "missing repository", + setup: func(t *testing.T) setupData { + return setupData{ + expectedErr: structerr.NewInvalidArgument(testhelper.GitalyOrPraefect( + "empty Repository", + "repo scoped: empty Repository", + )), + } + }, + }, + { + desc: "normal repository", + setup: func(t *testing.T) setupData { + repo, _, _ := setupRepoWithLFS(t, ctx, cfg) + + return setupData{ + repo: repo, + expectedPointers: []*gitalypb.LFSPointer{ + lfsPointers[lfsPointer1], + lfsPointers[lfsPointer2], + lfsPointers[lfsPointer3], + lfsPointers[lfsPointer4], + lfsPointers[lfsPointer5], + lfsPointers[lfsPointer6], + }, + } + }, + }, + { + desc: "dangling pointer", + setup: func(t *testing.T) setupData { + repo, repoPath, _ := setupRepoWithLFS(t, ctx, cfg) + + hash := gittest.ExecOpts(t, cfg, gittest.ExecConfig{Stdin: strings.NewReader(lfsPointerContents)}, + "-C", repoPath, "hash-object", "-w", "--stdin", + ) + lfsPointerOID := text.ChompBytes(hash) + + return setupData{ + repo: repo, + expectedPointers: []*gitalypb.LFSPointer{ + { + Oid: lfsPointerOID, + Data: []byte(lfsPointerContents), + Size: int64(len(lfsPointerContents)), + }, + lfsPointers[lfsPointer1], + lfsPointers[lfsPointer2], + lfsPointers[lfsPointer3], + lfsPointers[lfsPointer4], + lfsPointers[lfsPointer5], + lfsPointers[lfsPointer6], + }, + } + }, + }, + { + desc: "empty quarantine directory", + setup: func(t *testing.T) setupData { + repo, _, _ := setupRepoWithLFS(t, ctx, cfg) - hash := gittest.ExecOpts(t, cfg, gittest.ExecConfig{Stdin: strings.NewReader(lfsPointerContents)}, - "-C", repoPath, "hash-object", "-w", "--stdin", - ) - lfsPointerOID := text.ChompBytes(hash) + quarantineDir, err := quarantine.New(ctx, gittest.RewrittenRepository(t, ctx, cfg, repo), config.NewLocator(cfg)) + require.NoError(t, err) - stream, err := client.ListAllLFSPointers(ctx, &gitalypb.ListAllLFSPointersRequest{ - Repository: repo, - }) - require.NoError(t, err) - lfsPointersEqual(t, []*gitalypb.LFSPointer{ - { - Oid: lfsPointerOID, - Data: []byte(lfsPointerContents), - Size: int64(len(lfsPointerContents)), - }, - lfsPointers[lfsPointer1], - lfsPointers[lfsPointer2], - lfsPointers[lfsPointer3], - lfsPointers[lfsPointer4], - lfsPointers[lfsPointer5], - lfsPointers[lfsPointer6], - }, receivePointers(t, stream)) - }) + repo.GitObjectDirectory = quarantineDir.QuarantinedRepo().GitObjectDirectory - t.Run("quarantine", func(t *testing.T) { - cfg, repoProto, repoPath, client := setupWithLFS(t, ctx) - - // We're emulating the case where git is receiving data via a push, where objects - // are stored in a separate quarantine environment. In this case, LFS pointer checks - // may want to inspect all newly pushed objects, denoted by a repository proto - // message which only has its object directory set to the quarantine directory. - quarantineDir := "objects/incoming-123456" - require.NoError(t, os.Mkdir(filepath.Join(repoPath, quarantineDir), perm.PublicDir)) - repoProto.GitObjectDirectory = quarantineDir - repoProto.GitAlternateObjectDirectories = nil - - // There are no quarantined objects yet, so none should be returned here. - stream, err := client.ListAllLFSPointers(ctx, &gitalypb.ListAllLFSPointersRequest{ - Repository: repoProto, - }) - require.NoError(t, err) - require.Empty(t, receivePointers(t, stream)) - - // Write a new object into the repository. Because we set GIT_OBJECT_DIRECTORY to - // the quarantine directory, objects will be written in there instead of into the - // repository's normal object directory. - repo := localrepo.NewTestRepo(t, cfg, repoProto) - var buffer, stderr bytes.Buffer - err = repo.ExecAndWait(ctx, git.Command{ - Name: "hash-object", - Flags: []git.Option{ - git.Flag{Name: "-w"}, - git.Flag{Name: "--stdin"}, + // There are no quarantined objects yet, so none should be returned + // here. + return setupData{ + repo: repo, + expectedPointers: nil, + } }, - }, git.WithStdin(strings.NewReader(lfsPointerContents)), git.WithStdout(&buffer), git.WithStderr(&stderr)) - require.NoError(t, err) - - stream, err = client.ListAllLFSPointers(ctx, &gitalypb.ListAllLFSPointersRequest{ - Repository: repoProto, - }) - require.NoError(t, err) - - // We only expect to find a single LFS pointer, which is the one we've just written - // into the quarantine directory. - lfsPointersEqual(t, []*gitalypb.LFSPointer{ - { - Oid: text.ChompBytes(buffer.Bytes()), - Data: []byte(lfsPointerContents), - Size: int64(len(lfsPointerContents)), + }, + { + desc: "populated quarantine directory", + setup: func(t *testing.T) setupData { + repo, _, _ := setupRepoWithLFS(t, ctx, cfg) + + // We're emulating the case where git is receiving data via a push, + // where objects are stored in a separate quarantine environment. In + // this case, LFS pointer checks may want to inspect all newly + // pushed objects, denoted by a repository proto message which only + // has its object directory set to the quarantine directory. + quarantineDir, err := quarantine.New(ctx, gittest.RewrittenRepository(t, ctx, cfg, repo), config.NewLocator(cfg)) + require.NoError(t, err) + + // Note that we need to continue using the non-rewritten repository + // here as `localrepo.NewTestRepo()` already will try to rewrite it + // again. + repo.GitObjectDirectory = quarantineDir.QuarantinedRepo().GitObjectDirectory + + // Write a new object into the repository. Because we set + // GIT_OBJECT_DIRECTORY to the quarantine directory, objects will be + // written in there instead of into the repository's normal object + // directory. + quarantineRepo := localrepo.NewTestRepo(t, cfg, repo) + var buffer bytes.Buffer + require.NoError(t, quarantineRepo.ExecAndWait(ctx, git.Command{ + Name: "hash-object", + Flags: []git.Option{ + git.Flag{Name: "-w"}, + git.Flag{Name: "--stdin"}, + }, + }, git.WithStdin(strings.NewReader(lfsPointerContents)), git.WithStdout(&buffer))) + + return setupData{ + repo: repo, + expectedPointers: []*gitalypb.LFSPointer{ + { + Oid: text.ChompBytes(buffer.Bytes()), + Data: []byte(lfsPointerContents), + Size: int64(len(lfsPointerContents)), + }, + }, + } }, - }, receivePointers(t, stream)) - }) + }, + } { + tc := tc - t.Run("no repository provided", func(t *testing.T) { - _, _, _, client := setupWithLFS(t, ctx) - stream, err := client.ListAllLFSPointers(ctx, &gitalypb.ListAllLFSPointersRequest{ - Repository: nil, + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + + setup := tc.setup(t) + + stream, err := client.ListAllLFSPointers(ctx, &gitalypb.ListAllLFSPointersRequest{ + Repository: setup.repo, + }) + require.NoError(t, err) + + var pointers []*gitalypb.LFSPointer + for { + resp, err := stream.Recv() + if err == io.EOF { + break + } + + testhelper.RequireGrpcError(t, setup.expectedErr, err) + if err != nil { + break + } + + pointers = append(pointers, resp.GetLfsPointers()...) + } + lfsPointersEqual(t, setup.expectedPointers, pointers) }) - require.NoError(t, err) - _, err = stream.Recv() - testhelper.RequireGrpcError(t, status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect( - "empty Repository", - "repo scoped: empty Repository", - )), err) - }) + } } func TestGetLFSPointers(t *testing.T) { @@ -420,15 +461,6 @@ func lfsPointersEqual(tb testing.TB, expected, actual []*gitalypb.LFSPointer) { } } -func setupWithLFS(tb testing.TB, ctx context.Context) (config.Cfg, *gitalypb.Repository, string, gitalypb.BlobServiceClient) { - tb.Helper() - - cfg, client := setupWithoutRepo(tb, ctx) - repo, repoPath, _ := setupRepoWithLFS(tb, ctx, cfg) - - return cfg, repo, repoPath, client -} - type lfsRepoInfo struct { // defaultCommitID is the object ID of the commit pointed to by the default branch. defaultCommitID git.ObjectID |