fix: TriggerEventAsync runs in renderer sync context to avoid race condition with DOM updates

fixes #687

Author: egil

CHANGELOG.md

@@ -14,6 +14,8 @@ All notable changes to **bUnit** will be documented in this file. The project ad
 
 - A race condition existed between `WaitForState` / `WaitForAssertion` and `FindComponents`, if the first used the latter. Reported by [@rmihael](https://github.com/rmihael), [@SviatoslavK](https://github.com/SviatoslavK), and [@RaphaelMarcouxCTRL](https://github.com/RaphaelMarcouxCTRL). Fixed by [@egil](https://github.com/egil) and [@linkdotnet](https://github.com/linkdotnet).
 
+- Triggering of event handlers now runs entirely inside the renderers synchronization context, avoiding race condition between elements in the DOM tree being updated by the renderer and the event triggering logic traversing the DOM tree to find event handlers to trigger. Reported by [@FlukeFan](https://github.com/FlukeFan). Fixed by [@egil](https://github.com/egil).
+
 ## [1.8.15] - 2022-05-19
 
 ### Added

src/bunit.web/EventDispatchExtensions/TriggerEventDispatchExtensions.cs

@@ -1,4 +1,5 @@
 using System.Globalization;
+using System.Runtime.ExceptionServices;
 using AngleSharp.Dom;
 using AngleSharp.Html.Dom;
 using AngleSharpWrappers;
@@ -56,7 +57,6 @@ public static void TriggerEvent(this IElement element, string eventName, EventAr
 	/// <param name="eventName">The name of the event to raise (using on-form, e.g. <c>onclick</c>).</param>
 	/// <param name="eventArgs">The event arguments to pass to the event handler.</param>
 	/// <returns>A <see cref="Task"/> that completes when the render caused by the triggering of the event finishes.</returns>
-	[SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "HTML events are standardize to lower case and safe in this context.")]
 	public static Task TriggerEventAsync(this IElement element, string eventName, EventArgs eventArgs)
 	{
 		if (element is null)
@@ -67,18 +67,32 @@ public static Task TriggerEventAsync(this IElement element, string eventName, Ev
 		var renderer = element.GetTestContext()?.Renderer
 			?? throw new InvalidOperationException($"Blazor events can only be raised on elements rendered with the Blazor test renderer '{nameof(ITestRenderer)}'.");
 
-		var isNonBubblingEvent = NonBubblingEvents.Contains(eventName.ToLowerInvariant());
+		// TriggerEventsAsync will traverse the DOM tree to find
+		// all event handlers that needs to be triggered. This is done
+		// in the renderes synchronization context to avoid a race condition
+		// between the DOM tree being updated and traversed.
+		var result = renderer.Dispatcher.InvokeAsync(
+			() => TriggerEventsAsync(renderer, element, eventName, eventArgs));
+
+		ThrowIfFailedSynchronously(result);
+
+		return result;
+	}
 
+	[SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "HTML events are standardize to lower case and safe in this context.")]
+	private static Task TriggerEventsAsync(ITestRenderer renderer, IElement element, string eventName, EventArgs eventArgs)
+	{
+		var isNonBubblingEvent = NonBubblingEvents.Contains(eventName.ToLowerInvariant());
 		var unwrappedElement = element.Unwrap();
 		if (isNonBubblingEvent)
 			return TriggerNonBubblingEventAsync(renderer, unwrappedElement, eventName, eventArgs);
 
 		return unwrappedElement switch
 		{
-			IHtmlInputElement { Type: "submit", Form: not null } input when eventName is "onclick" =>
-				TriggerFormSubmitBubblingEventAsync(renderer, input, eventArgs, input.Form),
-			IHtmlButtonElement { Type: "submit", Form: not null } button when eventName is "onclick" =>
-				TriggerFormSubmitBubblingEventAsync(renderer, button, eventArgs, button.Form),
+			IHtmlInputElement { Type: "submit", Form: not null } input when eventName is "onclick"
+				=> TriggerFormSubmitBubblingEventAsync(renderer, input, eventArgs, input.Form),
+			IHtmlButtonElement { Type: "submit", Form: not null } button when eventName is "onclick"
+				=> TriggerFormSubmitBubblingEventAsync(renderer, button, eventArgs, button.Form),
 			_ => TriggerBubblingEventAsync(renderer, unwrappedElement, eventName, eventArgs)
 		};
 	}
@@ -166,4 +180,19 @@ private static bool TryGetEventId(this IElement element, string blazorEventName,
 		var eventId = element.GetAttribute(blazorEventName);
 		return ulong.TryParse(eventId, NumberStyles.Integer, CultureInfo.InvariantCulture, out id);
 	}
+
+	private static void ThrowIfFailedSynchronously(Task result)
+	{
+		if (result.IsFaulted && result.Exception is not null)
+		{
+			if (result.Exception.InnerExceptions.Count == 1)
+			{
+				ExceptionDispatchInfo.Capture(result.Exception.InnerExceptions[0]).Throw();
+			}
+			else
+			{
+				ExceptionDispatchInfo.Capture(result.Exception).Throw();
+			}
+		}
+	}
 }

tests/.editorconfig

@@ -42,3 +42,5 @@ dotnet_diagnostic.CA2007.severity = none            # https://github.com/atc-net
 
 dotnet_diagnostic.CA1819.severity = suggestion		# CA1819: Properties should not return arrays
 dotnet_diagnostic.CA1849.severity = suggestion		# CA1849: Call async methods when in an async method
+
+dotnet_diagnostic.xUnit1026.severity = none			# xUnit1026: Theory methods should use all of their parameters

tests/bunit.testassets/SampleComponents/CounterComponentDynamic.razor

@@ -0,0 +1,23 @@
+<h1>Counter Dynamic</h1>
+@foreach (var index in _values)
+{
+    <button
+		data-id=@index
+		@onclick="() => OnClick(index)">Click</button>
+}
+
+@code {	
+    private int[] _values = new int[0];
+
+    protected override async Task OnInitializedAsync()
+    {
+        await Task.Delay(1);
+        _values = new[] { 1 };
+        await InvokeAsync(StateHasChanged);
+    }
+
+    private void OnClick(int index)
+    {
+        _values = Enumerable.Range(1, index + 1).ToArray();
+    }
+}

tests/bunit.web.tests/EventDispatchExtensions/GeneralEventDispatchExtensionsTest.cs

@@ -266,8 +266,22 @@ public void Test306()
 		Should.Throw<InvalidOperationException>(() => cut.Find("button").Submit());
 	}
 
+	public static IEnumerable<object[]> GetTenNumbers() => Enumerable.Range(0, 10)
+		.Select(i => new object[] { i });
+
+	// Runs the test multiple times to trigger the race condition
+	// reliably.
+	[SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "Needed to trigger multiple reruns of test.")]
+	[Theory(DisplayName = "TriggerEventAsync avoids race condition with DOM tree updates")]
+	[MemberData(nameof(GetTenNumbers))]
+	public void Test400(int i)
+	{
+		var cut = RenderComponent<CounterComponentDynamic>();
+
+		cut.WaitForAssertion(() => cut.Find("[data-id=1]"));
+
+		cut.InvokeAsync(() => cut.Find("[data-id=1]").Click());
 
-			Should.Throw<InvalidOperationException>(() => cut.Find("button").Submit());
-		}
+		cut.WaitForAssertion(() => cut.Find("[data-id=2]"));
 	}
 }