refactor(mirror): probe remotes with detailed reasons and provision all git mirrors
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

- Add probe_remote_reachable_detail and improved GitRunError metadata
- Print short failure reasons for unreachable remotes
- Provision each git mirror URL via ensure_remote_repository_for_url

https://chatgpt.com/share/6946956e-f738-800f-a446-e2c8bf5595f4
This commit is contained in:
Kevin Veen-Birkenbach
2025-12-20 13:23:24 +01:00
parent 10998e50ad
commit a2138c9985
10 changed files with 706 additions and 74 deletions

View File

@@ -113,17 +113,12 @@ class TestIntegrationMirrorCommands(unittest.TestCase):
)
)
# Deterministic remote probing (new refactor: probe_remote_reachable)
# Deterministic remote probing (refactor: probe_remote_reachable_detail)
# Patch where it is USED (setup_cmd imported it directly).
stack.enter_context(
_p(
"pkgmgr.core.git.queries.probe_remote_reachable",
return_value=True,
)
)
stack.enter_context(
_p(
"pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable",
return_value=True,
"pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable_detail",
return_value=(True, ""),
)
)

View File

@@ -0,0 +1,220 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-
"""
Integration test for mirror probing + provisioning after refactor.
We test the CLI entrypoint `handle_mirror_command()` directly to avoid
depending on repo-selection / config parsing for `--all`.
Covers:
- setup_cmd uses probe_remote_reachable_detail()
- check prints [OK]/[WARN] and 'reason:' lines for failures
- provision triggers ensure_remote_repo (preview-safe) for each git mirror
"""
from __future__ import annotations
import io
import tempfile
import unittest
from contextlib import redirect_stderr, redirect_stdout
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import MagicMock, PropertyMock, patch
from pkgmgr.cli.commands.mirror import handle_mirror_command
class TestIntegrationMirrorProbeDetailAndProvision(unittest.TestCase):
def _make_ctx(
self, *, repositories_base_dir: str, all_repositories: list[dict]
) -> MagicMock:
ctx = MagicMock()
ctx.repositories_base_dir = repositories_base_dir
ctx.all_repositories = all_repositories
# mirror merge may look at this; keep it present for safety
ctx.user_config_path = str(Path(repositories_base_dir) / "user.yml")
return ctx
def _make_dummy_repo_ctx(self, *, repo_dir: str) -> MagicMock:
"""
This is the RepoMirrorContext-like object returned by build_context().
"""
dummy = MagicMock()
dummy.identifier = "dummy-repo"
dummy.repo_dir = repo_dir
dummy.config_mirrors = {"origin": "git@github.com:alice/repo.git"}
dummy.file_mirrors = {"backup": "ssh://git@git.example:2201/alice/repo.git"}
type(dummy).resolved_mirrors = PropertyMock(
return_value={
"origin": "git@github.com:alice/repo.git",
"backup": "ssh://git@git.example:2201/alice/repo.git",
}
)
return dummy
def _run_handle(
self,
*,
subcommand: str,
preview: bool,
selected: list[dict],
dummy_repo_dir: str,
probe_detail_side_effect,
) -> str:
"""
Run handle_mirror_command() with patched side effects and capture output.
"""
args = SimpleNamespace(subcommand=subcommand, preview=preview)
# Fake ensure_remote_repo result (preview safe)
def _fake_ensure_remote_repo(spec, provider_hint=None, options=None):
if options is not None and getattr(options, "preview", False) is not True:
raise AssertionError(
"ensure_remote_repo called without preview=True (should never happen in tests)."
)
r = MagicMock()
r.status = "preview"
r.message = "Preview mode: no remote provisioning performed."
r.url = None
return r
buf = io.StringIO()
ctx = self._make_ctx(
repositories_base_dir=str(Path(dummy_repo_dir).parent),
all_repositories=selected,
)
dummy_repo_ctx = self._make_dummy_repo_ctx(repo_dir=dummy_repo_dir)
with (
patch(
"pkgmgr.actions.mirror.setup_cmd.build_context",
return_value=dummy_repo_ctx,
),
patch(
"pkgmgr.actions.mirror.setup_cmd.ensure_origin_remote",
return_value=None,
),
patch(
"pkgmgr.actions.mirror.git_remote.ensure_origin_remote",
return_value=None,
),
patch(
"pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable_detail",
side_effect=probe_detail_side_effect,
),
patch(
"pkgmgr.actions.mirror.remote_provision.ensure_remote_repo",
side_effect=_fake_ensure_remote_repo,
),
redirect_stdout(buf),
redirect_stderr(buf),
):
handle_mirror_command(ctx, args, selected)
return buf.getvalue()
def test_mirror_check_preview_prints_warn_reason(self) -> None:
"""
'mirror check --preview' should:
- probe both git mirrors
- print [OK] for origin
- print [WARN] for backup + reason line
"""
with tempfile.TemporaryDirectory() as tmp:
tmp_path = Path(tmp)
repo_dir = tmp_path / "dummy-repo"
repo_dir.mkdir(parents=True, exist_ok=True)
selected = [
{"provider": "github.com", "account": "alice", "repository": "repo"}
]
def probe_side_effect(url: str, cwd: str = "."):
if "github.com" in url:
# show "empty repo reachable" note; setup_cmd prints [OK] and does not print reason for ok
return (
True,
"remote reachable, but no refs found yet (empty repository)",
)
return False, "(exit 128) fatal: Could not read from remote repository."
out = self._run_handle(
subcommand="check",
preview=True,
selected=selected,
dummy_repo_dir=str(repo_dir),
probe_detail_side_effect=probe_side_effect,
)
self.assertIn("[MIRROR SETUP:REMOTE]", out)
# origin OK (even with a note returned; still OK)
self.assertIn("[OK] origin: git@github.com:alice/repo.git", out)
# backup WARN prints reason line
self.assertIn(
"[WARN] backup: ssh://git@git.example:2201/alice/repo.git", out
)
self.assertIn("reason:", out)
self.assertIn("Could not read from remote repository", out)
def test_mirror_provision_preview_provisions_each_git_mirror(self) -> None:
"""
'mirror provision --preview' should:
- print provisioning lines for each git mirror
- still probe and print [OK]/[WARN]
- call ensure_remote_repo only in preview mode (enforced by fake)
"""
with tempfile.TemporaryDirectory() as tmp:
tmp_path = Path(tmp)
repo_dir = tmp_path / "dummy-repo"
repo_dir.mkdir(parents=True, exist_ok=True)
selected = [
{
"provider": "github.com",
"account": "alice",
"repository": "repo",
"private": True,
"description": "desc",
}
]
def probe_side_effect(url: str, cwd: str = "."):
if "github.com" in url:
return True, ""
return False, "(exit 128) fatal: Could not read from remote repository."
out = self._run_handle(
subcommand="provision",
preview=True,
selected=selected,
dummy_repo_dir=str(repo_dir),
probe_detail_side_effect=probe_side_effect,
)
# provisioning should attempt BOTH mirrors
self.assertIn(
"[REMOTE ENSURE] ensuring mirror 'origin': git@github.com:alice/repo.git",
out,
)
self.assertIn(
"[REMOTE ENSURE] ensuring mirror 'backup': ssh://git@git.example:2201/alice/repo.git",
out,
)
# patched ensure_remote_repo prints PREVIEW status via remote_provision
self.assertIn("[REMOTE ENSURE]", out)
self.assertIn("PREVIEW", out.upper())
# probes after provisioning
self.assertIn("[OK] origin: git@github.com:alice/repo.git", out)
self.assertIn(
"[WARN] backup: ssh://git@git.example:2201/alice/repo.git", out
)
if __name__ == "__main__":
unittest.main()

View File

@@ -40,8 +40,8 @@ class TestCreateRepoPypiNotInGitConfig(unittest.TestCase):
with (
# Avoid any real network calls during mirror "remote probing"
patch(
"pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable",
return_value=True,
"pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable_detail",
return_value=(True, ""),
),
# Force templates to come from our temp directory
patch(

View File

@@ -38,7 +38,6 @@ class TestMirrorSetupCmd(unittest.TestCase):
ensure_remote=False,
)
# ensure_origin_remote(repo, ctx, preview) is called positionally in your code
m_ensure.assert_called_once()
args, kwargs = m_ensure.call_args
@@ -50,13 +49,13 @@ class TestMirrorSetupCmd(unittest.TestCase):
@patch("pkgmgr.actions.mirror.setup_cmd.build_context")
@patch("pkgmgr.actions.mirror.setup_cmd.determine_primary_remote_url")
@patch("pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable")
@patch("pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable_detail")
def test_setup_mirrors_remote_no_mirrors_probes_primary(
self, m_probe, m_primary, m_ctx
self, m_probe_detail, m_primary, m_ctx
) -> None:
m_ctx.return_value = self._ctx(repo_dir="/tmp/repo", resolved={})
m_primary.return_value = "git@github.com:alice/repo.git"
m_probe.return_value = True
m_probe_detail.return_value = (True, "")
repos = [{"provider": "github.com", "account": "alice", "repository": "repo"}]
setup_mirrors(
@@ -70,14 +69,14 @@ class TestMirrorSetupCmd(unittest.TestCase):
)
m_primary.assert_called()
m_probe.assert_called_once_with(
m_probe_detail.assert_called_once_with(
"git@github.com:alice/repo.git", cwd="/tmp/repo"
)
@patch("pkgmgr.actions.mirror.setup_cmd.build_context")
@patch("pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable")
@patch("pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable_detail")
def test_setup_mirrors_remote_with_mirrors_probes_each(
self, m_probe, m_ctx
self, m_probe_detail, m_ctx
) -> None:
m_ctx.return_value = self._ctx(
repo_dir="/tmp/repo",
@@ -86,7 +85,7 @@ class TestMirrorSetupCmd(unittest.TestCase):
"backup": "ssh://git@git.veen.world:2201/alice/repo.git",
},
)
m_probe.return_value = True
m_probe_detail.return_value = (True, "")
repos = [{"provider": "github.com", "account": "alice", "repository": "repo"}]
setup_mirrors(
@@ -99,12 +98,105 @@ class TestMirrorSetupCmd(unittest.TestCase):
ensure_remote=False,
)
self.assertEqual(m_probe.call_count, 2)
m_probe.assert_any_call("git@github.com:alice/repo.git", cwd="/tmp/repo")
m_probe.assert_any_call(
# Should probe BOTH git mirror URLs
self.assertEqual(m_probe_detail.call_count, 2)
m_probe_detail.assert_any_call("git@github.com:alice/repo.git", cwd="/tmp/repo")
m_probe_detail.assert_any_call(
"ssh://git@git.veen.world:2201/alice/repo.git", cwd="/tmp/repo"
)
@patch("pkgmgr.actions.mirror.setup_cmd.build_context")
@patch("pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable_detail")
@patch("pkgmgr.actions.mirror.setup_cmd.ensure_remote_repository_for_url")
def test_setup_mirrors_remote_with_mirrors_ensure_remote_provisions_each(
self, m_ensure_url, m_probe_detail, m_ctx
) -> None:
m_ctx.return_value = self._ctx(
repo_dir="/tmp/repo",
resolved={
"origin": "git@github.com:alice/repo.git",
"backup": "ssh://git@git.veen.world:2201/alice/repo.git",
},
)
m_probe_detail.return_value = (True, "")
repos = [
{
"provider": "github.com",
"account": "alice",
"repository": "repo",
"private": True,
"description": "desc",
}
]
setup_mirrors(
selected_repos=repos,
repositories_base_dir="/tmp",
all_repos=repos,
preview=True,
local=False,
remote=True,
ensure_remote=True,
)
# Provision both mirrors
self.assertEqual(m_ensure_url.call_count, 2)
m_ensure_url.assert_any_call(
url="git@github.com:alice/repo.git",
private_default=True,
description="desc",
preview=True,
)
m_ensure_url.assert_any_call(
url="ssh://git@git.veen.world:2201/alice/repo.git",
private_default=True,
description="desc",
preview=True,
)
# Still probes both
self.assertEqual(m_probe_detail.call_count, 2)
@patch("pkgmgr.actions.mirror.setup_cmd.build_context")
@patch("pkgmgr.actions.mirror.setup_cmd.determine_primary_remote_url")
@patch("pkgmgr.actions.mirror.setup_cmd.ensure_remote_repository_for_url")
@patch("pkgmgr.actions.mirror.setup_cmd.probe_remote_reachable_detail")
def test_setup_mirrors_remote_no_mirrors_ensure_remote_provisions_primary(
self, m_probe_detail, m_ensure_url, m_primary, m_ctx
) -> None:
m_ctx.return_value = self._ctx(repo_dir="/tmp/repo", resolved={})
m_primary.return_value = "git@github.com:alice/repo.git"
m_probe_detail.return_value = (True, "")
repos = [
{
"provider": "github.com",
"account": "alice",
"repository": "repo",
"private": False,
"description": "desc",
}
]
setup_mirrors(
selected_repos=repos,
repositories_base_dir="/tmp",
all_repos=repos,
preview=True,
local=False,
remote=True,
ensure_remote=True,
)
m_ensure_url.assert_called_once_with(
url="git@github.com:alice/repo.git",
private_default=False,
description="desc",
preview=True,
)
m_probe_detail.assert_called_once_with(
"git@github.com:alice/repo.git", cwd="/tmp/repo"
)
if __name__ == "__main__":
unittest.main()

View File

@@ -0,0 +1,145 @@
from __future__ import annotations
import importlib
import unittest
from unittest.mock import patch
from pkgmgr.core.git.errors import GitRunError
# IMPORTANT:
# Import the MODULE, not the function exported by pkgmgr.core.git.queries.__init__.
pr = importlib.import_module("pkgmgr.core.git.queries.probe_remote_reachable")
def _git_error(
*,
returncode: int,
stderr: str = "",
stdout: str = "",
message: str = "git failed",
) -> GitRunError:
"""
Create a GitRunError that mimics what pkgmgr.core.git.run attaches.
"""
exc = GitRunError(message)
exc.returncode = returncode
exc.stderr = stderr
exc.stdout = stdout
return exc
class TestProbeRemoteReachableHelpers(unittest.TestCase):
def test_first_useful_line_prefers_keyword_lines(self) -> None:
text = "\nerror:\n \nFATAL: Could not read from remote repository.\nmore\n"
self.assertEqual(
pr._first_useful_line(text),
"FATAL: Could not read from remote repository.",
)
def test_first_useful_line_skips_plain_error_if_possible(self) -> None:
text = "error:\nsome other info\n"
self.assertEqual(pr._first_useful_line(text), "some other info")
def test_first_useful_line_returns_empty_for_empty(self) -> None:
self.assertEqual(pr._first_useful_line(" \n\n"), "")
def test_looks_like_real_transport_error_true(self) -> None:
self.assertTrue(
pr._looks_like_real_transport_error(
"fatal: Could not read from remote repository."
)
)
def test_looks_like_real_transport_error_false(self) -> None:
self.assertFalse(pr._looks_like_real_transport_error("some harmless output"))
class TestProbeRemoteReachableDetail(unittest.TestCase):
@patch.object(pr, "run", return_value="")
def test_detail_success_returns_true_empty_reason(self, m_run) -> None:
ok, reason = pr.probe_remote_reachable_detail(
"git@github.com:alice/repo.git",
cwd="/tmp",
)
self.assertTrue(ok)
self.assertEqual(reason, "")
m_run.assert_called_once()
@patch.object(pr, "run")
def test_detail_rc2_without_transport_indicators_treated_as_reachable(
self, m_run
) -> None:
# rc=2 but no transport/auth indicators => treat as reachable (empty repo)
m_run.side_effect = _git_error(
returncode=2,
stderr="",
stdout="",
message="Git command failed (exit 2)",
)
ok, reason = pr.probe_remote_reachable_detail(
"git@github.com:alice/empty.git",
cwd="/tmp",
)
self.assertTrue(ok)
self.assertIn("empty repository", reason.lower())
@patch.object(pr, "run")
def test_detail_rc2_with_transport_indicators_is_not_reachable(self, m_run) -> None:
# rc=2 but stderr indicates transport/auth problem => NOT reachable
m_run.side_effect = _git_error(
returncode=2,
stderr="ERROR: Repository not found.",
stdout="",
message="Git command failed (exit 2)",
)
ok, reason = pr.probe_remote_reachable_detail(
"git@github.com:alice/missing.git",
cwd="/tmp",
)
self.assertFalse(ok)
self.assertIn("repository not found", reason.lower())
@patch.object(pr, "run")
def test_detail_rc128_reports_reason(self, m_run) -> None:
m_run.side_effect = _git_error(
returncode=128,
stderr="fatal: Could not read from remote repository.",
stdout="",
message="Git command failed (exit 128)",
)
ok, reason = pr.probe_remote_reachable_detail(
"ssh://git@host:2201/a/b.git",
cwd="/tmp",
)
self.assertFalse(ok)
self.assertIn("(exit 128)", reason.lower())
self.assertIn("could not read from remote repository", reason.lower())
@patch.object(pr, "run")
def test_detail_adds_hint_if_reason_is_generic(self, m_run) -> None:
# Generic failure: rc=128 but no stderr/stdout => should append hint
m_run.side_effect = _git_error(
returncode=128,
stderr="",
stdout="",
message="",
)
url = "git@github.com:alice/repo.git"
ok, reason = pr.probe_remote_reachable_detail(url, cwd="/tmp")
self.assertFalse(ok)
self.assertIn("hint:", reason.lower())
self.assertIn("git ls-remote --exit-code", reason.lower())
@patch.object(pr, "probe_remote_reachable_detail", return_value=(True, ""))
def test_probe_remote_reachable_delegates_to_detail(self, m_detail) -> None:
self.assertTrue(pr.probe_remote_reachable("x", cwd="/tmp"))
m_detail.assert_called_once_with("x", cwd="/tmp")
if __name__ == "__main__":
unittest.main()