-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Set up HttpClient
for ClientWebSocket.ConnectAsync
#73387
Set up HttpClient
for ClientWebSocket.ConnectAsync
#73387
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue DetailsFixes #72301 Detecting if we get Most part of PR is adding HttpClient/HttpMessageInvoker into all existing tests.
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsFixes #72301 Detecting if we get Most part of PR is adding HttpClient/HttpMessageInvoker into all existing tests.
|
src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs
Outdated
Show resolved
Hide resolved
…ockets/WebSocketHandle.Managed.cs Co-authored-by: Miha Zupan <[email protected]>
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
There's a lot of duplication of ConnectAsync
and GetConnectedWebSocket
in tests.
Can they be implemented as helper methods on ClientWebSocketTestBase
instead, with the only thing for tests to implement being
protected virtual HttpMessageInvoker? GetInvoker();
?
src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/SendReceiveTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/DeflateTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Miha Zupan <[email protected]>
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Failures in the outerloop builds are unrelated |
Fixes #72476
Detecting if we get
HttpClient
as a parameter inConnectAsync
-- in that case anotherSendAsync
method should be called to provide correct options.Most part of PR is adding HttpClient/HttpMessageInvoker into all existing tests.