-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
[3.5] bpo-36576: Skip test_ssl and test_asyncio tests failing with OpenSSL 1.1.1 #12694
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
Conversation
|
I wrote a similar change for Fedora Rawhide: https://src.fedoraproject.org/rpms/python35/pull-request/23 Somehow related, I wrote a change to add OpenSSL 1.1.1 support to Python 3.4:
I may also skip failing tests on Python 3.4. |
|
cc @stratakis |
|
@stratakis asked me to replace "OpenSSL 1.1" with "OpenSSL 1.1.0": done. |
Lib/test/test_asyncio/test_events.py
Outdated
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.
Is the version number 1.1 and not 1.1.0 here by accident or purpose?
| @unittest.skipIf(IS_OPENSSL_1_1, "bpo-26470: fail on OpenSSL 1.1") | |
| @unittest.skipIf(IS_OPENSSL_1_1, "bpo-26470: fail on OpenSSL 1.1.0") |
That would make it more consistent.
Also, have we checked that it's actually 1.1.0 and not 1.1.1? Should we say 1.1 everywhere instead?
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.
Also, have we checked that it's actually 1.1.0 and not 1.1.1?
No, and I'm not interested to test if it's exactly 1.1.0 or 1.1.1. I'm tired of the OpenSSL 1.1.1 mess, I consider that I already spent enough time on this topic :-)
Should we say 1.1 everywhere instead?
I fixed the test_asyncio comment to write OpenSSL 1.1.0, as I did in test_ssl.
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.
ok
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.
For the record, the tests pass with OpenSSL 1.1.0i.
Lib/test/test_asyncio/test_events.py
Outdated
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.
Woudl a oneliner be more readable? Or less? Something like:
IS_OPENSSL_1_1 = ssl is not None and ssl.OPENSSL_VERSION_INFO >= (1, 1, 0)Or even:
IS_OPENSSL_1_1 = ssl and ssl.OPENSSL_VERSION_INFO >= (1, 1, 0)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 prefer to write the code on 4 lines ;-)
hroncok
left a comment
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.
Good enough for 3.5.
hroncok
left a comment
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.
Even better now! Thanks.
|
Oh, the NEWS entry used the old bpo number. I also fixed that. |
…1.1.1 Some test_ssl and test_asyncio are written for OpenSSL 1.0 and TLS 1.0, but fail with OpenSSL 1.1.1 and TLS 1.3. Fixing these needs require to backport new ssl flags like ssl.OP_NO_TLSv1_3 or ssl.OP_NO_COMPRESSION which cannot be done in a minor 3.5.x release. Moreover, it is not really worth it: the code works fine, issues are in the tests.
|
Oh, my PR used IS_OPENSSL_1_1_1 but it didn't exist! I fixed that as well. |
|
@tiran I'm inclined to merge this patch. Do you want to review it before I merge, or should I just go ahead? |
|
FYI Fedora now uses this patch in Python 3.5: https://src.fedoraproject.org/rpms/python35/blob/master/f/00322-test_ssl-skip-openssl111.patch Even if right now, the package is still linked to OpenSSL 1.0: https://src.fedoraproject.org/rpms/python35/blob/master/f/python35.spec#_128 |
|
@larryhastings: Please replace |
|
Thanks for the 3.5 love, Victor! |
Some test_ssl and test_asyncio are written for OpenSSL 1.0 and TLS
1.0, but fail with OpenSSL 1.1.1 and TLS 1.3.
Fixing these needs require to backport new ssl flags like
ssl.OP_NO_TLSv1_3 or ssl.OP_NO_COMPRESSION which cannot be done in a
minor 3.5.x release. Moreover, it is not really worth it: the code
works fine, issues are in the tests.
https://bugs.python.org/issue36576