diff --git a/pygit2/branches.py b/pygit2/branches.py index 77cabfc1..5819333f 100644 --- a/pygit2/branches.py +++ b/pygit2/branches.py @@ -28,7 +28,7 @@ from collections.abc import Iterator from typing import TYPE_CHECKING -from ._pygit2 import Branch, Commit, Oid +from ._pygit2 import Branch, Commit, InvalidSpecError, Oid from .enums import BranchType, ReferenceType # Need BaseRepository for type hints, but don't let it cause a circular dependency @@ -72,11 +72,13 @@ def __getitem__(self, name: str) -> Branch: return branch - def get(self, key: str) -> Branch: + def get(self, key: str) -> Branch | None: try: return self[key] - except KeyError: - return None # type:ignore # next commit + except (KeyError, InvalidSpecError): + # As in References.get: git_branch_lookup reports + # GIT_EINVALIDSPEC for a name that is not a valid branch name. + return None def __iter__(self) -> Iterator[str]: for branch_name in self._repository.listall_branches(self._flag): diff --git a/pygit2/references.py b/pygit2/references.py index 533e90ab..4094bb56 100644 --- a/pygit2/references.py +++ b/pygit2/references.py @@ -30,6 +30,7 @@ from pygit2 import Oid +from ._pygit2 import InvalidSpecError from .enums import ReferenceFilter # Need BaseRepository for type hints, but don't let it cause a circular dependency @@ -48,7 +49,10 @@ def __getitem__(self, name: str) -> 'Reference': def get(self, key: str) -> 'Reference' | None: try: return self[key] - except KeyError: + except (KeyError, InvalidSpecError): + # git_reference_lookup reports GIT_EINVALIDSPEC as well as + # GIT_ENOTFOUND for a name it cannot resolve; only the latter + # arrives as a KeyError. return None def __iter__(self) -> Iterator[str]: diff --git a/test/test_branch.py b/test/test_branch.py index 14cb965f..e6f5940c 100644 --- a/test/test_branch.py +++ b/test/test_branch.py @@ -85,11 +85,13 @@ def test_branches_delete_error(testrepo: Repository) -> None: def test_branches_is_head(testrepo: Repository) -> None: branch = testrepo.branches.get('master') + assert branch is not None assert branch.is_head() def test_branches_is_not_head(testrepo: Repository) -> None: branch = testrepo.branches.get('i18n') + assert branch is not None assert not branch.is_head() @@ -98,34 +100,40 @@ def test_branches_rename(testrepo: Repository) -> None: assert new_branch.target == I18N_LAST_COMMIT new_branch_2 = testrepo.branches.get('new-branch') + assert new_branch_2 is not None assert new_branch_2.target == I18N_LAST_COMMIT def test_branches_rename_error(testrepo: Repository) -> None: original_branch = testrepo.branches.get('i18n') + assert original_branch is not None with pytest.raises(ValueError): original_branch.rename('master') def test_branches_rename_force(testrepo: Repository) -> None: original_branch = testrepo.branches.get('master') + assert original_branch is not None new_branch = original_branch.rename('i18n', True) assert new_branch.target == LAST_COMMIT def test_branches_rename_invalid(testrepo: Repository) -> None: original_branch = testrepo.branches.get('i18n') + assert original_branch is not None with pytest.raises(ValueError): original_branch.rename('abc@{123') def test_branches_name(testrepo: Repository) -> None: branch = testrepo.branches.get('master') + assert branch is not None assert branch.branch_name == 'master' assert branch.name == 'refs/heads/master' assert branch.raw_branch_name == branch.branch_name.encode('utf-8') branch = testrepo.branches.get('i18n') + assert branch is not None assert branch.branch_name == 'i18n' assert branch.name == 'refs/heads/i18n' assert branch.raw_branch_name == branch.branch_name.encode('utf-8') @@ -268,3 +276,16 @@ def test_branch_name(testrepo: Repository) -> None: branch = testrepo.lookup_branch('i18n') assert branch.branch_name == 'i18n' assert branch.name == 'refs/heads/i18n' + + +def test_branches_get_invalid_name(testrepo: Repository) -> None: + """Branches.get() returns None for a name that is not a valid branch name. + + git_branch_lookup reports GIT_EINVALIDSPEC for these, which arrives as + InvalidSpecError rather than KeyError. + """ + repo = testrepo + + assert repo.branches.get('my branch') is None + assert 'my branch' not in repo.branches + assert repo.branches.get('does-not-exist') is None diff --git a/test/test_branch_empty.py b/test/test_branch_empty.py index b0ef793c..7934c60b 100644 --- a/test/test_branch_empty.py +++ b/test/test_branch_empty.py @@ -42,6 +42,7 @@ def repo(emptyrepo: Repository) -> Generator[Repository, None, None]: def test_branches_remote_get(repo: Repository) -> None: branch = repo.branches.remote.get('origin/master') + assert branch is not None assert branch.target == ORIGIN_MASTER_COMMIT assert repo.branches.remote.get('origin/not-exists') is None diff --git a/test/test_packbuilder.py b/test/test_packbuilder.py index 06410699..c3552c0c 100644 --- a/test/test_packbuilder.py +++ b/test/test_packbuilder.py @@ -75,6 +75,7 @@ def test_pack_with_delegate(testrepo: Repository, tmp_path: Path) -> None: def pack_delegate(pb: PackBuilder) -> None: for branch in pb._repo.branches: br = pb._repo.branches.get(branch) + assert br is not None for commit in br.log(): pb.add_recur(commit.oid_new) diff --git a/test/test_refs.py b/test/test_refs.py index 80c59fc9..f2e35fdb 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -466,6 +466,26 @@ def test_lookup_reference(testrepo: Repository) -> None: assert reference.name == 'refs/heads/master' +def test_references_get_invalid_name(testrepo: Repository) -> None: + """A name libgit2 rejects as a spec is still "not found" to get(). + + git_reference_lookup reports GIT_EINVALIDSPEC as well as GIT_ENOTFOUND, + and only the latter reaches pygit2 as a KeyError. + """ + repo = testrepo + + # 'master' is not a full reference name, so libgit2 calls it invalid. + assert repo.references.get('master') is None + assert 'master' not in repo.references + + # A well-formed name that does not exist behaves the same way. + assert repo.references.get('refs/heads/does-not-exist') is None + + # __getitem__ still raises, as documented. + with pytest.raises((KeyError, InvalidSpecError)): + repo.references['master'] + + def test_lookup_reference_dwim(testrepo: Repository) -> None: repo = testrepo