-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Make OCSP stapling tests more stable #97979
Conversation
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones Issue DetailsThe instability of the tests was caused by the tests racing with refreshing the OCSP staple on the background. This PR adds waiting for the background refresh to complete where it was assumed it will complete fast enough. I kept the tests running for a bit on my machine, and did not reproduce the failure after the changes.
|
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs
Show resolved
Hide resolved
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
@@ -18,6 +18,7 @@ namespace System.Net.Security.Tests; | |||
public class SslStreamCertificateContextOcspLinuxTests | |||
{ | |||
[Fact] | |||
[PlatformSpecific(TestPlatforms.Linux)] |
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.
The class is named "OcspLinuxTests", can/should this attribute just be put on the class? (I think it works at that level)
CI Failures are unrelated. |
Fixes #97836, #97779 and #97936.
The instability of the tests was caused by the tests racing with refreshing the OCSP staple on the background. This PR adds waiting for the background refresh to complete where it was assumed it will complete fast enough.
I kept the tests running for a bit on my machine, and did not reproduce the failure after the changes.