Skip the SNI test until it can verify SNI again - #1078
Merged
Conversation
The test requests an HTTPS host and expects a 200, which only demonstrates anything if that host serves a different certificate when no SNI is sent. It no longer does, so a transport which quietly stopped sending SNI would still pass. The host has meanwhile also started answering 403 to every client, including a plain `curl`, so the test fails for reasons entirely unrelated to the library. Pointing it at another host would get the suite green again, but it would also leave a test standing which looks like it covers something it does not. Skipping it says what is actually the case, and keeps the intent visible for whoever picks it up. Refs #1077, which has the measurements and the options for getting real coverage back.
jrfnl
approved these changes
Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
testSNISupportrequests an HTTPS host and expects a200, which only demonstrates something if that host serves a different certificate when no SNI is sent. It no longer does, so a transport which quietly stopped sending SNI would still pass the test.On top of that, the host it uses now answers
403to every client, including a plaincurl, so the test currently fails for reasons which have nothing to do with the library. That is what brought this to light.Repointing it at another host would get the suite green again, but it would leave a test standing which looks like it covers something it does not. Skipping it states what is actually the case, and keeps the intent visible for whoever picks it up. The original docblock stays, with a note explaining the situation, and both the note and the skip message link to the issue.
Details, including the certificate comparison across several hosts and the options for getting real coverage back, are in #1077.
Refs #1077