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
Ensure load_verify_locations raises SSLError for all backends #1812
Conversation
This also adds TestSSL to the classes tested in SecureTransport and PyOpenSSL, since: 1. TestSSL was the most natural place for this test. 2. The test only makes sense when run against all SSL backends. Co-authored-by: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr>
@pilou- I can't assign you, but would love a review too! Thanks |
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.
Love this! 🎉 Couple of questions
""" | ||
with pytest.raises(SSLError) as exc: | ||
ssl_wrap_socket(None, ca_certs=os.devnull) | ||
assert exc.type == SSLError |
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.
Isn't this assert redundant with the pytest.raises()
?
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.
Nice catch, thanks! Fixed.
pytest.raises() already checks this.
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.
LGTM
Thanks @sethmlarson for the lint fix and the approval. Thank you @pilou- for your contribution! |
…3#1812) * Ensure load_verify_locations raises SSLError for all backends This also adds TestSSL to the classes tested in SecureTransport and PyOpenSSL, since: 1. TestSSL was the most natural place for this test. 2. The test only makes sense when run against all SSL backends. Co-authored-by: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr> * Remove redundant check in test pytest.raises() already checks this. * Update test_socketlevel.py Co-authored-by: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr> Co-authored-by: Seth Michael Larson <sethmichaellarson@gmail.com>
…3#1812) * Ensure load_verify_locations raises SSLError for all backends This also adds TestSSL to the classes tested in SecureTransport and PyOpenSSL, since: 1. TestSSL was the most natural place for this test. 2. The test only makes sense when run against all SSL backends. Co-authored-by: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr> * Remove redundant check in test pytest.raises() already checks this. * Update test_socketlevel.py Co-authored-by: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr> Co-authored-by: Seth Michael Larson <sethmichaellarson@gmail.com>
This also adds TestSSL to the classes tested in SecureTransport and PyOpenSSL, since:
This is my take on #1517 from @pilou-: I've backported it to master, and made a few tweaks. In particular, we can't test the exact message, it's different with every backend.
Closes #1517