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

Clean up test warnings #2991

Merged
merged 6 commits into from
Jun 21, 2023
Merged

Clean up test warnings #2991

merged 6 commits into from
Jun 21, 2023

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Jun 9, 2023

What was wrong?

There were a bunch of warnings being emitted in various tests.

How was it fixed?

Cleaned up some, mostly by closing loops and sessions.

  • Limiting requests to <=2.29.0 will get rid of the DeprecationWarning: /home/circleci/repo/.tox/py310-core/lib/python3.10/site-packages/urllib3/poolmanager.py:315: DeprecationWarning: The 'strict' parameter is no longer needed on Python 3+. This will raise an error in urllib3 v2.1.0., but I opted not to do that. I think they'll clean it up eventually.

  • The warnings from tests/core/tools/pytest_ethereum/test_deployer.py::test_user_code_with_fixture are due to this file, and specifically the w3 and start_websocket_server fixtures.

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@kclowes kclowes changed the title Cleanup test warnings Clean up test warnings Jun 12, 2023
@kclowes kclowes force-pushed the cleanup-test-warnings branch 4 times, most recently from ea2e23c to 3ccd376 Compare June 19, 2023 19:54
@kclowes kclowes marked this pull request as ready for review June 19, 2023 20:17
@kclowes kclowes requested review from reedsa, fselmo and pacrob June 19, 2023 20:17
Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

lgtm

async_cache_and_return_session(endpoint_uri)
)
try:
event_loop = asyncio.new_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.

How is the event_loop defined here if the try fails?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice catch. Removed the try and no more warnings were issued.

Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

lgtm, once fselmo's question is resolved

Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

lgtm!

@kclowes kclowes merged commit 0a1da2d into ethereum:main Jun 21, 2023
@kclowes kclowes deleted the cleanup-test-warnings branch June 21, 2023 16:33
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.

4 participants