dotnet / dotnet/reactive

Some tests depend on a SynchronizationContext

Open
#1,885 3 comments 1 reaction 0 assignees View on GitHub
[area] Rx test-failure
Dominant language
C#
Stars
7.2k
Forks
798
PR merge metrics
No merged PRs in 30d

Description

The `TaskLikeSupportTest.Basics` test, and the `EventLoop_ScheduleActionDue` and `EventLoop_ScheduleActionNested` tests in `EventLoopSchedulerTest` were hanging sporadically after moving from xUnit to MSTest. This is indicative of some underlying problem—the success or failure of a test shouldn't depend on which particular test framework you are using.

One difference between the test frameworks is that xUnit always supplies a non-null `SynchronizationContext.Current` to tests (apparently to support `async void` tests). MSTest does not. (It supplies a code analyzer that detects `async void` tests and tells you not to use them.) The presence of a synchronization context can cause Rx to execute different code paths,.

This appears to be why the `TaskLikeSupportTest.Basics` test was failing. We've modified the code to set up a `SynchronizationContext` explicitly, and this appears to have got rid of the hange, but this is only a workaround. We need to establish whether there's something about the test that truly requires a `SynchronizationContext` for the code to be valid (which is possible, but if so, it's not obvious why), or whether we really should expect it to work with or without a `SynchronizationContext`. If it's the latter, that indicates a subtle bug somewhere, in which case the long term solution should be to have two forms of the test, one with the `SynchronizationContext` and one without, and make whatever fix is required for the test to pass reliably on both.

As for the two problematic tests in `EventLoopSchedulerTest`, these continued to hang even after explicitly setting up a `SynchronizationContext`. So whatever it is about xUnit that was different that they depended on, it apparently wasn't just the presence/absence of a `SynchronizationContext`.

For now, those two tests have been annotated with `[Ignore]` so that we can move forward, but these problems need to be fixed properly.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.