Skip to content

Replace VerifyWithRevocation test with DynamicRevocationTests #42687

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

Merged
merged 3 commits into from
Sep 29, 2020

Conversation

vcsjones
Copy link
Member

VerifyWithRevocation is a flaky test. It needs internet to contact a CA for CRL/AIA and can occasionally fail. This replaces the offline revocation checking part of that test with DynamicChainTests which use our own local CA.

Fixes #41952

@ghost
Copy link

ghost commented Sep 24, 2020

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @jeffhandley
See info in area-owners.md if you want to be subscribed.

@vcsjones
Copy link
Member Author

Some justification:

I ran VerifyWithRevocation on an ARM64 AWS instance a few thousand times, dumping the CRL and OCSP cache between each run by deleting ~/.dotnet/corefx/cryptography/crls and ~/.dotnet/corefx/cryptography/ocsp. I was unable to get it to fail with a stable network connection, so I am not aware of an actual underlying issue that the test is catching.

@danmoseley danmoseley requested a review from krwq September 24, 2020 17:45
@vcsjones
Copy link
Member Author

This could use an outerloop run when anyone has a moment: /azp run runtime-libraries-coreclr outerloop

@bartonjs
Copy link
Member

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bartonjs
Copy link
Member

None of the failures in HTTP (which look to be various kinds of timeouts) should have been caused by this test-only change (at least, it's not obvious that the timeouts are caused by some sort of automagical call to RevocationResponder.Stop()), so merging.

@bartonjs bartonjs merged commit d062c63 into dotnet:master Sep 29, 2020
@vcsjones vcsjones deleted the fix-41952 branch September 29, 2020 15:46
@vcsjones
Copy link
Member Author

@bartonjs I don't know what the bar is for porting test enhancements, but would it make sense to port this and perhaps #42680 to release/5.0 to stabilize the branch?

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failure: System.Security.Cryptography.X509Certificates.Tests.ChainTests.VerifyWithRevocation
3 participants