Skip to content

Commit 5fc7673

Browse files
committed
fixes tests
1 parent f03a765 commit 5fc7673

File tree

2 files changed

+35
-14
lines changed

2 files changed

+35
-14
lines changed

packages/common-library/src/common_library/async_tools.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import asyncio
22
import functools
3+
import logging
34
from collections.abc import Awaitable, Callable
45
from concurrent.futures import Executor
56
from inspect import isawaitable
67
from typing import ParamSpec, TypeVar, overload
78

9+
_logger = logging.getLogger(__name__)
10+
811
R = TypeVar("R")
912
P = ParamSpec("P")
1013

@@ -67,17 +70,19 @@ async def maybe_await(
6770
async def cancel_and_wait(task: asyncio.Task) -> None:
6871
"""Cancels the given task and waits for it to finish.
6972
70-
Accounts for the case where the parent function is being cancelled
71-
and the task is cancelled as a result. In that case, it suppresses the
72-
`asyncio.CancelledError` if the task was cancelled, but propagates it
73-
if the task was not cancelled (i.e., it was still running when the parent
74-
function was cancelled).
73+
Accounts for the case where the tasks's owner function is being cancelled
7574
"""
7675
task.cancel()
7776
try:
77+
# NOTE shield ensures that cancellation of the caller function won’t stop you
78+
# from observing the cancellation/finalization of task.
7879
await asyncio.shield(task)
7980
except asyncio.CancelledError:
8081
if not task.cancelled():
81-
# parent function is being cancelled -> propagate cancellation
82+
# task owner function is being cancelled -> propagate cancellation
8283
raise
83-
# else: task was cancelled, suppress
84+
# else: task cancellation is complete, we can safely ignore it
85+
_logger.debug(
86+
"Task %s cancellation is complete",
87+
task.get_name(),
88+
)

packages/common-library/tests/test_async_tools.py

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ def sync_function(x: int, y: int) -> int:
1313

1414
@make_async()
1515
def sync_function_with_exception() -> None:
16-
raise ValueError("This is an error!")
16+
msg = "This is an error!"
17+
raise ValueError(msg)
1718

1819

1920
@pytest.mark.asyncio
@@ -126,17 +127,23 @@ async def test_cancel_and_wait_propagates_external_cancel():
126127
the CancelledError is not swallowed.
127128
"""
128129

129-
async def inner_coro():
130+
async def coro():
130131
try:
131-
await asyncio.sleep(10)
132+
await asyncio.sleep(4)
132133
except asyncio.CancelledError:
133-
await asyncio.sleep(0.1) # simulate cleanup
134+
await asyncio.sleep(1) # simulate cleanup
134135
raise
135136

136-
task = asyncio.create_task(inner_coro())
137+
inner_task = asyncio.create_task(coro())
137138

138139
async def outer_coro():
139-
await cancel_and_wait(task)
140+
try:
141+
await cancel_and_wait(inner_task)
142+
except asyncio.CancelledError:
143+
assert (
144+
not inner_task.cancelled()
145+
), "Internal Task should not be cancelled yet (shielded)"
146+
raise
140147

141148
# Cancel the wrapper after a short delay
142149
outer_task = asyncio.create_task(outer_coro())
@@ -146,4 +153,13 @@ async def outer_coro():
146153
with pytest.raises(asyncio.CancelledError):
147154
await outer_task
148155

149-
assert task.cancelled()
156+
# Ensure the task was cancelled
157+
assert inner_task.cancelled() is False, "Task should not be cancelled initially"
158+
159+
done_event = asyncio.Event()
160+
161+
def on_done(_):
162+
done_event.set()
163+
164+
inner_task.add_done_callback(on_done)
165+
await done_event.wait()

0 commit comments

Comments
 (0)