Add IHttpUpgradeFeature to TestServer for SignalR WebSocket support #33595
Conversation
| @@ -54,5 +55,48 @@ public async Task ConnectAsync_ShouldSetRequestProperties(string requestUri, str | |||
| Assert.Equal(expectedHost, capturedHost); | |||
| Assert.Equal("/connect", capturedPath); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public async Task CanAcceptWebSocket() | |||
There was a problem hiding this comment.
You want a different test for the normal TestServer scenario that shows the upgrade and WebSocket features are present, but an upgrade isn't possible.
|
@Pilchie for preview6 consideration |
| }); | ||
| }))) | ||
| { | ||
| var client = testServer.CreateWebSocketClient(); |
There was a problem hiding this comment.
Sorry, I meant that you'd send a non-websocket request here like you do in the signalr negotiation scenario. Including app.UseWebSockets(); in this test makes sense and the checking that it does add the IHttpWebSocketFeature SignalR needs.
|
Can you send tactics a mail for approval at this point? Since builds have been sent out to start validation, I don't want to reset them without approval. |
Darn, I don't think it's worth resetting for this. |
|
Actually, I might have confused Preview 6 with servicing builds, I don't think those are out, but most teams are mailing Tactics for approval. |
|
@dotnet/aspnet-build Tactics approved over email this morning, merge when convenient! |
…33595) * Add IHttpUpgradeFeature to TestServer for SignalR WebSocket support * fb * more test
amcasey
added
area-networking
SignalR checks for
IHttpWebSocketFeaturewhich is set by the WebSocket middleware if theIHttpUpgradeFeatureexists.aspnetcore/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs
Lines 377 to 380 in 4fc4748
I don't know how much validation we need if any for saying that the request supports websockets. Also, currently throwing from
IHttpUpgradeFeature.UpgradeAsyncsince TestServer doesn't use that for the stream.Going to see if we can get this into preview6 since the new
WebSocketFactoryoption was added to SignalR in preview5 with the intent to allow TestServer websockets to work with the client.