ForceAsyncAwaiter Tests #792
Conversation
mikaelm12
requested review from
BrennanConroy,
analogrelay,
davidfowl and
moozzyk
|
@mikaelm12, |
| var hubConnection = new HubConnection(connection, new LoggerFactory()); | ||
| Assert.NotNull(SynchronizationContext.Current); | ||
| await hubConnection.StartAsync(); | ||
| Assert.Null(SynchronizationContext.Current); |
There was a problem hiding this comment.
This looks wrong. That doesn't happen...
There was a problem hiding this comment.
Yeah, tbh I was kind of confused by this behavior
There was a problem hiding this comment.
How should we be testing this at the HubConnection level?
|
There were issues with using the default SyncContext. Things should look better now. |
|
I think it would be better if - instead of/in addition to using event handlers - you created a mock transport (possibly wrapping an existing transport if it makes it easier) and checking sync context in the transport methods. If you use this pattern you can also test HubConnection async methods by mocking IConnection. |
|
I was able to add tests at the HubConnection level by checking the SyncContexts in the Connected and Closed events. Do you think I should still mock the transport and implement the tests that way? I'd have to brush up on how to use Mocks 😅 |
| { | ||
| await Task.Yield(); | ||
| Assert.Null(SynchronizationContext.Current); | ||
| await Task.CompletedTask; |
There was a problem hiding this comment.
You don't need to await Task.CompletedTask
|
@mikaelm12 ping |
|
Closing for now as this isn't critical atm |
Some tests for the ForceAsyncAwaiter