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

bpo-32972: Add unittest.AsyncioTestCase #10296

Closed

Conversation

Projects
None yet
6 participants
@dave-shawley
Copy link

commented Nov 2, 2018

This PR adds co-routine support to unittest by adding a new sub-class of unittest.TestCase named unittest.AsyncioTestCase. It extends unittest.TestCase by adding the following functionality:

  • a new event loop is created and terminated with each test method & the same event loop is used throughout the entire test method execution from setup through cleanup
  • asynchronous test setup and teardown are supported by new methods - asyncSetUp and asyncTearDown
  • test methods that are decorated with the async keyword are run on the event loop. Undecorated methods are run as normal methods.
  • cleanup hooks that are decorated with the async keyword are run on the event loop. Undecorated methods are run as normal methods.

I made the following conscious decisions during the implementation that may be somewhat controversial so I want to mention them here (in no particular order) along with the rationale.

  1. there is no direct support for other event loop implementation: other event loop implementations can be plugged in using a custom EventLoopPolicy implementation. It can be inserted by overriding setUpClass. I believe that other loop implementations can either plug in using the event loop policy or provide their own asynchronous test case implementation (e.g., tornado.testing).
  2. asyncio.iscoroutine is used to detect async methods: the two places that I need to detect whether a callable is a co-routine or not are: (a) test methods and (b) cleanup hooks added with addCleanup. Both are explicit (opt-in) actions so they are completely controlled by the test writer.
  3. outstanding asyncio Tasks are not terminated: I'm still on the fence about whether the test should fail if there are outstanding tasks or if it should gather them.
  4. async test case is a separate class: I decided to use a sub-class so that it is immediately obvious that a test methods are intended to run on the event loop. This also insulates unittest.TestCase from unexpected breakages and maintains it's current behavior.

Open Questions

  1. What should happen when len(asyncio.all_tasks()) is non-zero after cleanups are run?
  2. Should unittest.AsyncioTestCase include a global test run timeout?

I still have to amend commits to update documentation but I wanted to have the discussion about the implementation choices before I started writing up examples and updating the existing documentation suite.

https://bugs.python.org/issue32972

dave-shawley added some commits Oct 15, 2018

unittest.TestRunner: introduce _runTest method.
This is the core of running a test.  It is also the portion of run()
that requires specialization for asyncio support.
unittest: Refactor TestCase.debug.
This method is essentially ``_runTest`` with a Outcome object that does
nothing.  This commit inserts a new class that is a no-op Outcome so
that the method can be re-used.
unittest: Introduce TestCase._terminateTest.
This makes AsyncioTestCase.doCleanups() safe to call at any time.  The
previous version terminated the event loop unnecessarily.
@matrixise

This comment has been minimized.

Copy link
Member

commented Nov 2, 2018

Hi,

Thanks for your PR, but this one is not related to the issue. In this PR, you add the support for asyncio but it is not the topic of the bpo issue.

Maybe you have to open an other issue and assign this PR to the issue.

@dave-shawley

This comment has been minimized.

Copy link
Author

commented Nov 2, 2018

@matrixise ick ... early morning mistake. This is associated with BPO 32972, renaming the PR

@dave-shawley dave-shawley changed the title bpo-32971: Add unittest.AsyncioTestCase bpo-32972: Add unittest.AsyncioTestCase Nov 2, 2018

@matrixise
Copy link
Member

left a comment

Thank for your contribution but could you add a NEWS entry with the blurb tool.

pip install blurb

is stopped and closed.
When subclassing AsyncioTestCase, the event loop is available
via the ``event_loop`` property. Tests can assume that

This comment has been minimized.

Copy link
@matrixise

matrixise Nov 4, 2018

Member

about event_loop I am not sure, usually we found loop in the documentation of asyncio

self.__event_loop = None

def doCleanups(self):
"""

This comment has been minimized.

Copy link
@matrixise
@@ -81,6 +82,16 @@ def testPartExecutor(self, test_case, isTest=False):
self.success = self.success and old_success


class _IgnoredOutcome(object):

This comment has been minimized.

Copy link
@matrixise

matrixise Nov 4, 2018

Member

In Python3, we prefer to use the new style
https://docs.python.org/3/reference/compound_stmts.html#class-definitions

# new style
class _IgnoredOutcome:
    pass

is equivalent to

# old style
class _IgnoredOutcome(object):
    pass

so in this case, use the new style and not the old one.

import unittest


class Test(object):

This comment has been minimized.

Copy link
@matrixise

matrixise Nov 4, 2018

Member

See my previous comment about the construction of a class

class Test:
    pass

is better

self.tearDown()

@property
def event_loop(self):

This comment has been minimized.

Copy link
@matrixise

matrixise Nov 4, 2018

Member

in the documentation, we prefer to use the loop variable for the reference to the event loop.

Examples:
:meth:`loop.call_soon` or :meth:`loop.set_debug` etc...

maybe you should do the same.

raise AssertionError(f'test {test_class} unexpectedly failed')


class EventLoopTracking(object):

This comment has been minimized.

Copy link
@matrixise

matrixise Nov 4, 2018

Member

just class EventLoopTracking:

dave-shawley added some commits Nov 4, 2018

bpo-32972: Rename AsyncioTestCase.event_loop.
`loop` is consistent with the Standard Library spelling of the current
event loop.

See #10296
@dave-shawley

This comment has been minimized.

Copy link
Author

commented Nov 4, 2018

@matrixise - thanks for the review. I amended a few commits to address your feedback. I still intend to add some documentation in Doc/library/unittest.rst and Doc/library/asyncio.rst as well. I wanted to make sure that the implementation was acceptable before starting the documentation related tasks.

if self.__loop is None:
self.__loop = asyncio.new_event_loop()
self.__loop.set_debug(True)
asyncio.set_event_loop(self.__loop)

This comment has been minimized.

Copy link
@PetterS

PetterS Nov 13, 2018

Contributor

Does the test case have to modify the global state like this? It is calling self.loop.run_until_complete when running things.

This comment has been minimized.

Copy link
@dave-shawley

dave-shawley Nov 15, 2018

Author

I think that it does. This is the only way to ensure that the code under test gets the correct loop.

@asvetlov

This comment has been minimized.

Copy link
Contributor

commented May 17, 2019

Please take a look on #13386

@asvetlov

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

Implemented by #13386

@asvetlov asvetlov closed this May 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.