From def144df06ecce4f864864d9f0b604f15ac591d6 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 17 Sep 2026 12:47:57 -0400 Subject: [PATCH] fix(tests): a Result's .all() is sync, and the module has a fourth tool (#4104) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- tests/test_retrieval_migration.py | 8 +++++++- tests/test_retrieval_tuning.py | 7 +++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/test_retrieval_migration.py b/tests/test_retrieval_migration.py index 0856bea..e46820e 100644 --- a/tests/test_retrieval_migration.py +++ b/tests/test_retrieval_migration.py @@ -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 diff --git a/tests/test_retrieval_tuning.py b/tests/test_retrieval_tuning.py index 0c0ac5c..d55169f 100644 --- a/tests/test_retrieval_tuning.py +++ b/tests/test_retrieval_tuning.py @@ -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", ]