diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-08-05 13:48:02 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-08-11 13:35:15 +0300 |
commit | 79b8b738faf547ac112360b8ff19ede3964763e3 (patch) | |
tree | 4826e9c5ed9c5f5d27f42b665f5e1c8e66a7927b | |
parent | f20b8259797f3a1f3b59142c9789af97fcb0f69f (diff) |
gittest: Make CreateRepository handle repo creation without gRPC service
We have two sets of functions to create repositories for testing
purposes:
- CreateRepository creates the repository by calling the respective
RPCs of the RepositoryService. This allows us to properly test
with Praefect as a proxy, which requires the RPC to be called so
that it can set up the repositories in the database.
- InitRepo and CloneRepo, which both will create the repository
without doing an RPC call. These should only be used in contexts
where we don't have a gRPC service available.
It's confusing at times which of both should be used, and we're not
quite consistent.
Unify all functionality to create repositories into CreateRepository and
add a new option `SkipCreationViaService`. If set, this will behave the
same as InitRepo and CloneRepo, which allows us to consolidate the
functions into a single one. Callers that want to skip creation via the
RepositoryService will thus need to explicitly opt-in to this behaviour
and cannot just use InitRepo respectively CloneRepo without knowing
about the ramifications.
-rw-r--r-- | internal/git/gittest/repo.go | 90 |
1 files changed, 56 insertions, 34 deletions
diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go index fb9529b1e..4c8787ab5 100644 --- a/internal/git/gittest/repo.go +++ b/internal/git/gittest/repo.go @@ -94,6 +94,13 @@ type CreateRepositoryConfig struct { // Seed determines which repository is used to seed the created repository. If unset, the repository // is just created. The value should be one of the test repositories in _build/testrepos. Seed string + // SkipCreationViaService skips creation of the repository by calling the respective RPC call. + // In general, this should not be skipped so that we end up in a state that is consistent + // and expected by both Gitaly and Praefect. It may be required though when testing at a + // level where there are no gRPC services available. + SkipCreationViaService bool + // ObjectFormat overrides the object format used by the repository. + ObjectFormat string } func dialService(ctx context.Context, t testing.TB, cfg config.Cfg) *grpc.ClientConn { @@ -118,14 +125,11 @@ func CreateRepository(ctx context.Context, t testing.TB, cfg config.Cfg, configs opts = configs[0] } - conn := opts.ClientConn - if conn == nil { - conn = dialService(ctx, t, cfg) - t.Cleanup(func() { conn.Close() }) + if ObjectHashIsSHA256() || opts.ObjectFormat != "" { + require.Empty(t, opts.Seed, "seeded repository creation not supported with non-default object format") + require.True(t, opts.SkipCreationViaService, "repository creation via service not supported with non-default object format") } - client := gitalypb.NewRepositoryServiceClient(conn) - storage := cfg.Storages[0] if (opts.Storage != config.Storage{}) { storage = opts.Storage @@ -143,47 +147,65 @@ func CreateRepository(ctx context.Context, t testing.TB, cfg config.Cfg, configs GlProjectPath: GlProjectPath, } - if opts.Seed != "" { - if ObjectHashIsSHA256() { - require.FailNow(t, "seeded repository creation not supported with SHA256") + var repoPath string + if !opts.SkipCreationViaService { + conn := opts.ClientConn + if conn == nil { + conn = dialService(ctx, t, cfg) + t.Cleanup(func() { testhelper.MustClose(t, conn) }) } - - _, err := client.CreateRepositoryFromURL(ctx, &gitalypb.CreateRepositoryFromURLRequest{ - Repository: repository, - Url: testRepositoryPath(t, opts.Seed), - Mirror: true, - }) - require.NoError(t, err) - } else { - if ObjectHashIsSHA256() { - require.FailNow(t, "CreateRepository does not yet support creating SHA256 repositories") + client := gitalypb.NewRepositoryServiceClient(conn) + + if opts.Seed != "" { + _, err := client.CreateRepositoryFromURL(ctx, &gitalypb.CreateRepositoryFromURLRequest{ + Repository: repository, + Url: testRepositoryPath(t, opts.Seed), + Mirror: true, + }) + require.NoError(t, err) + } else { + _, err := client.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{ + Repository: repository, + }) + require.NoError(t, err) } - _, err := client.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{ - Repository: repository, - }) - require.NoError(t, err) - } + t.Cleanup(func() { + // The ctx parameter would be canceled by now as the tests defer the cancellation. + _, err := client.RemoveRepository(context.TODO(), &gitalypb.RemoveRepositoryRequest{ + Repository: repository, + }) - t.Cleanup(func() { - // The ctx parameter would be canceled by now as the tests defer the cancellation. - _, err := client.RemoveRepository(context.TODO(), &gitalypb.RemoveRepositoryRequest{ - Repository: repository, + if st, ok := status.FromError(err); ok && st.Code() == codes.NotFound { + // The tests may delete the repository, so this is not a failure. + return + } + + require.NoError(t, err) }) - if st, ok := status.FromError(err); ok && st.Code() == codes.NotFound { - // The tests may delete the repository, so this is not a failure. - return + repoPath = filepath.Join(storage.Path, getReplicaPath(ctx, t, conn, repository)) + } else { + if opts.Seed != "" { + CloneRepo(t, cfg, storage, CloneRepoOpts{ + RelativePath: relativePath, + SourceRepo: opts.Seed, + }) + } else { + InitRepo(t, cfg, storage, InitRepoOpts{ + WithRelativePath: relativePath, + ObjectFormat: opts.ObjectFormat, + }) } - require.NoError(t, err) - }) + repoPath = filepath.Join(storage.Path, repository.RelativePath) + } // Return a cloned repository so the above clean up function still targets the correct repository // if the tests modify the returned repository. clonedRepo := proto.Clone(repository).(*gitalypb.Repository) - return clonedRepo, filepath.Join(storage.Path, getReplicaPath(ctx, t, conn, repository)) + return clonedRepo, repoPath } // GetReplicaPathConfig allows for configuring the GetReplicaPath call. |