Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

bpo-43921: Fix test_ssl.test_pha_required_nocert()#26489

Merged
vstinner merged 1 commit into
python:mainpython/cpython:mainfrom
vstinner:pha_required_nocertCopy head branch name to clipboard
Jun 2, 2021
Merged

bpo-43921: Fix test_ssl.test_pha_required_nocert()#26489
vstinner merged 1 commit into
python:mainpython/cpython:mainfrom
vstinner:pha_required_nocertCopy head branch name to clipboard

Conversation

@vstinner

@vstinner vstinner commented Jun 2, 2021

Copy link
Copy Markdown
Member

Fix test_pha_required_nocert() of test_ssl: catch two more EOF cases
(when the recv() method returns an empty string).

https://bugs.python.org/issue43921

Fix test_pha_required_nocert() of test_ssl: catch two more EOF cases
(when the recv() method returns an empty string).
@vstinner

vstinner commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

Without this change, python -m test test_ssl -u all -v -F -j5 -m test_pha_required_nocert fails in less than 2 minutes.

With this change, the command didn't fail, I ran it for like 30 minutes.

@vstinner

vstinner commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

cc @tiran @pablogsal: I plan to merge this fix tomorrow, test_ssl random failures are super annoying, and I'm sure that this change fix the issue (I ran a functional test, see my previous comment).

@tiran

tiran commented Jun 2, 2021

Copy link
Copy Markdown
Member

LGTM, although it's more of a patch than a proper fix. The test connection should not fail with an EOF error. There is some foul play. It's good enough for me.

@tiran tiran added the needs backport to 3.10 only security fixes label Jun 2, 2021
@vstinner

vstinner commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

The test connection should not fail with an EOF error.

I don't have the bandwidth to investigate that. I'm just an employee who like to see a green CI. The EOF check was already there before my change :-p

@vstinner

vstinner commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

If someone wants to enhance the test, I would suggest to check for TLS alerts, rather than checking how the client socket fails or how it's closed.

@vstinner vstinner merged commit 320eaa7 into python:main Jun 2, 2021
@vstinner vstinner deleted the pha_required_nocert branch June 2, 2021 20:29
@vstinner vstinner added needs backport to 3.10 only security fixes and removed needs backport to 3.10 only security fixes labels Jun 2, 2021
@vstinner

vstinner commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

Come on little bot, you can do it! I believe in you!

@vstinner

vstinner commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

@miss-islington: knock knock knock

@vstinner

vstinner commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

The backport PR is not created automatically :-( python/miss-islington#462

@Mariatta Mariatta added needs backport to 3.10 only security fixes and removed needs backport to 3.10 only security fixes labels Jun 2, 2021
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@Mariatta Mariatta added needs backport to 3.10 only security fixes and removed needs backport to 3.10 only security fixes labels Jun 2, 2021
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jun 2, 2021
@bedevere-bot

Copy link
Copy Markdown

GH-26494 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 2, 2021
Fix test_pha_required_nocert() of test_ssl: catch two more EOF cases
(when the recv() method returns an empty string).
(cherry picked from commit 320eaa7)

Co-authored-by: Victor Stinner <vstinner@python.org>
miss-islington added a commit that referenced this pull request Jun 2, 2021
Fix test_pha_required_nocert() of test_ssl: catch two more EOF cases
(when the recv() method returns an empty string).
(cherry picked from commit 320eaa7)

Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.