Adding async to Django testing

Django Internals Async
1

Hi all,

NB: It’s worth prefacing this with acknowledging I’ve never used async before and apologies for any silly mistakes below…

I’ve been working on adding a an async option to django’s test suite to:

  • Better test async code
  • In theory speed up testing

As a starting point I’ve tried to add async to SimpleTestCase without employing an @async_to_sync decorator. The idea is to have an self.client.a.get method that can asynchronously test a get request.

That seems to require async-ing all the way down and having finally gotten to this error I figured this was a decent point to ask advice:

runtests.py -k AsyncClientTest --parallel=1 --pdb

Testing against Django installed in '/django/django'
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/warnings.py", line 522, in _warn_unawaited_coroutine
    warn(msg, category=RuntimeWarning, stacklevel=2, source=coro)
RuntimeWarning: coroutine 'SimpleTestCase.__init__' was never awaited
Traceback (most recent call last):
  File "/django/tests/runtests.py", line 566, in <module>
    options.start_at, options.start_after, options.pdb,
  File "/django/tests/runtests.py", line 308, in django_tests
    extra_tests=extra_tests,
  File "/django/django/test/runner.py", line 682, in run_tests
    suite = self.build_suite(test_labels, extra_tests)
  File "/django/django/test/runner.py", line 569, in build_suite
    tests = self.test_loader.discover(start_dir=label, **kwargs)
  File "/usr/local/lib/python3.7/unittest/loader.py", line 349, in discover
    tests = list(self._find_tests(start_dir, pattern))
  File "/usr/local/lib/python3.7/unittest/loader.py", line 406, in _find_tests
    full_path, pattern, namespace)
  File "/usr/local/lib/python3.7/unittest/loader.py", line 460, in _find_test_path
    return self.loadTestsFromModule(module, pattern=pattern), False
  File "/usr/local/lib/python3.7/unittest/loader.py", line 124, in loadTestsFromModule
    tests.append(self.loadTestsFromTestCase(obj))
  File "/usr/local/lib/python3.7/unittest/loader.py", line 93, in loadTestsFromTestCase
    loaded_suite = self.suiteClass(map(testCaseClass, testCaseNames))
  File "/usr/local/lib/python3.7/unittest/suite.py", line 24, in __init__
    self.addTests(tests)
  File "/usr/local/lib/python3.7/unittest/suite.py", line 57, in addTests
    for test in tests:
TypeError: __init__() should return None, not 'coroutine'

You can see the latest here: https://github.com/spool/django/commit/0bf95772e76d207f08aeba7e15420152f1f748c4

I’m getting the sense that adding an asynchronous event loop around each test is a solution and I think that’s what’s beeing added in python 3.8:

https://docs.python.org/3.8/whatsnew/3.8.html#unittest

There’s also a pytest-asyncio plugin worth checking out.

Thanks, hope that makes sense.

2

The way I see it, there are two possible goals of adding async support to tests.

  1. Allowing the tests to run faster by running different switching to another test while the current one waits for IO (i.e.: running all tests through an eventloop)
  2. Making it possible to test async view functionalities within a test (i.e.: having a test client that returns and uses an ASGIRequest instead of a WSGIRequest)

It seems you’re attempting both here.

I don’t think (1) adds much right now since there are very few places within Django where we are waiting for IO. This may change as we get more async support or if it becomes the standard.

I do think it’s important to add (2) soon to be able to add a full test case for the async functionalities and migrate the existing end-to-end tests to support async (they should work as they are and if we replace the internal calls with async ones).

I think you’re on the right track for (2) in your commit, but I think you are making too many things async that don’t need to be (particularly the __init__() calls). I’ll add some comments on your commit.

3

Cool. Figured I was making a mistake and thanks for the suggestions. Finally back from a trip so will look properly soon. Thanks for the detailed reply :slight_smile:

4

I agree, (2) is the better approach here, and you can mostly achieve it by wrapping each test method in sync_to_async or an equivalent. Take a look at the pytest-asyncio package if you want to see how they did it there, too.

5

@andrewgodwin thanks for the reply. Slightly kicking myself for only just reading this after making a commit. Guess I’ll see what you think of the new one and I’m happy to go back to replace my changes with sync_to_async decorators. That would also help if it means we can support ealier versions of python than 3.8.

To be specific: I switched to python 3.8 to use the new IsolatedAsyncioTestCase. Even with removing a lot of unneeded async conversions I still ened up with some requirement to async the _pre_setup(), tearDownClass and other similar methods. I may have just made that mistake repeatedly, but this fixed it with the caveat of needing to comment out # super().__init__(**defaults) in the __init__ method of the AsyncClient class which now inherits from both AsyncRequestFactory and Client. Rough and probably problematic, but at least it got me further. Comments on fixing that better much appreciated.

The error I get to now is the sort I hoped for which I think means I can now delve into the details of the scope and body parameters of the ASGIClientHandler assuming this is the right way to continue:

Testing against Django installed in '/django/django'
System check identified no issues (14 silenced).
E
Opening PDB: KeyError('path')
> /django/django/core/handlers/asgi.py(42)__init__()
-> self.path_info = scope['path']
(Pdb)

======================================================================
ERROR: test_get_view (test_client.tests.AsyncClientTest)
Test an async GET view
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/unittest/async_case.py", line 65, in _callTestMethod
    self._callMaybeAsync(method)
  File "/usr/local/lib/python3.8/unittest/async_case.py", line 88, in _callMaybeAsync
    return self._asyncioTestLoop.run_until_complete(fut)
  File "/usr/local/lib/python3.8/asyncio/base_events.py", line 608, in run_until_complete     return future.result()
  File "/usr/local/lib/python3.8/unittest/async_case.py", line 102, in _asyncioLoopRunner     ret = await awaitable
  File "/django/tests/test_client/tests.py", line 861, in test_get_view
    response = await self.client.a.get('/get_view/', data)
  File "/django/django/test/client.py", line 881, in request
    response = await self.handler(environ, BytesIO())
  File "/django/django/test/client.py", line 820, in __call__
    request = ASGIRequest(scope, body)
  File "/django/django/core/handlers/asgi.py", line 42, in __init__
    self.path_info = scope['path']
KeyError: 'path'

----------------------------------------------------------------------
Ran 1 test in 0.084s

FAILED (errors=1)

So in short: does it make sense to stick with python 3.8, and if not should I effectively try to copy that new feature into our own test suite so we can support earlier versions of python? If we are targetting pre python 3.8, I’m guessing adding the async_to_sync decorators may be the easiest way forward. The crucial advantage of the 3.8 feature is it adds in an events list to append to which effectively saves us the trouble of building our own (though it’s also not too hard to copy that ourselves).

Also: you may note that I passed BytesIO() as the body parameter to ASGIRequest as a sort of place holder. Any thoughts on that place holder appreciated too. And @nicolaslara thanks again for the detailed comments and feel free to go back to that again on this commit. I’ve left lots of sections just commented out to remind myself what I tried before. Will remove those when we hopefully get to something worth commiting to the branch.

Thanks again for comments and patience.

6

Yeah, we have to target 3.6 and up (and maybe even 3.5, I forget the deprecation schedule). I wish there was something that clean already but hopefully it’s not too hard to backport?

7

Hey sorry I’m a bit swamped at the moment. Have done a simple backport of IsolatedAsyncioTestCase (just removed a couple of python 3.8 syntax in function definitions) and got to the same error. There’s a new commit in my reop. Wondering if there’s a more useful place to put that but I figure if I can get the new test to pass I can refactor.

8

No worries, I’m quite swamped as well (as you may be able to tell by the lack of async progress in general).