From 2fe3c5bd50fd3e75676f358114d81664749c0161 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Tue, 7 Sep 2021 21:10:57 +0100 Subject: [PATCH 1/4] Ensure tests are run correctly by verifying call order. --- tests/test_timeout.py | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/tests/test_timeout.py b/tests/test_timeout.py index f006ff7..868d2a9 100644 --- a/tests/test_timeout.py +++ b/tests/test_timeout.py @@ -1,12 +1,23 @@ import asyncio import sys import time +from typing import Callable, List +from functools import wraps import pytest from async_timeout import Timeout, timeout, timeout_at +def log_func(func: Callable, msg: str, call_order: List[str]): + """Simple wrapper to add a log to call_order when the function is called.""" + @wraps(func) + def wrapper(*args, **kwargs): + call_order.append(msg) + return func(*args, **kwargs) + return wrapper + + @pytest.mark.asyncio async def test_timeout() -> None: canceled_raised = False @@ -364,19 +375,25 @@ async def test_task(deadline: float, loop: asyncio.AbstractEventLoop) -> None: with Timeout(deadline, loop): await asyncio.sleep(10) + call_order = [] + Timeout._do_exit = log_func(Timeout._do_exit, "exit", call_order) + Timeout._on_timeout = log_func(Timeout._on_timeout, "timeout", call_order) + loop = asyncio.get_running_loop() deadline = loop.time() + 1 t = asyncio.create_task(test_task(deadline, loop)) - loop.call_at(deadline, t.cancel) + loop.call_at(deadline, log_func(t.cancel, "cancel", call_order)) # If we get a TimeoutError, then the code is broken. with pytest.raises(asyncio.CancelledError): await t + # This test is very timing dependant, so we check the order that calls + # happened to be sure the test itself ran correctly. + assert call_order == ["cancel", "timeout", "exit"] -@pytest.mark.xfail( - reason="The test is CPU performance sensitive, might fail on slow CI box" -) -@pytest.mark.skipif(sys.version_info < (3, 7), reason="Not supported in 3.6") + +@pytest.mark.xfail(reason="Can't see a way to fix this currently.") +@pytest.mark.skipif(sys.version_info < (3, 9), reason="Can't be fixed in <3.9.") @pytest.mark.asyncio async def test_race_condition_cancel_after() -> None: """Test race condition when cancelling after timeout. @@ -393,10 +410,18 @@ async def test_task(deadline: float, loop: asyncio.AbstractEventLoop) -> None: with Timeout(deadline, loop): await asyncio.sleep(10) + call_order = [] + Timeout._do_exit = log_func(Timeout._do_exit, "exit", call_order) + Timeout._on_timeout = log_func(Timeout._on_timeout, "timeout", call_order) + loop = asyncio.get_running_loop() deadline = loop.time() + 1 t = asyncio.create_task(test_task(deadline, loop)) - loop.call_at(deadline + 0.0000000000001, t.cancel) + loop.call_at(deadline + .000001, log_func(t.cancel, "cancel", call_order)) # If we get a TimeoutError, then the code is broken. with pytest.raises(asyncio.CancelledError): await t + + # This test is very timing dependant, so we check the order that calls + # happened to be sure the test itself ran correctly. + assert call_order == ["timeout", "cancel", "exit"] From 558cbf694bd846e2994050b496cae09df2a7cf6e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 7 Sep 2021 20:11:34 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_timeout.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_timeout.py b/tests/test_timeout.py index 868d2a9..4b15819 100644 --- a/tests/test_timeout.py +++ b/tests/test_timeout.py @@ -1,8 +1,8 @@ import asyncio import sys import time -from typing import Callable, List from functools import wraps +from typing import Callable, List import pytest @@ -11,10 +11,12 @@ def log_func(func: Callable, msg: str, call_order: List[str]): """Simple wrapper to add a log to call_order when the function is called.""" + @wraps(func) def wrapper(*args, **kwargs): call_order.append(msg) return func(*args, **kwargs) + return wrapper @@ -417,7 +419,7 @@ async def test_task(deadline: float, loop: asyncio.AbstractEventLoop) -> None: loop = asyncio.get_running_loop() deadline = loop.time() + 1 t = asyncio.create_task(test_task(deadline, loop)) - loop.call_at(deadline + .000001, log_func(t.cancel, "cancel", call_order)) + loop.call_at(deadline + 0.000001, log_func(t.cancel, "cancel", call_order)) # If we get a TimeoutError, then the code is broken. with pytest.raises(asyncio.CancelledError): await t From b10ee3a47402893c597fae221ea9f06928dc18c7 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Tue, 7 Sep 2021 21:24:27 +0100 Subject: [PATCH 3/4] Cleanup --- tests/test_timeout.py | 65 +++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/tests/test_timeout.py b/tests/test_timeout.py index 4b15819..93ec5c4 100644 --- a/tests/test_timeout.py +++ b/tests/test_timeout.py @@ -2,22 +2,24 @@ import sys import time from functools import wraps -from typing import Callable, List +from typing import Any, Callable, List, TypeVar import pytest from async_timeout import Timeout, timeout, timeout_at +_Func = TypeVar("_Func", bound=Callable[..., Any]) -def log_func(func: Callable, msg: str, call_order: List[str]): + +def log_func(func: _Func, msg: str, call_order: List[str]) -> _Func: """Simple wrapper to add a log to call_order when the function is called.""" @wraps(func) - def wrapper(*args, **kwargs): + def wrapper(*args: Any, **kwargs: Any) -> Any: # type: ignore[misc] call_order.append(msg) return func(*args, **kwargs) - return wrapper + return wrapper # type: ignore[return-value] @pytest.mark.asyncio @@ -360,15 +362,8 @@ async def test_deprecated_with() -> None: await asyncio.sleep(0) -@pytest.mark.skipif(sys.version_info < (3, 7), reason="Not supported in 3.6") -@pytest.mark.asyncio -async def test_race_condition_cancel_before() -> None: - """Test race condition when cancelling before timeout. - - If cancel happens immediately before the timeout, then - the timeout may overrule the cancellation, making it - impossible to cancel some tasks. - """ +async def race_condition(offset: float = 0) -> List[str]: + """Common code for below race condition tests.""" async def test_task(deadline: float, loop: asyncio.AbstractEventLoop) -> None: # We need the internal Timeout class to specify the deadline (not delay). @@ -377,18 +372,34 @@ async def test_task(deadline: float, loop: asyncio.AbstractEventLoop) -> None: with Timeout(deadline, loop): await asyncio.sleep(10) - call_order = [] - Timeout._do_exit = log_func(Timeout._do_exit, "exit", call_order) - Timeout._on_timeout = log_func(Timeout._on_timeout, "timeout", call_order) + call_order: List[str] = [] + f_exit = log_func(Timeout._do_exit, "exit", call_order) + Timeout._do_exit = f_exit # type: ignore[assignment] + f_timeout = log_func(Timeout._on_timeout, "timeout", call_order) + Timeout._on_timeout = f_timeout # type: ignore[assignment] loop = asyncio.get_running_loop() deadline = loop.time() + 1 t = asyncio.create_task(test_task(deadline, loop)) - loop.call_at(deadline, log_func(t.cancel, "cancel", call_order)) + loop.call_at(deadline + offset, log_func(t.cancel, "cancel", call_order)) # If we get a TimeoutError, then the code is broken. with pytest.raises(asyncio.CancelledError): await t + return call_order + + +@pytest.mark.skipif(sys.version_info < (3, 7), reason="Not supported in 3.6") +@pytest.mark.asyncio +async def test_race_condition_cancel_before() -> None: + """Test race condition when cancelling before timeout. + + If cancel happens immediately before the timeout, then + the timeout may overrule the cancellation, making it + impossible to cancel some tasks. + """ + call_order = await race_condition() + # This test is very timing dependant, so we check the order that calls # happened to be sure the test itself ran correctly. assert call_order == ["cancel", "timeout", "exit"] @@ -404,25 +415,7 @@ async def test_race_condition_cancel_after() -> None: immediately after the timeout (but before the __exit__), then the explicit cancel can get overruled again. """ - - async def test_task(deadline: float, loop: asyncio.AbstractEventLoop) -> None: - # We need the internal Timeout class to specify the deadline (not delay). - # This is needed to create the precise timing to reproduce the race condition. - with pytest.warns(DeprecationWarning): - with Timeout(deadline, loop): - await asyncio.sleep(10) - - call_order = [] - Timeout._do_exit = log_func(Timeout._do_exit, "exit", call_order) - Timeout._on_timeout = log_func(Timeout._on_timeout, "timeout", call_order) - - loop = asyncio.get_running_loop() - deadline = loop.time() + 1 - t = asyncio.create_task(test_task(deadline, loop)) - loop.call_at(deadline + 0.000001, log_func(t.cancel, "cancel", call_order)) - # If we get a TimeoutError, then the code is broken. - with pytest.raises(asyncio.CancelledError): - await t + call_order = await race_condition(0.000001) # This test is very timing dependant, so we check the order that calls # happened to be sure the test itself ran correctly. From 38a2ae560a07c6b88bddff0f353c4fb063a31e5b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 7 Sep 2021 20:24:38 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_timeout.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_timeout.py b/tests/test_timeout.py index 93ec5c4..1917a7a 100644 --- a/tests/test_timeout.py +++ b/tests/test_timeout.py @@ -8,6 +8,7 @@ from async_timeout import Timeout, timeout, timeout_at + _Func = TypeVar("_Func", bound=Callable[..., Any])