Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Fix async tests with no explicit event_loop usage/declaration #352

Merged
merged 4 commits into from Apr 28, 2020

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Apr 27, 2020

What do these changes do?

Fix CI/CD tests with pytest-asyncio and no explicit declaration of event_loop fixture (as it is not used). This only affects CI/CD and local development, not the framework itself.

Blocks #350 #351 and all new PRs.

Description

Part 1: Reveals itself as async tests freezing with no reaction for long time: due to fixtures (e.g. aresponses, aiohttp server, etc) created in different event loops than the test itself.

Originally caused by pytest-asyncio upgrade from 0.10.0 to 0.11.0 (diff).

Fixed by auto-adding asyncio markers for all async tests even earlier than before, so that pytest-asyncio could notice them on item creation.


Part 2: In that case, some tests fail due to event loop is not running when they try to create event-loop dependent objects. E.g. aresponses says: The object should be created from async function.

Caused by: pytest-dev/pytest-asyncio#154 (since 0.11.0). Fixed in: pytest-dev/pytest-asyncio#156 (not yet merged and released to the moment).


As an all-in-one fix, bot prepare it for pytest-asyncio==0.11.0 by fixing the freezes, but roll back to 0.10.0 (and therefore, pytest<5.4.0) for now, until the event_loop issue is resolved.

Issues/PRs

Issues: #273

Related: #13

Type of changes

  • Mostly CI/CD automation, contribution experience

Checklist

  • The code addresses only the mentioned problem, and this problem only
  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt

@nolar nolar added the automation CI/CD: testing, linting, releasing automatically label Apr 27, 2020
@zincr
Copy link

zincr bot commented Apr 27, 2020

🤖 zincr found 0 problems , 1 warning

ℹ️ Dependency Licensing
✅ Large Commits
✅ Approvals
✅ Specification

Details on how to resolve are provided below


Dependency Licensing

All dependencies specified in package manager files must be reviewed, banned dependency licenses will block the merge, all new dependencies introduced in this pull request will give a warning, but not block the merge

Please ensure that only dependencies with licenses compatible with the license of this project is included in the pull request.

  • ℹ️ Could not process requirements.txt for new dependencies
     

@zincr
Copy link

zincr bot commented Apr 27, 2020

🤖 zincr found 1 problem , 1 warning

❌ Approvals
ℹ️ Dependency Licensing
✅ Large Commits
✅ Specification

Details on how to resolve are provided below


Approvals

All proposed changes must be reviewed by project maintainers before they can be merged

Not enough people have approved this pull request - please ensure that 1 additional user, who have not contributed to this pull request approve the changes.

  • ✅ Approved by PR author @nolar
  • ❌ 1 additional approval needed
     

Dependency Licensing

All dependencies specified in package manager files must be reviewed, banned dependency licenses will block the merge, all new dependencies introduced in this pull request will give a warning, but not block the merge

Please ensure that only dependencies with licenses compatible with the license of this project is included in the pull request.

  • ℹ️ Could not process requirements.txt for new dependencies
     

@nolar nolar merged commit 8439252 into zalando-incubator:master Apr 28, 2020
@nolar nolar deleted the fix-asyncio-tests branch April 28, 2020 08:09
@nolar nolar added this to the 0.27 milestone May 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automation CI/CD: testing, linting, releasing automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants