-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Enable MsQuicTests.ConnectWithClientCertificate on Windows #70015
Conversation
Tagging subscribers to this area: @dotnet/ncl |
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
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
The test is still failing on Edit: I see #69871 was merged, so I will try rerunning it. |
/azp run runtime |
Azure Pipelines successfully started running 1 pipeline(s). |
8fb7358
to
35d1171
Compare
/azp run runtime-extra-platforms |
I think we should use the S2022 to figure out why the test is hanging. Since it does not support client certs, it should be easy to investigate IMHO. And I would apply those changes before enabling Win11. |
well, we have disabled those tests on S2022, we only kept the test disabled on all other windows because it kept failing on Win11, which now seems to be gone now. |
right. We can change that for local investigation. My goal is to make sure it simply fails on failure instead of hanging whole test suite. |
I checked what is going on WinSrv2022 and it seems that our recent changes have made the code to deterministically throw and not hang. (at least there were no hangs for me when I ran the tests in a loop. So I think we can merge this and monitor test results if it starts happening again. |
Test failures are unrelated (this PR was a Windows test-only change) |
Fixes #64944.
Newer MsQuic version is now being consumed since #69127 was merged, so reenabling the test should be okay.
Context: #69843 (comment)
CC: @wfurt.