GeezerVison is showing github.com — gh-82082: Make our test suite pass on an IPv6-only Linux host by gpshead · Pull Request #26225 · python/cpython
Conversation
gpshead
commented
May 19, 2021
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Background: A true IPv6-only host has no IPv4 localhost 127.0.0.1 or IPv4 AF_INET stack enabled at all. I realize these are rare to most people. They're not to me & I have buildbot ready and waiting to go once I get things in our tree into decent shape. I'll post how that was setup in a gist later.
This is a collection of changes, not intended for submission via this branch. Likely to be split up across a variety of PRs addressing each individual test and a pre-PR for the test.support.socket_helper changes. But it makes for good initial viewing and study of patterns by showing this branch as a draft PR.
Status
Where I started
24 tests failed:
test_asynchat test_asyncore test_docxmlrpc test_eintr test_epoll
test_ftplib test_httplib test_imaplib test_multiprocessing_fork
test_multiprocessing_forkserver test_multiprocessing_spawn
test_nntplib test_os test_poplib test_robotparser test_smtplib
test_socket test_ssl test_support test_sys test_telnetlib
test_urllib2_localnet test_venv test_wsgiref
Where I'm at
At the time of writing this message, I have fixed all of the above to work in this branch!
At least four tests were hanging and still hang. Separate BPO issues are open for them: test_asyncio test_httpservers test_logging test_xmlrpc
I haven't yet run regrtest with all of the -u network flags enabled so I'm no doubt missing some more failures or hangs.
https://bugs.python.org/issue37901
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Tests that are agnostic about the IP protocol used will use whichever
the socket_helper module deems appropriate for the host.
there's a _lot_ more cleanup that could be done in here.
the capability skip decorators are applied in an inconsistent
manner. some on the base class, others way down on the leaf
Test classes. yuck.
Also makes the test_multiprocessing suites work on IPv6-only hosts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to start with a PR only to add get_family() and get_bound_ip_socket_and_port() (and IPV4_ENABLED, needed by get_family) to socket_helper. You can pick a few modified tests which only use these functions, like test_asynchat. It's easier to review a smaller PR.
Then you can write a PR for multiprocessing which adds tcp_socket().
When you add a function in test.support, it's good to document it at:
https://docs.python.org/dev/library/test.html#module-test.support.socket_helper
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"TODO(gpshead): We should support a https://pypi.org/project/portpicker/ portserver or equivalent process running on our buildbot hosts and use that portpicker library"
What is the advantage compared to find_unused_port()?
One find_unused_port() flaw is that there is a race condition if two processes call it at the same time: https://bugs.python.org/issue38697
But I'm not convinced by the find_unused_port() design. It sounds better to simply... not use it at all. Like create a socket with port=0 and then get the port number from the socket object.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
I suggest to start with a PR only to add get_family() and get_bound_ip_socket_and_port() (and IPV4_ENABLED, needed by get_family) to socket_helper. You can pick a few modified tests which only use these functions, like test_asynchat. It's easier to review a smaller PR.
This is a draft PR that isn't intended for direct submission. It'll be broken up into sub-PRs exactly like that once I'm happy with how it looks. :)
For starters, this is a way to test my branch in CI and on buildbots as I evolve the changes.
"TODO(gpshead): We should support a https://pypi.org/project/portpicker/ portserver or equivalent process running on our buildbot hosts and use that portpicker library"
What is the advantage compared to find_unused_port()?
Thats a long term idea that I don't intend to solve as it'd require infrastructure changes on every buildbot worker's part. I'll probably just drop the todo. It has advantages in that it is centralized port management so that multiple tests running in parallel don't hit a race condition when binding to port 0 to obtain a "unique" port to reuse. It'd solve issue38697 in the face of tests that still want to use the unfortunate "give me a port number" approach.
find_unused_port is always the worst API option. I'm not happy with the changes to it in socket_helper either. I'll step back and see which existing messy tests can easily be refactored to not use it after I get more things working.
Some things tests by their nature and code API structure need to start with a port number rather than a bound socket. I'm attempting to avoid introducing non-test API changes and features when possible for now.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
This PR is stale because it has been open for 30 days with no activity.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
arhadthedev
changed the title
bpo-37901: Make our test suite pass on an IPv6-only Linux host
gh-82082: Make our test suite pass on an IPv6-only Linux host
Apr 29, 2023
|
Removing
sprint
because the PR missed the ones in 2021, 2022, and 2023.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
test_nntplib was removed in gh-104894, asyncore/asynchat in gh-96580.
Could you resolve merge conflicts to rerun the tests, please?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
i'm not intending to continue with this work.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|