Skip to content
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

Allow additional ClientOSError for expected failure #17899

Closed
wants to merge 1 commit into from

Conversation

emlowe
Copy link
Contributor

@emlowe emlowe commented Apr 19, 2024

in test_farmer - when testing invalid certificates, also allow ClientOSError in addition to ServerDisconnectError

The exact error isn't that important when the connection fails

@emlowe emlowe requested a review from a team as a code owner April 19, 2024 22:02
@emlowe emlowe added CI CI changes Changed Required label for PR that categorizes merge commit message as "Changed" for changelog labels Apr 19, 2024
@emlowe emlowe requested a review from altendky April 25, 2024 20:15
Copy link
Contributor

@altendky altendky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raising a concern to block merging while I inspect in a bit more detail.

@altendky
Copy link
Contributor

Chatting with Earle about some details here and what might be a plausible improvement since we haven't confirmed that this newly accepted error is in fact indicative of the case we are trying to test vs. the existing flakiness of this sort of error being more common now in this test than others.

@emlowe
Copy link
Contributor Author

emlowe commented Apr 30, 2024

This is replaced by the fix in #17921

@emlowe emlowe closed this Apr 30, 2024
@emlowe emlowe deleted the EL.ssl-test-fix branch May 2, 2024 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog CI CI changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants