Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_contextlib_async produces several RuntimeWarnings #110378

Closed
sobolevn opened this issue Oct 5, 2023 · 3 comments
Closed

test_contextlib_async produces several RuntimeWarnings #110378

sobolevn opened this issue Oct 5, 2023 · 3 comments
Assignees
Labels
tests Tests in the Lib/test dir topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Oct 5, 2023

Bug report

» ./python.exe -m test test_contextlib_async
Using random seed 908291980
0:00:00 load avg: 4.08 Run 1 test sequentially
0:00:00 load avg: 4.08 [1/1] test_contextlib_async
/Users/sobolev/Desktop/cpython/Lib/contextlib.py:701: RuntimeWarning: coroutine method 'aclose' of 'AsyncContextManagerTestCase.test_contextmanager_trap_second_yield.<locals>.whoo' was never awaited
  async def __aenter__(self):
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
/Users/sobolev/Desktop/cpython/Lib/contextlib.py:701: RuntimeWarning: coroutine method 'aclose' of 'AsyncContextManagerTestCase.test_contextmanager_trap_yield_after_throw.<locals>.whoo' was never awaited
  async def __aenter__(self):
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
/Users/sobolev/Desktop/cpython/Lib/contextlib.py:701: RuntimeWarning: coroutine method 'aclose' of 'TestAbstractAsyncContextManager.test_async_gen_propagates_generator_exit.<locals>.gen' was never awaited
  async def __aenter__(self):
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

== Tests result: SUCCESS ==

1 test OK.

Total duration: 121 ms
Total tests: run=58
Total test files: run=1/1
Result: SUCCESS

This can be fixed by moving away from legacy @_async_test and by using stabel IsolatedAsyncioTestCase.

def _async_test(func):
"""Decorator to turn an async function into a test case."""
@functools.wraps(func)
def wrapper(*args, **kwargs):
coro = func(*args, **kwargs)
asyncio.run(coro)
return wrapper

Linked PRs

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir topic-asyncio labels Oct 5, 2023
@sobolevn sobolevn self-assigned this Oct 5, 2023
sobolevn added a commit to sobolevn/cpython that referenced this issue Oct 5, 2023
sobolevn added a commit to sobolevn/cpython that referenced this issue Oct 5, 2023
@serhiy-storchaka serhiy-storchaka self-assigned this Oct 7, 2023
@serhiy-storchaka
Copy link
Member

There are two issues here.

  1. test_async_gen_propagates_generator_exit (added in bpo-33786/@asynccontextmanager doesn't work well with async generators #77967, PR bpo-33786: Fix asynchronous generators to handle GeneratorExit in athrow() #7467) creates an asynchronous generator, but do not iterate it to the end (it would close it implicitly) and do not close it explicitly. Actually, this test does not work at all. It is passed with unpatched code, it only prints a traceback from implicitly called destructor. It also contains code not related to the issue. This test should be rewritten.
  2. test_contextmanager_trap_yield_after_throw and test_contextmanager_trap_second_yield test asynccontextmanager with an asynchronous generator with second yield. asynccontextmanager raises RuntimeError, but leaves the underlying asynchronous generator object not closed. Such generator is a programming error, and we don't have to clean up after this, but the "gen" attribute of asynccontextmanager object is an implementation detail, so it is better to not leave it to user and do clean up. The __exit__ method should close the underlying asynchronous generator object.

Moving to IsolatedAsyncioTestCase looks like a good thing, but only after fixing the deeper issues.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Oct 7, 2023
…contextmanager

contextmanager and asynccontextmanager context managers now close an invalid
underlying generator object that yields more then one value.
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Oct 7, 2023
…_contextlib_async

It now fails if the original bug is not fixed, and no longer produce
ResourceWarning with fixed code.
ambv pushed a commit that referenced this issue Oct 10, 2023
…tmanager (GH-110499)

contextmanager and asynccontextmanager context managers now close an invalid
underlying generator object that yields more then one value.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 10, 2023
…contextmanager (pythonGH-110499)

contextmanager and asynccontextmanager context managers now close an invalid
underlying generator object that yields more then one value.
(cherry picked from commit 96fed66)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 10, 2023
…contextmanager (pythonGH-110499)

contextmanager and asynccontextmanager context managers now close an invalid
underlying generator object that yields more then one value.
(cherry picked from commit 96fed66)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
ambv pushed a commit that referenced this issue Oct 10, 2023
…ccontextmanager (GH-110499) (#110588)

contextmanager and asynccontextmanager context managers now close an invalid
underlying generator object that yields more then one value.
(cherry picked from commit 96fed66)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
ambv added a commit that referenced this issue Oct 10, 2023
…ccontextmanager (GH-110499) (#110589)

contextmanager and asynccontextmanager context managers now close an invalid
underlying generator object that yields more then one value.
(cherry picked from commit 96fed66)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
ambv pushed a commit that referenced this issue Oct 10, 2023
…xtlib_async (#110500)

It now fails if the original bug is not fixed, and no longer produce ResourceWarning with fixed code.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 10, 2023
…_contextlib_async (pythonGH-110500)

It now fails if the original bug is not fixed, and no longer produce ResourceWarning with fixed code.
(cherry picked from commit 5aa62a8)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 10, 2023
…_contextlib_async (pythonGH-110500)

It now fails if the original bug is not fixed, and no longer produce ResourceWarning with fixed code.
(cherry picked from commit 5aa62a8)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
ambv pushed a commit that referenced this issue Oct 10, 2023
…t_contextlib_async (GH-110500) (#110610)

It now fails if the original bug is not fixed, and no longer produce ResourceWarning with fixed code.
(cherry picked from commit 5aa62a8)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
ambv pushed a commit that referenced this issue Oct 10, 2023
…t_contextlib_async (GH-110500) (#110611)

It now fails if the original bug is not fixed, and no longer produce ResourceWarning with fixed code.
(cherry picked from commit 5aa62a8)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@ambv
Copy link
Contributor

ambv commented Oct 10, 2023

This is solved, thanks! ✨ 🍰 ✨

@ambv ambv closed this as completed Oct 10, 2023
@serhiy-storchaka
Copy link
Member

Thank you for your reviews @ambv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

3 participants