-
Notifications
You must be signed in to change notification settings - Fork 5k
Enable GetAsync_CancelDuringResponseBodyReceived_Buffered_TaskCanceledQuickly #32032
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 GetAsync_CancelDuringResponseBodyReceived_Buffered_TaskCanceledQuickly #32032
Conversation
…uickly is enabled with a timeout
What would we see on failure? Should we add more instrumentation so we can nail root cause if it fails ever again? |
@wfurt Currently, it will manifest itself as just "Operation was cancelled" since I call CancelAfter on CTS. Could you please explain what instrumentation can be used here to collect more data? |
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs
Outdated
Show resolved
Hide resolved
If we cause a core file to be created we would have pretty much everything. We could also add _output.WriteLine() to track progress or add state variable with the only purpose to track progress. |
/azp run runtime-libraries outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
@wfurt I'm concerned that constructing a core dump file automatically can cause a huge load on CI infra in case we accidentally break a number of tests due to some mistake. May be we need to discuss it with the test infra team and plan as a separate task. |
My assumption is that failures are rare and hard to reproduce, right? (if not, we can debug it it) All you would need to do in this case would be assert. The rest is already ready to go. |
You can cause core file to be created by calling
We do construct core dump automatically on hard crashes, and the CI is able to handle it. It is not unusual for all tests in the core runtime PRs to crash and create core dump. |
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 list |
Postponed. |
It enables GetAsync_CancelDuringResponseBodyReceived_Buffered_TaskCanceledQuickly test and limits its execution time by 2 minutes.
Fixes #25760