fix(tests): await future completion before asserting done() (#5299)

* 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 = <Future at 0x7f62241b22d0 state=pending>.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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Parthiban Sivakumar 2026-09-08 21:35:10 +05:30 committed by GitHub
parent dfaeef3772
commit f5e51b1d4f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -2380,7 +2380,9 @@ class TestThreadSafety:
asyncio.run(schedule_from_caller()) asyncio.run(schedule_from_caller())
assert completed.wait(timeout=10), "work pinned to the persistent subagent loop must run after caller-loop teardown" 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 assert handles[0].result(timeout=10) is None
def test_multiple_executors_in_parallel(self, classes, base_config, msg): def test_multiple_executors_in_parallel(self, classes, base_config, msg):