fix(tests): a Result's .all() is sync, and the module has a fourth tool (#4104)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 42s
CI & Build / TypeScript typecheck (push) Successful in 56s
CI & Build / Build & push image (push) Canceled after 0s
CI & Build / Python tests (push) Canceled after 1m37s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 42s
CI & Build / TypeScript typecheck (push) Successful in 56s
CI & Build / Build & push image (push) Canceled after 0s
CI & Build / Python tests (push) Canceled after 1m37s
Two failures in the tests added with the step, both mine. `make_mock_session` is an AsyncMock and every child of an AsyncMock is one too, so leaving `.all` as it came handed `migrate_floor` a coroutine where it reads a list — the same trap the helper's own docstring already flags for `add`. And the registration test enumerated three tools by name, which is exactly what it is for: `migrate_retrieval_floor` made it four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -32,8 +32,14 @@ def _logs(pairs):
|
||||
|
||||
|
||||
def _session_with(rows):
|
||||
"""`.all()` is SYNCHRONOUS on a Result, so it needs a MagicMock.
|
||||
|
||||
`make_mock_session` is an AsyncMock, and every child of an AsyncMock is one
|
||||
too — leaving `.all` as it comes hands the service a coroutine where it
|
||||
expects a list, the same trap the helper's docstring flags for `add`.
|
||||
"""
|
||||
session = make_mock_session()
|
||||
session.execute.return_value.all.return_value = rows
|
||||
session.execute.return_value = MagicMock(all=MagicMock(return_value=rows))
|
||||
return session
|
||||
|
||||
|
||||
|
||||
@@ -183,13 +183,16 @@ def test_the_tool_teaches_reading_the_records_not_the_percentile():
|
||||
assert "69" in doc
|
||||
|
||||
|
||||
def test_all_three_tools_are_registered():
|
||||
def test_every_tool_in_the_module_is_registered():
|
||||
from scribe.mcp.tools import retrieval_tuning as tool
|
||||
|
||||
from tests.helpers import FakeMCP
|
||||
|
||||
mcp = FakeMCP()
|
||||
tool.register(mcp)
|
||||
# Order is the module's, and asserted rather than sorted: an unregistered
|
||||
# tool is invisible to every caller, so the list is worth reading literally.
|
||||
assert mcp.names == [
|
||||
"retrieval_surfaces", "tune_retrieval", "retrieval_tuning_history",
|
||||
"retrieval_surfaces", "migrate_retrieval_floor",
|
||||
"tune_retrieval", "retrieval_tuning_history",
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user