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

Update dependencies and adjust code #585

Merged
merged 3 commits into from
May 23, 2020

Conversation

Askaholic
Copy link
Collaborator

@Askaholic Askaholic commented May 17, 2020

Adjusting a few fixtures to get around the event loop being closed problem. I also ignore some deprecation warnings in the tests because they do not cause us any issues.

Closes #584.

@Askaholic Askaholic requested a review from cleborys May 17, 2020 01:31
@Askaholic Askaholic force-pushed the issue/#584-update-dependencies branch 2 times, most recently from 5bf1ec3 to 87661de Compare May 21, 2020 21:25
@Askaholic Askaholic requested a review from norraxx May 23, 2020 03:20

await db.close()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I wish, we work with database without dirtying it.
Like it's here.
https://github.com/norraxx/faf-aio-replayserver/blob/master/tests/fixtures/db_fixtures.py#L7

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And ofc IF we have .commit() in code, we should mock it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The database connection used during tests gets rolled back. This one is just for loading the test-data file once at the start of the session. And for some reason with the newest version of pytest-asyncio having an asyc finalizer on this session scoped fixture would throw an error due to closed event loop.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also rollback these changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the test data is not rolled back

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=T_T=

tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
Copy link
Member

@cleborys cleborys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have nothing to remark :)

@Askaholic Askaholic force-pushed the issue/#584-update-dependencies branch from 87661de to 35b0bdc Compare May 23, 2020 19:38
@Askaholic Askaholic force-pushed the issue/#584-update-dependencies branch from 35b0bdc to e660f2e Compare May 23, 2020 23:14
@Askaholic Askaholic merged commit fc378f9 into FAForever:develop May 23, 2020
Askaholic added a commit that referenced this pull request Jun 17, 2020
* Update dependencies and adjust code

* Ignore hypothesis deprecation warnings on a per test basis

* Make devserver command use `dev-config.yml` file.
@Askaholic Askaholic deleted the issue/#584-update-dependencies branch June 28, 2020 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update dependencies
3 participants