Author yselivanov
Recipients Petter S, asvetlov, njs, pdxjohnny, r.david.murray, yselivanov, zach.ware
Date 2018-03-12.20:27:42
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1520886462.85.0.467229070634.issue32972@psf.upfronthosting.co.za>
In-reply-to
Content
First, John and Peter, let's not have two competing PRs.  I'd prefer to have only one to make things easier to review.  So far it looks like Peter's is slightly more developed.  And this also seems to be a complex issue, so there's plenty of work to be done here and you can work on it jointly if you want.

I think a lot of Andrew's recommendations (see [1]) about doSetUp etc make sense.

The problem is how to marry methods like `asyncSetUpClass`, `asyncSetUp` and `runTest` together: all are async, all should be executed *within* one event loop.  They should also be executed *before* you call the actual test method, so it's *incorrect* to call the test case and decide if async set ups should be called or not.  This is another argument to have a separate AsyncioTestCase base class.

BTW, we can use asyncio.run() to execute only *one* coroutine.  If you have more than one, they will be executed in *different* event loops!  This is why the "coroutine runner" proposal is fundamentally incompatible with having `asyncSetUp` methods.

Speaking of asyncSetUp, let me re-iterate why we can't *just* reuse setUp:

  class B:

    async def setUp(self):
        print('bar')

  class A(B):

    def setUp(self):
        super().setUp()
        print('foo')

If we execute tests in A, 'bar' will never be printed.

So I'm suggesting to try another approach: have a generic ABC `unittets.AsyncTestCase(ABC)` with one abstract method: `makeAsyncExecutor`.  This will be a class intended to be used by framework developers.

We then add a concrete `unittest.AsyncioTestCase(unittest.AsyncTestCase)` implementation class to unittest (or should it be asyncio? I'm fine either way).

So what will `makeAsyncExecutor` do?  It should return an instance of `unittest.AsyncExecutor(ABC)` object which roughly would look like the following (this is just an example design, we need to discuss it further):

  class AsyncExecutor(abc.ABC):

      def init(self)

      def runAsync(self, coro: typing.Coroutine)

      def shutdown(self)

Why do we need a proxy object?  Well, due to some historical reasons, `asyncio.loop.close()` just closes the loop.  The actual correct shutdown protocol for an asyncio program is a little bit more elaborate, you can take a look at how asyncio.run is implemented in Python 3.7: [2].  Correct initialization of an event loop might also require extra work.  And keep in mind, we want to have a more-or-less generic and reusable implementation here so that frameworks like Trio could use this new API too (that's not the main goal, but would be nice to have).

A few open questions: 

- Should we have 'asyncSetUpClass'?

- Should we have an event loop per one test case method, or per one class?  Should this be configurable?  In my experience both things are needed from time to time.

- If we have 'asyncSetUp' should it be called before or after 'setUp'?


[1] https://github.com/python/cpython/pull/6051
[2] https://github.com/python/cpython/blob/master/Lib/asyncio/runners.py#L8
History
Date User Action Args
2018-03-12 20:27:42yselivanovsetrecipients: + yselivanov, r.david.murray, njs, asvetlov, zach.ware, pdxjohnny, Petter S
2018-03-12 20:27:42yselivanovsetmessageid: <1520886462.85.0.467229070634.issue32972@psf.upfronthosting.co.za>
2018-03-12 20:27:42yselivanovlinkissue32972 messages
2018-03-12 20:27:42yselivanovcreate