-
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
Test failure: SendAsync_SlowServerRespondsAfterDefaultReceiveTimeout_ThrowsHttpRequestException #20675
Comments
This test is never going to be 100% reliable in CI. In general, testing cancellation is problematic because cancellation is "cooperative" and not guaranteed. So, in some cases when the CI machine is busy, it might be possible for the cancellation request to take longer than the actual HTTP request. Which means the request succeeds and thus the test fails because the request was not cancelled. |
Why can't it just go against a loopback page that never finishes the request? Then cancellation/timeout can be issued at any point after we issue it. In fact we already have such a test: |
That could be an option. I had previously thought the WinHttpHandlerTest was already against a loopback server. |
Do we need this WinHttpHandler-specific test at all, since the test I linked to would seem to cover the same functionality? |
WinHttpHandler.ReceiveTimeout is different from HttpClient.Timeout. In fact, when HttpClient uses HttpClientHandler, it internally sets all the WinHttpHandler timeouts to "infinite". The cancellation mechansim is very different in the HttpClient.TImeout test compared to WinHttpHandler.ReceiveTimeout If we want good code coverage of the WinHttpHandler specific properties, then we need this test in some way. |
I see, I didn't notice it was a different timeout property. In that case the test we have could be duplicated and modified. |
Given that this is test issue only and that the product actually works and that we do not have infinite time, moving to Future. It would be still nice to have it fixed in 2.0. @hughbe are you interested? ;-) |
Triage: fix the test as suggested above. |
It's still failing after the change, (6 occurrences (2 fail, 4 pass on rerun)). |
Duplicate of #96481 |
https://ci.dot.net/job/dotnet_corefx/job/master/job/outerloop_windows_nt_debug_prtest/148/consoleText
https://ci.dot.net/job/dotnet_corefx/job/master/job/outerloop_windows_nt_debug_prtest/147/consoleText
The text was updated successfully, but these errors were encountered: