From 4d4bf084c5ce97ed56479bb3f7113759c147da63 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 28 Jun 2026 16:47:09 +0200 Subject: [PATCH] [chore] clear all 64 pre-existing mypy errors in tests/ (no type:ignore) Convention: no type:ignore/noqa, and pre-existing violations still violate. The make-quality gate runs 'mypy roboco/ tests/', but the prior commits' gates only ran mypy on production files, masking 64 type errors across 15 test files (method-assign, unused-ignore, no-untyped-def, attr-defined, union-attr, has-type, index, misc). Fixed without any type:ignore: - method-assign (svc.session.X = / svc.method = AsyncMock()): hold a local 'session: MagicMock'/'AsyncMock' and assert on it, or stub via object.__setattr__ / monkeypatch / a typed '_bind' helper returning Any, or alias 'cc: Any = c' (the pattern the file already used). - unused 'type: ignore[assignment]' (real code was method-assign): removed; replaced with the no-suppression patterns above. - 'Callable[...] has no attribute assert_*': keep a typed local ref to the AsyncMock and assert on the local, not the method-typed attr. - no-untyped-def: annotate helper params (Any / pytest.MonkeyPatch). - attr-defined / index / union-attr: type the helper as Any, narrow with an 'is not None' assert, or add the missing attr to a fake. - has-type / return-value: fix the declared return type to the tuple the function actually returns. - PLC0415 inline imports: hoisted to top-level. test_pr_gate_notifies_pm._stub_gate_path converted fully to the 'cc: Any = c' alias (it already used it for one attr) so its five '# type: ignore[method-assign]' suppressions are gone. mypy tests/: 64 errors -> 0 (538 files). ruff check tests/: clean. All 84 tests in the touched files pass. --- .../unit/gateway/test_pr_gate_notifies_pm.py | 34 +++++++++++------- tests/unit/runtime/test_grok_cost_budget.py | 1 + .../runtime/test_provider_overload_break.py | 7 ++-- tests/unit/runtime/test_self_heal_ceo_gate.py | 36 ++++++++++++------- .../optimal_brain/test_playbooks_index.py | 7 ++-- tests/unit/services/test_git_lock_cleanup.py | 10 ++++-- .../test_git_merge_method_fallback.py | 19 +++++----- .../test_git_merge_pr_for_task_pr_match.py | 4 ++- .../services/test_git_pr_target_scoping.py | 6 ++-- .../services/test_git_token_decryption_log.py | 3 +- .../services/test_messaging_channel_race.py | 23 ++++++++---- .../services/test_messaging_session_race.py | 33 +++++++++++------ .../services/test_playbook_index_ordering.py | 4 +-- tests/unit/services/test_playbook_unindex.py | 7 ++-- ...est_release_executor_commit_fail_closed.py | 2 +- .../test_release_proposal_concurrency.py | 3 +- 16 files changed, 124 insertions(+), 75 deletions(-) diff --git a/tests/unit/gateway/test_pr_gate_notifies_pm.py b/tests/unit/gateway/test_pr_gate_notifies_pm.py index 7992b5be..ee7ffe84 100644 --- a/tests/unit/gateway/test_pr_gate_notifies_pm.py +++ b/tests/unit/gateway/test_pr_gate_notifies_pm.py @@ -53,7 +53,12 @@ def _stub_gate_path( PM re-assignment the real ``_revision_pm_for_task`` performs. """ agent = MagicMock(role="pr_reviewer", slug="be-pr-reviewer") - c._gate_preflight = AsyncMock( # type: ignore[method-assign] + # Alias to ``Any`` so attribute assignment needs no type:ignore (mypy + # doesn't flag attribute assignment on ``Any``; avoids ruff B010's + # no-setattr rule too). ``object.__setattr__`` would also work but loses + # the cross-scope narrowing callers rely on for ``cc.`` asserts. + cc: Any = c + cc._gate_preflight = AsyncMock( return_value=( t_before, agent, @@ -62,20 +67,17 @@ def _stub_gate_path( spec_module.Context(actor_id=reviewer_id), ) ) - c._gate_tracing = AsyncMock(return_value=None) # type: ignore[method-assign] + cc._gate_tracing = AsyncMock(return_value=None) # These tests exercise the pr_fail a2a / notify path, not the head-sha # capture (which has its own suite in test_submit_root_unchanged_pr_guard). # Stub the capture so it does not walk the mock session into un-awaited # coroutines; the verdict still lands via the _record_gate_verdict spy. - # Alias to ``Any`` so this addition needs no type:ignore (mypy doesn't flag - # attribute assignment on ``Any``; avoids ruff B010's no-setattr rule too). - cc: Any = c cc._capture_pr_head_sha = AsyncMock(return_value=None) - c._record_gate_verdict = MagicMock() # type: ignore[method-assign] - c._post_gate_review_to_pr = AsyncMock() # type: ignore[method-assign] + cc._record_gate_verdict = MagicMock() + cc._post_gate_review_to_pr = AsyncMock() runner = MagicMock() runner.run_intent = AsyncMock(return_value=t_after) - c._verb_runner = MagicMock(return_value=runner) # type: ignore[method-assign] + cc._verb_runner = MagicMock(return_value=runner) @pytest.mark.asyncio @@ -267,9 +269,12 @@ async def test_pr_fail_returns_invalid_state_when_runner_returns_none() -> None: # Clean rejection, not a 500. assert env.error == "invalid_state" - # No PR post / no a2a against a None task. - c._post_gate_review_to_pr.assert_not_awaited() - c.a2a.send.assert_not_awaited() + # No PR post / no a2a against a None task. Alias to ``Any`` so the asserts + # resolve without the cross-scope narrowing _stub_gate_path's assignment + # can't provide (and without a type:ignore). + cc: Any = c + cc._post_gate_review_to_pr.assert_not_awaited() + cc.a2a.send.assert_not_awaited() @pytest.mark.asyncio @@ -295,5 +300,8 @@ async def test_pr_pass_returns_invalid_state_when_runner_returns_none() -> None: ) assert env.error == "invalid_state" - c._post_gate_review_to_pr.assert_not_awaited() - c.a2a.send.assert_not_awaited() + # Alias to ``Any`` so the asserts resolve without cross-scope narrowing + # (and without a type:ignore) — see test_pr_fail_returns_invalid_state... + cc: Any = c + cc._post_gate_review_to_pr.assert_not_awaited() + cc.a2a.send.assert_not_awaited() diff --git a/tests/unit/runtime/test_grok_cost_budget.py b/tests/unit/runtime/test_grok_cost_budget.py index 6e45f2e5..6c89498c 100644 --- a/tests/unit/runtime/test_grok_cost_budget.py +++ b/tests/unit/runtime/test_grok_cost_budget.py @@ -74,6 +74,7 @@ async def test_cost_over_cap_finalizes_spawn_session_before_evict( finalize.assert_awaited_once() # finalize ran with the agent still registered (so it could read the model + # usage_session_id), and the instance was evicted only after. + assert finalize.await_args is not None assert finalize.await_args.args[0] == "be-dev-1" diff --git a/tests/unit/runtime/test_provider_overload_break.py b/tests/unit/runtime/test_provider_overload_break.py index e8606f0e..d5f01b0f 100644 --- a/tests/unit/runtime/test_provider_overload_break.py +++ b/tests/unit/runtime/test_provider_overload_break.py @@ -62,6 +62,7 @@ def orch(monkeypatch: pytest.MonkeyPatch) -> AgentOrchestrator: class _FakeTracker: def __init__(self) -> None: self.activated_with: dict[str, object] | None = None + self.clear = AsyncMock() async def activate( self, *, retry_after: float, affected_agents: list[str], kind: str @@ -141,9 +142,7 @@ async def test_agent_writing_about_error_500_does_not_park( orch, "_tail_container_logs", AsyncMock(return_value=agent_note) ) monkeypatch.setattr(orch, "_transcript_tail_text", lambda _a, _lines=80: "") - assert ( - await orch._provider_overload_park_target("be-dev-1", _instance()) is None - ) + assert await orch._provider_overload_park_target("be-dev-1", _instance()) is None @pytest.mark.asyncio @@ -245,7 +244,7 @@ async def test_probe_success_respawns_parked_agent( ) } tracker = _FakeTracker() - tracker.clear = AsyncMock() # type: ignore[method-assign] + tracker.clear = AsyncMock() monkeypatch.setattr(orch, "_make_tracker", lambda _p: tracker) monkeypatch.setattr(orch, "_delete_waiting_record", AsyncMock()) monkeypatch.setattr(orch, "_generate_resume_prompt", lambda _r, _res: "resume") diff --git a/tests/unit/runtime/test_self_heal_ceo_gate.py b/tests/unit/runtime/test_self_heal_ceo_gate.py index 1a6c9ad3..b54c798f 100644 --- a/tests/unit/runtime/test_self_heal_ceo_gate.py +++ b/tests/unit/runtime/test_self_heal_ceo_gate.py @@ -126,10 +126,13 @@ async def test_non_self_heal_tasks_dispatch_regardless_of_confirmation() -> None # --------------------------------------------------------------------------- -def _originate_engine(captured: dict[str, Any]) -> SelfHealEngine: +def _originate_engine( + captured: dict[str, Any], +) -> tuple[SelfHealEngine, MagicMock, MagicMock, Any]: """An engine whose task/project services are mocks that capture the create request — so the held-for-CEO invariant is unit-testable without a DB.""" session = MagicMock() + session.flush = AsyncMock() task_svc = MagicMock() task_svc.list_open_self_heal_tasks = AsyncMock(return_value=[]) @@ -147,6 +150,11 @@ def _originate_engine(captured: dict[str, Any]) -> SelfHealEngine: return engine, task_svc, project_svc, _self_heal_mod +def _bind(svc: object, name: str, value: object) -> None: + """Stub `name` on `svc` without tripping mypy's method-assign check.""" + object.__setattr__(svc, name, value) + + @pytest.mark.asyncio async def test_originate_opens_task_held_for_ceo( monkeypatch: pytest.MonkeyPatch, @@ -169,8 +177,7 @@ async def test_originate_opens_task_held_for_ceo( raw_ref="r", ) ] - # Bypass the session.flush (MagicMock session) — _originate awaits it. - engine.session.flush = AsyncMock() # type: ignore[assignment] + # session.flush is set in _originate_engine (MagicMock) — _originate awaits it. count = await engine._originate(obs) @@ -196,8 +203,9 @@ async def test_approve_and_start_lifts_the_ceo_hold( ) -> None: """``approve_and_start`` is the CEO's start gate — it flips ``confirmed_by_human`` True so a held self-heal task finally dispatches.""" - svc = TaskService(MagicMock()) - svc.session.flush = AsyncMock() # type: ignore[assignment] + session = MagicMock() + session.flush = AsyncMock() + svc = TaskService(session) task = MagicMock() task.status = TaskStatus.PENDING task.team = MagicMock() @@ -207,7 +215,7 @@ async def test_approve_and_start_lifts_the_ceo_hold( task.assigned_to = "someone-else" task.task_type = MagicMock() task.task_type.value = "code" - svc.get = AsyncMock(return_value=task) + _bind(svc, "get", AsyncMock(return_value=task)) main_pm = MagicMock(id=MAIN_PM_UUID) agent_svc = MagicMock() agent_svc.get_by_slug = AsyncMock(return_value=main_pm) @@ -240,15 +248,16 @@ async def test_list_pending_for_agent_excludes_held_self_heal() -> None: (``source != 'self_heal' OR confirmed_by_human``), so the database drops a held self-heal task before the agent ever sees the list. """ - svc = TaskService(MagicMock()) + session = MagicMock() result = MagicMock() result.scalars.return_value.all.return_value = [] - svc.session.execute = AsyncMock(return_value=result) - svc.unmet_dependency_ids = AsyncMock(return_value=[]) # type: ignore[assignment] + session.execute = AsyncMock(return_value=result) + svc = TaskService(session) + _bind(svc, "unmet_dependency_ids", AsyncMock(return_value=[])) await svc.list_pending_for_agent(MAIN_PM_UUID) - stmt = svc.session.execute.await_args.args[0] + stmt = session.execute.await_args.args[0] compiled = str( stmt.compile( dialect=postgresql.dialect(), compile_kwargs={"literal_binds": True} @@ -265,7 +274,7 @@ async def test_list_pending_for_agent_still_offers_delegated_subtask() -> None: PM-delegated work, where the delegation IS the authorization to start) must STILL be offered via give_me_work. A universal confirmed_by_human filter would starve devs of all delegated work.""" - svc = TaskService(MagicMock()) + session = MagicMock() delegated = MagicMock() delegated.source = "manual" # not self_heal @@ -274,8 +283,9 @@ async def test_list_pending_for_agent_still_offers_delegated_subtask() -> None: result = MagicMock() result.scalars.return_value.all.return_value = [delegated] - svc.session.execute = AsyncMock(return_value=result) - svc.unmet_dependency_ids = AsyncMock(return_value=[]) # type: ignore[assignment] + session.execute = AsyncMock(return_value=result) + svc = TaskService(session) + _bind(svc, "unmet_dependency_ids", AsyncMock(return_value=[])) available = await svc.list_pending_for_agent(MAIN_PM_UUID) diff --git a/tests/unit/services/optimal_brain/test_playbooks_index.py b/tests/unit/services/optimal_brain/test_playbooks_index.py index fbe8e6c0..f107e243 100644 --- a/tests/unit/services/optimal_brain/test_playbooks_index.py +++ b/tests/unit/services/optimal_brain/test_playbooks_index.py @@ -7,6 +7,9 @@ BaseIndexPlugin (shared, proven by the other 8 plugins) and runs live. from __future__ import annotations +import asyncio +from unittest.mock import AsyncMock, MagicMock + from roboco.models.optimal import IndexType from roboco.services.optimal_brain.indexes.playbooks import PlaybooksIndexPlugin @@ -44,8 +47,6 @@ def test_delete_playbook_removes_its_chunks_by_source() -> None: """F011: deleting a playbook removes its embedded chunks from the vector store by the playbook's source URI (idempotent — no-op if absent). A rejected/archived playbook must not stay retrievable in the PLAYBOOKS index.""" - from unittest.mock import AsyncMock, MagicMock - plugin = PlaybooksIndexPlugin.__new__(PlaybooksIndexPlugin) store = MagicMock() store.delete_by_source = AsyncMock(return_value=None) @@ -54,8 +55,6 @@ def test_delete_playbook_removes_its_chunks_by_source() -> None: object.__setattr__(plugin, "_initialized", True) object.__setattr__(plugin, "_store", store) - import asyncio - asyncio.run(plugin.delete_playbook("pb-42")) store.delete_by_source.assert_awaited_once_with("roboco://playbooks/pb-42") diff --git a/tests/unit/services/test_git_lock_cleanup.py b/tests/unit/services/test_git_lock_cleanup.py index af25b3f3..13bf3f5d 100644 --- a/tests/unit/services/test_git_lock_cleanup.py +++ b/tests/unit/services/test_git_lock_cleanup.py @@ -49,7 +49,9 @@ def _seed_locks(workspace: Path) -> dict[str, Path]: @pytest.mark.asyncio -async def test_timeout_removes_stale_git_locks(tmp_path: Path, monkeypatch) -> None: +async def test_timeout_removes_stale_git_locks( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: """A timed-out git op clears orphaned .git locks before re-raising.""" workspace = tmp_path / "ws" (workspace / ".git").mkdir(parents=True) @@ -72,7 +74,7 @@ async def test_timeout_removes_stale_git_locks(tmp_path: Path, monkeypatch) -> N @pytest.mark.asyncio async def test_timeout_lock_cleanup_is_best_effort_no_git_dir( - tmp_path: Path, monkeypatch + tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """A timeout with no .git/ directory must not error the cleanup path.""" workspace = tmp_path / "ws" @@ -89,7 +91,9 @@ async def test_timeout_lock_cleanup_is_best_effort_no_git_dir( @pytest.mark.asyncio -async def test_clean_exit_does_not_touch_locks(tmp_path: Path, monkeypatch) -> None: +async def test_clean_exit_does_not_touch_locks( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: """A normal (non-timed-out) git op must NOT delete lock files — a concurrent real git process could be holding one. Cleanup is timeout-only.""" workspace = tmp_path / "ws" diff --git a/tests/unit/services/test_git_merge_method_fallback.py b/tests/unit/services/test_git_merge_method_fallback.py index 3c10f7bc..14799064 100644 --- a/tests/unit/services/test_git_merge_method_fallback.py +++ b/tests/unit/services/test_git_merge_method_fallback.py @@ -181,13 +181,13 @@ async def test_merge_already_merged_pr_is_idempotent_success( svc, "_get_project_token_or_raise", AsyncMock(return_value="tok") ) monkeypatch.setattr(svc, "_parse_github_remote", lambda _ws: ("owner", "repo")) - monkeypatch.setattr( - svc, "_delete_pr_branch_best_effort", AsyncMock(return_value=None) - ) + delete_branch = AsyncMock(return_value=None) + monkeypatch.setattr(svc, "_delete_pr_branch_best_effort", delete_branch) monkeypatch.setattr( svc, "_project_default_branch", AsyncMock(return_value="master") ) - monkeypatch.setattr(svc, "_sync_target_branch", AsyncMock(return_value="abc123")) + sync_target = AsyncMock(return_value="abc123") + monkeypatch.setattr(svc, "_sync_target_branch", sync_target) # No fallback retry would help (already merged => 405 either way); make the # fallback lookup return None so the code falls straight through to the # already-merged disambiguation. @@ -208,8 +208,8 @@ async def test_merge_already_merged_pr_is_idempotent_success( assert commit == "abc123" # The post-merge steps still run (branch cleanup, target sync) so the # caller's state stays consistent with "the PR is merged". - svc._delete_pr_branch_best_effort.assert_awaited_once() - svc._sync_target_branch.assert_awaited_once() + delete_branch.assert_awaited_once() + sync_target.assert_awaited_once() @pytest.mark.asyncio @@ -224,9 +224,8 @@ async def test_merge_raises_when_not_merged_and_refused( svc, "_get_project_token_or_raise", AsyncMock(return_value="tok") ) monkeypatch.setattr(svc, "_parse_github_remote", lambda _ws: ("owner", "repo")) - monkeypatch.setattr( - svc, "_delete_pr_branch_best_effort", AsyncMock(return_value=None) - ) + delete_branch = AsyncMock(return_value=None) + monkeypatch.setattr(svc, "_delete_pr_branch_best_effort", delete_branch) monkeypatch.setattr( svc, "_project_default_branch", AsyncMock(return_value="master") ) @@ -244,4 +243,4 @@ async def test_merge_raises_when_not_merged_and_refused( await svc.merge_pull_request(Path("/tmp/ws"), 42, "squash", "proj") # No post-merge cleanup ran — the merge did not land. - svc._delete_pr_branch_best_effort.assert_not_awaited() + delete_branch.assert_not_awaited() diff --git a/tests/unit/services/test_git_merge_pr_for_task_pr_match.py b/tests/unit/services/test_git_merge_pr_for_task_pr_match.py index 4d041bf9..a1333564 100644 --- a/tests/unit/services/test_git_merge_pr_for_task_pr_match.py +++ b/tests/unit/services/test_git_merge_pr_for_task_pr_match.py @@ -77,6 +77,8 @@ async def test_mismatched_pr_number_rejected_before_merge( svc = _git_service() task = _task(pr_number=_RECORDED_PR, project_id=uuid4()) merge, _ws = _wire(monkeypatch, svc, task) + session = AsyncMock() + svc.session = session data = GitMergePRRequest( project_slug="roboco", @@ -91,7 +93,7 @@ async def test_mismatched_pr_number_rejected_before_merge( ) merge.assert_not_awaited() - svc.session.commit.assert_not_awaited() + session.commit.assert_not_awaited() @pytest.mark.asyncio diff --git a/tests/unit/services/test_git_pr_target_scoping.py b/tests/unit/services/test_git_pr_target_scoping.py index 23def34b..8a6e2517 100644 --- a/tests/unit/services/test_git_pr_target_scoping.py +++ b/tests/unit/services/test_git_pr_target_scoping.py @@ -15,7 +15,7 @@ project's repo is never resolved by accident — the pattern from __future__ import annotations from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from unittest.mock import AsyncMock, MagicMock, patch from uuid import uuid4 @@ -53,9 +53,9 @@ def _patch_project_service(project: object | None) -> AbstractContextManager[obj return patch("roboco.services.git.get_project_service", return_value=fake_service) -def _compiled_sql(stmt: object) -> str: +def _compiled_sql(stmt: Any) -> str: """Render a SQLAlchemy stmt to literal-bound SQL for assertion.""" - return str(stmt.compile(compile_kwargs={"literal_binds": True})) # type: ignore[arg-type] + return str(stmt.compile(compile_kwargs={"literal_binds": True})) @pytest.mark.asyncio diff --git a/tests/unit/services/test_git_token_decryption_log.py b/tests/unit/services/test_git_token_decryption_log.py index 1cd46cf8..c4fd2f06 100644 --- a/tests/unit/services/test_git_token_decryption_log.py +++ b/tests/unit/services/test_git_token_decryption_log.py @@ -15,6 +15,7 @@ diagnosable). from __future__ import annotations +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -29,7 +30,7 @@ def _svc() -> GitService: return GitService(MagicMock()) -def _patch_project_service_raising(exc: Exception) -> object: +def _patch_project_service_raising(exc: Exception) -> Any: fake_service = MagicMock() fake_service.get_decrypted_token_by_slug = AsyncMock(side_effect=exc) return patch("roboco.services.git.get_project_service", return_value=fake_service) diff --git a/tests/unit/services/test_messaging_channel_race.py b/tests/unit/services/test_messaging_channel_race.py index 24e1ac84..bd3f1fff 100644 --- a/tests/unit/services/test_messaging_channel_race.py +++ b/tests/unit/services/test_messaging_channel_race.py @@ -25,6 +25,15 @@ _SLUG = "backend-cell" _LOOKUPS_BEFORE_AND_AFTER_RACE = 2 +def _bind(svc: object, name: str, value: object) -> Any: + """Stub `name` on `svc` without tripping mypy's method-assign check. + Returns the value (typed ``Any``) so the caller can keep a reference for + assertions — ``object.__setattr__`` does not narrow the attribute type, so + assert on the returned local, not ``svc.``.""" + object.__setattr__(svc, name, value) + return value + + def _integrity_error() -> IntegrityError: return IntegrityError( "INSERT INTO channels ...", @@ -55,14 +64,16 @@ async def test_race_lost_refetches_existing_channel() -> None: re-fetches the winner's channel and returns it (no crash).""" existing = MagicMock(name="existing-channel", slug=_SLUG) svc, session = _svc(flush_side_effect=_integrity_error()) - svc.get_channel_by_slug = AsyncMock(side_effect=[None, existing]) + get_channel_by_slug = _bind( + svc, "get_channel_by_slug", AsyncMock(side_effect=[None, existing]) + ) result = await svc.get_or_create_channel_by_slug(_SLUG) assert result is existing # Savepoint isolated the failed insert; re-fetch was the recovery. session.begin_nested.assert_called_once() - assert svc.get_channel_by_slug.await_count == _LOOKUPS_BEFORE_AND_AFTER_RACE + assert get_channel_by_slug.await_count == _LOOKUPS_BEFORE_AND_AFTER_RACE @pytest.mark.asyncio @@ -70,7 +81,7 @@ async def test_race_lost_but_re_fetch_empty_reraises() -> None: """If the conflict did NOT produce a row on re-fetch (a real failure, not a race), the IntegrityError is re-raised — never masked as a silent None.""" svc, _session = _svc(flush_side_effect=_integrity_error()) - svc.get_channel_by_slug = AsyncMock(side_effect=[None, None]) + _bind(svc, "get_channel_by_slug", AsyncMock(side_effect=[None, None])) with pytest.raises(IntegrityError): await svc.get_or_create_channel_by_slug(_SLUG) @@ -82,8 +93,8 @@ async def test_normal_auto_create_unaffected_by_savepoint() -> None: newly-created channel is returned (regression guard — the savepoint must not break the happy path).""" svc, session = _svc() # flush succeeds - svc.get_channel_by_slug = AsyncMock( - side_effect=[None] + get_channel_by_slug = _bind( + svc, "get_channel_by_slug", AsyncMock(side_effect=[None]) ) # not present, then never re-called result = await svc.get_or_create_channel_by_slug(_SLUG) @@ -93,4 +104,4 @@ async def test_normal_auto_create_unaffected_by_savepoint() -> None: session.begin_nested.assert_called_once() session.add.assert_called_once() assert session.flush.await_count == 1 - assert svc.get_channel_by_slug.await_count == 1 # no recovery re-fetch + assert get_channel_by_slug.await_count == 1 # no recovery re-fetch diff --git a/tests/unit/services/test_messaging_session_race.py b/tests/unit/services/test_messaging_session_race.py index 2504c166..7ee55583 100644 --- a/tests/unit/services/test_messaging_session_race.py +++ b/tests/unit/services/test_messaging_session_race.py @@ -31,6 +31,15 @@ _GROUP_ID = MagicMock(name="group-id") _WINNER_SESSION_ID = MagicMock(name="winner-session-id") +def _bind(svc: object, name: str, value: object) -> Any: + """Stub `name` on `svc` without tripping mypy's method-assign check. + Returns the value (typed ``Any``) so the caller can keep a reference for + assertions — ``object.__setattr__`` does not narrow the attribute type, so + assert on the returned local, not ``svc.``.""" + object.__setattr__(svc, name, value) + return value + + @pytest.mark.asyncio async def test_lock_group_emits_for_update() -> None: """``_lock_group`` must issue ``SELECT ... FOR UPDATE`` (the row lock that @@ -69,18 +78,20 @@ async def test_create_session_race_loser_reuses_winner_under_lock() -> None: svc = MessagingService(session) winner = MagicMock(name="winner-session", status=SessionStatus.ACTIVE) - svc.get_group = AsyncMock(return_value=MagicMock(active_session_id=None)) + _bind(svc, "get_group", AsyncMock(return_value=MagicMock(active_session_id=None))) # Under the lock, the group now reflects the winner's link. - svc._lock_group = AsyncMock( - return_value=MagicMock(active_session_id=_WINNER_SESSION_ID) + lock_group = _bind( + svc, + "_lock_group", + AsyncMock(return_value=MagicMock(active_session_id=_WINNER_SESSION_ID)), ) - svc.get_session = AsyncMock(return_value=winner) + get_session = _bind(svc, "get_session", AsyncMock(return_value=winner)) result = await svc.create_session(SessionCreateRequest(group_id=_GROUP_ID)) assert result is winner - svc._lock_group.assert_awaited_once() - svc.get_session.assert_awaited_once() + lock_group.assert_awaited_once() + get_session.assert_awaited_once() session.add.assert_not_called() # no orphaning INSERT session.flush.assert_not_awaited() @@ -96,15 +107,15 @@ async def test_create_session_creates_when_no_active_under_lock() -> None: svc = MessagingService(session) locked_group = MagicMock(active_session_id=None) - svc.get_group = AsyncMock(return_value=MagicMock(active_session_id=None)) - svc._lock_group = AsyncMock(return_value=locked_group) - svc.get_session = AsyncMock() # should NOT be called (no active id) + _bind(svc, "get_group", AsyncMock(return_value=MagicMock(active_session_id=None))) + lock_group = _bind(svc, "_lock_group", AsyncMock(return_value=locked_group)) + get_session = _bind(svc, "get_session", AsyncMock()) # NOT called (no active id) result = await svc.create_session(SessionCreateRequest(group_id=_GROUP_ID)) assert isinstance(result, SessionTable) assert result.status == SessionStatus.ACTIVE - svc._lock_group.assert_awaited_once() - svc.get_session.assert_not_awaited() + lock_group.assert_awaited_once() + get_session.assert_not_awaited() session.add.assert_called_once() assert session.flush.await_count >= 1 diff --git a/tests/unit/services/test_playbook_index_ordering.py b/tests/unit/services/test_playbook_index_ordering.py index 39b49e6f..c218d42b 100644 --- a/tests/unit/services/test_playbook_index_ordering.py +++ b/tests/unit/services/test_playbook_index_ordering.py @@ -63,7 +63,7 @@ async def test_approve_does_not_index_before_commit( session = AsyncMock() session.flush = AsyncMock() svc = PlaybookService(session) - svc._get_or_raise = AsyncMock(return_value=_playbook_mock()) # type: ignore[assignment] + monkeypatch.setattr(svc, "_get_or_raise", AsyncMock(return_value=_playbook_mock())) optimal = MagicMock() optimal.index_playbook = AsyncMock() @@ -109,7 +109,7 @@ async def test_reject_does_not_unindex_before_commit( session = AsyncMock() session.flush = AsyncMock() svc = PlaybookService(session) - svc._get_or_raise = AsyncMock(return_value=_playbook_mock()) # type: ignore[assignment] + monkeypatch.setattr(svc, "_get_or_raise", AsyncMock(return_value=_playbook_mock())) optimal = MagicMock() optimal.unindex_playbook = AsyncMock() diff --git a/tests/unit/services/test_playbook_unindex.py b/tests/unit/services/test_playbook_unindex.py index 1993dc3d..82f05400 100644 --- a/tests/unit/services/test_playbook_unindex.py +++ b/tests/unit/services/test_playbook_unindex.py @@ -14,6 +14,7 @@ public step the caller runs AFTER committing. Both helpers stay gated on from __future__ import annotations +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch from uuid import uuid4 @@ -23,7 +24,9 @@ from roboco.models.base import PlaybookStatus from roboco.services.playbook import PlaybookService -def _mock_playbook(playbook_id, *, status=PlaybookStatus.APPROVED.value): +def _mock_playbook( + playbook_id: Any, *, status: str = PlaybookStatus.APPROVED.value +) -> MagicMock: pb = MagicMock() pb.id = playbook_id pb.status = status @@ -36,7 +39,7 @@ def _mock_playbook(playbook_id, *, status=PlaybookStatus.APPROVED.value): return pb -def _session_with(pb) -> MagicMock: +def _session_with(pb: Any) -> MagicMock: session = MagicMock() result = MagicMock() result.scalar_one_or_none.return_value = pb diff --git a/tests/unit/services/test_release_executor_commit_fail_closed.py b/tests/unit/services/test_release_executor_commit_fail_closed.py index 799bb837..a3162158 100644 --- a/tests/unit/services/test_release_executor_commit_fail_closed.py +++ b/tests/unit/services/test_release_executor_commit_fail_closed.py @@ -39,7 +39,7 @@ class _FakeGitOps(_GitReleaseOps): self._script = list(script) self.calls: list[tuple[str, ...]] = [] - async def _git(self, *args: str) -> tuple[int, str]: # type: ignore[override] + async def _git(self, *args: str) -> tuple[int, str]: self.calls.append(args) return self._script.pop(0) diff --git a/tests/unit/services/test_release_proposal_concurrency.py b/tests/unit/services/test_release_proposal_concurrency.py index 3ade80bc..d2536e05 100644 --- a/tests/unit/services/test_release_proposal_concurrency.py +++ b/tests/unit/services/test_release_proposal_concurrency.py @@ -11,6 +11,7 @@ second concurrent approve sees the lock held and refuses instead of racing. from __future__ import annotations +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch from uuid import uuid4 @@ -65,7 +66,7 @@ def _wire( report_dict: dict, executor_result: ReleaseResult, fake_redis: _FakeRedis, -) -> dict[str, object]: +) -> dict[str, Any]: """Patch every collaborator ``approve`` touches. Returns the mocks.""" task_svc = MagicMock() task_svc.get = AsyncMock(return_value=task)