From f5e51b1d4f801f3b70fc6f1dde6863bbf1976ba6 Mon Sep 17 00:00:00 2001 From: Parthiban Sivakumar <228711334+parthiban-sivakumar@users.noreply.github.com> Date: Tue, 8 Sep 2026 21:35:10 +0530 Subject: [PATCH] fix(tests): await future completion before asserting done() (#5299) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(tests): await future completion before asserting done() `test_run_on_isolated_subagent_loop_survives_caller_loop_teardown` signals from inside the coroutine: async def deferred_work() -> None: completed.set() `run_on_isolated_subagent_loop` is `asyncio.run_coroutine_threadsafe`, whose `concurrent.futures.Future` is marked done by the loop only after the coroutine returns. The main thread can therefore wake from `completed.wait()` while the future is still pending, and `assert handles[0].done()` fails: assert False + where False = done() + where done = .done Observed on main at a2808e82 (shard 2) and on an unrelated PR at 852a94dd (shard 4) sixteen seconds apart, so it tracks runner load rather than any change under test. Assert the result first — `Future.result(timeout=10)` blocks until the future completes — then assert `done()`. Both assertions keep their original meaning and no sleep is introduced. Fixes #5298 Co-Authored-By: Claude Opus 5 * test: drop the now-redundant done() assertion Review follow-up. Once `result(timeout=10)` has returned normally the future is guaranteed to be FINISHED, so the `done()` assertion below it could no longer fail — it documented intent rather than checking anything. `result()` alone proves both halves of what the test is about: that the coroutine body ran after caller-loop teardown, and that the future resolved. The `completed.wait()` guard above still covers the "work never ran" case with a descriptive message. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- backend/tests/test_subagent_executor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/tests/test_subagent_executor.py b/backend/tests/test_subagent_executor.py index a6d915693..5cd10c46b 100644 --- a/backend/tests/test_subagent_executor.py +++ b/backend/tests/test_subagent_executor.py @@ -2380,7 +2380,9 @@ class TestThreadSafety: asyncio.run(schedule_from_caller()) assert completed.wait(timeout=10), "work pinned to the persistent subagent loop must run after caller-loop teardown" - assert handles[0].done() + # `completed` is set from inside the coroutine, so it can fire before + # the loop marks the future done. Blocking on the result covers both: + # it returns only once the coroutine ran and the future resolved. assert handles[0].result(timeout=10) is None def test_multiple_executors_in_parallel(self, classes, base_config, msg):