From 77bb3729a37f4592bddc34441702cb60ed55c180 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 21 Aug 2026 11:13:17 -0400 Subject: [PATCH] refactor(tests): per-model fakes, FakeMCP and session mocks come from tests/helpers (#2825, milestone 296 area 1, batch 2) Second pass over the tests/ ledger after bbee0d0. fake_record(**attrs) is the one MagicMock-with-real-attributes builder (to_dict mirrors them; the note-2109 hazard documented once); fake_note/fake_task/fake_snippet/ fake_project/fake_milestone/fake_system/fake_rulebook/fake_topic/fake_rule carry each model's ordinary defaults on top of it, replacing 14 per-file factories (two rulebook trios in tool-vs-service wordings, _fake_task, _fake_ms, _fake_project, _plan_note, _fake_snippet, two _snippet adapters now one-liners over fake_snippet). FakeMCP replaces the five closure-over-a-list registrar fakes (+ _Recorder); loc() and design_token_stub() replace the paired _loc / _token / _T stand-ins; every hand-built async_session mock (9 helper defs and 14 inline copies) now starts from make_mock_session(). Call sites rewritten by AST with each file's former defaults made explicit, so behaviour is unchanged. Co-Authored-By: Claude Fable 5 --- tests/helpers.py | 149 +++++++++++++++++--- tests/test_api_keys.py | 9 +- tests/test_design_cascade.py | 82 +++++------ tests/test_design_stylesheet.py | 49 +++---- tests/test_integration_snippet_locations.py | 13 +- tests/test_mcp_tool_design_systems.py | 32 ++--- tests/test_mcp_tool_milestones.py | 28 ++-- tests/test_mcp_tool_planning.py | 20 +-- tests/test_mcp_tool_processes.py | 15 +- tests/test_mcp_tool_projects.py | 49 ++----- tests/test_mcp_tool_rulebooks.py | 88 ++++-------- tests/test_mcp_tool_snippets.py | 46 ++---- tests/test_mcp_tool_systems.py | 19 +-- tests/test_mcp_tool_tags.py | 9 +- tests/test_mcp_tool_tasks.py | 54 +++---- tests/test_mcp_tool_trash.py | 14 +- tests/test_notes_description_field.py | 9 +- tests/test_recurrence.py | 25 +--- tests/test_retrieval_scopes.py | 6 +- tests/test_routes_design_systems.py | 12 +- tests/test_services_db_maintenance.py | 5 +- tests/test_services_dedup.py | 10 +- tests/test_services_project_summaries.py | 5 +- tests/test_services_rulebooks.py | 66 +-------- tests/test_services_supersession.py | 9 +- tests/test_shared_provenance.py | 9 +- tests/test_shared_write_access.py | 19 +-- tests/test_snippet_merge_provenance.py | 20 +-- tests/test_snippet_unmerge.py | 31 ++-- tests/test_trash_filtering.py | 5 +- tests/test_version_pinning_pin_unpin.py | 9 +- tests/test_version_pinning_prune.py | 8 +- 32 files changed, 375 insertions(+), 549 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index b0711b4..862c781 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -6,6 +6,8 @@ them; a module imports what it needs with ``from tests.helpers import ...``. """ from __future__ import annotations +from datetime import datetime, timezone +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock @@ -48,30 +50,135 @@ async def ensure_user(session, username: str, role: str = "user"): return user -def fake_note(**attrs) -> MagicMock: - """A MagicMock note with REAL values on every attribute the product reads - to label, scope, or render a record. +def fake_record(**attrs) -> MagicMock: + """A MagicMock record with REAL values on the attributes named, and a + ``to_dict()`` that mirrors them. The hazard this exists for (note 2109): an auto-created MagicMock attribute - is truthy and has a repr. The injected menu reads ``is_task`` / - ``task_kind`` / ``note_type`` for its kind marker, ``user_id`` to decide - whether a line needs a "shared by …" attribution, ``data`` for a snippet's - language tag, and ``deleted_at`` to spot trash — on a bare MagicMock every - record renders as another user's trashed task with a mock repr for a - language. Defaults below are the ORDINARY state (own note, live, no - structured data); override what the test is about. - - ``to_dict()`` returns the same values as a plain dict, so a tool that - repackages ``note.to_dict()`` sees keys that agree with the attributes. + is truthy and has a repr — so a bare MagicMock handed to the product reads + as trashed, shared, a task, and owned by a MagicMock. Name every attribute + the code under test will read; the per-model ``fake_*`` builders below + carry the ordinary defaults so a call site states only what the test is + about. ``created_at`` / ``updated_at`` are set as attributes but kept out + of ``to_dict()`` (no test serialises them, and the real models isoformat + them). """ - values = { + n = MagicMock() + for key, value in attrs.items(): + setattr(n, key, value) + n.to_dict.return_value = { + k: v for k, v in attrs.items() if k not in ("created_at", "updated_at") + } + return n + + +def _with_defaults(defaults: dict, attrs: dict) -> MagicMock: + values = dict(defaults) + values.update(attrs) + return fake_record(**values) + + +def _now(): + return datetime.now(timezone.utc) + + +def fake_note(**attrs) -> MagicMock: + """A stand-in Note: own (user_id=7, the caller `_bind_user` binds), live, + not a task, no structured data. The injected menu reads is_task / + task_kind / note_type for its kind marker, user_id for the "shared by …" + attribution, data for a snippet's language, deleted_at for trash.""" + return _with_defaults({ "id": 1, "title": "t", "body": "", "tags": [], "user_id": 7, "note_type": "note", "is_task": False, "task_kind": "work", "data": None, "deleted_at": None, - } - values.update(attrs) - n = MagicMock() - for key, value in values.items(): - setattr(n, key, value) - n.to_dict.return_value = dict(values) - return n + }, attrs) + + +def fake_task(**attrs) -> MagicMock: + """A stand-in task note — get_task reads parent_id, deleted_at, user_id.""" + return _with_defaults({ + "id": 1, "title": "t", "body": "", "status": "todo", "priority": "none", + "tags": [], "parent_id": None, "project_id": None, "is_task": True, + "task_kind": "work", "user_id": 7, "deleted_at": None, + }, attrs) + + +def fake_snippet(**attrs) -> MagicMock: + """A stand-in snippet note. ``data`` is explicitly None: snippet_fields + prefers `data` when truthy, and a MagicMock is truthy.""" + return _with_defaults({ + "id": 1, "title": "debounce — rate-limit a callback", + "body": "```js\nreturn 1\n```\n", "tags": ["js", "snippet"], + "note_type": "snippet", "is_task": False, "task_kind": "work", + "user_id": 7, "data": None, "deleted_at": None, + }, attrs) + + +def fake_project(**attrs) -> MagicMock: + """design_system_id is explicit: a truthy auto-attribute would route every + project through the design-system branch and out to a real database.""" + return _with_defaults({ + "id": 1, "title": "P", "description": "", "goal": "", "status": "active", + "color": None, "design_system_id": None, "user_id": 7, + }, attrs) + + +def fake_milestone(**attrs) -> MagicMock: + return _with_defaults({ + "id": 1, "project_id": 1, "title": "MS", "description": None, + "status": "active", "order_index": 0, + }, attrs) + + +def fake_system(**attrs) -> MagicMock: + return _with_defaults({"id": 1, "name": "Reader", "project_id": 5}, attrs) + + +def fake_rulebook(**attrs) -> MagicMock: + return _with_defaults({ + "id": 1, "owner_user_id": 7, "title": "FabledSword family", + "description": "", "created_at": _now(), "updated_at": _now(), + }, attrs) + + +def fake_topic(**attrs) -> MagicMock: + return _with_defaults({ + "id": 10, "rulebook_id": 1, "title": "git-workflow", "description": "", + "order_index": 0, "created_at": _now(), "updated_at": _now(), + }, attrs) + + +def fake_rule(**attrs) -> MagicMock: + return _with_defaults({ + "id": 1, "topic_id": 10, "title": "dev is home", + "statement": "Work directly on dev", "why": "", "how_to_apply": "", + "order_index": 0, "created_at": _now(), "updated_at": _now(), + }, attrs) + + +class FakeMCP: + """Stand-in for the FastMCP server a tool module's ``register(mcp)`` is + handed: records the ``name=`` of every ``@mcp.tool(...)`` registration in + ``names`` and leaves the function untouched, so a test can assert which + tools a module exposes.""" + + def __init__(self) -> None: + self.names: list[str] = [] + + def tool(self, name=None): + self.names.append(name) + return lambda fn: fn + + +def loc(path: str = "", repo: str = "", symbol: str = "") -> dict: + """One snippet location, in the shape the record stores.""" + return {"repo": repo, "path": path, "symbol": symbol} + + +def design_token_stub(name, value_by_mode, group_name=None, purpose=None, + order_index=0, supersedes=None) -> SimpleNamespace: + """A design-token row as the cascade / stylesheet code reads it.""" + return SimpleNamespace( + name=name, value_by_mode=value_by_mode, group_name=group_name, + purpose=purpose, order_index=order_index, supersedes=supersedes or [], + ) diff --git a/tests/test_api_keys.py b/tests/test_api_keys.py index dbe165b..7a0e312 100644 --- a/tests/test_api_keys.py +++ b/tests/test_api_keys.py @@ -13,6 +13,7 @@ from scribe.services.api_keys import ( revoke_api_key, lookup_key, ) +from tests.helpers import make_mock_session def test_generate_key_format(): @@ -45,9 +46,7 @@ async def test_create_api_key_returns_full_key(): mock_key_obj.to_dict.return_value = {"id": 1, "name": "test", "scope": "read", "key_prefix": "fmcp_xxx"} with patch("scribe.services.api_keys.async_session") as mock_session_ctx: - mock_session = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) + mock_session = make_mock_session() mock_session.add = MagicMock() mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock(side_effect=lambda obj: None) @@ -67,9 +66,7 @@ async def test_create_api_key_returns_full_key(): @pytest.mark.asyncio async def test_lookup_key_returns_none_for_unknown(): with patch("scribe.services.api_keys.async_session") as mock_session_ctx: - mock_session = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) + mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.first.return_value = None mock_session.execute = AsyncMock(return_value=mock_result) diff --git a/tests/test_design_cascade.py b/tests/test_design_cascade.py index 9083c33..167c1e8 100644 --- a/tests/test_design_cascade.py +++ b/tests/test_design_cascade.py @@ -7,6 +7,7 @@ own import-free module — see services/design_cascade.py. from types import SimpleNamespace from scribe.services.design_cascade import ancestry, resolve_tokens, would_cycle +from tests.helpers import design_token_stub # --- ancestry --------------------------------------------------------------- @@ -102,14 +103,6 @@ def test_the_guard_survives_a_hierarchy_that_is_already_corrupt(): # because resolve_tokens is pure and duck-typed — which is the whole reason it # lives here rather than inside the service. -def _token(name, value_by_mode, group_name=None, purpose=None, order_index=0, - supersedes=None): - return SimpleNamespace( - name=name, value_by_mode=value_by_mode, group_name=group_name, - purpose=purpose, order_index=order_index, supersedes=supersedes or [], - ) - - # A family (1) and an app inheriting from it (2) — the shape the model exists for. FAMILY, APP = 1, 2 PARENTS = {FAMILY: None, APP: FAMILY} @@ -124,7 +117,7 @@ def test_a_system_with_no_tokens_of_its_own_inherits_the_whole_family_set(): and the state every app system starts in.""" resolved = resolve_tokens( APP, PARENTS, - {FAMILY: [_token("--fs-obsidian", {"base": "#14171a"})], APP: []}, + {FAMILY: [design_token_stub(name="--fs-obsidian", value_by_mode={"base": "#14171a"})], APP: []}, ) assert [t.name for t in resolved] == ["--fs-obsidian"] assert resolved[0].value_by_mode == {"base": "#14171a"} @@ -138,8 +131,8 @@ def test_the_deepest_system_wins_and_says_what_it_overrode(): resolved = _by_name(resolve_tokens( APP, PARENTS, { - FAMILY: [_token("--fs-accent", {"base": "#6b2118"})], - APP: [_token("--fs-accent", {"base": "#5b4a8a"})], + FAMILY: [design_token_stub(name="--fs-accent", value_by_mode={"base": "#6b2118"})], + APP: [design_token_stub(name="--fs-accent", value_by_mode={"base": "#5b4a8a"})], }, )) accent = resolved["--fs-accent"] @@ -158,8 +151,8 @@ def test_overriding_one_mode_leaves_the_others_inherited(): resolved = _by_name(resolve_tokens( APP, PARENTS, { - FAMILY: [_token("--fs-accent", {"base": "#34a877", "dark": "#34a877"})], - APP: [_token("--fs-accent", {"base": "#15803d"})], + FAMILY: [design_token_stub(name="--fs-accent", value_by_mode={"base": "#34a877", "dark": "#34a877"})], + APP: [design_token_stub(name="--fs-accent", value_by_mode={"base": "#15803d"})], }, )) accent = resolved["--fs-accent"] @@ -172,7 +165,7 @@ def test_a_token_only_the_app_defines_is_not_an_override(): labelled both "overridden here" would misdescribe the first.""" resolved = _by_name(resolve_tokens( APP, PARENTS, - {FAMILY: [], APP: [_token("--fs-editor-caret", {"base": "#5b4a8a"})]}, + {FAMILY: [], APP: [design_token_stub(name="--fs-editor-caret", value_by_mode={"base": "#5b4a8a"})]}, )) caret = resolved["--fs-editor-caret"] assert caret.origin_by_mode == {"base": APP} @@ -183,8 +176,8 @@ def test_is_overridden_in_is_true_only_for_the_system_that_shadowed(): resolved = _by_name(resolve_tokens( APP, PARENTS, { - FAMILY: [_token("--fs-accent", {"base": "#6b2118"})], - APP: [_token("--fs-accent", {"base": "#5b4a8a"})], + FAMILY: [design_token_stub(name="--fs-accent", value_by_mode={"base": "#6b2118"})], + APP: [design_token_stub(name="--fs-accent", value_by_mode={"base": "#5b4a8a"})], }, )) accent = resolved["--fs-accent"] @@ -198,9 +191,9 @@ def test_three_levels_stack_nearest_first(): resolved = _by_name(resolve_tokens( 3, parents, { - 1: [_token("--fs-bg", {"base": "a"})], - 2: [_token("--fs-bg", {"base": "b"})], - 3: [_token("--fs-bg", {"base": "c"})], + 1: [design_token_stub(name="--fs-bg", value_by_mode={"base": "a"})], + 2: [design_token_stub(name="--fs-bg", value_by_mode={"base": "b"})], + 3: [design_token_stub(name="--fs-bg", value_by_mode={"base": "c"})], }, )) bg = resolved["--fs-bg"] @@ -214,8 +207,8 @@ def test_resolving_the_family_itself_ignores_its_children(): resolved = _by_name(resolve_tokens( FAMILY, PARENTS, { - FAMILY: [_token("--fs-accent", {"base": "#6b2118"})], - APP: [_token("--fs-accent", {"base": "#5b4a8a"})], + FAMILY: [design_token_stub(name="--fs-accent", value_by_mode={"base": "#6b2118"})], + APP: [design_token_stub(name="--fs-accent", value_by_mode={"base": "#5b4a8a"})], }, )) assert resolved["--fs-accent"].value_by_mode == {"base": "#6b2118"} @@ -226,7 +219,7 @@ def test_value_for_falls_back_to_the_base_mode(): "dark" must yield that rather than nothing — the read rule the storage shape implies.""" resolved = _by_name(resolve_tokens( - FAMILY, PARENTS, {FAMILY: [_token("--fs-radius-md", {"base": "8px"})]}, + FAMILY, PARENTS, {FAMILY: [design_token_stub(name="--fs-radius-md", value_by_mode={"base": "8px"})]}, )) radius = resolved["--fs-radius-md"] assert radius.value_for("dark") == "8px" @@ -236,7 +229,7 @@ def test_value_for_falls_back_to_the_base_mode(): def test_value_for_prefers_an_explicit_mode_over_the_fallback(): resolved = _by_name(resolve_tokens( FAMILY, PARENTS, - {FAMILY: [_token("--fs-bg", {"base": "#f7f5ef", "dark": "#14171a"})]}, + {FAMILY: [design_token_stub(name="--fs-bg", value_by_mode={"base": "#f7f5ef", "dark": "#14171a"})]}, )) assert resolved["--fs-bg"].value_for("dark") == "#14171a" @@ -248,11 +241,8 @@ def test_metadata_is_inherited_when_the_override_leaves_it_blank(): resolved = _by_name(resolve_tokens( APP, PARENTS, { - FAMILY: [_token( - "--fs-obsidian", {"base": "#14171a"}, - group_name="surface", purpose="page bg, deepest surface", - )], - APP: [_token("--fs-obsidian", {"base": "#101317"})], + FAMILY: [design_token_stub(name="--fs-obsidian", value_by_mode={"base": "#14171a"}, group_name="surface", purpose="page bg, deepest surface")], + APP: [design_token_stub(name="--fs-obsidian", value_by_mode={"base": "#101317"})], }, )) obsidian = resolved["--fs-obsidian"] @@ -265,8 +255,8 @@ def test_an_override_that_states_metadata_wins_it_too(): resolved = _by_name(resolve_tokens( APP, PARENTS, { - FAMILY: [_token("--fs-x", {"base": "a"}, purpose="family says")], - APP: [_token("--fs-x", {"base": "b"}, purpose="app says")], + FAMILY: [design_token_stub(name="--fs-x", value_by_mode={"base": "a"}, purpose="family says")], + APP: [design_token_stub(name="--fs-x", value_by_mode={"base": "b"}, purpose="app says")], }, )) assert resolved["--fs-x"].purpose == "app says" @@ -279,8 +269,8 @@ def test_an_override_at_default_order_keeps_the_familys_position(): resolved = _by_name(resolve_tokens( APP, PARENTS, { - FAMILY: [_token("--fs-x", {"base": "a"}, order_index=7)], - APP: [_token("--fs-x", {"base": "b"})], + FAMILY: [design_token_stub(name="--fs-x", value_by_mode={"base": "a"}, order_index=7)], + APP: [design_token_stub(name="--fs-x", value_by_mode={"base": "b"})], }, )) assert resolved["--fs-x"].order_index == 7 @@ -290,10 +280,10 @@ def test_the_effective_set_is_ordered_by_group_then_position_with_ungrouped_last resolved = resolve_tokens( FAMILY, PARENTS, {FAMILY: [ - _token("--fs-z", {"base": "1"}), # ungrouped - _token("--fs-b", {"base": "2"}, group_name="text", order_index=1), - _token("--fs-a", {"base": "3"}, group_name="surface", order_index=2), - _token("--fs-c", {"base": "4"}, group_name="surface", order_index=1), + design_token_stub(name="--fs-z", value_by_mode={"base": "1"}), # ungrouped + design_token_stub(name="--fs-b", value_by_mode={"base": "2"}, group_name="text", order_index=1), + design_token_stub(name="--fs-a", value_by_mode={"base": "3"}, group_name="surface", order_index=2), + design_token_stub(name="--fs-c", value_by_mode={"base": "4"}, group_name="surface", order_index=1), ]}, ) assert [t.name for t in resolved] == ["--fs-c", "--fs-a", "--fs-b", "--fs-z"] @@ -306,7 +296,7 @@ def test_resolution_terminates_on_a_corrupt_hierarchy(): parents = {1: 2, 2: 1} resolved = _by_name(resolve_tokens( 1, parents, - {1: [_token("--fs-a", {"base": "one"})], 2: [_token("--fs-b", {"base": "two"})]}, + {1: [design_token_stub(name="--fs-a", value_by_mode={"base": "one"})], 2: [design_token_stub(name="--fs-b", value_by_mode={"base": "two"})]}, )) assert set(resolved) == {"--fs-a", "--fs-b"} # Each system contributes exactly once, not endlessly. @@ -326,8 +316,8 @@ def test_supersedes_is_inherited_when_the_override_is_silent_about_it(): resolved = _by_name(resolve_tokens( APP, PARENTS, { - FAMILY: [_token("--fs-text", {"base": "#e8e4d8"}, supersedes=["#fff", "#ffffff"])], - APP: [_token("--fs-text", {"base": "#f0ece0"})], + FAMILY: [design_token_stub(name="--fs-text", value_by_mode={"base": "#e8e4d8"}, supersedes=["#fff", "#ffffff"])], + APP: [design_token_stub(name="--fs-text", value_by_mode={"base": "#f0ece0"})], }, )) text = resolved["--fs-text"] @@ -341,8 +331,8 @@ def test_an_override_that_states_its_own_supersedes_replaces_the_list(): resolved = _by_name(resolve_tokens( APP, PARENTS, { - FAMILY: [_token("--fs-text", {"base": "a"}, supersedes=["#fff", "#ffffff"])], - APP: [_token("--fs-text", {"base": "b"}, supersedes=["#fff"])], + FAMILY: [design_token_stub(name="--fs-text", value_by_mode={"base": "a"}, supersedes=["#fff", "#ffffff"])], + APP: [design_token_stub(name="--fs-text", value_by_mode={"base": "b"}, supersedes=["#fff"])], }, )) assert resolved["--fs-text"].supersedes == ("#fff",) @@ -352,7 +342,7 @@ def test_a_token_that_supersedes_nothing_resolves_to_an_empty_tuple(): """Most tokens replace nothing. That has to be an empty sequence rather than None, so no caller has to test for two kinds of nothing.""" resolved = _by_name(resolve_tokens( - FAMILY, PARENTS, {FAMILY: [_token("--fs-radius-md", {"base": "8px"})]}, + FAMILY, PARENTS, {FAMILY: [design_token_stub(name="--fs-radius-md", value_by_mode={"base": "8px"})]}, )) assert resolved["--fs-radius-md"].supersedes == () @@ -360,7 +350,7 @@ def test_a_token_that_supersedes_nothing_resolves_to_an_empty_tuple(): def test_supersedes_survives_serialisation_as_a_list(): resolved = _by_name(resolve_tokens( FAMILY, PARENTS, - {FAMILY: [_token("--fs-text", {"base": "#e8e4d8"}, supersedes=["#fff"])]}, + {FAMILY: [design_token_stub(name="--fs-text", value_by_mode={"base": "#e8e4d8"}, supersedes=["#fff"])]}, )) assert resolved["--fs-text"].to_dict()["supersedes"] == ["#fff"] @@ -372,7 +362,7 @@ def test_the_superseded_literal_need_not_match_the_tokens_own_value(): it was turned around.""" resolved = _by_name(resolve_tokens( FAMILY, PARENTS, - {FAMILY: [_token("--fs-text", {"base": "#e8e4d8"}, supersedes=["#fff"])]}, + {FAMILY: [design_token_stub(name="--fs-text", value_by_mode={"base": "#e8e4d8"}, supersedes=["#fff"])]}, )) text = resolved["--fs-text"] assert text.value_by_mode["base"] not in text.supersedes @@ -404,7 +394,7 @@ def test_rationale_cascades_like_purpose_and_is_a_different_question(): rationale="equals Moss, aligned by design", order_index=0, supersedes=[], )], - APP: [_token("--fs-success", {"base": "#3f5236"})], + APP: [design_token_stub(name="--fs-success", value_by_mode={"base": "#3f5236"})], }, )) token = resolved["--fs-success"] @@ -415,6 +405,6 @@ def test_rationale_cascades_like_purpose_and_is_a_different_question(): def test_a_token_without_a_rationale_resolves_to_none(): resolved = _by_name(resolve_tokens( - FAMILY, PARENTS, {FAMILY: [_token("--fs-x", {"base": "1px"})]}, + FAMILY, PARENTS, {FAMILY: [design_token_stub(name="--fs-x", value_by_mode={"base": "1px"})]}, )) assert resolved["--fs-x"].rationale is None diff --git a/tests/test_design_stylesheet.py b/tests/test_design_stylesheet.py index 245fc71..7f40cb7 100644 --- a/tests/test_design_stylesheet.py +++ b/tests/test_design_stylesheet.py @@ -17,13 +17,7 @@ from scribe.services.design_stylesheet import ( safe_value, selector_for_mode, ) - - -def _token(name, value_by_mode, group_name=None, purpose=None): - return SimpleNamespace( - name=name, value_by_mode=value_by_mode, - group_name=group_name, purpose=purpose, - ) +from tests.helpers import design_token_stub # --- safety ----------------------------------------------------------------- @@ -66,7 +60,7 @@ def test_a_rejected_value_is_dropped_not_cleaned_up(): """Rejecting beats stripping. A partially-sanitised value is one the operator never wrote, and the sheet's entire claim is that it IS the record — quietly rendering a different colour would break that claim invisibly.""" - css = render_stylesheet([_token("--fs-x", {"base": "red; } body { color: blue"})]) + css = render_stylesheet([design_token_stub(name="--fs-x", value_by_mode={"base": "red; } body { color: blue"})]) assert "body" not in css assert "value rejected" in css @@ -85,7 +79,7 @@ def test_a_malformed_token_name_is_dropped(): assert not is_valid_token_name("color: red") assert not is_valid_token_name("fs-obsidian") # no leading -- - css = render_stylesheet([_token("--bad name", {"base": "red"})]) + css = render_stylesheet([design_token_stub(name="--bad name", value_by_mode={"base": "red"})]) assert "bad name" not in css @@ -100,8 +94,8 @@ def test_the_sheet_declares_properties_and_styles_no_elements(): handful of values once per element and grow with the UI. Purpose tokens are stated once and reused; components are snippets that reference them.""" css = render_stylesheet([ - _token("--fs-obsidian", {"base": "#14171a"}, group_name="surface"), - _token("--fs-moss", {"base": "#4a5d3f"}, group_name="action"), + design_token_stub(name="--fs-obsidian", value_by_mode={"base": "#14171a"}, group_name="surface"), + design_token_stub(name="--fs-moss", value_by_mode={"base": "#4a5d3f"}, group_name="action"), ]) assert "--fs-obsidian: #14171a;" in css # No element or class rules — the sheet has exactly one block here, and @@ -117,7 +111,7 @@ def test_the_sheet_declares_properties_and_styles_no_elements(): def test_the_header_says_what_the_sheet_is_for(): """A generated file with no explanation gets hand-edited, and then it has diverged from the record it claims to be.""" - css = render_stylesheet([_token("--fs-x", {"base": "1px"})], title="FabledSword") + css = render_stylesheet([design_token_stub(name="--fs-x", value_by_mode={"base": "1px"})], title="FabledSword") assert "FabledSword" in css assert "Generated" in css assert "snippets" in css @@ -129,7 +123,7 @@ def test_base_goes_on_the_root_selector_and_other_modes_layer_over_it(): """Matches the convention already in the codebase, and the one-way scoping #251 recorded: light on `:root`, dark layered on an attribute selector.""" css = render_stylesheet([ - _token("--fs-bg", {"base": "#f5f1e8", "dark": "#14171a"}), + design_token_stub(name="--fs-bg", value_by_mode={"base": "#f5f1e8", "dark": "#14171a"}), ]) assert ":root {" in css assert '[data-theme="dark"] {' in css @@ -141,8 +135,8 @@ def test_a_mode_block_contains_only_what_that_mode_declares(): Repeating every token in every block would make the sheet claim each mode redefines the whole system.""" css = render_stylesheet([ - _token("--fs-bg", {"base": "#f5f1e8", "dark": "#14171a"}), - _token("--fs-radius-md", {"base": "8px"}), + design_token_stub(name="--fs-bg", value_by_mode={"base": "#f5f1e8", "dark": "#14171a"}), + design_token_stub(name="--fs-radius-md", value_by_mode={"base": "8px"}), ]) dark_block = css.split('[data-theme="dark"] {')[1] assert "--fs-bg" in dark_block @@ -153,7 +147,7 @@ def test_the_root_selector_is_caller_chosen(): """A container-scoped preview cannot use `:root`. A generator that hardcoded it could not serve the preview surface at all.""" css = render_stylesheet( - [_token("--fs-x", {"base": "1px"})], root_selector="[data-preview]" + [design_token_stub(name="--fs-x", value_by_mode={"base": "1px"})], root_selector="[data-preview]" ) assert "[data-preview] {" in css assert ":root {" not in css @@ -163,8 +157,8 @@ def test_the_root_selector_is_caller_chosen(): def test_tokens_are_grouped_by_purpose_with_the_group_named(): css = render_stylesheet([ - _token("--fs-obsidian", {"base": "#14171a"}, group_name="surface"), - _token("--fs-radius-md", {"base": "8px"}, group_name="radius"), + design_token_stub(name="--fs-obsidian", value_by_mode={"base": "#14171a"}, group_name="surface"), + design_token_stub(name="--fs-radius-md", value_by_mode={"base": "8px"}, group_name="radius"), ]) assert "/* surface */" in css assert "/* radius */" in css @@ -174,8 +168,7 @@ def test_a_purpose_becomes_an_inline_comment_on_the_base_layer_only(): """Repeating the same prose in every mode block is noise: the token means the same thing in dark mode.""" css = render_stylesheet([ - _token("--fs-obsidian", {"base": "#14171a", "dark": "#000000"}, - purpose="page bg, deepest surface"), + design_token_stub(name="--fs-obsidian", value_by_mode={"base": "#14171a", "dark": "#000000"}, purpose="page bg, deepest surface"), ]) assert css.count("page bg, deepest surface") == 1 @@ -185,8 +178,8 @@ def test_a_declared_token_with_no_value_appears_as_a_comment_not_a_silence(): that finding where the reader is already looking; dropping it would make the sheet look complete.""" css = render_stylesheet([ - _token("--fs-obsidian", {"base": "#14171a"}), - _token("--fs-radius-sm", {}), + design_token_stub(name="--fs-obsidian", value_by_mode={"base": "#14171a"}), + design_token_stub(name="--fs-radius-sm", value_by_mode={}), ]) assert "--fs-radius-sm" in css assert "no value set yet" in css @@ -210,9 +203,9 @@ def test_two_tokens_sharing_a_value_are_reported_not_refused(): purpose — "Success = Moss, by design" — so this reports and lets a human decide which it is.""" dupes = duplicate_values([ - _token("--fs-moss", {"base": "#4A5D3F"}), - _token("--fs-success", {"base": "#4a5d3f"}), - _token("--fs-obsidian", {"base": "#14171a"}), + design_token_stub(name="--fs-moss", value_by_mode={"base": "#4A5D3F"}), + design_token_stub(name="--fs-success", value_by_mode={"base": "#4a5d3f"}), + design_token_stub(name="--fs-obsidian", value_by_mode={"base": "#14171a"}), ]) assert dupes == {"#4a5d3f": ["--fs-moss", "--fs-success"]} @@ -221,15 +214,15 @@ def test_tokens_that_agree_in_one_mode_but_differ_in_another_are_not_duplicates( """A near-miss is a different, weaker finding, and calling it a duplicate would send someone to merge two tokens that genuinely diverge.""" assert duplicate_values([ - _token("--fs-a", {"base": "#fff", "dark": "#000"}), - _token("--fs-b", {"base": "#fff", "dark": "#111"}), + design_token_stub(name="--fs-a", value_by_mode={"base": "#fff", "dark": "#000"}), + design_token_stub(name="--fs-b", value_by_mode={"base": "#fff", "dark": "#111"}), ]) == {"#fff": ["--fs-a", "--fs-b"]} def test_valueless_tokens_never_count_as_duplicates_of_each_other(): """Otherwise every unfilled token would collide with every other one and the report would be nothing but noise on a fresh import.""" - assert duplicate_values([_token("--fs-a", {}), _token("--fs-b", {})]) == {} + assert duplicate_values([design_token_stub(name="--fs-a", value_by_mode={}), design_token_stub(name="--fs-b", value_by_mode={})]) == {} # --- reading the sheet from the other side ---------------------------------- diff --git a/tests/test_integration_snippet_locations.py b/tests/test_integration_snippet_locations.py index 9db9f29..7f5a363 100644 --- a/tests/test_integration_snippet_locations.py +++ b/tests/test_integration_snippet_locations.py @@ -31,14 +31,11 @@ from scribe.services.snippets import ( list_snippets, snippet_fields, ) +from tests.helpers import loc pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")] -def _loc(repo="", path="", symbol=""): - return {"repo": repo, "path": path, "symbol": symbol} - - @pytest_asyncio.fixture async def seeded(): """A user with four snippets covering the cases the filter has to separate. @@ -60,13 +57,13 @@ async def seeded(): data=compose_data(name=name, language="python", locations=locations), ) - nested = _snippet("nested", [_loc("Scribe", "frontend/src/lib/x.ts", "helper")]) - sibling = _snippet("sibling", [_loc("Scribe", "frontend/srcmap.ts", "other")]) + nested = _snippet("nested", [loc(repo="Scribe", path="frontend/src/lib/x.ts", symbol="helper")]) + sibling = _snippet("sibling", [loc(repo="Scribe", path="frontend/srcmap.ts", symbol="other")]) # Two locations, deliberately crossed: repo Scribe at src/a.py and repo # Portal at src/b.py. repo=Scribe + path=src/b.py must NOT match it. multi = _snippet( "multi", - [_loc("Scribe", "src/a.py", "alpha"), _loc("Portal", "src/b.py", "beta")], + [loc(repo="Scribe", path="src/a.py", symbol="alpha"), loc(repo="Portal", path="src/b.py", symbol="beta")], ) # No structured location at all — must never satisfy a location filter, # and must not error the query either. @@ -242,7 +239,7 @@ async def test_backfill_makes_a_pre_0070_snippet_findable_by_location(seeded): defaults to none_as_null=False) — a different state, covered by the next test. """ user_id, _ids = seeded - locations = [_loc("Legacy", "old/path/y.py", "legacy_helper")] + locations = [loc(repo="Legacy", path="old/path/y.py", symbol="legacy_helper")] async with async_session() as s: old = Note( user_id=user_id, diff --git a/tests/test_mcp_tool_design_systems.py b/tests/test_mcp_tool_design_systems.py index ffc61ff..d679470 100644 --- a/tests/test_mcp_tool_design_systems.py +++ b/tests/test_mcp_tool_design_systems.py @@ -5,26 +5,23 @@ calling convention meets the service's: an agent cannot omit an argument, so "leave unchanged", "clear" and "set" have to be encoded in the value. Getting that mapping wrong is silent — the call succeeds and changes the wrong thing. """ -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest from scribe.services.design_systems import DesignSystemCycle +from tests.helpers import design_token_stub, fake_record pytestmark = pytest.mark.usefixtures("_bind_user") -def _fake_system(): - s = MagicMock() - s.to_dict.return_value = {"id": 1, "title": "FabledSword", "parent_id": None} - return s +def _fake_design_system(): + return fake_record(id=1, title="FabledSword", parent_id=None) def _fake_token(): - t = MagicMock() - t.to_dict.return_value = {"id": 9, "name": "--fs-obsidian"} - return t + return fake_record(id=9, name="--fs-obsidian") # --- create ----------------------------------------------------------------- @@ -35,7 +32,7 @@ async def test_creating_without_a_parent_passes_none_not_zero(): system id — there is no system 0, so the create would fail an ACL check for a record that cannot exist.""" with patch("scribe.mcp.tools.design_systems.ds_svc") as svc: - svc.create_design_system = AsyncMock(return_value=_fake_system()) + svc.create_design_system = AsyncMock(return_value=_fake_design_system()) from scribe.mcp.tools.design_systems import create_design_system await create_design_system(title="FabledSword") assert svc.create_design_system.await_args.kwargs["parent_id"] is None @@ -44,7 +41,7 @@ async def test_creating_without_a_parent_passes_none_not_zero(): @pytest.mark.asyncio async def test_creating_with_a_parent_passes_it_through(): with patch("scribe.mcp.tools.design_systems.ds_svc") as svc: - svc.create_design_system = AsyncMock(return_value=_fake_system()) + svc.create_design_system = AsyncMock(return_value=_fake_design_system()) from scribe.mcp.tools.design_systems import create_design_system await create_design_system(title="Scribe", parent_id=4) assert svc.create_design_system.await_args.kwargs["parent_id"] == 4 @@ -65,7 +62,7 @@ async def test_create_raises_when_the_parent_is_not_writable(): async def test_update_with_parent_id_zero_leaves_the_parent_alone(): """The common case — renaming a system must not silently re-root it.""" with patch("scribe.mcp.tools.design_systems.ds_svc") as svc: - svc.update_design_system = AsyncMock(return_value=_fake_system()) + svc.update_design_system = AsyncMock(return_value=_fake_design_system()) from scribe.mcp.tools.design_systems import update_design_system await update_design_system(design_system_id=1, title="Renamed") fields = svc.update_design_system.await_args.kwargs @@ -79,7 +76,7 @@ async def test_update_with_parent_id_minus_one_clears_it(): None, which is the value the service reads as "become a root" — where omitting the key means "leave alone".""" with patch("scribe.mcp.tools.design_systems.ds_svc") as svc: - svc.update_design_system = AsyncMock(return_value=_fake_system()) + svc.update_design_system = AsyncMock(return_value=_fake_design_system()) from scribe.mcp.tools.design_systems import update_design_system await update_design_system(design_system_id=1, parent_id=-1) assert svc.update_design_system.await_args.kwargs["parent_id"] is None @@ -88,7 +85,7 @@ async def test_update_with_parent_id_minus_one_clears_it(): @pytest.mark.asyncio async def test_update_with_a_positive_parent_id_sets_it(): with patch("scribe.mcp.tools.design_systems.ds_svc") as svc: - svc.update_design_system = AsyncMock(return_value=_fake_system()) + svc.update_design_system = AsyncMock(return_value=_fake_design_system()) from scribe.mcp.tools.design_systems import update_design_system await update_design_system(design_system_id=1, parent_id=4) assert svc.update_design_system.await_args.kwargs["parent_id"] == 4 @@ -161,16 +158,11 @@ async def test_resolve_returns_serialised_tokens_with_their_provenance(): resolution was built to preserve.""" from scribe.services.design_cascade import resolve_tokens - class _T: - def __init__(self, name, value_by_mode): - self.name, self.value_by_mode = name, value_by_mode - self.group_name = self.purpose = None - self.order_index = 0 resolved = resolve_tokens( 2, {1: None, 2: 1}, - {1: [_T("--fs-accent", {"base": "#6b2118"})], - 2: [_T("--fs-accent", {"base": "#5b4a8a"})]}, + {1: [design_token_stub("--fs-accent", {"base": "#6b2118"})], + 2: [design_token_stub("--fs-accent", {"base": "#5b4a8a"})]}, ) with patch("scribe.mcp.tools.design_systems.ds_svc") as svc: svc.resolve_design_system = AsyncMock(return_value=resolved) diff --git a/tests/test_mcp_tool_milestones.py b/tests/test_mcp_tool_milestones.py index f4c1aef..907d0f1 100644 --- a/tests/test_mcp_tool_milestones.py +++ b/tests/test_mcp_tool_milestones.py @@ -6,20 +6,12 @@ import pytest from scribe.mcp.tools.milestones import ( list_milestones, get_milestone, create_milestone, update_milestone, ) +from tests.helpers import fake_milestone pytestmark = pytest.mark.usefixtures("_bind_user") -def _fake_ms(**overrides) -> MagicMock: - m = MagicMock() - base = {"id": 1, "project_id": 1, "title": "MS", "description": None, - "status": "active", "order_index": 0} - base.update(overrides) - m.to_dict.return_value = base - return m - - @pytest.mark.asyncio async def test_list_milestones_returns_dict_with_progress(): rows = [{"id": 1, "title": "MS1", "status": "active", "task_count": 2}] @@ -33,7 +25,7 @@ async def test_list_milestones_returns_dict_with_progress(): @pytest.mark.asyncio async def test_create_milestone_passes_through(): - m = _fake_ms(id=5) + m = fake_milestone(id=5) mock = AsyncMock(return_value=m) with patch("scribe.mcp.tools.milestones.milestones_svc.create_milestone", mock): out = await create_milestone(project_id=1, title="new", description="d") @@ -45,7 +37,7 @@ async def test_create_milestone_passes_through(): @pytest.mark.asyncio async def test_create_milestone_empty_description_becomes_none(): - m = _fake_ms() + m = fake_milestone() mock = AsyncMock(return_value=m) with patch("scribe.mcp.tools.milestones.milestones_svc.create_milestone", mock): await create_milestone(project_id=1, title="t", description="") @@ -55,7 +47,7 @@ async def test_create_milestone_empty_description_becomes_none(): @pytest.mark.asyncio async def test_create_milestone_passes_body_through(): """The milestone-as-plan body is forwarded to the service.""" - m = _fake_ms(id=5) + m = fake_milestone(id=5) mock = AsyncMock(return_value=m) with patch("scribe.mcp.tools.milestones.milestones_svc.create_milestone", mock): await create_milestone(project_id=1, title="t", body="## Goal\n\nship") @@ -64,7 +56,7 @@ async def test_create_milestone_passes_body_through(): @pytest.mark.asyncio async def test_create_milestone_empty_body_becomes_none(): - m = _fake_ms() + m = fake_milestone() mock = AsyncMock(return_value=m) with patch("scribe.mcp.tools.milestones.milestones_svc.create_milestone", mock): await create_milestone(project_id=1, title="t", body="") @@ -73,7 +65,7 @@ async def test_create_milestone_empty_body_becomes_none(): @pytest.mark.asyncio async def test_update_milestone_sends_body(): - m = _fake_ms() + m = fake_milestone() mock = AsyncMock(return_value=m) with patch("scribe.mcp.tools.milestones.milestones_svc.update_milestone", mock): await update_milestone(project_id=1, milestone_id=5, body="new plan") @@ -82,7 +74,7 @@ async def test_update_milestone_sends_body(): @pytest.mark.asyncio async def test_get_milestone_returns_body_steps_and_rules(): - m = _fake_ms(id=5, project_id=3, body="## Goal") + m = fake_milestone(id=5, project_id=3, body="## Goal") step = MagicMock() step.to_dict.return_value = {"id": 9, "title": "step 1", "status": "todo"} applicable = {"rules": [{"id": 1, "title": "r"}], "truncated": False, @@ -112,7 +104,7 @@ async def test_get_milestone_raises_when_not_found(): @pytest.mark.asyncio async def test_update_milestone_only_sends_non_default_fields(): - m = _fake_ms() + m = fake_milestone() mock = AsyncMock(return_value=m) with patch("scribe.mcp.tools.milestones.milestones_svc.update_milestone", mock): await update_milestone(project_id=1, milestone_id=5, status="done") @@ -124,7 +116,7 @@ async def test_update_milestone_only_sends_non_default_fields(): @pytest.mark.asyncio async def test_update_milestone_order_index_negative_is_omitted(): """order_index=-1 sentinel means leave unchanged.""" - m = _fake_ms() + m = fake_milestone() mock = AsyncMock(return_value=m) with patch("scribe.mcp.tools.milestones.milestones_svc.update_milestone", mock): await update_milestone(project_id=1, milestone_id=5, order_index=-1) @@ -134,7 +126,7 @@ async def test_update_milestone_order_index_negative_is_omitted(): @pytest.mark.asyncio async def test_update_milestone_order_index_zero_is_explicit(): """order_index=0 is a real value (top of list), not a sentinel.""" - m = _fake_ms() + m = fake_milestone() mock = AsyncMock(return_value=m) with patch("scribe.mcp.tools.milestones.milestones_svc.update_milestone", mock): await update_milestone(project_id=1, milestone_id=5, order_index=0) diff --git a/tests/test_mcp_tool_planning.py b/tests/test_mcp_tool_planning.py index 7fdc617..26d4897 100644 --- a/tests/test_mcp_tool_planning.py +++ b/tests/test_mcp_tool_planning.py @@ -1,6 +1,7 @@ -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest +from tests.helpers import fake_task pytestmark = pytest.mark.usefixtures("_bind_user") @@ -18,25 +19,12 @@ async def test_start_planning_tool_delegates_to_service(): assert mock.call_args.kwargs == {"user_id": 7, "project_id": 3, "title": "Plan it"} -def _plan_note(task_kind: str): - note = MagicMock() - note.parent_id = None - note.project_id = 3 - note.id = 9 - # Real values — get_task reads deleted_at and compares user_id to the bound - # caller, and a MagicMock is truthy on both (note 2109). - note.user_id = 7 - note.deleted_at = None - note.to_dict.return_value = {"id": 9, "task_kind": task_kind, "project_id": 3} - return note - - @pytest.mark.asyncio async def test_get_task_augments_plan_with_rules(): applicable = {"rules": [{"id": 1, "title": "r"}], "truncated": False, "subscribed_rulebooks": [{"id": 2, "title": "rb"}]} with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user", - AsyncMock(return_value=(_plan_note("plan"), "owner"))), \ + AsyncMock(return_value=(fake_task(task_kind="plan", id=9, project_id=3), "owner"))), \ patch("scribe.mcp.tools.tasks.rulebooks_svc.get_applicable_rules", AsyncMock(return_value=applicable)): from scribe.mcp.tools.tasks import get_task @@ -49,7 +37,7 @@ async def test_get_task_augments_plan_with_rules(): @pytest.mark.asyncio async def test_get_task_work_kind_has_no_rules(): with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user", - AsyncMock(return_value=(_plan_note("work"), "owner"))), \ + AsyncMock(return_value=(fake_task(task_kind="work", id=9, project_id=3), "owner"))), \ patch("scribe.mcp.tools.tasks.rulebooks_svc.get_applicable_rules", AsyncMock()) as mock_rules: from scribe.mcp.tools.tasks import get_task diff --git a/tests/test_mcp_tool_processes.py b/tests/test_mcp_tool_processes.py index d8f73a5..49e26ff 100644 --- a/tests/test_mcp_tool_processes.py +++ b/tests/test_mcp_tool_processes.py @@ -2,7 +2,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest -from tests.helpers import fake_note +from tests.helpers import FakeMCP, fake_note pytestmark = pytest.mark.usefixtures("_bind_user") @@ -172,18 +172,11 @@ def test_register_attaches_every_tool_in_the_module(): import inspect from scribe.mcp.tools import processes - names: list[str] = [] + mcp = FakeMCP() - class FakeMcp: - def tool(self, name): - names.append(name) - def deco(fn): - return fn - return deco - - processes.register(FakeMcp()) + processes.register(mcp) public = { name for name, obj in vars(processes).items() if inspect.iscoroutinefunction(obj) and not name.startswith("_") } - assert set(names) == public + assert set(mcp.names) == public diff --git a/tests/test_mcp_tool_projects.py b/tests/test_mcp_tool_projects.py index 94df878..a434ee4 100644 --- a/tests/test_mcp_tool_projects.py +++ b/tests/test_mcp_tool_projects.py @@ -7,6 +7,7 @@ from scribe.mcp.tools.projects import ( list_projects, get_project, create_project, update_project, enter_project, ) +from tests.helpers import FakeMCP, fake_project pytestmark = pytest.mark.usefixtures("_bind_user") @@ -56,22 +57,9 @@ def _no_bootstrap(): yield mock -def _fake_project(design_system_id=None, **overrides) -> MagicMock: - p = MagicMock() - base = {"id": 1, "title": "P", "description": "", "goal": "", - "status": "active", "color": None} - base.update(overrides) - p.to_dict.return_value = base - # Explicit, because a bare MagicMock hands back a truthy auto-attribute — - # which would route every project in this file through the design-system - # branch and out to a real database. - p.design_system_id = design_system_id - return p - - @pytest.mark.asyncio async def test_list_projects_wraps_in_dict(): - rows = [_fake_project(id=1), _fake_project(id=2)] + rows = [fake_project(id=1), fake_project(id=2)] with patch( "scribe.mcp.tools.projects.projects_svc.list_projects", AsyncMock(return_value=rows), @@ -82,7 +70,7 @@ async def test_list_projects_wraps_in_dict(): @pytest.mark.asyncio async def test_get_project_enriches_with_milestone_summary(): - p = _fake_project(id=5, title="found") + p = fake_project(id=5, title="found") milestone_summary = [{"id": 10, "title": "MS", "task_count": 3}] applicable_payload = { "rules": [], "truncated": False, "subscribed_rulebooks": [], @@ -107,7 +95,7 @@ async def test_get_project_includes_applicable_rules_and_subscribed_rulebooks(): """The augmented get_project response includes applicable_rules and subscribed_rulebooks pulled from services/rulebooks.get_applicable_rules. """ - p = _fake_project(id=3, title="Fabled Assistant") + p = fake_project(id=3, title="Fabled Assistant") milestone_summary = [] applicable_payload = { "rules": [ @@ -147,7 +135,7 @@ async def test_get_project_raises_when_not_found(): @pytest.mark.asyncio async def test_create_project_passes_color_empty_as_none(): - p = _fake_project() + p = fake_project() mock = AsyncMock(return_value=p) with patch("scribe.mcp.tools.projects.projects_svc.create_project", mock): await create_project(title="P", color="") @@ -156,7 +144,7 @@ async def test_create_project_passes_color_empty_as_none(): @pytest.mark.asyncio async def test_update_project_only_sends_non_default_fields(): - p = _fake_project() + p = fake_project() mock = AsyncMock(return_value=p) with patch("scribe.mcp.tools.projects.projects_svc.update_project", mock): await update_project(project_id=1, status="archived") @@ -179,7 +167,7 @@ async def test_update_project_raises_when_not_found(): async def test_enter_project_composes_full_context(): """enter_project pulls project + rules + milestone summary + open tasks + recent notes in one composed call.""" - p = _fake_project(id=5, title="P") + p = fake_project(id=5, title="P") applicable_payload = { "rules": [{"id": 1, "title": "r1", "statement": "s", "topic_title": "t", "rulebook_title": "rb"}], @@ -239,7 +227,7 @@ async def test_enter_project_surfaces_the_systems_vocabulary(): days after the feature landed — one System, nothing tagged since July 28 (#2546's audit). Trimmed to id/name/first-line: it rides on every session start, and the full charter is get_system's job.""" - p = _fake_project(id=5) + p = fake_project(id=5) sys1 = MagicMock() sys1.id = 3 sys1.name = "retrieval" @@ -294,7 +282,7 @@ async def test_enter_project_carries_the_bootstrap_ask_when_it_fires(): ask = "This project has 282 records and NO Systems modelled — ..." with contextlib.ExitStack() as stack: - for cm in _enter_project_stubs(_fake_project(id=5)): + for cm in _enter_project_stubs(fake_project(id=5)): stack.enter_context(cm) stack.enter_context(patch( "scribe.mcp.tools.projects.systems_tools.bootstrap_systems_ask", @@ -315,7 +303,7 @@ async def test_enter_project_never_asks_bootstrap_once_a_vocabulary_exists( sys1 = MagicMock() sys1.id = 3; sys1.name = "retrieval"; sys1.description = "" with contextlib.ExitStack() as stack: - for cm in _enter_project_stubs(_fake_project(id=5)): + for cm in _enter_project_stubs(fake_project(id=5)): stack.enter_context(cm) stack.enter_context(patch( "scribe.mcp.tools.projects.systems_svc.list_systems", @@ -336,7 +324,7 @@ async def test_enter_project_fires_the_coverage_seed_on_the_owner( enter stays fast.""" import contextlib - project = _fake_project(id=5) + project = fake_project(id=5) project.user_id = 42 # explicit: the OWNER, not the caller (ctx uid=7) with contextlib.ExitStack() as stack: for cm in _enter_project_stubs(project): @@ -357,7 +345,7 @@ async def test_enter_project_hands_back_the_design_system_when_the_project_has_o system binds the same way a rule does. Before this it was reachable only by an agent that already knew to call resolve_design_system — so the standards were present in the store and absent from the work.""" - p = _fake_project(id=5, design_system_id=9) + p = fake_project(id=5, design_system_id=9) design = {"id": 9, "title": "App kit", "guidance": [{"title": "House"}], "token_count": 95, "token_groups": ["surface"], "inherits_from": ["House"], "description": ""} @@ -399,14 +387,7 @@ async def test_enter_project_raises_when_project_not_found(): def test_enter_project_registered_in_register(): """register(mcp) registers enter_project alongside the existing tools.""" from scribe.mcp.tools.projects import register - registered: list[str] = [] + mcp = FakeMCP() - class FakeMCP: - def tool(self, name=None): - def decorator(fn): - registered.append(name) - return fn - return decorator - - register(FakeMCP()) - assert "enter_project" in registered + register(mcp) + assert "enter_project" in mcp.names diff --git a/tests/test_mcp_tool_rulebooks.py b/tests/test_mcp_tool_rulebooks.py index 4ddb48a..004c7d2 100644 --- a/tests/test_mcp_tool_rulebooks.py +++ b/tests/test_mcp_tool_rulebooks.py @@ -1,41 +1,16 @@ """Tests for MCP rulebook tools — patches the service layer.""" -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest +from tests.helpers import FakeMCP, fake_rule, fake_rulebook, fake_topic pytestmark = pytest.mark.usefixtures("_bind_user") -def _fake_rulebook(id=1, title="t"): - rb = MagicMock() - rb.id = id - rb.title = title - rb.to_dict.return_value = {"id": id, "title": title} - return rb - - -def _fake_topic(id=10, title="git"): - t = MagicMock() - t.id = id - t.title = title - t.to_dict.return_value = {"id": id, "title": title} - return t - - -def _fake_rule(id=100, title="r", statement="s"): - r = MagicMock() - r.id = id - r.title = title - r.topic_id = 10 - r.statement = statement - r.to_dict.return_value = {"id": id, "title": title, "statement": statement} - return r - - @pytest.mark.asyncio async def test_list_rulebooks_wraps_in_dict(): - rows = [_fake_rulebook(id=1), _fake_rulebook(id=2)] + rows = [fake_rulebook(id=1, title="t"), fake_rulebook(id=2, title="t")] with patch( "scribe.mcp.tools.rulebooks.rulebooks_svc.list_rulebooks", AsyncMock(return_value=rows), @@ -47,8 +22,8 @@ async def test_list_rulebooks_wraps_in_dict(): @pytest.mark.asyncio async def test_get_rulebook_includes_topics(): - rb = _fake_rulebook(id=1) - topics = [_fake_topic(id=10), _fake_topic(id=11)] + rb = fake_rulebook(id=1, title="t") + topics = [fake_topic(id=10, title="git"), fake_topic(id=11, title="git")] with patch( "scribe.mcp.tools.rulebooks.rulebooks_svc.get_rulebook", AsyncMock(return_value=rb), @@ -75,7 +50,7 @@ async def test_get_rulebook_raises_when_not_found(): @pytest.mark.asyncio async def test_create_rule_passes_required_fields(): - rule = _fake_rule() + rule = fake_rule(id=100, title="r", statement="s", topic_id=10) mock = AsyncMock(return_value=rule) with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule", mock): from scribe.mcp.tools.rulebooks import create_rule @@ -109,7 +84,7 @@ async def test_create_rule_force_bypasses_duplicate_gate(): find_mock = AsyncMock() with patch("scribe.mcp.tools.rulebooks.dedup_svc.find_duplicate_rule", find_mock), \ patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule", - AsyncMock(return_value=_fake_rule(id=5))): + AsyncMock(return_value=fake_rule(id=5, title="r", statement="s", topic_id=10))): from scribe.mcp.tools.rulebooks import create_rule out = await create_rule(topic_id=10, title="dev is home", statement="x", force=True) assert out["id"] == 5 @@ -118,7 +93,7 @@ async def test_create_rule_force_bypasses_duplicate_gate(): @pytest.mark.asyncio async def test_update_rule_only_sends_non_default_fields(): - rule = _fake_rule() + rule = fake_rule(id=100, title="r", statement="s", topic_id=10) mock = AsyncMock(return_value=rule) with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.update_rule", mock): from scribe.mcp.tools.rulebooks import update_rule @@ -131,7 +106,7 @@ async def test_update_rule_only_sends_non_default_fields(): @pytest.mark.asyncio async def test_delete_rule_without_confirmed_returns_warning(): """delete_rule with confirmed=False returns a preview, not an action.""" - rule = _fake_rule() + rule = fake_rule(id=100, title="r", statement="s", topic_id=10) with patch( "scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule", AsyncMock(return_value=rule), @@ -148,7 +123,7 @@ async def test_delete_rule_without_confirmed_returns_warning(): @pytest.mark.asyncio async def test_delete_rule_with_confirmed_soft_deletes(): - rule = _fake_rule() + rule = fake_rule(id=100, title="r", statement="s", topic_id=10) mock_delete = AsyncMock(return_value="batch-1") with patch( "scribe.mcp.tools.rulebooks.rulebooks_svc.get_rule", @@ -191,27 +166,20 @@ async def test_unsubscribe_project_from_rulebook_calls_service(): def test_register_attaches_all_sixteen_tools(): """register(mcp) should call mcp.tool(name=...) for all 16 tools.""" from scribe.mcp.tools.rulebooks import register - registered: list[str] = [] + mcp = FakeMCP() - class FakeMCP: - def tool(self, name=None): - def decorator(fn): - registered.append(name) - return fn - return decorator - - register(FakeMCP()) - assert len(registered) == 22 + register(mcp) + assert len(mcp.names) == 22 # spot-check a few names - assert "list_rulebooks" in registered - assert "create_rule" in registered - assert "subscribe_project_to_rulebook" in registered - assert "list_always_on_rules" in registered - assert "create_project_rule" in registered - assert "suppress_rule_for_project" in registered - assert "unsuppress_rule_for_project" in registered - assert "suppress_topic_for_project" in registered - assert "unsuppress_topic_for_project" in registered + assert "list_rulebooks" in mcp.names + assert "create_rule" in mcp.names + assert "subscribe_project_to_rulebook" in mcp.names + assert "list_always_on_rules" in mcp.names + assert "create_project_rule" in mcp.names + assert "suppress_rule_for_project" in mcp.names + assert "unsuppress_rule_for_project" in mcp.names + assert "suppress_topic_for_project" in mcp.names + assert "unsuppress_topic_for_project" in mcp.names @pytest.mark.asyncio @@ -227,7 +195,7 @@ async def test_list_always_on_rules_returns_empty_when_no_always_on_rulebooks(): @pytest.mark.asyncio async def test_list_always_on_rules_projects_each_rule(): - rules = [_fake_rule(id=100), _fake_rule(id=101)] + rules = [fake_rule(id=100, title="r", statement="s", topic_id=10), fake_rule(id=101, title="r", statement="s", topic_id=10)] with patch( "scribe.mcp.tools.rulebooks.rulebooks_svc.list_always_on_rules", AsyncMock(return_value=rules), @@ -241,7 +209,7 @@ async def test_list_always_on_rules_projects_each_rule(): @pytest.mark.asyncio async def test_update_rulebook_forwards_always_on_when_set(): - rb = _fake_rulebook(id=1, title="t") + rb = fake_rulebook(id=1, title="t") mock = AsyncMock(return_value=rb) with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.update_rulebook", mock): from scribe.mcp.tools.rulebooks import update_rulebook @@ -254,7 +222,7 @@ async def test_update_rulebook_forwards_always_on_when_set(): @pytest.mark.asyncio async def test_update_rulebook_omits_always_on_when_none(): - rb = _fake_rulebook(id=1, title="t") + rb = fake_rulebook(id=1, title="t") mock = AsyncMock(return_value=rb) with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.update_rulebook", mock): from scribe.mcp.tools.rulebooks import update_rulebook @@ -266,7 +234,7 @@ async def test_update_rulebook_omits_always_on_when_none(): @pytest.mark.asyncio async def test_create_project_rule_passes_required_fields(): - rule = _fake_rule() + rule = fake_rule(id=100, title="r", statement="s", topic_id=10) mock = AsyncMock(return_value=rule) with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock): from scribe.mcp.tools.rulebooks import create_project_rule @@ -284,7 +252,7 @@ async def test_create_project_rule_passes_required_fields(): @pytest.mark.asyncio async def test_create_project_rule_derives_title_from_statement(): - rule = _fake_rule() + rule = fake_rule(id=100, title="r", statement="s", topic_id=10) mock = AsyncMock(return_value=rule) with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock): from scribe.mcp.tools.rulebooks import create_project_rule @@ -299,7 +267,7 @@ async def test_create_project_rule_derives_title_from_statement(): @pytest.mark.asyncio async def test_create_project_rule_uses_explicit_title_when_given(): - rule = _fake_rule() + rule = fake_rule(id=100, title="r", statement="s", topic_id=10) mock = AsyncMock(return_value=rule) with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock): from scribe.mcp.tools.rulebooks import create_project_rule diff --git a/tests/test_mcp_tool_snippets.py b/tests/test_mcp_tool_snippets.py index 441d737..7773f14 100644 --- a/tests/test_mcp_tool_snippets.py +++ b/tests/test_mcp_tool_snippets.py @@ -2,32 +2,12 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from tests.helpers import FakeMCP, fake_snippet pytestmark = pytest.mark.usefixtures("_bind_user") -def _fake_snippet(user_id: int = 7): - n = MagicMock() - n.id = 1 - n.title = "debounce — rate-limit a callback" - n.body = "```js\nreturn 1\n```\n" - n.tags = ["js", "snippet"] - n.note_type = "snippet" - # Real int, matching the bound caller by default. The tools compare it to - # decide whether to attach a shared/owner marker; an auto-MagicMock would read - # as another user's record and send them off to look up a username. - n.user_id = user_id - # Explicitly None, not an auto-attribute: snippet_fields prefers `data` when - # truthy, and a MagicMock is truthy — every parsed field would come back as a - # MagicMock instead of a string. - n.data = None - n.to_dict.return_value = { - "id": 1, "title": n.title, "note_type": "snippet", "tags": n.tags, - } - return n - - @pytest.mark.asyncio async def test_create_snippet_requires_name_and_code(): from scribe.mcp.tools.snippets import create_snippet @@ -39,7 +19,7 @@ async def test_create_snippet_requires_name_and_code(): @pytest.mark.asyncio async def test_create_snippet_records_and_returns_parsed(): - created = _fake_snippet() + created = fake_snippet() with patch("scribe.services.dedup.find_duplicate_note", AsyncMock(return_value=None)), \ patch("scribe.services.snippets.create_snippet", AsyncMock(return_value=created)) as mock_create: @@ -88,7 +68,7 @@ async def test_update_snippet_missing_raises(): async def test_update_snippet_empty_string_clears_a_field(): # An omitted field must stay None ("leave alone"), but an explicit empty # string has to reach the service as "" so a stale field can be removed. - updated = _fake_snippet() + updated = fake_snippet() with patch("scribe.services.snippets.update_snippet", AsyncMock(return_value=updated)) as mock_update: from scribe.mcp.tools.snippets import update_snippet @@ -101,7 +81,7 @@ async def test_update_snippet_empty_string_clears_a_field(): @pytest.mark.asyncio async def test_update_snippet_project_id_conventions(): from scribe.services import snippets as snippets_svc - updated = _fake_snippet() + updated = fake_snippet() cases = {0: snippets_svc.UNSET, -1: None, 5: 5} for given, expected in cases.items(): with patch("scribe.services.snippets.update_snippet", @@ -115,7 +95,7 @@ async def test_update_snippet_project_id_conventions(): async def test_create_and_update_pass_locations_through(): locs = [{"repo": "a", "path": "a.py", "symbol": "f"}, {"repo": "b", "path": "b.py", "symbol": "g"}] - created = _fake_snippet() + created = fake_snippet() with patch("scribe.services.dedup.find_duplicate_note", AsyncMock(return_value=None)), \ patch("scribe.services.snippets.create_snippet", AsyncMock(return_value=created)) as mock_create: @@ -187,7 +167,7 @@ async def test_merge_snippets_requires_a_source(): @pytest.mark.asyncio async def test_merge_snippets_returns_survivor_and_merged_ids(): - survivor = _fake_snippet() + survivor = fake_snippet() with patch("scribe.services.snippets.merge_snippets", AsyncMock(return_value=(survivor, [2, 3]))) as mock_merge: from scribe.mcp.tools.snippets import merge_snippets @@ -209,18 +189,10 @@ async def test_merge_snippets_not_found_raises(): def test_register_attaches_all_tools(): from scribe.mcp.tools import snippets - names: list[str] = [] + mcp = FakeMCP() - class FakeMcp: - def tool(self, name): - names.append(name) - - def deco(fn): - return fn - return deco - - snippets.register(FakeMcp()) - assert set(names) == { + snippets.register(mcp) + assert set(mcp.names) == { "list_snippets", "create_snippet", "get_snippet", "update_snippet", "delete_snippet", "merge_snippets", "verify_snippet", "find_duplicate_snippets", "unmerge_snippet", diff --git a/tests/test_mcp_tool_systems.py b/tests/test_mcp_tool_systems.py index 25beb8f..45e3a32 100644 --- a/tests/test_mcp_tool_systems.py +++ b/tests/test_mcp_tool_systems.py @@ -2,21 +2,14 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest -from tests.helpers import fake_note - - -def _fake_system(sid=1, name="Reader", project_id=5): - s = MagicMock() - s.to_dict.return_value = {"id": sid, "name": name, "project_id": project_id} - s.project_id = project_id - return s +from tests.helpers import fake_note, fake_system @pytest.mark.asyncio async def test_create_system_returns_dict(): with patch("scribe.mcp.tools.systems.current_user_id", return_value=1), \ patch("scribe.mcp.tools.systems.systems_svc") as svc: - svc.create_system = AsyncMock(return_value=_fake_system(name="Reader")) + svc.create_system = AsyncMock(return_value=fake_system(name="Reader")) from scribe.mcp.tools.systems import create_system result = await create_system(project_id=5, name="Reader", description="pdf reader") assert result["name"] == "Reader" @@ -39,7 +32,7 @@ async def test_get_system_splits_records_by_kind(): note = MagicMock(); note.to_dict.return_value = {"id": 12}; note.task_kind = "work"; note.status = None with patch("scribe.mcp.tools.systems.current_user_id", return_value=1), \ patch("scribe.mcp.tools.systems.systems_svc") as svc: - svc.get_system = AsyncMock(return_value=_fake_system(sid=3)) + svc.get_system = AsyncMock(return_value=fake_system(id=3)) svc.list_records_for_system = AsyncMock(return_value=[issue, work, note]) from scribe.mcp.tools.systems import get_system result = await get_system(system_id=3) @@ -164,7 +157,7 @@ async def test_populated_vocabulary_never_counts_records(): @pytest.mark.asyncio async def test_create_system_same_normalized_name_is_duplicate_gated(): - existing = _fake_system(sid=7, name="Scrape Pipeline") + existing = fake_system(id=7, name="Scrape Pipeline") existing.id = 7 existing.name = "Scrape Pipeline" with patch("scribe.mcp.tools.systems.current_user_id", return_value=1), \ @@ -180,12 +173,12 @@ async def test_create_system_same_normalized_name_is_duplicate_gated(): @pytest.mark.asyncio async def test_create_system_distinct_name_passes_the_gate(): - other = _fake_system(sid=7, name="Workers") + other = fake_system(id=7, name="Workers") other.name = "Workers" with patch("scribe.mcp.tools.systems.current_user_id", return_value=1), \ patch("scribe.mcp.tools.systems.systems_svc") as svc: svc.list_systems = AsyncMock(return_value=[other]) - svc.create_system = AsyncMock(return_value=_fake_system(sid=8, name="Exporter")) + svc.create_system = AsyncMock(return_value=fake_system(id=8, name="Exporter")) from scribe.mcp.tools.systems import create_system result = await create_system(project_id=5, name="Exporter") assert result["name"] == "Exporter" diff --git a/tests/test_mcp_tool_tags.py b/tests/test_mcp_tool_tags.py index ea95a36..dcb2231 100644 --- a/tests/test_mcp_tool_tags.py +++ b/tests/test_mcp_tool_tags.py @@ -8,6 +8,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from scribe.mcp.tools.tags import list_tags, _aggregate_tag_counts +from tests.helpers import make_mock_session pytestmark = pytest.mark.usefixtures("_bind_user") @@ -31,9 +32,7 @@ async def test_fable_list_tags_returns_sorted_by_count_desc(): """End-to-end: query returns three rows, top tag wins.""" mock_result = MagicMock() mock_result.all.return_value = [(["a"],), (["a", "b"],), (["a"],)] - mock_session = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) + mock_session = make_mock_session() mock_session.execute = AsyncMock(return_value=mock_result) mock_ctx = MagicMock(return_value=mock_session) with patch("scribe.mcp.tools.tags.async_session", mock_ctx): @@ -47,9 +46,7 @@ async def test_fable_list_tags_returns_sorted_by_count_desc(): async def test_fable_list_tags_clamps_limit(): mock_result = MagicMock() mock_result.all.return_value = [] - mock_session = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) + mock_session = make_mock_session() mock_session.execute = AsyncMock(return_value=mock_result) mock_ctx = MagicMock(return_value=mock_session) with patch("scribe.mcp.tools.tags.async_session", mock_ctx): diff --git a/tests/test_mcp_tool_tasks.py b/tests/test_mcp_tool_tasks.py index fa44350..47db67a 100644 --- a/tests/test_mcp_tool_tasks.py +++ b/tests/test_mcp_tool_tasks.py @@ -8,35 +8,15 @@ from scribe.mcp.tools.tasks import ( list_tasks, get_task, create_task, update_task, add_task_log, ) +from tests.helpers import fake_task pytestmark = pytest.mark.usefixtures("_bind_user") -def _fake_task(*, parent_id: int | None = None, user_id: int = 7, - **overrides) -> MagicMock: - n = MagicMock() - n.parent_id = parent_id - base = { - "id": 1, "title": "t", "body": "", "status": "todo", - "priority": "none", "tags": [], "parent_id": parent_id, - "is_task": True, - } - base.update(overrides) - n.to_dict.return_value = base - n.title = base["title"] - n.id = base["id"] - # Real values, not auto-attributes: get_task reads deleted_at and compares - # user_id against the bound caller for the shared/owner marker — a MagicMock - # is truthy on both (note 2109). - n.user_id = user_id - n.deleted_at = None - return n - - @pytest.mark.asyncio async def test_list_tasks_passes_is_task_true_and_repackages(): - rows = [_fake_task(id=1), _fake_task(id=2)] + rows = [fake_task(id=1), fake_task(id=2)] mock = AsyncMock(return_value=(rows, 2)) with patch("scribe.mcp.tools.tasks.notes_svc.list_notes", mock): out = await list_tasks() @@ -63,7 +43,7 @@ async def test_list_tasks_empty_status_means_no_filter(): @pytest.mark.asyncio async def test_get_task_with_no_parent_returns_null_parent_title(): - fake = _fake_task(id=5, title="solo", parent_id=None) + fake = fake_task(id=5, title="solo", parent_id=None) with patch( "scribe.mcp.tools.tasks.notes_svc.get_note_for_user", AsyncMock(return_value=(fake, "owner")), @@ -77,8 +57,8 @@ async def test_get_task_with_no_parent_returns_null_parent_title(): @pytest.mark.asyncio async def test_get_task_enriches_with_parent_title(): """When parent_id is set, get_task fetches the parent and adds parent_title.""" - child = _fake_task(id=10, title="child", parent_id=5) - parent = _fake_task(id=5, title="parent of 10", parent_id=None) + child = fake_task(id=10, title="child", parent_id=5) + parent = fake_task(id=5, title="parent of 10", parent_id=None) # fetched twice: once for the child, once for the parent mock_get = AsyncMock(side_effect=[(child, "owner"), (parent, "owner")]) with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user", mock_get): @@ -90,7 +70,7 @@ async def test_get_task_enriches_with_parent_title(): @pytest.mark.asyncio async def test_get_task_parent_missing_returns_null(): """If parent_id is set but the parent is gone (orphaned), parent_title is None.""" - child = _fake_task(id=10, parent_id=5) + child = fake_task(id=10, parent_id=5) mock_get = AsyncMock(side_effect=[(child, "owner"), None]) with patch("scribe.mcp.tools.tasks.notes_svc.get_note_for_user", mock_get): out = await get_task(task_id=10) @@ -101,7 +81,7 @@ async def test_get_task_parent_missing_returns_null(): async def test_get_task_opens_a_shared_task_and_says_whose_it_is(): """A task in a shared project opens in the web UI, so the agent path must not answer "not found" for the same id — and must say it isn't the caller's.""" - theirs = _fake_task(id=5, title="Their task", user_id=9) + theirs = fake_task(id=5, title="Their task", user_id=9) with patch( "scribe.mcp.tools.tasks.notes_svc.get_note_for_user", AsyncMock(return_value=(theirs, "viewer")), @@ -127,7 +107,7 @@ async def test_get_task_raises_when_not_found(): @pytest.mark.asyncio async def test_create_task_passes_status(): - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.create_note", mock): await create_task(title="do x", status="todo") @@ -155,7 +135,7 @@ async def test_create_task_force_bypasses_duplicate_gate(): find_mock = AsyncMock() with patch("scribe.mcp.tools.tasks.dedup_svc.find_duplicate_note", find_mock), \ patch("scribe.mcp.tools.tasks.notes_svc.create_note", - AsyncMock(return_value=_fake_task(id=9))): + AsyncMock(return_value=fake_task(id=9))): out = await create_task(title="dup", force=True) assert out["id"] == 9 find_mock.assert_not_called() # gate not even consulted @@ -173,7 +153,7 @@ async def test_create_task_rejects_retired_plan_kind(): @pytest.mark.asyncio async def test_create_task_priority_empty_becomes_none(): - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.create_note", mock): await create_task(title="x", priority="") @@ -182,7 +162,7 @@ async def test_create_task_priority_empty_becomes_none(): @pytest.mark.asyncio async def test_create_task_zero_id_sentinels_become_none(): - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.create_note", mock): await create_task(title="x", project_id=0, milestone_id=0, parent_id=0) @@ -193,7 +173,7 @@ async def test_create_task_zero_id_sentinels_become_none(): @pytest.mark.asyncio async def test_update_task_only_sends_non_default_fields(): - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.update_note", mock): await update_task(task_id=1, status="done") @@ -205,7 +185,7 @@ async def test_update_task_only_sends_non_default_fields(): @pytest.mark.asyncio async def test_update_task_empty_priority_is_omitted(): """Priority="" is "leave unchanged" — must not reach service as empty string.""" - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.update_note", mock): await update_task(task_id=1, status="done", priority="") @@ -225,7 +205,7 @@ async def test_update_task_raises_when_not_found(): @pytest.mark.asyncio async def test_update_task_milestone_zero_is_omitted(): """milestone_id=0 is 'leave unchanged' — must not reach the service.""" - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.update_note", mock): await update_task(task_id=1, milestone_id=0) @@ -234,7 +214,7 @@ async def test_update_task_milestone_zero_is_omitted(): @pytest.mark.asyncio async def test_update_task_milestone_positive_is_set(): - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.update_note", mock): await update_task(task_id=1, milestone_id=42) @@ -244,7 +224,7 @@ async def test_update_task_milestone_positive_is_set(): @pytest.mark.asyncio async def test_update_task_milestone_negative_one_clears(): """milestone_id=-1 clears the milestone (sets the column NULL).""" - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.update_note", mock): await update_task(task_id=1, milestone_id=-1) @@ -254,7 +234,7 @@ async def test_update_task_milestone_negative_one_clears(): @pytest.mark.asyncio async def test_update_task_clearing_project_also_clears_milestone(): """project_id=-1 clears the project and, with it, the milestone.""" - fake = _fake_task() + fake = fake_task() mock = AsyncMock(return_value=fake) with patch("scribe.mcp.tools.tasks.notes_svc.update_note", mock): await update_task(task_id=1, project_id=-1) diff --git a/tests/test_mcp_tool_trash.py b/tests/test_mcp_tool_trash.py index 733442a..26fdc4c 100644 --- a/tests/test_mcp_tool_trash.py +++ b/tests/test_mcp_tool_trash.py @@ -2,6 +2,7 @@ from unittest.mock import AsyncMock, patch import pytest +from tests.helpers import FakeMCP pytestmark = pytest.mark.usefixtures("_bind_user") @@ -47,14 +48,7 @@ async def test_purge_trash_when_confirmed(): def test_register_attaches_three_tools(): from scribe.mcp.tools.trash import register - names: list[str] = [] + mcp = FakeMCP() - class FakeMCP: - def tool(self, name=None): - def deco(fn): - names.append(name) - return fn - return deco - - register(FakeMCP()) - assert set(names) == {"list_trash", "restore", "purge_trash"} + register(mcp) + assert set(mcp.names) == {"list_trash", "restore", "purge_trash"} diff --git a/tests/test_notes_description_field.py b/tests/test_notes_description_field.py index d519ce5..3884b8d 100644 --- a/tests/test_notes_description_field.py +++ b/tests/test_notes_description_field.py @@ -5,17 +5,16 @@ from `body` which (post-Task-as-Durable-Record) becomes the LLM-maintained consolidation summary. """ from unittest.mock import AsyncMock, MagicMock, patch +from tests.helpers import make_mock_session def _mock_session_for_update(mock_note): - mock_session = AsyncMock() + mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.first.return_value = mock_note mock_session.execute = AsyncMock(return_value=mock_result) mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) return mock_session @@ -56,12 +55,10 @@ async def test_create_note_forwards_description_to_model(): for k, v in kw.items(): setattr(self, k, v) - mock_session = AsyncMock() + mock_session = make_mock_session() mock_session.add = MagicMock() mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) with patch( "scribe.services.notes.async_session", return_value=mock_session diff --git a/tests/test_recurrence.py b/tests/test_recurrence.py index d11d3f3..bb89bfc 100644 --- a/tests/test_recurrence.py +++ b/tests/test_recurrence.py @@ -12,14 +12,12 @@ async def test_update_note_sets_started_at_on_in_progress(): mock_note.started_at = None mock_note.recurrence_rule = None - mock_session = AsyncMock() + mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.first.return_value = mock_note mock_session.execute = AsyncMock(return_value=mock_result) mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) with patch("scribe.services.notes.async_session", return_value=mock_session): from scribe.services.notes import update_note @@ -36,14 +34,12 @@ async def test_update_note_sets_completed_at_on_done(): mock_note.completed_at = None mock_note.recurrence_rule = None - mock_session = AsyncMock() + mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.first.return_value = mock_note mock_session.execute = AsyncMock(return_value=mock_result) mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) with patch("scribe.services.notes.async_session", return_value=mock_session): from scribe.services.notes import update_note @@ -60,14 +56,12 @@ async def test_update_note_clears_timestamps_on_todo(): mock_note.completed_at = datetime(2026, 3, 15, tzinfo=timezone.utc) mock_note.recurrence_rule = None - mock_session = AsyncMock() + mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.first.return_value = mock_note mock_session.execute = AsyncMock(return_value=mock_result) mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) with patch("scribe.services.notes.async_session", return_value=mock_session): from scribe.services.notes import update_note @@ -85,14 +79,12 @@ async def test_update_note_preserves_started_at_if_already_set(): mock_note.started_at = original_start mock_note.recurrence_rule = None - mock_session = AsyncMock() + mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.first.return_value = mock_note mock_session.execute = AsyncMock(return_value=mock_result) mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) with patch("scribe.services.notes.async_session", return_value=mock_session): from scribe.services.notes import update_note @@ -104,6 +96,7 @@ async def test_update_note_preserves_started_at_if_already_set(): # ── Recurrence rule validation ──────────────────────────────────────────────── import pytest +from tests.helpers import make_mock_session def test_validate_interval_rule_valid(): @@ -233,14 +226,12 @@ async def test_spawn_recurring_tasks_creates_child(): mock_task.due_date = date(2026, 3, 1) mock_task.recurrence_rule = {"type": "interval", "every": 3, "unit": "month"} - mock_session = AsyncMock() + mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.all.return_value = [mock_task] mock_session.execute = AsyncMock(return_value=mock_result) mock_session.get = AsyncMock(return_value=mock_task) mock_session.commit = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) mock_child = MagicMock() mock_child.id = 2 @@ -273,13 +264,11 @@ async def test_list_notes_multi_status_builds_in_clause(): """list_notes with a list of statuses executes without error.""" from unittest.mock import AsyncMock, MagicMock, patch - mock_session = AsyncMock() + mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.all.return_value = [] mock_session.execute = AsyncMock(return_value=mock_result) mock_session.scalar = AsyncMock(return_value=0) - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) with patch("scribe.services.notes.async_session", return_value=mock_session): from scribe.services.notes import list_notes diff --git a/tests/test_retrieval_scopes.py b/tests/test_retrieval_scopes.py index 2c2bfe0..fadbe2d 100644 --- a/tests/test_retrieval_scopes.py +++ b/tests/test_retrieval_scopes.py @@ -15,7 +15,7 @@ shared record would be findable by wording and invisible by meaning. from unittest.mock import AsyncMock, MagicMock, patch import pytest -from tests.helpers import fake_note +from tests.helpers import fake_note, make_mock_session pytestmark = pytest.mark.usefixtures("_no_supersession") @@ -95,9 +95,7 @@ async def test_hybrid_search_halves_agree_on_scope(): patch.object(knowledge, "readable_notes_clause", MagicMock(return_value=(Note.user_id == 7))) as read_clause, \ patch.object(knowledge, "async_session") as sess: - session = AsyncMock() - session.__aenter__ = AsyncMock(return_value=session) - session.__aexit__ = AsyncMock(return_value=False) + session = make_mock_session() result = MagicMock() result.scalars.return_value.all.return_value = [] session.execute = AsyncMock(return_value=result) diff --git a/tests/test_routes_design_systems.py b/tests/test_routes_design_systems.py index 8ad2389..803e247 100644 --- a/tests/test_routes_design_systems.py +++ b/tests/test_routes_design_systems.py @@ -5,6 +5,7 @@ deliberate: relaxing one to a pattern would stop it catching the thing it was written for, which is a capability landing on one surface and not the other. """ import inspect +from tests.helpers import FakeMCP def test_design_systems_blueprint_registered(): @@ -107,17 +108,12 @@ def test_every_mcp_tool_in_the_module_is_registered(): else in the codebase would notice.""" from scribe.mcp.tools import design_systems as tools - registered = [] + mcp = FakeMCP() - class _Recorder: - def tool(self, name): - registered.append(name) - return lambda fn: fn - - tools.register(_Recorder()) + tools.register(mcp) public = { name for name, obj in vars(tools).items() if inspect.iscoroutinefunction(obj) and not name.startswith("_") } - assert set(registered) == public + assert set(mcp.names) == public diff --git a/tests/test_services_db_maintenance.py b/tests/test_services_db_maintenance.py index 8b5860e..33a6358 100644 --- a/tests/test_services_db_maintenance.py +++ b/tests/test_services_db_maintenance.py @@ -9,6 +9,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from scribe.services.db_maintenance import MAINTENANCE_TABLES, run_maintenance +from tests.helpers import make_mock_session def _mock_engine(exec_side_effect=None): @@ -79,9 +80,7 @@ async def test_one_table_failure_does_not_abort_the_rest(): def _health_session(db_bytes, rows): - s = AsyncMock() - s.__aenter__ = AsyncMock(return_value=s) - s.__aexit__ = AsyncMock(return_value=False) + s = make_mock_session() size_res = MagicMock() size_res.scalar.return_value = db_bytes rows_res = MagicMock() diff --git a/tests/test_services_dedup.py b/tests/test_services_dedup.py index 4f0283b..873eaa1 100644 --- a/tests/test_services_dedup.py +++ b/tests/test_services_dedup.py @@ -9,14 +9,12 @@ from scribe.services.dedup import ( find_duplicate_note, find_duplicate_rule, ) -from tests.helpers import fake_note +from tests.helpers import fake_note, make_mock_session def _session_returning(note): """A mocked async_session() whose single execute() yields `note` (or None).""" - s = AsyncMock() - s.__aenter__ = AsyncMock(return_value=s) - s.__aexit__ = AsyncMock(return_value=False) + s = make_mock_session() result = MagicMock() result.scalars.return_value.first.return_value = note s.execute = AsyncMock(return_value=result) @@ -150,9 +148,7 @@ def _session_sequence(results): a location query and then a code query, and the whole point is that they answer differently. """ - s = AsyncMock() - s.__aenter__ = AsyncMock(return_value=s) - s.__aexit__ = AsyncMock(return_value=False) + s = make_mock_session() wrapped = [] for note in results: r = MagicMock() diff --git a/tests/test_services_project_summaries.py b/tests/test_services_project_summaries.py index 07dde57..745c615 100644 --- a/tests/test_services_project_summaries.py +++ b/tests/test_services_project_summaries.py @@ -17,14 +17,13 @@ project would pass a correctness test and reproduce the outage. from unittest.mock import AsyncMock, MagicMock, patch import pytest +from tests.helpers import make_mock_session def _session_factory(counter: list[int], results: list): """A session whose .execute() returns queued results, counting opens.""" def _make(): - s = AsyncMock() - s.__aenter__ = AsyncMock(return_value=s) - s.__aexit__ = AsyncMock(return_value=False) + s = make_mock_session() counter[0] += 1 async def _execute(*_a, **_kw): diff --git a/tests/test_services_rulebooks.py b/tests/test_services_rulebooks.py index 4e4bfc5..c5871f6 100644 --- a/tests/test_services_rulebooks.py +++ b/tests/test_services_rulebooks.py @@ -3,25 +3,9 @@ Mirrors the pattern in tests/test_events_service.py. """ from unittest.mock import AsyncMock, MagicMock, patch -from datetime import datetime, timezone import pytest -from tests.helpers import make_mock_session - - -def _fake_rulebook(id=1, owner_user_id=7, title="FabledSword family", description=""): - rb = MagicMock() - rb.id = id - rb.owner_user_id = owner_user_id - rb.title = title - rb.description = description - rb.created_at = datetime.now(timezone.utc) - rb.updated_at = datetime.now(timezone.utc) - rb.to_dict.return_value = { - "id": id, "owner_user_id": owner_user_id, - "title": title, "description": description or "", - } - return rb +from tests.helpers import fake_rule, fake_rulebook, fake_topic, make_mock_session @pytest.mark.asyncio @@ -39,7 +23,7 @@ async def test_create_rulebook_stores_to_db(): @pytest.mark.asyncio async def test_list_rulebooks_returns_owned_only(): - rb = _fake_rulebook(id=1) + rb = fake_rulebook(id=1) mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.all.return_value = [rb] @@ -67,7 +51,7 @@ async def test_get_rulebook_returns_none_when_not_owner(): @pytest.mark.asyncio async def test_update_rulebook_only_sets_provided_fields(): - rb = _fake_rulebook(id=1, title="old") + rb = fake_rulebook(id=1, title="old") mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalar_one_or_none.return_value = rb @@ -81,7 +65,7 @@ async def test_update_rulebook_only_sets_provided_fields(): @pytest.mark.asyncio async def test_delete_rulebook_calls_delete(): - rb = _fake_rulebook(id=1) + rb = fake_rulebook(id=1) mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalar_one_or_none.return_value = rb @@ -96,22 +80,6 @@ async def test_delete_rulebook_calls_delete(): # ── Topic CRUD ─────────────────────────────────────────────────────────── -def _fake_topic(id=1, rulebook_id=1, title="git-workflow", description="", order_index=0): - t = MagicMock() - t.id = id - t.rulebook_id = rulebook_id - t.title = title - t.description = description - t.order_index = order_index - t.created_at = datetime.now(timezone.utc) - t.updated_at = datetime.now(timezone.utc) - t.to_dict.return_value = { - "id": id, "rulebook_id": rulebook_id, "title": title, - "description": description or "", "order_index": order_index, - } - return t - - @pytest.mark.asyncio async def test_create_topic_requires_owned_rulebook(): """create_topic raises ValueError if the rulebook isn't owned by user.""" @@ -130,8 +98,8 @@ async def test_create_topic_requires_owned_rulebook(): @pytest.mark.asyncio async def test_list_topics_returns_topics_for_owned_rulebook(): - rb = _fake_rulebook(id=1) - topic = _fake_topic(id=10, rulebook_id=1, title="git-workflow") + rb = fake_rulebook(id=1) + topic = fake_topic(id=10, rulebook_id=1, title="git-workflow") # Two execute calls: ownership check, then topic select. mock_session = make_mock_session() @@ -151,26 +119,6 @@ async def test_list_topics_returns_topics_for_owned_rulebook(): # ── Rule CRUD ─────────────────────────────────────────────────────────── -def _fake_rule(id=1, topic_id=10, title="dev is home", - statement="Work directly on dev", why="", how_to_apply=""): - r = MagicMock() - r.id = id - r.topic_id = topic_id - r.title = title - r.statement = statement - r.why = why - r.how_to_apply = how_to_apply - r.order_index = 0 - r.created_at = datetime.now(timezone.utc) - r.updated_at = datetime.now(timezone.utc) - r.to_dict.return_value = { - "id": id, "topic_id": topic_id, "title": title, - "statement": statement, "why": why or "", - "how_to_apply": how_to_apply or "", "order_index": 0, - } - return r - - @pytest.mark.asyncio async def test_create_rule_requires_owned_topic(): mock_session = make_mock_session() @@ -189,7 +137,7 @@ async def test_create_rule_requires_owned_topic(): @pytest.mark.asyncio async def test_list_rules_filters_by_topic_id(): """list_rules(topic_id=X) returns rules in that topic, ownership-scoped.""" - rule = _fake_rule(id=1, topic_id=10) + rule = fake_rule(id=1, topic_id=10) mock_session = make_mock_session() mock_result = MagicMock() mock_result.scalars.return_value.all.return_value = [rule] diff --git a/tests/test_services_supersession.py b/tests/test_services_supersession.py index 9c7d59c..11aca14 100644 --- a/tests/test_services_supersession.py +++ b/tests/test_services_supersession.py @@ -15,13 +15,12 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest from scribe.services import supersession +from tests.helpers import make_mock_session def _session(scalars_sequence=None, get_returns=None): """A mocked async_session whose execute() yields successive scalar lists.""" - s = AsyncMock() - s.__aenter__ = AsyncMock(return_value=s) - s.__aexit__ = AsyncMock(return_value=False) + s = make_mock_session() results = [] for scalars in scalars_sequence or []: @@ -156,9 +155,7 @@ async def test_get_relations_partitions_both_directions_from_one_query(): Note 5 supersedes 2 and 3, and is itself superseded by 9. All four rows come back from a single OR query and are partitioned by which column holds 5. """ - session = AsyncMock() - session.__aenter__ = AsyncMock(return_value=session) - session.__aexit__ = AsyncMock(return_value=False) + session = make_mock_session() result = MagicMock() result.all.return_value = [(5, 3), (5, 2), (9, 5)] # (superseder, superseded) session.execute = AsyncMock(return_value=result) diff --git a/tests/test_shared_provenance.py b/tests/test_shared_provenance.py index 08c7c1b..33334c4 100644 --- a/tests/test_shared_provenance.py +++ b/tests/test_shared_provenance.py @@ -9,14 +9,13 @@ passive surface Scribe has (each entry becomes an auto-surfacing skill file). from unittest.mock import AsyncMock, MagicMock, patch import pytest +from tests.helpers import make_mock_session def _session_returning_rows(rows): result = MagicMock() result.all.return_value = rows - session = AsyncMock() - session.__aenter__ = AsyncMock(return_value=session) - session.__aexit__ = AsyncMock(return_value=False) + session = make_mock_session() session.execute = AsyncMock(return_value=result) return session @@ -62,9 +61,7 @@ async def test_provenance_names_the_owner_and_permission(): from scribe.services.access import describe_provenance note = MagicMock(id=1, user_id=9) owner = MagicMock(username="alex") - session = AsyncMock() - session.__aenter__ = AsyncMock(return_value=session) - session.__aexit__ = AsyncMock(return_value=False) + session = make_mock_session() session.get = AsyncMock(return_value=owner) with patch("scribe.services.access.async_session") as cls, \ patch("scribe.services.access.get_note_permission", diff --git a/tests/test_shared_write_access.py b/tests/test_shared_write_access.py index 50c90bb..c0d689e 100644 --- a/tests/test_shared_write_access.py +++ b/tests/test_shared_write_access.py @@ -10,24 +10,17 @@ A record readable-but-not-writable raises PermissionError rather than reporting "not found" — the caller can plainly open it, so not-found would be a lie that sends them hunting for a missing id. """ -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest +from tests.helpers import fake_snippet def _snippet(id=1, owner=9): - n = MagicMock() - n.id = id - n.user_id = owner - n.title = "formatDuration — humanize a millisecond count" - n.body = "```ts\nexport const f = 1\n```\n" - n.tags = ["ts", "snippet"] - n.note_type = "snippet" - n.deleted_at = None - # Explicitly None — snippet_fields prefers `data` when truthy, and an - # auto-MagicMock attribute is truthy (see note 2109). - n.data = None - return n + return fake_snippet( + id=id, user_id=owner, title="formatDuration — humanize a millisecond count", + body="```ts\nexport const f = 1\n```\n", tags=["ts", "snippet"], + ) @pytest.mark.asyncio diff --git a/tests/test_snippet_merge_provenance.py b/tests/test_snippet_merge_provenance.py index b465f1b..d4ea2de 100644 --- a/tests/test_snippet_merge_provenance.py +++ b/tests/test_snippet_merge_provenance.py @@ -6,26 +6,20 @@ only in the trash — and only for someone who already knew to go looking. So th survivor carries `merged_from`, written to the body and the indexed mirror from one value, and ordinary edits must carry it forward rather than erase it. """ -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest from scribe.services import snippets as s +from tests.helpers import fake_snippet def _snippet(id=1, owner=9, body=None, data=None, tags=None): - n = MagicMock() - n.id = id - n.user_id = owner - n.title = "formatDuration — humanize a millisecond count" - n.body = body if body is not None else s.compose_body(code="x = 1", language="ts") - n.tags = tags if tags is not None else ["ts", "snippet"] - n.note_type = "snippet" - n.deleted_at = None - # Explicitly None — snippet_fields prefers `data` when truthy, and an - # auto-MagicMock attribute is truthy (note 2109). - n.data = data - return n + return fake_snippet( + id=id, user_id=owner, title="formatDuration — humanize a millisecond count", + body=body if body is not None else s.compose_body(code="x = 1", language="ts"), + tags=tags if tags is not None else ["ts", "snippet"], data=data, + ) async def _run_merge(target, sources, source_ids): diff --git a/tests/test_snippet_unmerge.py b/tests/test_snippet_unmerge.py index 1009504..d898c02 100644 --- a/tests/test_snippet_unmerge.py +++ b/tests/test_snippet_unmerge.py @@ -7,15 +7,12 @@ that source actually added. These tests pin that rule, and the refusal that guards the case where the attribution isn't there. """ from types import SimpleNamespace -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, patch import pytest from scribe.services import snippets as s - - -def _loc(path, repo="r", symbol=""): - return {"repo": repo, "path": path, "symbol": symbol} +from tests.helpers import loc def _survivor(locations, merged_from, tags=None, owner=7): @@ -54,7 +51,7 @@ async def _run_unmerge(survivor, *, source_alive=None, restore=1): async def test_unmerge_strips_only_what_the_source_contributed(): """The survivor's own location survives; the source's is removed.""" - mine, theirs = _loc("mine.py"), _loc("theirs.py") + mine, theirs = loc(path="mine.py", repo="r"), loc(path="theirs.py", repo="r") survivor = _survivor( [mine, theirs], [{"id": 2, "locations": [theirs], "tags": ["helper"]}], @@ -70,7 +67,7 @@ async def test_a_location_the_survivor_also_owned_is_never_stripped(): """The central hazard the task named. If a source brought a location the survivor ALREADY had, merge attributes nothing to it — so reversing must leave that call site in place.""" - shared = _loc("shared.py") + shared = loc(path="shared.py", repo="r") survivor = _survivor([shared], [{"id": 2, "locations": [], "tags": ["t"]}]) kwargs = await _run_unmerge(survivor) assert kwargs["data"]["locations"] == [shared] @@ -78,8 +75,8 @@ async def test_a_location_the_survivor_also_owned_is_never_stripped(): async def test_the_reversed_entry_leaves_the_provenance_list(): survivor = _survivor( - [_loc("a.py"), _loc("b.py")], - [{"id": 2, "locations": [_loc("b.py")]}, {"id": 3, "locations": []}], + [loc(path="a.py", repo="r"), loc(path="b.py", repo="r")], + [{"id": 2, "locations": [loc(path="b.py", repo="r")]}, {"id": 3, "locations": []}], ) kwargs = await _run_unmerge(survivor) assert s.merged_from_ids(kwargs["data"]["merged_from"]) == [3] @@ -89,7 +86,7 @@ async def test_the_reversed_entry_leaves_the_provenance_list(): async def test_unmerging_the_last_source_clears_the_provenance_line(): - survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": [], "tags": ["t"]}]) + survivor = _survivor([loc(path="a.py", repo="r")], [{"id": 2, "locations": [], "tags": ["t"]}]) kwargs = await _run_unmerge(survivor) assert "Merged from" not in kwargs["body"] @@ -101,8 +98,8 @@ async def test_an_already_restored_source_is_repaired_not_refused(): """The scenario that motivated the feature: the operator restored the source from the trash by hand, so both records claim its call sites and nothing ever stripped the survivor's copy. Un-merge must fix that, not reject it.""" - theirs = _loc("theirs.py") - survivor = _survivor([_loc("mine.py"), theirs], + theirs = loc(path="theirs.py", repo="r") + survivor = _survivor([loc(path="mine.py", repo="r"), theirs], [{"id": 2, "locations": [theirs]}]) alive = SimpleNamespace(id=2, user_id=7, note_type="snippet", deleted_at=None, title="g — x", tags=["snippet"], body="", data=None) @@ -110,13 +107,13 @@ async def test_an_already_restored_source_is_repaired_not_refused(): kwargs = await _run_unmerge(survivor, source_alive=alive) # Nothing to revive — it's already alive — but the subtraction still happens. revive.assert_not_called() - assert kwargs["data"]["locations"] == [_loc("mine.py")] + assert kwargs["data"]["locations"] == [loc(path="mine.py", repo="r")] async def test_a_purged_source_is_refused_and_the_survivor_is_untouched(): """If the source can't come back, stripping the survivor would lose the locations entirely — no record would claim them.""" - survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": [_loc("a.py")]}]) + survivor = _survivor([loc(path="a.py", repo="r")], [{"id": 2, "locations": [loc(path="a.py", repo="r")]}]) async def fake_get(_uid, sid): return survivor if sid == 1 else None @@ -139,7 +136,7 @@ async def test_an_entry_without_attribution_is_refused_not_guessed(): """Bare-id provenance comes from parsing the body, which can only hold ids. Subtracting a guess could strip call sites the survivor owns — so refuse and say what to do instead.""" - survivor = _survivor([_loc("a.py")], [2]) + survivor = _survivor([loc(path="a.py", repo="r")], [2]) async def fake_get(_uid, sid): return survivor if sid == 1 else None @@ -155,7 +152,7 @@ async def test_an_entry_without_attribution_is_refused_not_guessed(): async def test_unmerging_something_never_absorbed_is_refused(): - survivor = _survivor([_loc("a.py")], [{"id": 99, "locations": []}]) + survivor = _survivor([loc(path="a.py", repo="r")], [{"id": 99, "locations": []}]) with ( patch.object(s, "get_snippet", AsyncMock(return_value=survivor)), patch("scribe.services.access.can_write_note", AsyncMock(return_value=True)), @@ -166,7 +163,7 @@ async def test_unmerging_something_never_absorbed_is_refused(): async def test_unmerge_requires_write_access(): """Same rule as merge: a read-only share can see the record, not rearrange it.""" - survivor = _survivor([_loc("a.py")], [{"id": 2, "locations": []}]) + survivor = _survivor([loc(path="a.py", repo="r")], [{"id": 2, "locations": []}]) with ( patch.object(s, "get_snippet", AsyncMock(return_value=survivor)), patch("scribe.services.access.can_write_note", AsyncMock(return_value=False)), diff --git a/tests/test_trash_filtering.py b/tests/test_trash_filtering.py index 34861d2..fb5a3b3 100644 --- a/tests/test_trash_filtering.py +++ b/tests/test_trash_filtering.py @@ -7,12 +7,11 @@ compiled SQL of every statement passed to execute, then assert the from unittest.mock import AsyncMock, MagicMock, patch import pytest +from tests.helpers import make_mock_session def _capturing_session(captured: list[str]): - s = AsyncMock() - s.__aenter__ = AsyncMock(return_value=s) - s.__aexit__ = AsyncMock(return_value=False) + s = make_mock_session() s.commit = AsyncMock() s.scalar = AsyncMock(return_value=0) diff --git a/tests/test_version_pinning_pin_unpin.py b/tests/test_version_pinning_pin_unpin.py index 8fab843..f7d7efe 100644 --- a/tests/test_version_pinning_pin_unpin.py +++ b/tests/test_version_pinning_pin_unpin.py @@ -1,16 +1,15 @@ """Tests for manual pin / unpin on note versions.""" from unittest.mock import AsyncMock, MagicMock, patch +from tests.helpers import make_mock_session def _mock_session_for_version(mock_version): - mock_session = AsyncMock() + mock_session = make_mock_session() result = MagicMock() result.scalars.return_value.first.return_value = mock_version mock_session.execute = AsyncMock(return_value=result) mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) return mock_session @@ -92,14 +91,12 @@ async def test_pin_version_rejects_overlong_label(): async def test_pin_version_returns_none_when_not_found(): - mock_session = AsyncMock() + mock_session = make_mock_session() result = MagicMock() result.scalars.return_value.first.return_value = None mock_session.execute = AsyncMock(return_value=result) mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) with patch( "scribe.services.version_pinning.async_session", diff --git a/tests/test_version_pinning_prune.py b/tests/test_version_pinning_prune.py index a7fbe86..8787bdb 100644 --- a/tests/test_version_pinning_prune.py +++ b/tests/test_version_pinning_prune.py @@ -10,7 +10,7 @@ async def test_create_version_prune_sql_filters_to_unpinned(): """The DELETE statement issued by create_version's prune step must include `pin_kind IS NULL` in the inner SELECT so pinned versions aren't counted toward MAX_VERSIONS and can't be pruned by it.""" - mock_session = AsyncMock() + mock_session = make_mock_session() select_result = MagicMock() # No prior version → skips the throttle/dedupe early-return paths and # proceeds straight to insert + prune. @@ -18,8 +18,6 @@ async def test_create_version_prune_sql_filters_to_unpinned(): mock_session.add = MagicMock() mock_session.commit = AsyncMock() mock_session.refresh = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) captured_sql: list[str] = [] @@ -48,10 +46,8 @@ async def test_create_version_prune_sql_filters_to_unpinned(): async def test_prune_auto_pins_filters_to_auto_kind(): """prune_auto_pins must filter to pin_kind='auto' so manual pins and rolling rows aren't touched, and must bind MAX_AUTO_PINS as the OFFSET.""" - mock_session = AsyncMock() + mock_session = make_mock_session() mock_session.commit = AsyncMock() - mock_session.__aenter__ = AsyncMock(return_value=mock_session) - mock_session.__aexit__ = AsyncMock(return_value=False) captured_sql: list[str] = [] captured_params: list[dict] = []