Fix: non-async tests hang sometimes when UIThread = true

[josetr]

Xunit.Sdk.AsyncTestSyncContext seems to be the problem and it's only purpose is to support async void test methods.

So the problem is solved if we dodge it when the test method is not async and UIThread = true.

[sharwell]

It might be worth considering an alternative where the test is immediately reported as failing if it's a UI thread test but isn't async Task. It's limiting in theory, but it probably isn't what you want anyway.

[josetr]

Apparently async void tests suffer from the same problem. The call to WaitForCompletionAsync never completes on both cases. async Task tests do work because they do not call WaitForCompletionAsync at all.

Task taskFromResult = GetTaskFromResult(CallTestMethod(testClassInstance));
if (taskFromResult != null)
    await taskFromResult;
else
{
    Exception ex = await asyncSyncContext.WaitForCompletionAsync();
  
}

[sharwell]

It's interesting that xunit has a way to run async void tests in most cases, but it's not especially valuable. async void should never be used in unit tests or otherwise. Visual Studio extension code will typically be running the vs-threading analyzers which reports a warning for violations of this, so I would not expect async void tests to appear in code using this library.

[josetr]

Agree I just wanted to point out that it didn't work since my pull request didn't account for it.

I'm still not sure whether we shouldn't allow synchronous UI tests to just work.

It's much simpler to play with this library if it does, and I'm guessing there are still methods that just call ThreadHelper.ThrowIfNotOnMainThread() at the beginning in the wild.

Should those be avoided if possible by the way?

I'm just making them async and then calling SwitchToMainThreadAsync but just wondering what's the best practice here.

[sharwell]

💭 If you want to undo the AsyncTestSyncContext, you can override CallTestMethod and add this before calling the base implementation:

SynchronizationContext.SetSynchronizationContext(GetEffectiveSynchronizationContext());

Where GetEffectiveSynchronizationContext comes from here:

https://github.com/dotnet/roslyn/blob/948a9e1da970618d8e04cfd254b62659139ae9af/src/Workspaces/CoreTestUtilities/TestExportJoinableTaskContext.cs#L73-L91

[sharwell]

Not sure if that helps or not.