Skip to content

self.doCleanups() cannot be called from a test method of a unittest.IsolatedAsyncioTestCase subclass #101018

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

Open
zware opened this issue Jan 13, 2023 · 6 comments
Labels
3.11 only security fixes 3.12 only security fixes stdlib Python modules in the Lib dir topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@zware
Copy link
Member

zware commented Jan 13, 2023

Example:

import unittest


events = []

class Test(unittest.IsolatedAsyncioTestCase):

    def test(self):
        events.append('started')
        self.addCleanup(events.append, 'cleanup')
        self.doCleanups()
        events.append('end')

unittest.main(exit=False)

assert events == ['started', 'cleanup', 'end'], events

The above passes in 3.9 and 3.10, but fails in 3.11 with

E
======================================================================
ERROR: test (__main__.Test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/.../cpython/Lib/unittest/async_case.py", line 75, in _callCleanup
    self._callMaybeAsync(function, *args, **kwargs)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.../cpython/Lib/unittest/async_case.py", line 95, in _callMaybeAsync
    return self._asyncioTestContext.run(func, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: cannot enter context: <_contextvars.Context object at 0x7fb2f93a9900> is already entered

----------------------------------------------------------------------
Ran 1 test in 0.002s

FAILED (errors=1)
Traceback (most recent call last):
  File "<stdin>", line 16, in <module>
AssertionError: ['started', 'end']

git bisect found the culprit to be gh-91150/GH-31837.

@zware zware added type-bug An unexpected behavior, bug, or error topic-asyncio stdlib Python modules in the Lib dir 3.11 only security fixes 3.12 only security fixes labels Jan 13, 2023
@github-project-automation github-project-automation bot moved this to Todo in asyncio Jan 13, 2023
@gvanrossum
Copy link
Member

gvanrossum commented Jan 13, 2023

I don't understand that test class very well. Do you have an analysis of the root cause (beyond pointing to the PR that introduced the regression)? Or a proposed fix?

@zware zware changed the title self.doCleanups() cannot be called from a synchronous test method of a unittest.IsolatedAsyncioTestCase subclass self.doCleanups() cannot be called from a test method of a unittest.IsolatedAsyncioTestCase subclass Jan 17, 2023
@zware
Copy link
Member Author

zware commented Jan 17, 2023

The root cause is that self._callMaybeAsync is called from within a self._callMaybeAsync call, which tries to run self._asyncioTestContext.run recursively. At a deeper level, though, the problem is that we're trying to switch back and forth between sync and async code without knowing which direction we might actually be going, which I think means that doCleanups is just broken in IsolatedAsyncioTestCase. Some testing1 shows that an async cleanup can't be called via doCleanups in an async test case even in previous versions. Changing the title accordingly.

I don't have a great suggestion for a fix. I'm not sure if we can actually support calling doCleanups both during and after the test method. I think we could support calling it after and awaiting it during, but that's going to be ugly and doesn't sound easy to explain, either.

Footnotes

  1. Using the same script as above, but with async def test and events.append wrapped in an async def, we get a RuntimeError: This event loop is already running in 3.9 and RuntimeError: Runner.run() cannot be called from a running event loop in 3.11

@kumaraditya303
Copy link
Contributor

I don't think that doCleanups should be callable from a test method, if this is a supported use-case then our test suite would have caught this regression. From asyncio side this will be hard to support, someone would have to do more research.

@zware
Copy link
Member Author

zware commented Jan 17, 2023

The documentation for doCleanups says "If you need cleanup functions to be called prior to tearDown() then you can call doCleanups() yourself. doCleanups() pops methods off the stack of cleanup functions one at a time, so it can be called at any time."

Our test suite does not have complete coverage, but doCleanups is used in a few of our tests, including in tearDown, which is also broken for IsolatedAsyncioTestCase.

@kumaraditya303
Copy link
Contributor

Okay thanks but I am not myself a user of unittest and the comment https://github.com/python/cpython/blame/c5660ae96f2ab5732c68c301ce9a63009f432d93/Lib/unittest/async_case.py#L14 is mostly out of date so I am not sure of the semantics here.

@serhiy-storchaka
Copy link
Member

I concur with @kumaraditya303 that doCleanups was not purposed to be called from a test method. Calling it from tearDown may have sense, and we should investigate whether it is feasible.

I worked on a patch which makes calling tearDown a part of doCleanups. It is definitely a breaking change, although it does not break Python tests (except tests for low level details of unittest) and makes things more consistent (tearDown and callbacks registered with addCallback will be always called in LIFO order). I am wondering whether this will break your tests even more or make them simpler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 only security fixes stdlib Python modules in the Lib dir topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
Status: Todo
Status: Todo
Development

No branches or pull requests

4 participants