refactor(git): introduce GitRunError hierarchy, surface non-repo errors, and improve verification queries
Some checks failed
Mark stable commit / test-unit (push) Has been cancelled
Mark stable commit / test-integration (push) Has been cancelled
Mark stable commit / test-env-virtual (push) Has been cancelled
Mark stable commit / test-env-nix (push) Has been cancelled
Mark stable commit / test-e2e (push) Has been cancelled
Mark stable commit / test-virgin-user (push) Has been cancelled
Mark stable commit / test-virgin-root (push) Has been cancelled
Mark stable commit / lint-shell (push) Has been cancelled
Mark stable commit / lint-python (push) Has been cancelled
Mark stable commit / mark-stable (push) Has been cancelled
Some checks failed
Mark stable commit / test-unit (push) Has been cancelled
Mark stable commit / test-integration (push) Has been cancelled
Mark stable commit / test-env-virtual (push) Has been cancelled
Mark stable commit / test-env-nix (push) Has been cancelled
Mark stable commit / test-e2e (push) Has been cancelled
Mark stable commit / test-virgin-user (push) Has been cancelled
Mark stable commit / test-virgin-root (push) Has been cancelled
Mark stable commit / lint-shell (push) Has been cancelled
Mark stable commit / lint-python (push) Has been cancelled
Mark stable commit / mark-stable (push) Has been cancelled
* Replace legacy GitError usage with a clearer exception hierarchy: * GitBaseError as the common root for all git-related failures * GitRunError for subprocess execution failures * GitQueryError for read-only query failures * GitCommandError for state-changing command failures * GitNotRepositoryError to explicitly signal “not a git repository” situations * Update git runner to detect “not a git repository” stderr and raise GitNotRepositoryError with rich context (cwd, command, stderr) * Refactor repository verification to use dedicated query helpers instead of ad-hoc subprocess calls: * get_remote_head_commit (ls-remote) for pull mode * get_head_commit for local mode * get_latest_signing_key (%GK) for signature verification * Add strict vs best-effort behavior in verify_repository: * Best-effort collection for reporting (does not block when no verification config exists) * Strict retrieval and explicit error messages when verification is configured * Clear failure cases when commit/signing key cannot be determined * Add new unit tests covering: * get_latest_signing_key output stripping and error wrapping * get_remote_head_commit parsing, empty output, and error wrapping * verify_repository success/failure scenarios and “do not swallow GitNotRepositoryError” * Adjust imports and exception handling across actions/commands/queries to align with GitRunError-based handling while keeping GitNotRepositoryError uncaught for debugging clarity https://chatgpt.com/share/6943173c-508c-800f-8879-af75d131c79b
This commit is contained in:
@@ -2,7 +2,7 @@ import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.actions.branch.close_branch import close_branch
|
||||
from pkgmgr.core.git.errors import GitError
|
||||
from pkgmgr.core.git.errors import GitRunError
|
||||
from pkgmgr.core.git.commands import GitDeleteRemoteBranchError
|
||||
|
||||
|
||||
@@ -90,7 +90,7 @@ class TestCloseBranch(unittest.TestCase):
|
||||
delete_local_branch.assert_called_once_with("feature-x", cwd=".", force=False)
|
||||
delete_remote_branch.assert_called_once_with("origin", "feature-x", cwd=".")
|
||||
|
||||
@patch("pkgmgr.actions.branch.close_branch.get_current_branch", side_effect=GitError("fail"))
|
||||
@patch("pkgmgr.actions.branch.close_branch.get_current_branch", side_effect=GitRunError("fail"))
|
||||
def test_close_branch_errors_if_cannot_detect_branch(self, _current) -> None:
|
||||
with self.assertRaises(RuntimeError):
|
||||
close_branch(None)
|
||||
|
||||
@@ -2,7 +2,7 @@ import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.actions.branch.drop_branch import drop_branch
|
||||
from pkgmgr.core.git.errors import GitError
|
||||
from pkgmgr.core.git.errors import GitRunError
|
||||
from pkgmgr.core.git.commands import GitDeleteRemoteBranchError
|
||||
|
||||
|
||||
@@ -50,7 +50,7 @@ class TestDropBranch(unittest.TestCase):
|
||||
delete_local.assert_called_once_with("feature-x", cwd=".", force=False)
|
||||
delete_remote.assert_called_once_with("origin", "feature-x", cwd=".")
|
||||
|
||||
@patch("pkgmgr.actions.branch.drop_branch.get_current_branch", side_effect=GitError("fail"))
|
||||
@patch("pkgmgr.actions.branch.drop_branch.get_current_branch", side_effect=GitRunError("fail"))
|
||||
def test_drop_branch_errors_if_no_branch_detected(self, _current) -> None:
|
||||
with self.assertRaises(RuntimeError):
|
||||
drop_branch(None)
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.core.git.errors import GitNotRepositoryError, GitRunError
|
||||
from pkgmgr.core.git.queries.get_latest_signing_key import (
|
||||
GitLatestSigningKeyQueryError,
|
||||
get_latest_signing_key,
|
||||
)
|
||||
|
||||
|
||||
class TestGetLatestSigningKey(unittest.TestCase):
|
||||
@patch("pkgmgr.core.git.queries.get_latest_signing_key.run", return_value="ABCDEF1234567890\n")
|
||||
def test_strips_output(self, _mock_run) -> None:
|
||||
out = get_latest_signing_key(cwd="/tmp/repo")
|
||||
self.assertEqual(out, "ABCDEF1234567890")
|
||||
|
||||
@patch("pkgmgr.core.git.queries.get_latest_signing_key.run", side_effect=GitRunError("boom"))
|
||||
def test_wraps_git_run_error(self, _mock_run) -> None:
|
||||
with self.assertRaises(GitLatestSigningKeyQueryError):
|
||||
get_latest_signing_key(cwd="/tmp/repo")
|
||||
|
||||
@patch("pkgmgr.core.git.queries.get_latest_signing_key.run", side_effect=GitNotRepositoryError("no repo"))
|
||||
def test_does_not_catch_not_repository_error(self, _mock_run) -> None:
|
||||
with self.assertRaises(GitNotRepositoryError):
|
||||
get_latest_signing_key(cwd="/tmp/no-repo")
|
||||
@@ -0,0 +1,32 @@
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.core.git.errors import GitNotRepositoryError, GitRunError
|
||||
from pkgmgr.core.git.queries.get_remote_head_commit import (
|
||||
GitRemoteHeadCommitQueryError,
|
||||
get_remote_head_commit,
|
||||
)
|
||||
|
||||
|
||||
class TestGetRemoteHeadCommit(unittest.TestCase):
|
||||
@patch("pkgmgr.core.git.queries.get_remote_head_commit.run", return_value="abc123\tHEAD\n")
|
||||
def test_parses_first_token_as_hash(self, mock_run) -> None:
|
||||
out = get_remote_head_commit(cwd="/tmp/repo")
|
||||
self.assertEqual(out, "abc123")
|
||||
mock_run.assert_called_once()
|
||||
|
||||
@patch("pkgmgr.core.git.queries.get_remote_head_commit.run", return_value="")
|
||||
def test_returns_empty_string_on_empty_output(self, _mock_run) -> None:
|
||||
out = get_remote_head_commit(cwd="/tmp/repo")
|
||||
self.assertEqual(out, "")
|
||||
|
||||
@patch("pkgmgr.core.git.queries.get_remote_head_commit.run", side_effect=GitRunError("boom"))
|
||||
def test_wraps_git_run_error(self, _mock_run) -> None:
|
||||
with self.assertRaises(GitRemoteHeadCommitQueryError) as ctx:
|
||||
get_remote_head_commit(cwd="/tmp/repo")
|
||||
self.assertIn("Failed to query remote head commit", str(ctx.exception))
|
||||
|
||||
@patch("pkgmgr.core.git.queries.get_remote_head_commit.run", side_effect=GitNotRepositoryError("no repo"))
|
||||
def test_does_not_catch_not_repository_error(self, _mock_run) -> None:
|
||||
with self.assertRaises(GitNotRepositoryError):
|
||||
get_remote_head_commit(cwd="/tmp/no-repo")
|
||||
@@ -6,7 +6,7 @@ from __future__ import annotations
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.core.git import GitError
|
||||
from pkgmgr.core.git import GitRunError
|
||||
from pkgmgr.core.git.queries.probe_remote_reachable import probe_remote_reachable
|
||||
|
||||
|
||||
@@ -32,7 +32,7 @@ class TestProbeRemoteReachable(unittest.TestCase):
|
||||
|
||||
@patch("pkgmgr.core.git.queries.probe_remote_reachable.run")
|
||||
def test_probe_remote_reachable_failure_returns_false(self, mock_run) -> None:
|
||||
mock_run.side_effect = GitError("Git command failed (simulated)")
|
||||
mock_run.side_effect = GitRunError("Git command failed (simulated)")
|
||||
|
||||
ok = probe_remote_reachable(
|
||||
"ssh://git@code.example.org:2201/alice/repo.git",
|
||||
|
||||
@@ -3,7 +3,7 @@ from __future__ import annotations
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.core.git import GitError
|
||||
from pkgmgr.core.git import GitRunError
|
||||
from pkgmgr.core.git.queries.resolve_base_branch import (
|
||||
GitBaseBranchNotFoundError,
|
||||
resolve_base_branch,
|
||||
@@ -21,7 +21,7 @@ class TestResolveBaseBranch(unittest.TestCase):
|
||||
@patch("pkgmgr.core.git.queries.resolve_base_branch.run")
|
||||
def test_resolves_fallback(self, mock_run):
|
||||
mock_run.side_effect = [
|
||||
GitError("fatal: Needed a single revision"), # treat as "missing"
|
||||
GitRunError("fatal: Needed a single revision"), # treat as "missing"
|
||||
"dummy",
|
||||
]
|
||||
result = resolve_base_branch("main", "master", cwd=".")
|
||||
@@ -32,8 +32,8 @@ class TestResolveBaseBranch(unittest.TestCase):
|
||||
@patch("pkgmgr.core.git.queries.resolve_base_branch.run")
|
||||
def test_raises_when_no_branch_exists(self, mock_run):
|
||||
mock_run.side_effect = [
|
||||
GitError("fatal: Needed a single revision"),
|
||||
GitError("fatal: Needed a single revision"),
|
||||
GitRunError("fatal: Needed a single revision"),
|
||||
GitRunError("fatal: Needed a single revision"),
|
||||
]
|
||||
with self.assertRaises(GitBaseBranchNotFoundError):
|
||||
resolve_base_branch("main", "master", cwd=".")
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import unittest
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from pkgmgr.core.git.errors import GitError
|
||||
from pkgmgr.core.git.errors import GitRunError
|
||||
from pkgmgr.core.git.run import run
|
||||
|
||||
|
||||
@@ -55,7 +55,7 @@ class TestGitRun(unittest.TestCase):
|
||||
exc.stderr = "ERR!"
|
||||
|
||||
with patch("pkgmgr.core.git.run.subprocess.run", side_effect=exc):
|
||||
with self.assertRaises(GitError) as ctx:
|
||||
with self.assertRaises(GitRunError) as ctx:
|
||||
run(["status"], cwd="/bad/repo", preview=False)
|
||||
|
||||
msg = str(ctx.exception)
|
||||
|
||||
87
tests/unit/pkgmgr/core/repository/test_verify_repository.py
Normal file
87
tests/unit/pkgmgr/core/repository/test_verify_repository.py
Normal file
@@ -0,0 +1,87 @@
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.core.git.errors import GitNotRepositoryError
|
||||
from pkgmgr.core.git.queries.get_latest_signing_key import GitLatestSigningKeyQueryError
|
||||
from pkgmgr.core.git.queries.get_remote_head_commit import GitRemoteHeadCommitQueryError
|
||||
from pkgmgr.core.repository.verify import verify_repository
|
||||
|
||||
|
||||
class TestVerifyRepository(unittest.TestCase):
|
||||
def test_no_verified_info_returns_ok_and_best_effort_values(self) -> None:
|
||||
repo = {"id": "demo"} # no "verified"
|
||||
with patch("pkgmgr.core.repository.verify.get_head_commit", return_value="deadbeef"), patch(
|
||||
"pkgmgr.core.repository.verify.get_latest_signing_key",
|
||||
return_value="KEYID",
|
||||
):
|
||||
ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local")
|
||||
self.assertTrue(ok)
|
||||
self.assertEqual(errors, [])
|
||||
self.assertEqual(commit, "deadbeef")
|
||||
self.assertEqual(key, "KEYID")
|
||||
|
||||
def test_best_effort_swallows_query_errors_when_no_verified_info(self) -> None:
|
||||
repo = {"id": "demo"}
|
||||
with patch(
|
||||
"pkgmgr.core.repository.verify.get_head_commit",
|
||||
return_value=None,
|
||||
), patch(
|
||||
"pkgmgr.core.repository.verify.get_latest_signing_key",
|
||||
side_effect=GitLatestSigningKeyQueryError("fail signing key"),
|
||||
):
|
||||
ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local")
|
||||
self.assertTrue(ok)
|
||||
self.assertEqual(errors, [])
|
||||
self.assertEqual(commit, "")
|
||||
self.assertEqual(key, "")
|
||||
|
||||
def test_verified_commit_mismatch_fails(self) -> None:
|
||||
repo = {"verified": {"commit": "expected", "gpg_keys": None}}
|
||||
with patch("pkgmgr.core.repository.verify.get_head_commit", return_value="actual"), patch(
|
||||
"pkgmgr.core.repository.verify.get_latest_signing_key",
|
||||
return_value="",
|
||||
):
|
||||
ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local")
|
||||
|
||||
self.assertFalse(ok)
|
||||
self.assertIn("Expected commit: expected, found: actual", errors)
|
||||
self.assertEqual(commit, "actual")
|
||||
self.assertEqual(key, "")
|
||||
|
||||
def test_verified_gpg_key_missing_fails(self) -> None:
|
||||
repo = {"verified": {"commit": None, "gpg_keys": ["ABC"]}}
|
||||
with patch("pkgmgr.core.repository.verify.get_head_commit", return_value=""), patch(
|
||||
"pkgmgr.core.repository.verify.get_latest_signing_key",
|
||||
return_value="",
|
||||
):
|
||||
ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="local")
|
||||
|
||||
self.assertFalse(ok)
|
||||
self.assertTrue(any("no signing key was found" in e for e in errors))
|
||||
self.assertEqual(commit, "")
|
||||
self.assertEqual(key, "")
|
||||
|
||||
def test_strict_pull_collects_remote_error_message(self) -> None:
|
||||
repo = {"verified": {"commit": "expected", "gpg_keys": None}}
|
||||
with patch(
|
||||
"pkgmgr.core.repository.verify.get_remote_head_commit",
|
||||
side_effect=GitRemoteHeadCommitQueryError("remote fail"),
|
||||
), patch(
|
||||
"pkgmgr.core.repository.verify.get_latest_signing_key",
|
||||
return_value="",
|
||||
):
|
||||
ok, errors, commit, key = verify_repository(repo, "/tmp/repo", mode="pull")
|
||||
|
||||
self.assertFalse(ok)
|
||||
self.assertIn("remote fail", " ".join(errors))
|
||||
self.assertEqual(commit, "")
|
||||
self.assertEqual(key, "")
|
||||
|
||||
def test_not_repository_error_is_not_caught(self) -> None:
|
||||
repo = {"verified": {"commit": "expected", "gpg_keys": None}}
|
||||
with patch(
|
||||
"pkgmgr.core.repository.verify.get_head_commit",
|
||||
side_effect=GitNotRepositoryError("no repo"),
|
||||
):
|
||||
with self.assertRaises(GitNotRepositoryError):
|
||||
verify_repository(repo, "/tmp/no-repo", mode="local")
|
||||
@@ -4,7 +4,7 @@
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.core.git.errors import GitError
|
||||
from pkgmgr.core.git.errors import GitRunError
|
||||
from pkgmgr.core.git.run import run
|
||||
from pkgmgr.core.git.queries import get_tags, get_head_commit, get_current_branch
|
||||
|
||||
@@ -35,7 +35,7 @@ class TestGitRun(unittest.TestCase):
|
||||
stderr="error\n",
|
||||
)
|
||||
|
||||
with self.assertRaises(GitError) as ctx:
|
||||
with self.assertRaises(GitRunError) as ctx:
|
||||
run(["status"], cwd="/tmp/repo")
|
||||
|
||||
msg = str(ctx.exception)
|
||||
@@ -66,7 +66,7 @@ class TestGitQueries(unittest.TestCase):
|
||||
|
||||
@patch("pkgmgr.core.git.queries.get_head_commit.run")
|
||||
def test_get_head_commit_failure_returns_none(self, mock_run):
|
||||
mock_run.side_effect = GitError("fail")
|
||||
mock_run.side_effect = GitRunError("fail")
|
||||
commit = get_head_commit(cwd="/tmp/repo")
|
||||
self.assertIsNone(commit)
|
||||
|
||||
@@ -78,7 +78,7 @@ class TestGitQueries(unittest.TestCase):
|
||||
|
||||
@patch("pkgmgr.core.git.queries.get_current_branch.run")
|
||||
def test_get_current_branch_failure_returns_none(self, mock_run):
|
||||
mock_run.side_effect = GitError("fail")
|
||||
mock_run.side_effect = GitRunError("fail")
|
||||
branch = get_current_branch(cwd="/tmp/repo")
|
||||
self.assertIsNone(branch)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user