Author dave-shawley
Recipients Petter S, asvetlov, dave-shawley, njs, pdxjohnny, r.david.murray, yselivanov, zach.ware
Date 2019-02-07.12:31:12
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1549542672.3.0.80397634516.issue32972@roundup.psfhosted.org>
In-reply-to
Content
PR 10296 is my implementation of a unittest.TestCase subclass solution to this issue.  This comment explains the approach and rationale in detail.  Let's discuss this and see if the implementation meets expectations or should be abandoned.

I refactored unittest.TestCase to isolate the running of test methods into a new helper method named _runTest and add a new hook named _terminateTest.  The _runTest method is used to customize test method execution in sub-classes.  The _terminateTest method is called from within the finally block in unittest.TestCase.run.  It is an empty method in unittest.TestCase.  This was the only change to unittest.TestCase.  A new class unittest.AsyncioTestCase was added that implements async-based testing.  It is a direct sub-class of unittest.TestCase that:

* uses a @property named loop to lazily create an event loop instance
* destroys the event loop instance in _terminateTest
* re-implements _runTest to call new asynchronous hook methods
* adds asyncSetUp and asyncTearDown methods that simply call the synchronous methods
* re-implements doCleanups to call co-routines asynchronously

Rationale
---------
I used asyncio.iscoroutinefunction to detect if test methods or callbacks are co-routines.  You explicitly opt-in to async behavior using the async marker on things that you want to run on the loop.  This will cause problems with using the patch decorator on test methods since they will no not be detected as co-routines.  I took this approach primarily to simplify the code and enforce explicitness.  Since the implementation is a new sub-class, it cannot break existing code and new code can place the patch inside of the test method instead of decorating the method.

I believe that creating an destroying the loop with each test method execution is the safest approach for managing the lifecycle.  I view having the loop exist at the class level is an unnecessary optimization.  I also ensure that code under test that calls asyncio.get_event_loop or asyncio.get_running_loop will receive the loop by calling asyncio.set_event_loop with the new loop.  This came up in PR review with Petter S.

The management of the loop is isolated into a property which makes it possible to create custom sub-classes that instantiate 3rd party loops that are asyncio compatible.  This is the only concession that my implementation makes to supporting other loop classes.

If it is not clear, I believe that a new sub-class of unittest.TestCase is necessary for a clean implementation.  It preserves unittest.TestCase so the risk of breaking existing code is minimized and the async functionality is isolated in a new class that is explicitly meant to test async code. This is also necessary if the implementation should exist in the asyncio module.
History
Date User Action Args
2019-02-07 12:31:14dave-shawleysetrecipients: + dave-shawley, r.david.murray, njs, asvetlov, zach.ware, yselivanov, pdxjohnny, Petter S
2019-02-07 12:31:12dave-shawleysetmessageid: <1549542672.3.0.80397634516.issue32972@roundup.psfhosted.org>
2019-02-07 12:31:12dave-shawleylinkissue32972 messages
2019-02-07 12:31:12dave-shawleycreate