mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* [420e5e68] Fix mypy errors in tests/unit/ and create tests/__init__.py (#154) * [420e5e68] fix(tests): resolve all mypy errors in tests/unit/ and create tests/__init__.py - Create tests/__init__.py as empty package marker - Add Any import and fix list type annotation in test_flow_server_intent_public_mapping.py - Move AsyncIterator to TYPE_CHECKING block and fix m.cls.__name__ attr error in test_app.py - Add return type annotations to _stub_get_optimal, _source, and factory functions - Implement abstract methods (index_type, prepare_metadata, build_source_uri) in _FakePlugin - Add pyproject.toml per-file-ignore for ARG002 on test_optimal_grounding.py stub - Remove 4 stale # type: ignore comments from test_rate_limit_tracker.py - Fix method-assignment patterns in test_rate_limit_sweep.py via patch.object - All 487 source files pass mypy with 0 errors; 2312 unit tests pass * [420e5e68] fix(tests): move stdlib/third-party imports to TYPE_CHECKING blocks across tests/unit/ Resolves 6 remaining ruff TC002/TC003 errors from the quality gate: - test_handlers.py: Iterator → TYPE_CHECKING - test_quality_gate.py: pathlib → TYPE_CHECKING - test_board_dispatch.py: AsyncIterator + httpx → TYPE_CHECKING - test_streaming.py: Iterator → TYPE_CHECKING - test_notification.py: AsyncIterator → TYPE_CHECKING All files have from __future__ import annotations so annotations are strings at runtime; no runtime NameError risk from moving to TYPE_CHECKING. * [420e5e68] fix(tests): use forward-ref cast() and drop unused TYPE_CHECKING import in 4 test files * [420e5e68] chore(Makefile): scope lint mypy target to roboco/ to match gate and quality targets --------- * [b0c9d41b] Fix mypy errors in tests/integration/ tests/foundation/ tests/property/ and update Makefile quality gates (#155) * [b0c9d41b] fix(tests): resolve all mypy errors in tests/integration/, tests/foundation/, tests/property/ - Add missing type annotations to inner functions (_override_db, _override_agent_id, _req, etc.) - Use cast("UUID", ...) to fix SQLAlchemy UUID vs uuid.UUID arg-type mismatches - Remove stale # type: ignore comments from test_full_lifecycle_real_db.py and test_task_service_lifecycle_misc.py - Update Makefile quality/quality-fast targets to run mypy on roboco/ tests/ - No runtime logic changed — annotations and cast() only * [b0c9d41b] fix(tests): apply ruff TC006 quoted-cast and AsyncGenerator[T] fixes to complete mypy gate - Quote all cast() type arguments per ruff TC006 rule (cast("T", x)) - Change AsyncGenerator[T, None] to AsyncGenerator[T] (Python 3.12 form) - Move runtime-only imports to TYPE_CHECKING blocks (Path, Table, Generator, etc.) - No runtime logic changed — annotation-only changeset * [b0c9d41b] fix(Makefile): align lint target mypy scope with gate target (roboco/ only) The lint target used `uv run mypy .` (all files) while gate uses `uv run mypy roboco/`. This inconsistency caused the pre-submit gate to fail on 161 pre-existing tests/unit/ errors (being fixed by sibling task 420e5e68). The quality/quality-fast targets already check `roboco/ tests/` — the lint target now matches gate scope. --------- --------- Co-authored-by: Backend Developer 1 <be-dev-1@agents.roboco.dev> Co-authored-by: Backend Developer 2 <be-dev-2@agents.roboco.dev>
307 lines
12 KiB
Python
307 lines
12 KiB
Python
"""Task #161: GitService diff base falls back to default branch.
|
|
|
|
A leaf dev branch's parent (per parent_branch_for) is the cell-PM
|
|
branch feature/{team}/{root}--{cellpm}, which is NEVER pushed — only
|
|
devs push their own leaf branch. Diffing against a non-existent
|
|
origin/<parent> returns an empty diff, so QA / docs saw nothing.
|
|
_resolve_diff_base must fall back to the repo default branch when the
|
|
parent ref is absent on origin.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
from typing import Any
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
import pytest
|
|
from roboco.services.git import GitService
|
|
|
|
|
|
def _git_service() -> GitService:
|
|
return GitService.__new__(GitService)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_resolve_diff_base_uses_parent_when_pushed() -> None:
|
|
"""When origin/<parent> exists, use it (normal case)."""
|
|
svc = _git_service()
|
|
svc._run_git = AsyncMock() # type: ignore[method-assign]
|
|
svc._ref_exists = AsyncMock(return_value=True) # type: ignore[method-assign]
|
|
ws = Path("/tmp/ws")
|
|
|
|
base = await svc._resolve_diff_base(
|
|
ws, "feature/backend/root1234--cellpm56--dev78901"
|
|
)
|
|
# parent_branch_for strips the last --segment.
|
|
assert base == "origin/feature/backend/root1234--cellpm56"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_resolve_diff_base_falls_back_when_parent_absent() -> None:
|
|
"""When origin/<parent> does NOT exist (cell-PM branch never pushed),
|
|
fall back to the repo default branch via origin/HEAD."""
|
|
svc = _git_service()
|
|
svc._run_git = AsyncMock() # type: ignore[method-assign]
|
|
# parent ref absent → _ref_exists False for the parent check.
|
|
svc._ref_exists = AsyncMock(return_value=False) # type: ignore[method-assign]
|
|
svc._default_branch_ref = AsyncMock( # type: ignore[method-assign]
|
|
return_value="origin/master"
|
|
)
|
|
ws = Path("/tmp/ws")
|
|
|
|
base = await svc._resolve_diff_base(
|
|
ws, "feature/backend/root1234--cellpm56--dev78901"
|
|
)
|
|
assert base == "origin/master"
|
|
svc._default_branch_ref.assert_awaited_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_default_branch_ref_prefers_origin_head() -> None:
|
|
"""origin/HEAD symbolic-ref is the canonical default-branch pointer."""
|
|
svc = _git_service()
|
|
|
|
async def fake_run(_ws: Any, args: list[str], **_kw: Any) -> Any:
|
|
if args[:2] == ["symbolic-ref", "--quiet"]:
|
|
return type(
|
|
"R", (), {"returncode": 0, "stdout": "refs/remotes/origin/main\n"}
|
|
)()
|
|
return type("R", (), {"returncode": 1, "stdout": ""})()
|
|
|
|
with patch.object(svc, "_run_git", new=fake_run):
|
|
ref = await svc._default_branch_ref(Path("/tmp/ws"))
|
|
assert ref == "origin/main"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_default_branch_ref_fallback_when_no_head() -> None:
|
|
"""No origin/HEAD → probe origin/master then origin/main; final
|
|
hard fallback is origin/master so the git invocation stays valid."""
|
|
svc = _git_service()
|
|
|
|
async def fake_run(_ws: Any, _args: list[str], **_kw: Any) -> Any:
|
|
# symbolic-ref fails; fetches succeed but ref never verifies.
|
|
return type("R", (), {"returncode": 1, "stdout": ""})()
|
|
|
|
svc._ref_exists = AsyncMock(return_value=False) # type: ignore[method-assign]
|
|
with patch.object(svc, "_run_git", new=fake_run):
|
|
ref = await svc._default_branch_ref(Path("/tmp/ws"))
|
|
assert ref == "origin/master"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Task #161 (facet): the diff HEAD side must resolve in the inspecting
|
|
# clone. The local <branch> ref only exists in the dev's own clone (the
|
|
# clone that ran `git checkout -b` at claim). QA / doc / PM diff from
|
|
# their OWN clones, which only have origin/<branch> after a fetch. Diffing
|
|
# against the bare local name there yields an empty diff (smoke-14: QA saw
|
|
# no changes on a real PR). _resolve_head_ref + diff()/list_changed_files
|
|
# must prefer the local branch, then origin/<branch>.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
_BR = "feature/backend/root1234--cellpm56--dev78901"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_resolve_head_ref_prefers_local_branch_in_dev_clone() -> None:
|
|
"""Dev's own clone has the local branch — use it unchanged."""
|
|
svc = _git_service()
|
|
svc._run_git = AsyncMock() # type: ignore[method-assign]
|
|
svc._ref_exists = AsyncMock(return_value=True) # type: ignore[method-assign]
|
|
|
|
head = await svc._resolve_head_ref(Path("/tmp/ws"), _BR)
|
|
assert head == _BR
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_resolve_head_ref_falls_back_to_origin_in_foreign_clone() -> None:
|
|
"""QA/doc/PM clone has no local branch but origin/<branch> exists
|
|
(open_pr pushed it) — diff must target origin/<branch>."""
|
|
svc = _git_service()
|
|
svc._run_git = AsyncMock() # type: ignore[method-assign]
|
|
|
|
async def ref_exists(_ws: Any, ref: str) -> bool:
|
|
# local branch absent; only the remote-tracking ref resolves.
|
|
return ref == f"origin/{_BR}"
|
|
|
|
with patch.object(svc, "_ref_exists", new=ref_exists):
|
|
head = await svc._resolve_head_ref(Path("/tmp/ws"), _BR)
|
|
assert head == f"origin/{_BR}"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_resolve_head_ref_fetches_branch_before_resolving() -> None:
|
|
"""The branch is fetched into the workspace so origin/<branch> is
|
|
available even on a clone that never saw it."""
|
|
svc = _git_service()
|
|
calls: list[list[str]] = []
|
|
|
|
async def fake_run(_ws: Any, args: list[str], **_kw: Any) -> Any:
|
|
calls.append(args)
|
|
return type("R", (), {"returncode": 0, "stdout": ""})()
|
|
|
|
svc._ref_exists = AsyncMock(return_value=True) # type: ignore[method-assign]
|
|
with patch.object(svc, "_run_git", new=fake_run):
|
|
await svc._resolve_head_ref(Path("/tmp/ws"), _BR)
|
|
assert ["fetch", "origin", _BR] in calls
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_diff_targets_origin_head_in_foreign_clone() -> None:
|
|
"""Regression for smoke-14: diff() from QA's clone must compare
|
|
base...origin/<branch>, not base...<bare-local-branch> (which is
|
|
unresolvable there and silently produced an empty diff)."""
|
|
svc = _git_service()
|
|
svc._workspace_for_branch = AsyncMock( # type: ignore[method-assign]
|
|
return_value=Path("/tmp/qa-ws")
|
|
)
|
|
svc._resolve_diff_base = AsyncMock( # type: ignore[method-assign]
|
|
return_value="origin/master"
|
|
)
|
|
svc._resolve_head_ref = AsyncMock( # type: ignore[method-assign]
|
|
return_value=f"origin/{_BR}"
|
|
)
|
|
svc._token_for_branch = AsyncMock( # type: ignore[method-assign]
|
|
return_value="tok"
|
|
)
|
|
captured: list[list[str]] = []
|
|
|
|
async def fake_run(_ws: Any, args: list[str], **_kw: Any) -> Any:
|
|
captured.append(args)
|
|
return type("R", (), {"returncode": 0, "stdout": "diff body"})()
|
|
|
|
with patch.object(svc, "_run_git", new=fake_run):
|
|
out = await svc.diff(branch_name=_BR)
|
|
assert out == "diff body"
|
|
assert captured == [["diff", f"origin/master...origin/{_BR}"]]
|
|
# #168: the resolved project token is threaded into ref resolution so
|
|
# the fetches authenticate (unauth fails on private repos).
|
|
svc._resolve_head_ref.assert_awaited_once_with(Path("/tmp/qa-ws"), _BR, token="tok")
|
|
svc._resolve_diff_base.assert_awaited_once_with(
|
|
Path("/tmp/qa-ws"), _BR, token="tok"
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_list_changed_files_targets_origin_head_in_foreign_clone() -> None:
|
|
"""Same fix on the files_changed path (#154 evidence)."""
|
|
svc = _git_service()
|
|
svc._workspace_for_branch = AsyncMock( # type: ignore[method-assign]
|
|
return_value=Path("/tmp/qa-ws")
|
|
)
|
|
svc._resolve_diff_base = AsyncMock( # type: ignore[method-assign]
|
|
return_value="origin/master"
|
|
)
|
|
svc._resolve_head_ref = AsyncMock( # type: ignore[method-assign]
|
|
return_value=f"origin/{_BR}"
|
|
)
|
|
svc._token_for_branch = AsyncMock( # type: ignore[method-assign]
|
|
return_value="tok"
|
|
)
|
|
captured: list[list[str]] = []
|
|
|
|
async def fake_run(_ws: Any, args: list[str], **_kw: Any) -> Any:
|
|
captured.append(args)
|
|
return type("R", (), {"returncode": 0, "stdout": "README.md\nsrc/app.py\n"})()
|
|
|
|
with patch.object(svc, "_run_git", new=fake_run):
|
|
files = await svc.list_changed_files(branch_name=_BR)
|
|
assert files == ["README.md", "src/app.py"]
|
|
assert captured == [["diff", "--name-only", f"origin/master...origin/{_BR}"]]
|
|
svc._resolve_diff_base.assert_awaited_once_with(
|
|
Path("/tmp/qa-ws"), _BR, token="tok"
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_diff_honours_explicit_base_with_resolved_head() -> None:
|
|
"""The incremental dev path (base=HEAD~1) still works: explicit base
|
|
is preserved, head still goes through _resolve_head_ref."""
|
|
svc = _git_service()
|
|
svc._workspace_for_branch = AsyncMock( # type: ignore[method-assign]
|
|
return_value=Path("/tmp/dev-ws")
|
|
)
|
|
svc._resolve_diff_base = AsyncMock( # type: ignore[method-assign]
|
|
return_value="SHOULD_NOT_BE_USED"
|
|
)
|
|
svc._resolve_head_ref = AsyncMock( # type: ignore[method-assign]
|
|
return_value=_BR
|
|
)
|
|
svc._token_for_branch = AsyncMock( # type: ignore[method-assign]
|
|
return_value=None
|
|
)
|
|
captured: list[list[str]] = []
|
|
|
|
async def fake_run(_ws: Any, args: list[str], **_kw: Any) -> Any:
|
|
captured.append(args)
|
|
return type("R", (), {"returncode": 0, "stdout": ""})()
|
|
|
|
with patch.object(svc, "_run_git", new=fake_run):
|
|
await svc.diff(branch_name=_BR, base="HEAD~1")
|
|
assert captured == [["diff", f"HEAD~1...{_BR}"]]
|
|
svc._resolve_diff_base.assert_not_awaited()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Task #168: the diff base must be CURRENT. In an inspecting clone
|
|
# origin/HEAD is set, so _default_branch_ref early-returns the ref NAME
|
|
# without fetching, leaving the base the stale clone-time tip — a
|
|
# three-dot diff then spans the whole repo delta (smoke-15: 41 files vs a
|
|
# 1-line change). _resolve_diff_base must re-fetch the resolved base, and
|
|
# every fetch in the diff path must authenticate (unauth fails on a
|
|
# private repo: "could not read Username for github.com").
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_resolve_diff_base_refetches_default_branch_with_token() -> None:
|
|
"""Parent absent → default branch resolved → it must be re-fetched
|
|
authenticated so the base is current, not the stale clone-time ref."""
|
|
svc = _git_service()
|
|
calls: list[tuple[list[str], str | None]] = []
|
|
|
|
async def fake_run(_ws: Any, args: list[str], **kw: Any) -> Any:
|
|
calls.append((args, kw.get("token")))
|
|
return type("R", (), {"returncode": 0, "stdout": ""})()
|
|
|
|
# parent ref never exists → fall back to default branch.
|
|
svc._ref_exists = AsyncMock(return_value=False) # type: ignore[method-assign]
|
|
svc._default_branch_ref = AsyncMock( # type: ignore[method-assign]
|
|
return_value="origin/master"
|
|
)
|
|
|
|
with patch.object(svc, "_run_git", new=fake_run):
|
|
base = await svc._resolve_diff_base(Path("/tmp/ws"), _BR, token="tok")
|
|
assert base == "origin/master"
|
|
# The resolved default branch ('master') was fetched, authenticated.
|
|
assert (["fetch", "origin", "master"], "tok") in calls
|
|
# The parent fetch is also authenticated.
|
|
assert all(tok == "tok" for args, tok in calls if args[:2] == ["fetch", "origin"])
|
|
svc._default_branch_ref.assert_awaited_once_with(Path("/tmp/ws"), token="tok")
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_resolve_head_ref_fetch_is_authenticated() -> None:
|
|
"""The branch fetch in _resolve_head_ref must carry the token too."""
|
|
svc = _git_service()
|
|
seen: list[tuple[list[str], str | None]] = []
|
|
|
|
async def fake_run(_ws: Any, args: list[str], **kw: Any) -> Any:
|
|
seen.append((args, kw.get("token")))
|
|
return type("R", (), {"returncode": 0, "stdout": ""})()
|
|
|
|
svc._ref_exists = AsyncMock(return_value=True) # type: ignore[method-assign]
|
|
with patch.object(svc, "_run_git", new=fake_run):
|
|
await svc._resolve_head_ref(Path("/tmp/ws"), _BR, token="tok")
|
|
assert (["fetch", "origin", _BR], "tok") in seen
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_token_for_branch_is_best_effort_none() -> None:
|
|
"""Unresolvable branch/project must yield None (degrade to unauth),
|
|
never raise inside the evidence-assembly path."""
|
|
svc = _git_service()
|
|
svc._task_for_branch = AsyncMock(return_value=None) # type: ignore[method-assign]
|
|
assert await svc._token_for_branch(_BR) is None
|