diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c9dd081a4e637dd24c2b42030eb2478bb6add1af_LmdpdGxhYi1jaS55bWw=..b872a3a97aacaa11a1cb835009062c2dbcbcc225_LmdpdGxhYi1jaS55bWw= 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -33,16 +33,9 @@ .tests: script: - # The hgitaly project currently only has one branch: default, and we'll - # have a stable branch only after Heptapod 0.17.0 release. - # At this point, the simplest will be to have the same branch conventions - # in py-heptapod and HGitaly (already implemented below). - # So either both have a `heptapod-0-17` branch - # or we decide that the new versioning rules (see heptapod#352) allow us to - # simply have a `stable` branch in both. - # TODO switch to clone/update, but a first attempt cloning in ./py-heptapod - # led to shadowing problems and bad applied hg config. Using archives is not - # *that* inefficient + # TODO switch to clone/update (chainsaw), but a first attempt cloning + # in ./py-heptapod led to shadowing problems and bad applied hg config. + # Using archives is not *that* inefficient thanks to caching in Workhorse - pip3 install https://foss.heptapod.net/heptapod/py-heptapod/-/archive/branch/${CI_COMMIT_HG_BRANCH}/py-heptapod-branch-${CI_COMMIT_HG_BRANCH}.tar.bz2 # usually the base image should have all that's needed # but in case of changes in test dependencies, we may still @@ -53,9 +46,9 @@ - mkdir -p /run/sshd - ./run-all-tests -tests-current: - rules: - - when: always +tests-hg-current: + variables: + HGITALY_TESTS_HG_EXE_PATH: /usr/local/bin/hg extends: - .tests stage: main @@ -59,6 +52,12 @@ extends: - .tests stage: main + image: ${BASE_IMAGES_MERCURIAL}/hg-current:${BASE_IMAGES_TAG} + +gitaly-comparison: + extends: + - .tests + stage: compat image: ${BASE_IMAGES_COLLECTION}/heptapod-gitaly:${CI_COMMIT_HG_BRANCH} variables: GITALY_INSTALL_DIR: /opt/gitlab/gitaly diff --git a/.hgtags b/.hgtags index c9dd081a4e637dd24c2b42030eb2478bb6add1af_LmhndGFncw==..b872a3a97aacaa11a1cb835009062c2dbcbcc225_LmhndGFncw== 100644 --- a/.hgtags +++ b/.hgtags @@ -50,7 +50,8 @@ fbcf2df5173d0d42ca8ebdc5529077791a89aaf5 0.27.2 e4016c35defc483ed6485a467e939b5c281008af 0.28.0 54ba6c3133acc7b21a24ad3e1bd715bafe98f0f8 0.29.0 +494d44b60baab709836bdcb4851437d4ebfee1b0 0.29.1 06eff55f3eb96f808f791142f132b541d32a9b31 0.30.0 47380d3aa05550b0873c9100dbfbdd4abd1e695c 0.31.0 e559b74680c410d065b079af04584211a57850bc 0.32.0 d7fd281d6227cc126448a03b7c3defbc79125d4a 0.32.1 @@ -53,4 +54,5 @@ 06eff55f3eb96f808f791142f132b541d32a9b31 0.30.0 47380d3aa05550b0873c9100dbfbdd4abd1e695c 0.31.0 e559b74680c410d065b079af04584211a57850bc 0.32.0 d7fd281d6227cc126448a03b7c3defbc79125d4a 0.32.1 +e67d981935c35d2bc48736fa29182e0c4d1df2cc 0.32.2 diff --git a/hgitaly/repository.py b/hgitaly/repository.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS9yZXBvc2l0b3J5LnB5..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS9yZXBvc2l0b3J5LnB5 100644 --- a/hgitaly/repository.py +++ b/hgitaly/repository.py @@ -52,6 +52,19 @@ repo.vfs.write(GITLAB_PROJECT_FULL_PATH_FILENAME, full_path) +def get_gitlab_project_full_path(repo): + """Get the full path of GitLab Project. + + See also :func:`set_gitlab_project_full_path`. + + :return: the full path, or ``None`` if not set. + """ + try: + return repo.vfs.read(GITLAB_PROJECT_FULL_PATH_FILENAME) + except FileNotFoundError: + return None + + def unbundle(repo, bundle_path: str, rails_sync=False): """Call unbundle with proper options and conversions. diff --git a/hgitaly/revision.py b/hgitaly/revision.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS9yZXZpc2lvbi5weQ==..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS9yZXZpc2lvbi5weQ== 100644 --- a/hgitaly/revision.py +++ b/hgitaly/revision.py @@ -11,6 +11,7 @@ error, scmutil, ) +from mercurial.context import changectx from hgext3rd.heptapod.branch import get_default_gitlab_branch from hgext3rd.heptapod.keep_around import ( parse_keep_around_ref, @@ -94,7 +95,10 @@ # TODO we should probably give precedence to tags, as Mercurial # does, although we should check what Git(aly) really does in # case of naming conflicts - return scmutil.revsingle(repo.unfiltered(), revision) + ctx = scmutil.revsingle(repo.unfiltered(), revision) + # ctx can be, e.g., workingctx (see heptapod#717) + # The null node is not to be filtered (it may be used in diffs) + return ctx if isinstance(ctx, changectx) else None except error.RepoLookupError: return None # voluntarily explicit diff --git a/hgitaly/service/commit.py b/hgitaly/service/commit.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS9zZXJ2aWNlL2NvbW1pdC5weQ==..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS9zZXJ2aWNlL2NvbW1pdC5weQ== 100644 --- a/hgitaly/service/commit.py +++ b/hgitaly/service/commit.py @@ -7,6 +7,7 @@ # SPDX-License-Identifier: GPL-2.0-or-later import itertools import logging +import os from grpc import StatusCode from google.protobuf.timestamp_pb2 import Timestamp @@ -15,6 +16,7 @@ pycompat, logcmdutil, hgweb, + node as nodemod, ) from hgext3rd.heptapod.branch import get_default_gitlab_branch @@ -110,6 +112,13 @@ base_logger = logging.getLogger(__name__) +PSEUDO_REVS = (nodemod.wdirrev, nodemod.nullrev) +PSEUDO_REVLOG_NODES = { + nodemod.nullid, + nodemod.wdirid, +} +PSEUDO_REVLOG_NODES.update(nodemod.wdirfilenodeids) + class CommitServicer(CommitServiceServicer, HGitalyServicer): @@ -113,6 +122,8 @@ class CommitServicer(CommitServiceServicer, HGitalyServicer): + STATUS_CODE_STORAGE_NOT_FOUND = StatusCode.INVALID_ARGUMENT + def CommitIsAncestor(self, request: CommitIsAncestorRequest, context) -> CommitIsAncestorResponse: @@ -142,10 +153,12 @@ context) -> TreeEntryResponse: """Return an entry of a tree. - The name could be confusing with the entry for a tree. + The name could be confusing with the entry for a tree: the entry + can be of any type. + Actually, it always yields one response message, using the empty response in case the given path does not resolve. """ repo = self.load_repo(request.repository, context).unfiltered() changeset = gitlab_revision_changeset(repo, request.revision) if changeset is None: @@ -146,12 +159,13 @@ Actually, it always yields one response message, using the empty response in case the given path does not resolve. """ repo = self.load_repo(request.repository, context).unfiltered() changeset = gitlab_revision_changeset(repo, request.revision) if changeset is None: - # unknown revision is not an error, and expected in some cases. - yield TreeEntryResponse() - return + # in case of unknown revision, Gitaly error details are + # still about the path only (as of v15.4.6) + context.abort(StatusCode.NOT_FOUND, + 'not found: ' + os.fsdecode(request.path)) sha = changeset.hex().decode('ascii') # early testing shows that even for leaf files, Gitaly ignores @@ -163,6 +177,12 @@ except error.ManifestLookupError: filectx = None + # TODO investigate why it's not the usual WRITE_BUFFER_SIZE + # The only occurrence we could find so far was a 16384 in grpc Golang + # lib (size of HTTP/2 frames). + # Could be because Gitaly implementation uses CopyN to send to + # its chunker (in `streamio.go`) and CopyN has a buffer. + buffer_size = 16384 if filectx is not None: otype = TreeEntryResponse.ObjectType.BLOB oid = blob_oid(repo, sha, path) @@ -181,15 +201,21 @@ data = data[:limit] offset = 0 - while offset <= size: - yield TreeEntryResponse( - type=otype, - oid=oid, - size=size, - data=data[offset:offset+WRITE_BUFFER_SIZE], - mode=mode) - offset += WRITE_BUFFER_SIZE + while offset < size: + # only the first response of the stream carries the metadata + if offset: + resp = TreeEntryResponse() + else: + resp = TreeEntryResponse(type=otype, + oid=oid, + size=size, + mode=mode) + + resp.data = data[offset:offset+buffer_size] + offset += buffer_size + + yield resp return subtrees, file_paths = manifest.miner(changeset).ls_dir(path) if not subtrees and not file_paths: @@ -192,9 +218,9 @@ return subtrees, file_paths = manifest.miner(changeset).ls_dir(path) if not subtrees and not file_paths: - yield TreeEntryResponse() - return + context.abort(StatusCode.NOT_FOUND, + 'not found: ' + os.fsdecode(path)) # path is an actual directory @@ -808,7 +834,8 @@ for chunk in chunked(pycompat.sysbytes(oid) for oid in request.oid): try: chunk_commits = [message.commit(repo[rev]) - for rev in repo.revs(b'%ls', chunk)] + for rev in repo.revs(b'%ls', chunk) + if rev not in PSEUDO_REVS] except lookup_error_classes: # lookup errors aren't surprising: the client uses this # method for prefix resolution @@ -825,8 +852,10 @@ pass else: if len(revs) == 1: - chunk_commits.append( - message.commit(repo[revs.first()])) + rev = revs.first() + if rev in PSEUDO_REVS: + continue + chunk_commits.append(message.commit(repo[rev])) yield ListCommitsByOidResponse(commits=chunk_commits) def ListCommitsByRefName(self, request: ListCommitsByRefNameRequest, diff --git a/hgitaly/service/ref.py b/hgitaly/service/ref.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS9zZXJ2aWNlL3JlZi5weQ==..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS9zZXJ2aWNlL3JlZi5weQ== 100644 --- a/hgitaly/service/ref.py +++ b/hgitaly/service/ref.py @@ -127,11 +127,7 @@ request: FindDefaultBranchNameRequest, context) -> FindDefaultBranchNameResponse: logger = LoggerAdapter(base_logger, context) - try: - repo = self.load_repo(request.repository, context) - except KeyError as exc: - context.abort(StatusCode.NOT_FOUND, - "repository not found: " + repr(exc.args)) + repo = self.load_repo(request.repository, context) branch = get_default_gitlab_branch(repo) if branch is None: diff --git a/hgitaly/service/repository.py b/hgitaly/service/repository.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS9zZXJ2aWNlL3JlcG9zaXRvcnkucHk=..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS9zZXJ2aWNlL3JlcG9zaXRvcnkucHk= 100644 --- a/hgitaly/service/repository.py +++ b/hgitaly/service/repository.py @@ -52,6 +52,7 @@ validate_relative_path, ) from ..repository import ( + get_gitlab_project_full_path, set_gitlab_project_full_path, unbundle, ) @@ -81,6 +82,8 @@ FetchBundleResponse, FindMergeBaseRequest, FindMergeBaseResponse, + FullPathRequest, + FullPathResponse, GetRawChangesRequest, GetRawChangesResponse, RepositoryExistsRequest, @@ -128,6 +131,8 @@ """RepositoryServiceService implementation. """ + STATUS_CODE_STORAGE_NOT_FOUND = StatusCode.INVALID_ARGUMENT + def FindMergeBase(self, request: FindMergeBaseRequest, context) -> FindMergeBaseResponse: @@ -155,5 +160,5 @@ request: RepositoryExistsRequest, context) -> RepositoryExistsResponse: try: - self.load_repo(request.repository, context) + self.load_repo_inner(request.repository, context) exists = True @@ -159,3 +164,8 @@ exists = True - except KeyError: + except KeyError as exc: + if exc.args[0] == 'storage': + context.abort( + StatusCode.INVALID_ARGUMENT, + f'GetStorageByName: no such storage: "{exc.args[1]}"' + ) exists = False @@ -161,10 +171,6 @@ exists = False - # TODO would be better to have a two-layer helper - # in servicer: load_repo() for proper gRPC error handling and - # load_repo_raw_exceptions() (name to be improved) to get the - # raw exceptions - context.set_code(StatusCode.OK) - context.set_details('') + except ValueError as exc: + context.abort(StatusCode.INVALID_ARGUMENT, exc.args[0]) return RepositoryExistsResponse(exists=exists) @@ -369,11 +375,9 @@ try: repo_path = self.repo_disk_path(request.repository, context) except KeyError as exc: - if exc.args[0] == 'storage': - context.abort(StatusCode.INVALID_ARGUMENT, - "Unkbown storage %r" % exc.args[1]) - - raise # pragma no cover (not triggerable at this point) + self.handle_key_error(context, exc.args) + except ValueError as exc: + self.handle_value_error(context, exc.args) if not os.path.exists(repo_path): # same error message as Gitaly (probably no need to repeat @@ -396,21 +400,7 @@ def SetFullPath(self, request: SetFullPathRequest, context) -> SetFullPathResponse: - try: - repo = self.load_repo(request.repository, context) - except KeyError as exc: - kind, what = exc.args - if kind == 'storage': - context.abort(StatusCode.INVALID_ARGUMENT, - 'setting config: GetStorageByName: ' - 'no such storage: "%s"' % what) - else: - # (H)Gitaly relative paths are always ASCII, but the - # root might not be (Gitaly does disclose the full expected - # path in the error message) - context.abort(StatusCode.NOT_FOUND, - 'setting config: GetRepoPath: not a Mercurial ' - 'repository: "%s"' % os.fsdecode(what)) + repo = self.load_repo(request.repository, context) if not request.path: context.abort(StatusCode.INVALID_ARGUMENT, "no path provided") @@ -418,6 +408,18 @@ set_gitlab_project_full_path(repo, request.path.encode('utf-8')) return SetFullPathResponse() + def FullPath(self, request: FullPathRequest, + context) -> FullPathResponse: + repo = self.load_repo(request.repository, context) + + path = get_gitlab_project_full_path(repo) + if not path: # None or (not probable) empty string + # Gitaly simply returns the `git config` stderr output for now. + # Pretty ridiculous to mimick it, we can hope not much to + # depend on it. + context.abort(StatusCode.INTERNAL, "Full path not set") + return FullPathResponse(path=path) + def CreateBundle(self, request: CreateBundleRequest, context) -> CreateBundleResponse: repo = self.load_repo(request.repository, context).unfiltered() diff --git a/hgitaly/service/tests/test_commit.py b/hgitaly/service/tests/test_commit.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfY29tbWl0LnB5..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfY29tbWl0LnB5 100644 --- a/hgitaly/service/tests/test_commit.py +++ b/hgitaly/service/tests/test_commit.py @@ -10,6 +10,9 @@ from mercurial import ( pycompat, ) +from mercurial_testhelpers import ( + as_bytes, +) from google.protobuf.timestamp_pb2 import Timestamp from hgext3rd.heptapod.branch import set_default_gitlab_branch @@ -250,6 +253,10 @@ # with unknown changeset assert not find_commit(b'unknown').HasField('commit') + # special cases of invalid commit ids (prefix or not) + assert not find_commit(b'f' * 39).HasField('commit') + assert not find_commit(b'f' * 40).HasField('commit') + def test_find_commits(commit_fixture_empty_repo): fixture = commit_fixture_empty_repo @@ -301,6 +308,10 @@ assert find_commits_ids(revision=b'..0') == [] # when revision does not exists assert find_commits_ids(revision=b'does_not_exists') == [] + assert find_commits_ids(revision=b'1234deadbeaf') == [] + # special node ids + assert find_commits_ids(revision=b'ffffffff' * 5) == [] + assert find_commits_ids(revision=b'ffffffff') == [] # with all, return all the commits assert find_commits_ids(all=True) == [sha0, sha1, sha2, sha3, sha4] # with offset @@ -645,6 +656,10 @@ assert do_rpc([unknown_sha]) == [] assert do_rpc([unknown_sha, short]) == [ctx.hex()] + # heptapod#717, both code paths + assert do_rpc(["ffffffff"]) == [] + assert do_rpc([unknown_sha, "ffffffff", short]) == [ctx.hex()] + # heptapod#640: obsolete changeset given by its full hash wrapper.command('amend', message=b'amended') assert do_rpc([ctx2.hex()]) == [ctx2.hex()] @@ -791,12 +806,13 @@ assert 'bigger than the maximum allowed size (5)' in err_details # revision or file not found - resp = rpc_single_response(b'not/there') - assert not resp.data - assert not resp.oid - resp = rpc_single_response(b'foo', revision=b'branch/unknown') - assert not resp.data - assert not resp.oid + with pytest.raises(grpc.RpcError) as exc_info: + do_rpc(b'not/there') + assert exc_info.value.code() == grpc.StatusCode.NOT_FOUND + + with pytest.raises(grpc.RpcError) as exc_info: + do_rpc(b'foo', revision=b'branch/unknown') + assert exc_info.value.code() == grpc.StatusCode.NOT_FOUND # with obsolescence wrapper.command('amend', message=b'amended') @@ -806,6 +822,23 @@ assert resp.mode == 0o100644 assert resp.type == TreeEntryResponse.ObjectType.BLOB + # chunking + thirty_two = b'ba' * 16384 + for content_suffix in (b'', b'x'): + content = thirty_two + content_suffix + rev_hex = wrapper.commit_file(b'bar', content).hex() + resps = do_rpc(b'bar', revision=rev_hex) + + assert len(resps) == 3 if content_suffix else 2 + # metadata are carried by the first response only + assert resps[0].type == TreeEntryResponse.ObjectType.BLOB + assert resps[0].mode == 0o100644 + for r in resps[1:]: + assert not r.type + assert not r.mode + + assert b''.join(r.data for r in resps) == content + def test_get_tree_entries(commit_fixture_empty_repo): fixture = commit_fixture_empty_repo @@ -1113,6 +1146,10 @@ # with unknown revision assert do_rpc(b"23fire32" * 5) == [] + # special cases of invalid commit ids (prefix or full) + assert do_rpc(b"f" * 39) == [] + assert do_rpc(b"f" * 40) == [] + # with obsolescence wrapper.command('amend', message=b'amended') assert do_rpc(sha2) == [b'bar', b'foo', b'zoo'] @@ -1169,7 +1206,11 @@ commit_message_patterns=[b'FOO'], ignore_case=True) == [sha2] - # unknown revision - with pytest.raises(grpc.RpcError) as exc_info: - list_commits(b'branch/unknown') + # unknown revision, including special cases: + for unknown in ('branch/unknown', + 'f' * 39, # wdir node id, prefix + 'f' * 40, # wdir full node id + ): + with pytest.raises(grpc.RpcError) as exc_info: + list_commits(as_bytes(unknown)) @@ -1175,6 +1216,6 @@ - assert exc_info.value.code() == grpc.StatusCode.INTERNAL - assert 'branch/unknown' in exc_info.value.details() + assert exc_info.value.code() == grpc.StatusCode.INTERNAL + assert unknown in exc_info.value.details() # invalid arguments with pytest.raises(grpc.RpcError) as exc_info: diff --git a/hgitaly/service/tests/test_repository_service.py b/hgitaly/service/tests/test_repository_service.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfcmVwb3NpdG9yeV9zZXJ2aWNlLnB5..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS9zZXJ2aWNlL3Rlc3RzL3Rlc3RfcmVwb3NpdG9yeV9zZXJ2aWNlLnB5 100644 --- a/hgitaly/service/tests/test_repository_service.py +++ b/hgitaly/service/tests/test_repository_service.py @@ -43,6 +43,7 @@ CreateBundleFromRefListRequest, FetchBundleRequest, FindMergeBaseRequest, + FullPathRequest, GetArchiveRequest, HasLocalBranchesRequest, RemoveRepositoryRequest, @@ -145,6 +146,12 @@ return self.stub.SetFullPath(SetFullPathRequest(repository=grpc_repo, path=path)) + def full_path(self, grpc_repo=None): + if grpc_repo is None: + grpc_repo = self.grpc_repo + + return self.stub.FullPath(FullPathRequest(repository=grpc_repo)).path + def create_bundle(self, save_path): with open(save_path, 'wb') as fobj: for chunk in self.stub.CreateBundle(CreateBundleRequest( @@ -246,6 +253,18 @@ fixture.grpc_repo.relative_path = 'does/not/exist' assert not fixture.exists() + # storage does not exist + fixture.grpc_repo.storage_name = 'dream' + with pytest.raises(grpc.RpcError) as exc_info: + fixture.exists() + assert exc_info.value.code() == grpc.StatusCode.INVALID_ARGUMENT + + # missing repo argument + fixture.grpc_repo = None + with pytest.raises(grpc.RpcError) as exc_info: + fixture.exists() + assert exc_info.value.code() == grpc.StatusCode.INVALID_ARGUMENT + def test_has_local_branches(fixture_with_repo): fixture = fixture_with_repo @@ -260,6 +279,12 @@ assert not fixture.has_local_branches() + # missing repo argument + fixture.grpc_repo = None + with pytest.raises(grpc.RpcError) as exc_info: + fixture.has_local_branches() + assert exc_info.value.code() == grpc.StatusCode.INVALID_ARGUMENT + def test_write_ref(fixture_with_repo): fixture = fixture_with_repo @@ -548,6 +573,12 @@ relative_path='no/such/path')) assert exc_info.value.code() == grpc.StatusCode.NOT_FOUND + # missing repo argument + fixture.grpc_repo = None + with pytest.raises(grpc.RpcError) as exc_info: + fixture.remove_repository(grpc_repo=None) + assert exc_info.value.code() == grpc.StatusCode.INVALID_ARGUMENT + def test_set_full_path(fixture_with_repo): fixture = fixture_with_repo @@ -558,6 +589,7 @@ assert (wrapper.path / '.hg' / GITLAB_PROJECT_FULL_PATH_FILENAME.decode('ascii') ).read_text(encoding='utf-8') == expected + assert fixture.full_path() == expected def call_then_assert(full_path): fixture.set_full_path(full_path) @@ -583,6 +615,28 @@ storage_name='unknown')) +def test_full_path_errors(fixture_with_repo): + fixture = fixture_with_repo + grpc_repo = fixture.grpc_repo + + def call_then_assert_error(error_code, grpc_repo=None): + with pytest.raises(grpc.RpcError) as exc_info: + fixture.full_path(grpc_repo=grpc_repo) + assert exc_info.value.code() == error_code + + # existing repo, but full path not set + call_then_assert_error(grpc.StatusCode.INTERNAL) + + call_then_assert_error(grpc.StatusCode.NOT_FOUND, + grpc_repo=Repository( + storage_name=grpc_repo.storage_name, + relative_path='no/such/repo')) + call_then_assert_error(grpc.StatusCode.INVALID_ARGUMENT, + grpc_repo=Repository( + relative_path=grpc_repo.relative_path, + storage_name='unknown')) + + def test_fetch_bundle(fixture_with_repo, tmpdir): fixture = fixture_with_repo wrapper = fixture.repo_wrapper diff --git a/hgitaly/servicer.py b/hgitaly/servicer.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS9zZXJ2aWNlci5weQ==..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS9zZXJ2aWNlci5weQ== 100644 --- a/hgitaly/servicer.py +++ b/hgitaly/servicer.py @@ -60,5 +60,9 @@ uis are derived. In particular, it bears the common configuration. """ + STATUS_CODE_STORAGE_NOT_FOUND = StatusCode.NOT_FOUND + STATUS_CODE_REPO_NOT_FOUND = StatusCode.NOT_FOUND + STATUS_CODE_MISSING_REPO_ARG = StatusCode.INVALID_ARGUMENT + def __init__(self, storages): self.storages = storages @@ -63,6 +67,32 @@ def __init__(self, storages): self.storages = storages - self.ui = uimod.ui.load() + self.init_ui() + + def init_ui(self): + """Prepare the base ``ui`` instance from which all will derive. + + See also :meth:`hgweb.__init__` + """ + + ui = self.ui = uimod.ui.load() + # prevents `ui.interactive()` to crash (see heptapod#717) + ui.setconfig(b'ui', b'nontty', b'true', b'hgitaly') + + # progress bars in stdout (log file at best) would be rather useless + ui.setconfig(b'progress', b'disable', b'true', b'hgitaly') + + # other settings done in hgweb.__init__(): + # + # - forcing file pattern resolution to be relative to root would be + # nice, but perhaps need more adaptation, and would have to be + # done in load_repo() + # - `signal-safe-locks=no` worth being considered, but this not being + # WSGI, we control the server and its signal handling (see hgweb's + # comment) + # - `report_unstrusted=off`: if some perms are unaligned, reporting + # the untrust could be the only lead for an obscure behaviour + # (typically ignoring some settings that can be critical to + # operation) def load_repo(self, repository: Repository, context): """Load the repository from storage name and relative path @@ -75,8 +105,38 @@ Error treatment: the caller doesn't have to do anything specific, the status code and the details are already set in context, and these are automatically propagated to the client (see corresponding test - in `test_servicer.py`). Still, the caller can still catch the - raised exception and change the code and details as they wish. + in `test_servicer.py`). For specific error treatment, use + :meth:`load_repo_inner` and catch the exceptions it raises. + """ + try: + return self.load_repo_inner(repository, context) + except KeyError as exc: + self.handle_key_error(context, exc.args) + except ValueError as exc: + self.handle_value_error(context, exc.args) + + def handle_value_error(self, context, exc_args): + context.abort(self.STATUS_CODE_MISSING_REPO_ARG, exc_args[0]) + + def handle_key_error(self, context, exc_args): + ktype = exc_args[0] + if ktype == 'storage': + context.abort(self.STATUS_CODE_STORAGE_NOT_FOUND, + "No storage named %r" % exc_args[1]) + elif ktype == 'repo': + context.abort(self.STATUS_CODE_REPO_NOT_FOUND, + exc_args[1]) + + def load_repo_inner(self, repository: Repository, context): + """Load the repository from storage name and relative path + + :param repository: Repository Gitaly Message, encoding storage name + and relative path + :param context: gRPC context (used in error raising) + :raises: + - ``KeyError('storage', storage_name)`` if storage is not found + - ``KeyError('repo', path, details)`` if repo not found or + cannot be loaded. """ global repos_counter if repos_counter % GARBAGE_COLLECTING_RATE_GEN2 == 0: @@ -95,9 +155,8 @@ try: repo = hg.repository(self.ui, repo_path) except error.RepoError as exc: - context.set_code(StatusCode.NOT_FOUND) - context.set_details(repr(exc.args)) - raise KeyError('repo', repo_path) + raise KeyError('repo', repo_path, repr(exc.args)) + weakref.finalize(repo, clear_repo_class, repo.unfiltered().__class__) srcrepo = hg.sharedreposource(repo) if srcrepo is not None: @@ -109,6 +168,6 @@ def storage_root_dir(self, storage_name, context): """Return the storage directory. - If the storage is unknown, this sets error code and details - on the context and raises ``KeyError('storage', storage_name)`` + If the storage is unknown, this raises + ``KeyError('storage', storage_name)`` """ @@ -114,3 +173,8 @@ """ + if not storage_name: + # this is the best detection of a missing `repository` field + # in request, without having the request object itself + raise ValueError('empty repository') + root_dir = self.storages.get(storage_name) if root_dir is None: @@ -115,7 +179,5 @@ root_dir = self.storages.get(storage_name) if root_dir is None: - context.set_code(StatusCode.NOT_FOUND) - context.set_details("No storage named %r" % storage_name) raise KeyError('storage', storage_name) return root_dir @@ -141,4 +203,26 @@ :param bool ensure: if ``True``, the temporary directory is created if it does not exist yet. + """ + try: + return self.temp_dir_inner(storage_name, context, ensure=ensure) + except KeyError as exc: + self.handle_key_error(context, exc.args) + except ValueError as exc: + self.handle_value_error(context, exc.args) + except OSError as exc: + context.abort(StatusCode.INTERNAL, + "Error ensuring temporary dir: %s" % exc) + + def temp_dir_inner(self, storage_name, context, ensure=True): + """Return the path to temporary directory for the given storage + + Similar to what Gitaly uses, with a dedicated path in order + to be really sure not to overwrite anything. The important feature + is that the temporary directory is under the root directory of + the storage, hence on the same file system (atomic renames of + other files from the storage, etc.) + + :param bool ensure: if ``True``, the temporary directory is created + if it does not exist yet. :raises KeyError: if the storage is unknown @@ -144,5 +228,5 @@ :raises KeyError: if the storage is unknown - :raises OSError: if creation fail + :raises OSError: if creation fails. """ temp_dir = os.path.join(self.storage_root_dir(storage_name, context), b'+hgitaly', b'tmp') @@ -154,8 +238,8 @@ # work well) to_create = [] current = temp_dir - try: - while not os.path.exists(current): - to_create.append(current) - current = os.path.dirname(current) + + while not os.path.exists(current): + to_create.append(current) + current = os.path.dirname(current) @@ -161,12 +245,7 @@ - while to_create: - # same mode as in Gitaly, hence we don't care about groups - # although this does propagate the setgid bit - os.mkdir(to_create.pop(), mode=0o755) - except OSError as exc: - context.set_code(StatusCode.INTERNAL) - context.set_details("Error ensuring temporary dir %r: %s" % ( - temp_dir, exc)) - raise + while to_create: + # same mode as in Gitaly, hence we don't care about groups + # although this does propagate the setgid bit + os.mkdir(to_create.pop(), mode=0o755) return temp_dir diff --git a/hgitaly/tests/test_revision.py b/hgitaly/tests/test_revision.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS90ZXN0cy90ZXN0X3JldmlzaW9uLnB5..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS90ZXN0cy90ZXN0X3JldmlzaW9uLnB5 100644 --- a/hgitaly/tests/test_revision.py +++ b/hgitaly/tests/test_revision.py @@ -6,6 +6,8 @@ # SPDX-License-Identifier: GPL-2.0-or-later import pytest +from mercurial.node import nullrev + from heptapod.testhelpers import ( LocalRepoWrapper, ) @@ -43,6 +45,14 @@ assert gitlab_revision_changeset(repo, ctx.hex()) == ctx + # special node ids + # collision very unlikely, but still a prefix + assert gitlab_revision_changeset(repo, b'f' * 39) is None + assert gitlab_revision_changeset(repo, b'0' * 39).rev() == nullrev + # full node hex + assert gitlab_revision_changeset(repo, b'f' * 40) is None + assert gitlab_revision_changeset(repo, b'00000000' * 5).rev() == nullrev + wrapper.command('amend', message=b'amended') obs_ctx = gitlab_revision_changeset(repo, ctx.hex()) diff --git a/hgitaly/tests/test_servicer.py b/hgitaly/tests/test_servicer.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_aGdpdGFseS90ZXN0cy90ZXN0X3NlcnZpY2VyLnB5..b872a3a97aacaa11a1cb835009062c2dbcbcc225_aGdpdGFseS90ZXN0cy90ZXN0X3NlcnZpY2VyLnB5 100644 --- a/hgitaly/tests/test_servicer.py +++ b/hgitaly/tests/test_servicer.py @@ -31,5 +31,13 @@ from ..stub.repository_pb2_grpc import RepositoryServiceStub +class AbortContext(Exception): + """Raised by FakeContext.abort. + + gRPC's context.abort raises `Exception()` (sic), which is + inconvenient for testing. + """ + + class FakeContext(FakeServicerContext): @@ -34,5 +42,10 @@ class FakeContext(FakeServicerContext): + def abort(self, code, message): + self.code = code + self.message = message + raise AbortContext() + def set_code(self, code): self.code = code @@ -61,6 +74,12 @@ assert loaded.root == wrapper.repo.root with pytest.raises(KeyError) as exc_info: + servicer.load_repo_inner(Repository(storage_name='dream', + relative_path='dream-proj.hg'), + context) + assert exc_info.value.args == ('storage', 'dream') + + with pytest.raises(AbortContext): servicer.load_repo(Repository(storage_name='dream', relative_path='dream-proj.hg'), context) @@ -64,7 +83,6 @@ servicer.load_repo(Repository(storage_name='dream', relative_path='dream-proj.hg'), context) - assert exc_info.value.args == ('storage', 'dream') assert context.code == grpc.StatusCode.NOT_FOUND @@ -102,7 +120,7 @@ assert len(frt) == 0 -def test_not_found_propagation(grpc_channel, server_repos_root): +def test_errors_propagation(grpc_channel, server_repos_root): # Taking a random RPC to check that the client receives the # proper error response repo_stub = RepositoryServiceStub(grpc_channel) @@ -112,7 +130,7 @@ repository=Repository(storage_name='dream', relative_path=''))) exc = exc_info.value - assert exc.code() == grpc.StatusCode.NOT_FOUND + assert exc.code() == grpc.StatusCode.INVALID_ARGUMENT assert 'dream' in exc.details() with pytest.raises(grpc.RpcError) as exc_info: @@ -139,5 +157,5 @@ assert path.exists() with pytest.raises(KeyError) as exc_info: - servicer.temp_dir('unknown', context) + servicer.temp_dir_inner('unknown', context) assert exc_info.value.args == ('storage', 'unknown') @@ -143,3 +161,6 @@ assert exc_info.value.args == ('storage', 'unknown') + + with pytest.raises(AbortContext) as exc_info: + servicer.temp_dir('unknown', context) assert context.code == grpc.StatusCode.NOT_FOUND @@ -144,5 +165,11 @@ assert context.code == grpc.StatusCode.NOT_FOUND + # this is how it looks with a missing repository argument in the + # gRPC request, and temp_dir(request.repository.storage_name) + with pytest.raises(AbortContext) as exc_info: + servicer.temp_dir('', context) + assert context.code == grpc.StatusCode.INVALID_ARGUMENT + def test_temp_dir_failed_creation(tmpdir): # context is used for error raising only @@ -152,6 +179,6 @@ servicer = HGitalyServicer(dict(broken=as_bytes(broken))) - with pytest.raises(OSError): + with pytest.raises(AbortContext): servicer.temp_dir('broken', context, ensure=True) assert context.code == grpc.StatusCode.INTERNAL diff --git a/tests_with_gitaly/comparison.py b/tests_with_gitaly/comparison.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_dGVzdHNfd2l0aF9naXRhbHkvY29tcGFyaXNvbi5weQ==..b872a3a97aacaa11a1cb835009062c2dbcbcc225_dGVzdHNfd2l0aF9naXRhbHkvY29tcGFyaXNvbi5weQ== 100644 --- a/tests_with_gitaly/comparison.py +++ b/tests_with_gitaly/comparison.py @@ -7,6 +7,7 @@ """Fixture for Gitaly comparison tests based on Heptapod's Git mirroring.""" import attr import contextlib +from copy import deepcopy import functools import pytest import random @@ -114,6 +115,9 @@ response_sha_attrs=(), feature_flags=(), normalizer=None, + error_details_normalizer=None, + chunked_fields_remover=None, + chunks_concatenator=None, streaming=False): self.comparison = comparison self.stubs = dict(git=stub_cls(comparison.gitaly_channel), @@ -130,6 +134,9 @@ self.streaming = streaming self.feature_flags = list(feature_flags) self.normalizer = normalizer + self.error_details_normalizer = error_details_normalizer + self.chunked_fields_remover = chunked_fields_remover + self.chunks_concatenator = chunks_concatenator def grpc_metadata(self): return feature.as_grpc_metadata(self.feature_flags) @@ -230,7 +237,14 @@ for k, v in defaults.items(): kwargs.setdefault(k, v) - def assert_compare(self, **hg_kwargs): + def call_backends(self, **hg_kwargs): + """Call Gitaly and HGitaly with uniform request kwargs. + + To be used only if no error is expected. + + :param hg_kwargs: used as-is to construct the request for HGitaly, + converted to Git and then to a request for Gitaly. + """ self.apply_request_defaults(hg_kwargs) git_kwargs = self.request_kwargs_to_git(hg_kwargs) @@ -238,8 +252,11 @@ hg_response = self.rpc('hg', **hg_kwargs) git_response = self.rpc('git', **git_kwargs) + return hg_response, git_response + + def normalize_responses(self, hg_response, git_response): self.response_to_git(hg_response) norm = self.normalizer if norm is not None: norm(self, hg_response, vcs='hg') norm(self, git_response, vcs='git') @@ -241,7 +258,11 @@ self.response_to_git(hg_response) norm = self.normalizer if norm is not None: norm(self, hg_response, vcs='hg') norm(self, git_response, vcs='git') + + def assert_compare(self, **hg_kwargs): + hg_response, git_response = self.call_backends(**hg_kwargs) + self.normalize_responses(hg_response, git_response) assert hg_response == git_response @@ -246,5 +267,53 @@ assert hg_response == git_response + def assert_compare_aggregated(self, + compare_first_chunks=True, + check_both_chunked=True, + **hg_kwargs): + """Compare streaming responses with appropriate concatenation. + + Sometimes, it's unreasonable to expect HGitaly chunking to + exactly match Gitaly's. This method allows to compare after + regrouping the chunks, with the provided :attr:`chunks_concatenator`. + + Usually Gitaly returns small values within the first response only, + to avoid the bandwidth waste of repetiting them. This helper + checks that HGitaly does the same by comparing after applying + :attr: `chunked_fields_remover` to as many responses as possible + (typically the number of responses would differ). + + :param bool compare_first_chunk: if ``True``, the first chunks of + both responses are directly compared (including main content). + :param bool check_both_chunked: if ``True` checks that we get + more than one response for both HGitaly and Gitaly + :return: a pair: the first chunk of responses for Gitaly and HGitaly + respectively, taken before normalization. This can be useful, e.g., + for pagination parameters. + """ + assert self.streaming # for consistency + + hg_resps, git_resps = self.call_backends(**hg_kwargs) + original_first_chunks = deepcopy((git_resps[0], hg_resps[0])) + + self.normalize_responses(hg_resps, git_resps) + if compare_first_chunks: + assert hg_resps[0] == git_resps[0] + + if check_both_chunked: + assert len(hg_resps) > 1 + assert len(git_resps) > 1 + + concatenator = getattr(self, 'chunks_concatenator') + fields_remover = getattr(self, 'chunked_fields_remover') + assert concatenator(hg_resps) == concatenator(git_resps) + + for hg_resp, git_resp in zip(hg_resps, git_resps): + fields_remover(hg_resp) + fields_remover(git_resp) + assert hg_resp == git_resp + + return original_first_chunks + def assert_compare_errors(self, same_details=True, **hg_kwargs): self.apply_request_defaults(hg_kwargs) @@ -258,7 +327,13 @@ assert exc_hg.code() == exc_git.code() if same_details: - assert exc_hg.details() == exc_git.details() + norm = self.error_details_normalizer + hg_details = exc_hg.details() + git_details = exc_git.details() + if norm is not None: # pragma no cover + hg_details = norm(hg_details, vcs='hg') + git_details = norm(git_details, vcs='git') + assert hg_details == git_details # trailing metadata can bear a typed error gRPC message, which # is more important to compare than "details" (in the sense of diff --git a/tests_with_gitaly/test_blob_tree.py b/tests_with_gitaly/test_blob_tree.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9ibG9iX3RyZWUucHk=..b872a3a97aacaa11a1cb835009062c2dbcbcc225_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9ibG9iX3RyZWUucHk= 100644 --- a/tests_with_gitaly/test_blob_tree.py +++ b/tests_with_gitaly/test_blob_tree.py @@ -4,4 +4,5 @@ # GNU General Public License version 2 or any later version. # # SPDX-License-Identifier: GPL-2.0-or-later +from copy import deepcopy import pytest @@ -7,6 +8,4 @@ import pytest -import grpc - from hgitaly.oid import ( blob_oid, @@ -10,6 +9,10 @@ from hgitaly.oid import ( blob_oid, + tree_oid, +) +from hgitaly.revision import ( + gitlab_revision_changeset, ) from hgitaly.stream import WRITE_BUFFER_SIZE @@ -32,28 +35,6 @@ pytestmark = pytest.mark.skip -class TreeBlobFixture: - - def __init__(self, gitaly_comparison): - self.comparison = gitaly_comparison - self.hg_repo_wrapper = gitaly_comparison.hg_repo_wrapper - self.git_repo = gitaly_comparison.git_repo - - self.gitaly_repo = gitaly_comparison.gitaly_repo - self.commit_stubs = dict( - git=CommitServiceStub(self.comparison.gitaly_channel), - hg=CommitServiceStub(self.comparison.hgitaly_channel)) - self.blob_stubs = dict( - git=BlobServiceStub(self.comparison.gitaly_channel), - hg=BlobServiceStub(self.comparison.hgitaly_channel)) - - def tree_entry(self, vcs, path, revision=b'branch/default', - limit=0, max_size=0): - request = TreeEntryRequest(repository=self.gitaly_repo, - revision=revision, - limit=limit, - max_size=max_size, - path=path) - resp = self.commit_stubs[vcs].TreeEntry(request) - return [r for r in resp] +def concat_data_fields(responses): + """Callback for RpcHelper.assert_compare_aggregated. @@ -59,30 +40,5 @@ - def assert_compare_tree_entry(self, path, several_responses=False, **kw): - hg_entries = self.tree_entry('hg', path, **kw) - git_entries = self.tree_entry('hg', path, **kw) - - for entries in (hg_entries, git_entries): - for r in entries: - # oid should be the only difference in comparison - r.oid = '' - - assert hg_entries == git_entries - if several_responses: - assert len(hg_entries) > 1 - - def assert_error_compare_tree_entry(self, path, **kw): - with pytest.raises(grpc.RpcError) as hg_err_info: - self.tree_entry('hg', path, **kw) - with pytest.raises(grpc.RpcError) as git_err_info: - self.tree_entry('git', path, **kw) - - assert hg_err_info.value.code() == git_err_info.value.code() - assert hg_err_info.value.details() == git_err_info.value.details() - - def get_blob(self, vcs, oid, limit=-1): - request = GetBlobRequest(repository=self.gitaly_repo, - limit=limit, - oid=oid) - - return [r for r in self.blob_stubs[vcs].GetBlob(request)] + Works for all response gRPC messages with a `data` field. + """ + return b''.join(r.data for r in responses) @@ -88,11 +44,2 @@ - def get_blobs(self, vcs, rev_paths, limit=-1, **request_kw): - rev_path_msgs = [ - GetBlobsRequest.RevisionPath(revision=rev, path=path) - for rev, path in rev_paths - ] - request = GetBlobsRequest(repository=self.gitaly_repo, - revision_paths=rev_path_msgs, - limit=limit, - **request_kw) @@ -98,3 +45,4 @@ - return [r for r in self.blob_stubs[vcs].GetBlobs(request)] +def remove_data_field(response): + """Callback for RpcHelper.assert_compare_aggregated. @@ -100,50 +48,6 @@ - def get_tree_entries_raw(self, vcs, path, revision=b'branch/default', - pagination=True, - cursor='', - limit=10, - trees_first=False, - skip_flat_paths=False, - recursive=False): - pagination_params = PaginationParameter( - page_token=cursor, limit=limit) if pagination else None - if trees_first: - sort = GetTreeEntriesRequest.SortBy.TREES_FIRST - else: - sort = GetTreeEntriesRequest.SortBy.DEFAULT - request = GetTreeEntriesRequest(repository=self.gitaly_repo, - revision=revision, - pagination_params=pagination_params, - sort=sort, - skip_flat_paths=skip_flat_paths, - recursive=recursive, - path=path) - - return self.commit_stubs[vcs].GetTreeEntries(request) - - def get_tree_entries(self, vcs, path, **kw): - return [entry - for chunk in self.get_tree_entries_raw(vcs, path, **kw) - for entry in chunk.entries] - - def assert_compare_get_tree_entries(self, path, **kw): - hg_tree_entries = self.get_tree_entries('hg', path, **kw) - git_tree_entries = self.get_tree_entries('git', path, **kw) - - # TODO itertools - for entry in (e for elist in (git_tree_entries, hg_tree_entries) - for e in elist): - entry.oid = entry.root_oid = '' - - assert hg_tree_entries == git_tree_entries - - def assert_error_compare_get_tree_entries(self, *a, **kw): - with pytest.raises(grpc.RpcError) as hg_err_info: - self.get_tree_entries('hg', *a, **kw) - with pytest.raises(grpc.RpcError) as git_err_info: - self.get_tree_entries('git', *a, **kw) - git_err, hg_err = git_err_info.value, hg_err_info.value - assert git_err.code() == hg_err.code() - assert git_err.details() == hg_err.details() + Allows to compare all fields but `data`. + """ + response.data = b'' @@ -148,7 +52,29 @@ -@pytest.fixture -def tree_blob_fixture(gitaly_comparison): - yield TreeBlobFixture(gitaly_comparison) +def oid_mapping(gitaly_comp, rev_paths): + """Provide the OID mappings between Mercurial and Git for blobs. + + :param fixture: an instance of `GitalyComparison` + :param rev_paths: an iterable of tuples (revision, path, type), where + type is 'tree' or 'blob' + :return: a :class:`dict` mapping Mercurial oids to Git oids + + Does not need HGitaly blob/tree methods to work, only those of Gitaly. + """ + res = {} + gitaly_meth = CommitServiceStub(gitaly_comp.gitaly_channel).TreeEntry + hg_repo = gitaly_comp.hg_repo_wrapper.repo + hg_oid_meths = dict(tree=tree_oid, blob=blob_oid) + for rev, path, t in rev_paths: + changeset = gitlab_revision_changeset(hg_repo, rev) + hg_oid = hg_oid_meths[t](hg_repo, changeset.hex().decode(), path) + gitaly_resp = gitaly_meth( + TreeEntryRequest(repository=gitaly_comp.gitaly_repo, + revision=rev, + path=path, + limit=1) + ) + res[hg_oid] = next(iter(gitaly_resp)).oid + return res @@ -153,7 +79,24 @@ -def test_compare_tree_entry_request(tree_blob_fixture): - fixture = tree_blob_fixture +def oid_normalizer(oid2git): + """Return a response normalizer for oid fields. + + :param oid_mapping: :class:`dict` instance mapping HGitaly OIDs to + Gitaly OIDs + """ + def normalizer(rpc_helper, responses, vcs='hg'): + if vcs != 'hg': + return + + for entry in responses: + if entry.oid: + entry.oid = oid2git[entry.oid] + + return normalizer + + +def test_compare_tree_entry_request(gitaly_comparison): + fixture = gitaly_comparison wrapper = fixture.hg_repo_wrapper wrapper.write_commit('foo', message="Some foo") @@ -166,6 +109,12 @@ wrapper.commit(rel_paths=['sub/bar', 'sub/ba2'], message="zebar", add_remove=True) - # precondition for the test: mirror worked - assert fixture.git_repo.branch_titles() == {b'branch/default': b"zebar"} + default_rev = b'branch/default' + oid2git = oid_mapping(fixture, + ((default_rev, b'foo', 'blob'), + (default_rev, b'sub', 'tree'), + (default_rev, b'sub/bar', 'blob'), + (default_rev, b'sub/ba2', 'blob'), + )) + normalizer = oid_normalizer(oid2git) @@ -171,5 +120,27 @@ - for path in (b'sub', b'sub/bar', b'sub/', b'.', b'do-not-exist'): - fixture.assert_compare_tree_entry(path) + rpc_helper = fixture.rpc_helper(stub_cls=CommitServiceStub, + method_name='TreeEntry', + request_cls=TreeEntryRequest, + request_defaults=dict( + revision=default_rev, + limit=0, + max_size=0), + streaming=True, + normalizer=normalizer, + chunked_fields_remover=remove_data_field, + chunks_concatenator=concat_data_fields, + ) + assert_compare = rpc_helper.assert_compare + assert_compare_errors = rpc_helper.assert_compare_errors + + # precondition for the test: mirror worked + assert fixture.git_repo.branch_titles() == {default_rev: b"zebar"} + + # basic case + for path in (b'sub', b'sub/bar', b'sub/'): + assert_compare(path=path) + + for path in (b'.', b'do-not-exist'): + assert_compare_errors(path=path) # limit and max_size (does not apply to Trees) @@ -174,6 +145,6 @@ # limit and max_size (does not apply to Trees) - fixture.assert_compare_tree_entry(b'foo', limit=4) - fixture.assert_error_compare_tree_entry(b'foo', max_size=4) - fixture.assert_compare_tree_entry(b'sub', max_size=1) + assert_compare(path=b'foo', limit=4) + assert_compare_errors(path=b'foo', max_size=4) + assert_compare(path=b'sub', max_size=1) @@ -179,5 +150,5 @@ - # unknown revision (not an error) - fixture.assert_compare_tree_entry(b'sub', revision=b'unknown') + # unknown revision + assert_compare_errors(path=b'sub', revision=b'unknown') # chunking for big Blob entry @@ -182,6 +153,18 @@ # chunking for big Blob entry - wrapper.write_commit('bigfile', message="A big file", - content=b"big" + b'ff' * WRITE_BUFFER_SIZE) - fixture.assert_compare_tree_entry(b'bigfile', several_responses=True) + wrapper.write_commit('bigfile', message="A big file with 2 or 3 chunks", + content=b'a' * 1023 + b'ff' * 16384) + # default_rev now resolves to another changeset, hence a different + # HGitaly OID given the current implementation + oid2git.update(oid_mapping(fixture, ((default_rev, b'bigfile', 'blob'), + (default_rev, b'sub', 'tree'), + (default_rev, b'sub/bar', 'blob'), + (default_rev, b'sub/ba2', 'blob'), + (default_rev, b'foo', 'blob'), + ))) + rpc_helper.assert_compare_aggregated(path=b'bigfile') + + # + # reusing content to test GetTreeEntries, hence with a new rpc_helper + # @@ -187,5 +170,22 @@ - # reusing content to test GetTreeEntries + def gte_normalizer(rpc_helper, responses, vcs='hg'): + for resp in responses: + for entry in resp.entries: + if entry.oid and vcs == 'hg': + entry.oid = oid2git[entry.oid] + entry.root_oid = b'' + + rpc_helper = fixture.rpc_helper(stub_cls=CommitServiceStub, + method_name='GetTreeEntries', + request_cls=GetTreeEntriesRequest, + request_defaults=dict( + revision=default_rev, + ), + streaming=True, + normalizer=gte_normalizer, + ) + assert_compare = rpc_helper.assert_compare + for path in (b'.', b'sub'): for recursive in (False, True): for skip_flat in (False, True): @@ -189,9 +189,7 @@ for path in (b'.', b'sub'): for recursive in (False, True): for skip_flat in (False, True): - fixture.assert_compare_get_tree_entries( - path, - skip_flat_paths=skip_flat, - recursive=recursive - ) + assert_compare(path=path, + skip_flat_paths=skip_flat, + recursive=recursive) @@ -197,4 +195,4 @@ - fixture.assert_compare_get_tree_entries(b'.', revision=b'unknown') + assert_compare(path=b'.', revision=b'unknown') # sort parameter @@ -199,3 +197,4 @@ # sort parameter + SortBy = GetTreeEntriesRequest.SortBy for recursive in (False, True): @@ -201,6 +200,6 @@ for recursive in (False, True): - fixture.assert_compare_get_tree_entries(b'.', recursive=recursive, - trees_first=True) + assert_compare(path=b'.', recursive=recursive, + sort=SortBy.TREES_FIRST) # tree first and nested trees nested = sub / 'nested' @@ -208,4 +207,15 @@ (nested / 'deeper').write_text('deep thoughts') wrapper.commit_file('sub/nested/deeper', message='deeper') assert fixture.git_repo.branch_titles() == {b'branch/default': b"deeper"} + + # see comment above about update of OID mapping + oid2git.update(oid_mapping(fixture, ( + (default_rev, b'bigfile', 'blob'), + (default_rev, b'sub', 'tree'), + (default_rev, b'sub/bar', 'blob'), + (default_rev, b'sub/ba2', 'blob'), + (default_rev, b'sub/nested', 'tree'), + (default_rev, b'sub/nested/deeper', 'blob'), + (default_rev, b'foo', 'blob'), + ))) for skip_flat in (False, True): @@ -211,6 +221,7 @@ for skip_flat in (False, True): - fixture.assert_compare_get_tree_entries(b'.', recursive=True, - skip_flat_paths=skip_flat, - trees_first=True) + assert_compare(path=b'.', + recursive=True, + skip_flat_paths=skip_flat, + sort=SortBy.TREES_FIRST) @@ -215,7 +226,7 @@ -def test_compare_get_tree_entries_pagination(tree_blob_fixture): - fixture = tree_blob_fixture +def test_compare_get_tree_entries_pagination(gitaly_comparison): + fixture = gitaly_comparison wrapper = fixture.hg_repo_wrapper wrapper.write_commit('foo', message="Some foo") @@ -233,11 +244,53 @@ wrapper.commit(rel_paths=rel_paths, message="zebar", add_remove=True) - def assert_compare_entries_amount(*resp_collections): - distinct_amounts = set(sum(len(resp.entries) for resp in resps) - for resps in resp_collections) - assert len(distinct_amounts) == 1 - return next(iter(distinct_amounts)) + def normalizer(rpc_helper, responses, **kw): + for resp in responses: + for entry in resp.entries: + entry.oid = entry.root_oid = b'' + resp.pagination_cursor.next_cursor = '' + + def concat_entries(responses): + return list(e for r in responses for e in r.entries) + + def remove_entries(response): + del response.entries[:] + + cursor2git = {} # hg cursor -> git cursor + + rpc_helper = fixture.rpc_helper(stub_cls=CommitServiceStub, + method_name='GetTreeEntries', + request_cls=GetTreeEntriesRequest, + request_defaults=dict( + revision=b'branch/default', + ), + streaming=True, + normalizer=normalizer, + chunked_fields_remover=remove_entries, + chunks_concatenator=concat_entries, + ) + + def request_kwargs_to_git(hg_kwargs): + """Swapping the cursor is too specific for the current RpcHelper. + + We might grow something for all paginated methods, though. + """ + pagination = hg_kwargs.get('pagination_params') + if pagination is None: + return hg_kwargs + + hg_cursor = pagination.page_token + if not hg_cursor: + return hg_kwargs + + git_kwargs = deepcopy(hg_kwargs) + git_kwargs['pagination_params'].page_token = cursor2git[hg_cursor] + return git_kwargs + + rpc_helper.request_kwargs_to_git = request_kwargs_to_git + + assert_compare = rpc_helper.assert_compare + assert_compare_aggregated = rpc_helper.assert_compare_aggregated # asking more than the expected Gitaly first chunk size (2888 entries) # but still less than the total @@ -241,10 +294,10 @@ # asking more than the expected Gitaly first chunk size (2888 entries) # but still less than the total - git_resps, hg_resps = [ - list(fixture.get_tree_entries_raw(vcs, b'sub', - recursive=True, - limit=2890)) - for vcs in ('git', 'hg') - ] + first_resps = assert_compare_aggregated( + path=b'sub', + recursive=True, + pagination_params=PaginationParameter(limit=2890), + compare_first_chunks=False, + ) @@ -250,12 +303,5 @@ - # the page token (aka cursor) being an oid, comparison can only be - # indirect. Chunk sizes are different between Gitaly and HGitaly - assert len(git_resps) > 1 - assert len(hg_resps) > 1 - - assert_compare_entries_amount(git_resps, hg_resps) - - git_cursor, hg_cursor = [resps[0].pagination_cursor.next_cursor - for resps in (git_resps, hg_resps)] + git_cursor, hg_cursor = [resp.pagination_cursor.next_cursor + for resp in first_resps] assert git_cursor assert hg_cursor @@ -260,10 +306,5 @@ assert git_cursor assert hg_cursor - - # cursor is only on first responses (that's probably suboptimal, hence - # prone to change) - assert not any(resp.HasField('pagination_cursor') - for resps in (git_resps, hg_resps) - for resp in resps[1:]) + cursor2git[hg_cursor] = git_cursor # using the cursor @@ -268,13 +309,8 @@ # using the cursor - git_resps, hg_resps = [ - list(fixture.get_tree_entries_raw(vcs, b'sub', - recursive=True, - cursor=cursor, - limit=9000)) - for vcs, cursor in (('git', git_cursor), - ('hg', hg_cursor)) - ] - assert_compare_entries_amount(git_resps, hg_resps) + assert_compare(path=b'sub', + recursive=True, + pagination_params=PaginationParameter(page_token=hg_cursor, + limit=9000)) # negative limit means all results, and there's no cursor if no next page @@ -279,11 +315,10 @@ # negative limit means all results, and there's no cursor if no next page - git_resps, hg_resps = [ - list(fixture.get_tree_entries_raw(vcs, b'sub', - recursive=True, - limit=-1)) - for vcs in ('git', 'hg') - ] - assert_compare_entries_amount(git_resps, hg_resps) - assert git_resps[0].pagination_cursor == hg_resps[0].pagination_cursor + git_resp0, hg_resp0 = assert_compare_aggregated( + path=b'sub', + recursive=True, + pagination_params=PaginationParameter(limit=-1), + compare_first_chunks=False, + ) + assert git_resp0.pagination_cursor == hg_resp0.pagination_cursor @@ -289,10 +324,7 @@ - # case of limit=0 - git_resps, hg_resps = [ - list(fixture.get_tree_entries_raw(vcs, b'sub', - recursive=True, - limit=0)) - for vcs in ('git', 'hg') - ] - assert git_resps == hg_resps # both are empty + # case of limit=0 (empty results) + assert_compare(path=b'sub', + recursive=True, + pagination_params=PaginationParameter(limit=0), + ) @@ -298,13 +330,8 @@ - # case of no params - git_resps, hg_resps = [ - list(fixture.get_tree_entries_raw(vcs, b'sub', - recursive=True, - pagination=False, - limit=0)) - for vcs in ('git', 'hg') - ] - assert_compare_entries_amount(git_resps, hg_resps) + # case of no pagination params + assert_compare_aggregated(path=b'sub', + recursive=True, + compare_first_chunks=False) # case of a cursor that doesn't match any entry (can happen if content # changes between requests) @@ -308,10 +335,13 @@ # case of a cursor that doesn't match any entry (can happen if content # changes between requests) - - fixture.assert_error_compare_get_tree_entries(b'sub', - recursive=True, - cursor="surely not an OID", - limit=10) + cursor = "surely not an OID" + cursor2git[cursor] = cursor + rpc_helper.assert_compare_errors(path=b'sub', + recursive=True, + pagination_params=PaginationParameter( + page_token=cursor, + limit=10) + ) @@ -316,7 +346,14 @@ -def test_compare_get_blob_request(tree_blob_fixture): - fixture = tree_blob_fixture +def rev_path_messages(rev_paths): + """Convert (revision, path) pairs into a list of `RevisionPath` messages. + """ + return [GetBlobsRequest.RevisionPath(revision=rev, path=path) + for rev, path in rev_paths] + + +def test_compare_get_blob_request(gitaly_comparison): + fixture = gitaly_comparison git_repo = fixture.git_repo wrapper = fixture.hg_repo_wrapper @@ -325,5 +362,8 @@ wrapper.commit_file('small', message="Small file") changeset = wrapper.commit_file('foo', message="Large foo", content=large_data) + hg_oid = blob_oid(wrapper.repo, changeset.hex().decode(), b'foo') + + default_rev = b'branch/default' # mirror worked @@ -328,4 +368,4 @@ # mirror worked - assert git_repo.branch_titles() == {b'branch/default': b"Large foo"} + assert git_repo.branch_titles() == {default_rev: b"Large foo"} @@ -331,10 +371,6 @@ - oids = dict( - git=fixture.tree_entry('git', b'foo', limit=1)[0].oid, - hg=blob_oid(wrapper.repo, changeset.hex().decode(), b'foo') - ) - - git_resps = fixture.get_blob('git', oids['git'], limit=12) - # important assumption for hg implementation: - assert git_resps[0].oid == oids['git'] + oid2git = oid_mapping(fixture, + ((default_rev, b'foo', 'blob'), + (default_rev, b'small', 'blob'))) + normalizer = oid_normalizer(oid2git) @@ -340,10 +376,13 @@ - hg_resps = fixture.get_blob('hg', oids['hg'], limit=12) - assert len(hg_resps) == 1 # double-check: already done in direct hg test - assert len(git_resps) == 1 - git_resp, hg_resp = git_resps[0], hg_resps[0] - assert hg_resp.size == git_resp.size - assert hg_resp.data == git_resp.data - - git_resps = fixture.get_blob('git', oids['git']) + rpc_helper = fixture.rpc_helper(stub_cls=BlobServiceStub, + method_name='GetBlob', + request_cls=GetBlobRequest, + request_defaults=dict( + limit=-1, + ), + streaming=True, + normalizer=normalizer, + chunked_fields_remover=remove_data_field, + chunks_concatenator=concat_data_fields, + ) @@ -349,14 +388,4 @@ - hg_resps = fixture.get_blob('hg', oids['hg']) - # Gitaly chunking is not fully deterministic, so the most - # we can check is that chunking occurs for both servers - # and that the first and second responses have the same metadata - assert len(hg_resps) > 1 - assert len(git_resps) > 1 - - assert hg_resps[0].oid == oids['hg'] - assert git_resps[0].oid == oids['git'] - assert hg_resps[1].oid == git_resps[1].oid - for hgr, gitr in zip(hg_resps[:2], git_resps[:2]): - assert hgr.size == gitr.size + def request_kwargs_to_git(hg_kwargs): + """Swapping oid is too specific for the current RpcHelper @@ -362,7 +391,8 @@ - assert ( - b''.join(r.data for r in hg_resps) - == - b''.join(r.data for r in git_resps) - ) + We might provide it with a direct `git` subprocess or with dulwich, + though. + """ + git_kwargs = deepcopy(hg_kwargs) + git_kwargs['oid'] = oid2git[hg_kwargs['oid']] + return git_kwargs @@ -368,7 +398,3 @@ - # now with get_blobs - rev_paths = ((b'branch/default', b'small'), - (b'branch/default', b'does-not-exist'), - (b'no-such-revision', b'small'), - ) + rpc_helper.request_kwargs_to_git = request_kwargs_to_git @@ -374,4 +400,6 @@ - hg_resps = fixture.get_blobs('hg', rev_paths) - git_resps = fixture.get_blobs('git', rev_paths) + rpc_helper.assert_compare(oid=hg_oid, limit=12) + + rpc_helper.assert_compare_aggregated(oid=hg_oid, + compare_first_chunks=False) @@ -377,6 +405,14 @@ - for resp in hg_resps: - resp.oid = '' - for resp in git_resps: - resp.oid = '' + # now with GetBlobs + rpc_helper = fixture.rpc_helper(stub_cls=BlobServiceStub, + method_name='GetBlobs', + request_cls=GetBlobsRequest, + request_defaults=dict( + limit=-1, + ), + streaming=True, + normalizer=normalizer, + chunked_fields_remover=remove_data_field, + chunks_concatenator=concat_data_fields, + ) @@ -382,4 +418,7 @@ - assert hg_resps == git_resps - + rev_paths = rev_path_messages(((b'branch/default', b'small'), + (b'branch/default', b'does-not-exist'), + (b'no-such-revision', b'small'), + )) + rpc_helper.assert_compare(revision_paths=rev_paths) # with limits (the limit is per file) @@ -385,12 +424,4 @@ # with limits (the limit is per file) - hg_resps = fixture.get_blobs('hg', rev_paths, limit=3) - git_resps = fixture.get_blobs('git', rev_paths, limit=3) - - for resp in hg_resps: - resp.oid = '' - for resp in git_resps: - resp.oid = '' - - assert hg_resps == git_resps + rpc_helper.assert_compare(revision_paths=rev_paths, limit=3) # chunking in get_blobs, again non-deterministic for Gitaly @@ -395,23 +426,6 @@ # chunking in get_blobs, again non-deterministic for Gitaly - rev_paths = ((b'branch/default', b'small'), - (b'branch/default', b'foo'), - ) - hg_resps = fixture.get_blobs('hg', rev_paths) - git_resps = fixture.get_blobs('git', rev_paths) - assert len(hg_resps) > 2 - assert len(git_resps) > 2 - assert hg_resps[0].oid != "" - assert git_resps[0].oid != "" - assert hg_resps[1].oid != "" - assert git_resps[1].oid != "" - assert hg_resps[2].oid == "" - assert git_resps[2].oid == "" - for hgr, gitr in zip(hg_resps[:3], git_resps[:3]): - assert hgr.size == gitr.size - - assert ( # content of the big file at 'foo' - b''.join(r.data for r in hg_resps[1:]) - == - b''.join(r.data for r in git_resps[1:]) - ) + rev_paths = rev_path_messages(((b'branch/default', b'small'), + (b'branch/default', b'foo'), + )) + rpc_helper.assert_compare_aggregated(revision_paths=rev_paths) diff --git a/tests_with_gitaly/test_commit.py b/tests_with_gitaly/test_commit.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9jb21taXQucHk=..b872a3a97aacaa11a1cb835009062c2dbcbcc225_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9jb21taXQucHk= 100644 --- a/tests_with_gitaly/test_commit.py +++ b/tests_with_gitaly/test_commit.py @@ -10,6 +10,7 @@ import time from hgitaly.stub.shared_pb2 import ( PaginationParameter, + Repository, ) from hgitaly.stub.commit_pb2 import ( FindCommitsRequest, @@ -474,3 +475,11 @@ assert_compare_errors(revisions=[b'branch/default'], commit_message_patterns=[b'[]'], # invalid regexp same_details=False) + + assert_compare_errors(revisions=[ctx4.hex()], + repository=Repository(storage_name='unknown', + relative_path='/no/matter'), + same_details=False) + assert_compare_errors(revisions=[ctx4.hex()], + repository=None, + same_details=False) diff --git a/tests_with_gitaly/test_ref.py b/tests_with_gitaly/test_ref.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZWYucHk=..b872a3a97aacaa11a1cb835009062c2dbcbcc225_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZWYucHk= 100644 --- a/tests_with_gitaly/test_ref.py +++ b/tests_with_gitaly/test_ref.py @@ -62,6 +62,10 @@ rpc_helper.assert_compare(name=gl_branch) + # invalid case + rpc_helper.assert_compare_errors(name=gl_branch, repository=None, + same_details=False) + @parametrize('branch_style', ['new-style', 'old-style']) def test_compare_find_local_branches(gitaly_comparison, branch_style): diff --git a/tests_with_gitaly/test_repository_service.py b/tests_with_gitaly/test_repository_service.py index c9dd081a4e637dd24c2b42030eb2478bb6add1af_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZXBvc2l0b3J5X3NlcnZpY2UucHk=..b872a3a97aacaa11a1cb835009062c2dbcbcc225_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZXBvc2l0b3J5X3NlcnZpY2UucHk= 100644 --- a/tests_with_gitaly/test_repository_service.py +++ b/tests_with_gitaly/test_repository_service.py @@ -20,6 +20,7 @@ CreateBundleFromRefListRequest, CreateRepositoryFromBundleRequest, FindMergeBaseRequest, + FullPathRequest, RemoveRepositoryRequest, SetFullPathRequest, ) @@ -187,6 +188,83 @@ relative_path='no/such/path')) +def test_repository_exists(gitaly_comparison, server_repos_root): + fixture = gitaly_comparison + grpc_repo = fixture.gitaly_repo + rpc_helper = fixture.rpc_helper( + stub_cls=RepositoryServiceStub, + method_name='RepositoryExists', + request_cls=RemoveRepositoryRequest, + ) + assert_compare = rpc_helper.assert_compare + assert_compare_errors = rpc_helper.assert_compare_errors + + assert_compare(repository=grpc_repo) + assert_compare( + repository=Repository(storage_name=grpc_repo.storage_name, + relative_path='no/such/path')) + + # missing repo *message* (!) + assert_compare_errors(repository=None, same_details=False) + + # unknown storage + assert_compare_errors(same_details=False, + repository=Repository(storage_name='unknown', + relative_path='/some/path')) + + +def test_has_local_branches(gitaly_comparison, server_repos_root): + fixture = gitaly_comparison + grpc_repo = fixture.gitaly_repo + rpc_helper = fixture.rpc_helper( + stub_cls=RepositoryServiceStub, + method_name='HasLocalBranches', + request_cls=RemoveRepositoryRequest, + ) + assert_compare = rpc_helper.assert_compare + assert_compare_errors = rpc_helper.assert_compare_errors + + assert_compare(repository=grpc_repo) + + # repo does not exist + assert_compare_errors( + same_details=False, + repository=Repository(storage_name=grpc_repo.storage_name, + relative_path='no/such/path')) + + # missing repo *message* (!) + assert_compare_errors(repository=None, same_details=False) + + # unknown storage + assert_compare_errors(same_details=False, + repository=Repository(storage_name='unknown', + relative_path='/some/path')) + + +def test_full_path(gitaly_comparison, server_repos_root): + fixture = gitaly_comparison + grpc_repo = fixture.gitaly_repo + + rpc_helper = fixture.rpc_helper( + stub_cls=RepositoryServiceStub, + method_name='FullPath', + request_cls=FullPathRequest, + ) + assert_compare_errors = rpc_helper.assert_compare_errors + + # path not set on existing repo + assert_compare_errors(same_details=False) + + # unknown storage and missing repo + assert_compare_errors(same_details=False, + repository=Repository(storage_name='unknown', + relative_path='/some/path')) + assert_compare_errors( + same_details=False, + repository=Repository(storage_name=grpc_repo.storage_name, + relative_path='no/such/path')) + + def test_set_full_path(gitaly_comparison, server_repos_root): fixture = gitaly_comparison @@ -190,7 +268,8 @@ def test_set_full_path(gitaly_comparison, server_repos_root): fixture = gitaly_comparison - repo_stubs = dict( - hg=RepositoryServiceStub(fixture.hgitaly_channel), - git=RepositoryServiceStub(fixture.gitaly_channel) + rpc_helper = fixture.rpc_helper( + stub_cls=RepositoryServiceStub, + method_name='SetFullPath', + request_cls=SetFullPathRequest, ) @@ -196,23 +275,5 @@ ) - - def do_rpc(vcs, path, grpc_repo=fixture.gitaly_repo): - return repo_stubs[vcs].SetFullPath( - SetFullPathRequest(repository=grpc_repo, path=path)) - - def assert_compare(path): - assert do_rpc('hg', path) == do_rpc('git', path) - - def assert_error_compare(path, - grpc_repo=None, - msg_normalizer=lambda m: m): - kwargs = dict(grpc_repo=grpc_repo) if grpc_repo is not None else {} - with pytest.raises(Exception) as hg_exc_info: - do_rpc('hg', path, **kwargs) - with pytest.raises(Exception) as git_exc_info: - do_rpc('git', path, **kwargs) - hg_exc, git_exc = hg_exc_info.value, git_exc_info.value - assert hg_exc.code() == git_exc.code() - hg_msg, git_msg = hg_exc.details(), git_exc.details() - assert msg_normalizer(hg_msg) == msg_normalizer(git_msg) + assert_compare = rpc_helper.assert_compare + assert_error_compare = rpc_helper.assert_compare_errors # success case @@ -217,7 +278,7 @@ # success case - assert_compare('group/project') + assert_compare(path='group/project') # error cases assert_error_compare('') @@ -220,11 +281,8 @@ # error cases assert_error_compare('') - def normalize_repo_not_found(msg): - return msg.replace('git repo', 'Mercurial repo') - - assert_error_compare('some/full/path', - grpc_repo=Repository( + assert_error_compare(path='some/full/path', + repository=Repository( storage_name=fixture.gitaly_repo.storage_name, relative_path='no/such/repo'), @@ -229,5 +287,5 @@ storage_name=fixture.gitaly_repo.storage_name, relative_path='no/such/repo'), - msg_normalizer=normalize_repo_not_found, + same_details=False, ) @@ -232,7 +290,8 @@ ) - assert_error_compare('some/full/path', - grpc_repo=Repository( + assert_error_compare(path='some/full/path', + same_details=False, + repository=Repository( relative_path=fixture.gitaly_repo.relative_path, storage_name='unknown'))