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

Making sure we can run async def tests #4987

Merged
merged 2 commits into from Feb 18, 2021
Merged

Conversation

wRAR
Copy link
Member

@wRAR wRAR commented Feb 12, 2021

With twisted.trial we are able to write asynchronous test functions by returning a Deferred from them. We also have the deferred_f_from_coro_f higher order function that can be used as a decorator for an async def test function and everything should work as expected. So I'm adding this small test case to check that we indeed can write working tests this way.

Unfortunately, a naked async def test function will always succeed, with a warning that a coroutine was not awaited (ideally we should get rid if warnings from tests, but this is still easy to miss).

I added these tests because I've just found for #4978 that such tests fail with old Twisted. So this PR should fail too.

@wRAR wRAR added the CI label Feb 12, 2021
@wRAR wRAR changed the title [WIP] Making sure we can run async def tests Making sure we can run async def tests Feb 12, 2021
@codecov
Copy link

codecov bot commented Feb 12, 2021

Codecov Report

Merging #4987 (06a928d) into master (ea92b49) will increase coverage by 0.18%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4987      +/-   ##
==========================================
+ Coverage   87.82%   88.00%   +0.18%     
==========================================
  Files         158      158              
  Lines        9723     9723              
  Branches     1433     1433              
==========================================
+ Hits         8539     8557      +18     
+ Misses        929      911      -18     
  Partials      255      255              
Impacted Files Coverage Δ
scrapy/robotstxt.py 97.53% <0.00%> (+22.22%) ⬆️

@wRAR
Copy link
Member Author

wRAR commented Feb 18, 2021

As the only thing we use pytest-twisted for is installing the asyncio reactor (we had a dep on it before this code was merged but it was unused at that time) and we cannot easily tune the asyncio loop (which we need for Twisted 17.9.0), and we already have code to install a reactor, I've just put this code into conftest.py and dropped pytest-twisted.

Ironically, pytest-twisted has code for running async def tests but looks like it's not suitable for methods.

@wRAR wRAR marked this pull request as ready for review February 18, 2021 13:34
@wRAR wRAR merged commit fa3ebb1 into scrapy:master Feb 18, 2021
@wRAR wRAR deleted the asyncdef-tests branch February 18, 2021 13:34
@wRAR wRAR added the asyncio label Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants