Fix flaky HubConnectionHandler test #18388
Conversation
|
Not looking at the details of this. But, I'm very glad to see the fix is coming. Thanks @BrennanConroy❕ |
| @@ -2813,6 +2875,9 @@ public async Task HubMethodInvokeDoesNotCountTowardsClientTimeout() | |||
|
|
|||
| using (var client = new TestClient(new JsonHubProtocol())) | |||
| { | |||
| var customDuplex = new CustomDuplex(client.Connection.Transport); | |||
| client.Connection.Transport = customDuplex; | |||
|
|
|||
| var connectionHandlerTask = await client.ConnectAsync(connectionHandler); | |||
| // This starts the timeout logic | |||
| await client.SendHubMessageAsync(PingMessage.Instance); | |||
There was a problem hiding this comment.
Is there a risk that the client will timeout between sending the ping and invoking the long running method given the ClientTimeoutInterval is 0 seconds? Can we use a mock system clock or something like that?
There was a problem hiding this comment.
Is there a risk that the client will timeout between sending the ping and invoking the long running method given the ClientTimeoutInterval is 0 seconds? Can we use a mock system clock or something like that?
No risk because that logic lives in HttpConnectionManager and we are only working with the HubConnectionHandler layer.
|
|
||
| public bool IsWaitingForRead() | ||
| { | ||
| return _waitingForRead; |
There was a problem hiding this comment.
Could you make the test pass faster by using a TCS to implement a WaitForRead method that returns a task that completes once a read starts?
There was a problem hiding this comment.
I thought about that but was worried I'd have to write a bunch of synchronization logic to make sure the TCS is reset properly. I'll try it out, the test logic would look a lot nicer if we used a TCS...
analogrelay
added
the
tell-mode
|
Test-only change, so tell-mode. If we can get review signoff (poke @halter73) I can merge this whenever 3.0 is open (and I think it still is today) |
|
Yes, let's take this today while dependencies are still flowing. |
There was a race where the test assumed certain state and ran some code before that state was true.