Convert Sync RemoteExecutors to Async on Http Tests #115422
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request updates several test methods that previously used synchronous RemoteExecutor calls to now use asynchronous calls via await and DisposeAsync, improving test reliability and potential resource management.
- Converted test methods in RuntimeSettingParserTest, TelemetryTest, DiagnosticsTests, HttpClientAuthenticationTest, and HttpClientHandlerTest.Proxy from void to async Task.
- Updated RemoteExecutor.Invoke calls to be awaited with DisposeAsync for proper asynchronous disposal.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Net.Http/tests/UnitTests/RuntimeSettingParserTest.cs | Converted synchronous test methods to async patterns. |
| src/libraries/System.Net.Http/tests/FunctionalTests/TelemetryTest.cs | Adjusted async lambda and disposal for async testing. |
| src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs | Updated async test methods with proper async disposal. |
| src/libraries/System.Net.Http/tests/EnterpriseTests/HttpClientAuthenticationTest.cs | Modified synchronous authentication tests to async. |
| src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs | Changed sync proxy test to async using await for disposal. |
|
Tagging subscribers to this area: @dotnet/ncl |
|
should the awaits have some timeout? While this makes it more friendly to Unit, we can still block indefinitely (or very long time), right? |
No, RemoteExecutor already has timeout mechanism by default it's 60 sec IIRC. |
Applied a generic regex to convert Sync RemoteExecutors tests to Async, might fix some cases, as we observed in the past.
See for more info: #102699