Investigate running test code and renderer code in the same thread/sync context

[egil]

The library has some rather complex locking checks in place to avoid memory coherence problems, that can cause tests to fail when the test code and renderer code is not running on the same core/cpu.

It might be worth switching to a model, where both test code and renderer share the same dispatcher/sync context. So let's investigate. Here are a few questions to answer:

• How do we avoid test and component under test/renderer deadlocking each other?
• What compromises do we have to make for the user, e.g. will writing tests become more complicated for 90% of the cases, or just for the 10% that involves async rendering?
• Are there any special requirements from Blazor?
• Can a general solution be created that is independent of test framework, e.g. nunit/xunit/mstest?

Related issue: #118

[egil]

Inspiration from @jspuij: I've written my own for BlazorWebView, because I have a dedicated thread running the Blazor Renderer. This is more like the classic desktop "UI" model, where you have a single thread running the UI. The thread hopping on the thread pool that the server side renderer exhibits might confuse desktop users. https://github.com/jspuij/BlazorWebView/blob/master/src/BlazorWebView/PlatformDispatcher.cs
and https://github.com/jspuij/BlazorWebView/blob/master/src/BlazorWebView/PlatformSynchronizationContext.cs

[jspuij]

Your safest bet would be to use a variant of my dispatcher. Then run the tests on the threadpool (most test runners probably will, but dedicated threads will probably as well). Create a SynchronizationContext for async operations or use this one: https://github.com/StephenCleary/AsyncEx/wiki/AsyncContext.
You can probably remove all locks / volatile reads / fences if you make sure that you run on the AsyncContext or invoke back to it.
To pass around state, I'd create a "TestContext" that is passed around with all calls that has all state for the currently executing test. If you don't want to do that, or hide that state, you can use AsyncLocal.

[egil]

@jspuij xUnit runs tests on the thread pool and has, as far as I know, it's own async context. I am pretty sure the other test frameworks does something similar.

Could a strategy be to adopt the test frameworks async context/dispatcher? I.e. simply pass the already active async context when the test runs to the Renderer.

[jspuij]

Yes, that will work. You can probably check SynchronizationContext.Current to see if there is one. If there is one (xunit) good, if there isn't one, create your own. Some test frameworks won't provide them, some will only provide them if the test is async (e.g. returns task).

[JeroenBos]

It might be worth switching to a model, where both test code and renderer share the same dispatcher/sync context.

I will argue that this is impossible.

I arrived at this issue because I ran into another Dispatcher.InvokeAsync(Action).Wait() being called instead of Dispatcher.InvokeAsync(Func<Task>).Wait() in Bunit.Rendering.TestRenderer.RenderFragmentInsideWrapper(RenderFragment renderFragment).
I was about the file a bug report until I realized that there's not even a way around this. The reason is a restriction posed by Blazor: namely that the delegate Microsoft.AspNetCore.Components.RenderFragment is void-returning.

A void-returning async method is in principle not awaitable. Under the assumption that you cannot modify the void-returning async method, there's no operation you can do to ensure that all its asynchronous continuations are finished: see e.g. this stackoverflow question.

I realize that you're already talking about creating a custom SynchronizationContext, which I think may be the only way but then still I don't see it. Admittedly that's over my head. If that route might still be possible I just wanted to point out this complication.

---

Also in #177 I asked the question "Why do we need the WaitFor* methods, why can't the bUnit API not just return tasks?"

I got the answer that returning void is simpler for the users, which I found a bit unsatisfactory because to me tasks are simpler. But now I understand the real answer: it's impossible to return tasks because blazor doesn't return the tasks.

Finally I remark that this argument is imho worth mentioning to explain why you need those methods e.g. here.

[egil]

@JeroenBos thanks for the input. My gut feeling is that you are correct.

In some places Blazor does return a Task, in particular when we trigger event handlers, so in some cases it might work.

Can you describe the scenario you mention in more details?

[JeroenBos]

Well my particular scenario might not be so interesting because I'm shooting myself in the foot by using MSTest.
Nevertheless the details could help so I'll explain. I did something similar to in SnapshotTest:

public class MyTests // : RazorTestBase 
{
        [TestMethod]
	public async Task Run() // similar to SnapshotTest.Run
	{
		Services.AddDefaultTestContextServices();
		var (cutId, cut) = this.Renderer.RenderComponent<MyComponent>(parameters);
		var icut = cut.ToIRenderedComponent(cutId, this.Services);
		using var waiter = new WaitForStateHelper(icut, somePredicate);
		// some assertion
	}
}


/// MyComponent.razor:
@code 
  {
	public override async Task SetParametersAsync(ParameterView parameters)
	{
		await Task.Delay(100);
	}
}

But this doesn't work and can never work without the cooperation of the SynchronizationContext.
The line icut = cut.ToIRenderedComponent is never reached because MSTest uses the default SynchronizationContext.
The flow is like this:

• We arrive in the Run method above from a test (with MSTest we're on the process's main thread).
• Renderer.RenderComponent is called which creates a RenderFragment
• that RenderFragment is dispatched
• The scheduler (Microsoft.AspNetCore.Components.Rendering.RendererSynchronizationContext) just executes the render synchronously
• We arrive in the SetParametersAsync still synchronously on the main thread, we do the await statement but the SynchronizationContext.Current is null (because MSTest), so the continuation is scheduled on the thread pool
• The main thread has nothing to do anymore because there's nothing to await because the chain of async-all-the-way-down has been broken by a void-returning async method (the RenderFragment)
• so the program terminates mid-test.
• Note that it terminated without there ever having been the change to register the WaitForStateHelper.

---

Without any task to await on the main thread you'd have to resort to hacks to keep the process alive.

My first reaction was that I should be able to await Renderer.RenderComponent to keep the main thread alive, but in reality it's because Blazor swallows the task, which is why I don't consider it an issue with bUnit. Although I would still love a SynchronizationContext such that I could await Renderer.RenderComponent (and thereby being closer to fully supporting MSTest) and do await with WaitFor-helpers, I realize that Blazor is making it difficult.

[egil]

Thanks @JeroenBos.

Which version of MSTest are you using, and does it allow you to specify a custom sync context? I ask because if not, then I probably need to stop telling folks that bUnit and MSTest works together.

Btw., im curious, why are you using WaitForStateHelper directly instead of the WaitFor... extension methods?

[JeroenBos]

I reference MSTest like so: <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.4.0" />. The latest version is 16.7.1 so that is unlikely to make a difference.

MSTest doesn't provide a way to set a sync context, nor do I know of a sufficient sync context out of the box. My solution looks something like this using the AsyncEx library (also mentioned above by jspuij). Using that context everything works like a charm.

As for the WaitForStateHelper. I don't have a good reason, by accidental fluke. I never changed it, but I will now! It's simply because I discovered the helper before the extension methods. Not because the documentation isn't good, but because I'm limited by linux/vscode and Omnisharp's intellisense just isn't good enough to do library exploration so I manually browsed the source code.

Another remark about whether MSTest and bUnit are compatible: Blazor has a RendererSynchronizationContext but it just isn't set to SynchronizationContext.Current. I thought it would maybe suffice to just manually do so (via reflection), but alas. So I'm afraid I would agree that they're not really compatible, however they can easily be made so by merely setting the sync context to Stephen Cleary's.

[egil]

OK. It would be super helpful to other would-be MSTest+bUnit folks if you could create small hello world sample test project where you show this. No fancy tests, just the bare minimum.

Then I can update the docs or perhaps create a template for those that need to use MSTest.

[egil]

Moving this to the backlog. It might be possible, but currently it doesn't seem like it.

[jspuij]

My first reaction was that I should be able to await Renderer.RenderComponent to keep the main thread alive, but in reality it's because Blazor swallows the task, which is why I don't consider it an issue with bUnit. Although I would still love a SynchronizationContext such that I could await Renderer.RenderComponent (and thereby being closer to fully supporting MSTest) and do await with WaitFor-helpers, I realize that Blazor is making it difficult.

This is true and by design, otherwise Blazor would never be able to render anything untill the last task is awaited, so it runs through a synchronous path, while firing off async tasks that will complete in the future (and might trigger another render) and are never awaited.

The easiest way to make sure that the test finished is to create a TaskCompletionSource on the main thread that you await, and trigger the completion of the TCS inside the async Blazor Lifecycle methods.

[egil]

The easiest way to make sure that the test finished is to create a TaskCompletionSource on the main thread that you await, and trigger the completion of the TCS inside the async Blazor Lifecycle methods.

That would probably be a workaround for making RenderComponent async/awaitable.

But if we are in a scenario where a components life cycle methods awaits an uncompleted task during the initial render, e.g. a timer task, the second render will happen later after the await of RenderComponent is over in the test. So for certain test cases, where users want to verify the component instance/markup between the first and second render, they would need a "wait for" method, like the ones in bUnit already, to await one or more renders, before continuing the test, e.g. yield to the renderer so it can process any awaiting async renders.

Unless I am missing something, putting things in the same sync context wont really make a difference to the APIs then.

[jspuij]

The easiest way to make sure that the test finished is to create a TaskCompletionSource on the main thread that you await, and trigger the completion of the TCS inside the async Blazor Lifecycle methods.

That would probably be a workaround for making RenderComponent async/awaitable.

But if we are in a scenario where a components life cycle methods awaits an uncompleted task during the initial render, e.g. a timer task, the second render will happen later after the await of RenderComponent is over in the test. So for certain test cases, where users want to verify the component instance/markup between the first and second render, they would need a "wait for" method, like the ones in bUnit already, to await one or more renders, before continuing the test, e.g. yield to the renderer so it can process any awaiting async renders.

Unless I am missing something, putting things in the same sync context wont really make a difference to the APIs then.

You are right. I'm not familiar with bUnit (sorry, shame on me), so I understand that you've got WaitFor methods that wait for elements to occur. I was just suggesting for @JeroenBos to use a TCS inside his own test, but he can use WaitFor just as well. It's inherent to the Blazor architecture and no SynchronizationContext will change this.

[egil]

OK, in that case I feel confident enough to close this issue. If somebody figures out a good way to do this that improves the usability of bUnit, we can revisit this.