Skip to content

bpo-37228: Fix warnings in test_asyncio.test_base_events#17577

Merged
ambv merged 1 commit intopython:masterfrom
aeros:bpo37228-fix-asyncio_test_base_events-warnings
Dec 12, 2019
Merged

bpo-37228: Fix warnings in test_asyncio.test_base_events#17577
ambv merged 1 commit intopython:masterfrom
aeros:bpo37228-fix-asyncio_test_base_events-warnings

Conversation

@aeros
Copy link
Copy Markdown
Contributor

@aeros aeros commented Dec 12, 2019 •
edited
Loading

Co-authored-by: @tirkarthi

The security patch #17311 (which was backported to 3.8, 3.7, and 3.6) added a few subtle warnings, including a ResourceWarning and two DeprecationWarnings in test_asyncio.test_base_events. @tirkarthi discovered the warnings, and proposed a fix for the ResourceWarning. See the bpo issue for more details.

https://bugs.python.org/issue37228

@aeros aeros requested review from 1st1 and asvetlov as code owners December 12, 2019 07:35
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Dec 12, 2019
@aeros aeros requested review from ambv and ned-deily December 12, 2019 07:58
@ambv
Copy link
Copy Markdown
Contributor

ambv commented Dec 12, 2019

Wasn't the point of the security fix to not use reuse_addr=?

@aeros
Copy link
Copy Markdown
Contributor Author

aeros commented Dec 12, 2019 •
edited
Loading

@ambv

Wasn't the point of the security fix to not use reuse_addr=?

The primary point of the security fix was to specifically remove usage of reuse_addr=True for create_datagram_endpoint() and no longer use it as a default. But as a part of it we also deprecated the reuse_address parameter itself since it was turned into a no-op. This PR is simply addressing some minor test changes that I missed, which @tirkarthi picked up on by running the tests with -Wall.

I accidentally introduced a couple deprecation warnings that were raised from previous tests that still had reuse_address=False, which should have been removed. Also, in the actual test for the deprecation warning, the transport and protocol were not closed, leading to a resource warning.

It wouldn't be the end of the world if this didn't make it into the next releases, but it would definitely be preferable to not have additional warnings lingering in the regression tests. Especially in this case where the fix is quite simple.

@ambv ambv merged commit 1988344 into python:master Dec 12, 2019
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Dec 12, 2019

@ambv: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Dec 12, 2019

Thanks @aeros for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8.
🐍🍒⛏🤖

@ambv
Copy link
Copy Markdown
Contributor

ambv commented Dec 12, 2019

Thanks! ✨ 🍰 ✨

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Dec 12, 2019

GH-17579 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
@aeros @miss-islington
Fix warnings in test_asyncio.test_base_events (pythonGH-17577)
9499a7b 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <[email protected]>
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Dec 12, 2019

GH-17580 is a backport of this pull request to the 3.7 branch.

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Dec 12, 2019

GH-17581 is a backport of this pull request to the 3.6 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
@aeros @miss-islington
Fix warnings in test_asyncio.test_base_events (pythonGH-17577)
e0afcd7 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
@aeros @miss-islington
Fix warnings in test_asyncio.test_base_events (pythonGH-17577)
0230f47 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <[email protected]>
@tirkarthi
Copy link
Copy Markdown
Member

tirkarthi commented Dec 12, 2019

Thanks @aeros :)

ambv pushed a commit that referenced this pull request Dec 12, 2019
@miss-islington @aeros @ambv
Fix warnings in test_asyncio.test_base_events (GH-17577) (#17581)
b35ec00 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <[email protected]>
ned-deily pushed a commit that referenced this pull request Dec 17, 2019
@miss-islington @aeros @ned-deily
Fix warnings in test_asyncio.test_base_events (GH-17577) (GH-17579)
d21ad67 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <[email protected]>
ned-deily pushed a commit that referenced this pull request Dec 17, 2019
@miss-islington @aeros @ned-deily
Fix warnings in test_asyncio.test_base_events (GH-17577) (GH-17580)
c70c8ca 
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <[email protected]>
ned-deily pushed a commit to ned-deily/cpython that referenced this pull request Dec 18, 2019
@miss-islington @aeros @ned-deily
Fix warnings in test_asyncio.test_base_events (pythonGH-17577) (pytho…
9346209 
…nGH-17580)

Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <[email protected]>
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@aeros @shihai1991
Fix warnings in test_asyncio.test_base_events (python#17577)
b8ba5ef 
Co-authored-by: tirkarthi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@1st1 1st1 Awaiting requested review from 1st1

@asvetlov asvetlov Awaiting requested review from asvetlov

@ned-deily ned-deily Awaiting requested review from ned-deily

@ambv ambv Awaiting requested review from ambv

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir topic-asyncio type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@aeros @ambv @bedevere-bot @miss-islington @tirkarthi @the-knights-who-say-ni