Merge pull request #1053 from bUnit-dev/release/v1.19

Release of new minor version v1.19

Author: linkdotnet

.config/dotnet-tools.json

@@ -3,7 +3,7 @@
   "isRoot": true,
   "tools": {
     "dotnet-serve": {
-      "version": "1.7.125",
+      "version": "1.10.155",
       "commands": [
         "dotnet-serve"
       ]

.editorconfig

@@ -505,3 +505,4 @@ dotnet_diagnostic.S3459.severity = none # S3459: Unassigned members should be re
 dotnet_diagnostic.S3871.severity = none # S3871: Exception types should be "public"
 dotnet_diagnostic.S1186.severity = none # S1186: Methods should not be empty
 dotnet_diagnostic.S1133.severity = none # S1133: Deprecated code should be removed
+dotnet_diagnostic.S3963.severity = none # S3963: "static" fields should be initialized inline (covered by CA1810)

CHANGELOG.md

@@ -6,6 +6,14 @@ All notable changes to **bUnit** will be documented in this file. The project ad
 
 ## [Unreleased]
 
+### Fixed
+
+- Custom elements with attributes throw `ArgumentException` with `MarkupMatches`. Reported by [@candritzky](https://github.com/candritzky). Fixed by [@linkdotnet](https://github.com/linkdotnet).
+
+### Changed 
+
+- Changed test renderer such that updates to rendered components markup happen in the same synchronization context as the test framework is using (if any), if any, to avoid memory race conditions. By [@egil](https://github.com/egil).
+
 ## [1.18.4] - 2023-02-26
 
 ### Fixed

Directory.Build.props

@@ -19,9 +19,9 @@
 	<PropertyGroup Label="Compile settings">
 		<Nullable>enable</Nullable>
 		<LangVersion>11.0</LangVersion>
-		<AccelerateBuildsInVisualStudio>true</AccelerateBuildsInVisualStudio>
+		<AccelerateBuildsInVisualStudio>false</AccelerateBuildsInVisualStudio>
 		<ImplicitUsings>enable</ImplicitUsings>
-		<NoWarn>CA1014,NU5104</NoWarn>
+		<NoWarn>CA1014,NU5104,NETSDK1138</NoWarn>
 		<CheckEolTargetFramework>false</CheckEolTargetFramework>
 
 		<!-- Used by code coverage -->
@@ -50,7 +50,7 @@
 	<!-- Shared code analyzers used for all projects in the solution -->
 	<ItemGroup Label="Code Analyzers">
 		<PackageReference Include="AsyncFixer" Version="1.6.0" PrivateAssets="All" />
-		<PackageReference Include="SonarAnalyzer.CSharp" Version="8.53.0.62665" PrivateAssets="All" />
+		<PackageReference Include="SonarAnalyzer.CSharp" Version="8.56.0.67649" PrivateAssets="All" />
 	</ItemGroup>
 
 	<ItemGroup Label="Implicit usings"

src/bunit.core/Extensions/WaitForHelpers/WaitForFailedException.cs

@@ -1,5 +1,3 @@
-using System.ComponentModel;
-
 namespace Bunit.Extensions.WaitForHelpers;
 
 /// <summary>

src/bunit.core/Rendering/RootComponent.cs

@@ -12,9 +12,10 @@ internal sealed class RootComponent : IComponent
 
 	public void Attach(RenderHandle renderHandle) => this.renderHandle = renderHandle;
 
-	public Task SetParametersAsync(ParameterView parameters) => throw new InvalidOperationException($"{nameof(RootComponent)} shouldn't receive any parameters");
+	public Task SetParametersAsync(ParameterView parameters)
+		=> throw new InvalidOperationException($"{nameof(RootComponent)} shouldn't receive any parameters");
 
 	public void Render() => renderHandle.Render(renderFragment);
 
 	public void Detach() => renderHandle.Render(_ => { });
-}
+}

src/bunit.core/Rendering/RootRenderTree.cs

@@ -91,16 +91,9 @@ public RenderFragment Wrap(RenderFragment target)
 	public int GetCountOf<TComponent>()
 		where TComponent : IComponent
 	{
-		var result = 0;
 		var countType = typeof(TComponent);
 
-		for (int i = 0; i < registrations.Count; i++)
-		{
-			if (countType == registrations[i].ComponentType)
-				result++;
-		}
-
-		return result;
+		return registrations.Count(t => countType == t.ComponentType);
 	}
 
 	private static RenderFragment<RenderFragment> CreateRenderFragmentBuilder<TComponent>(Action<ComponentParameterCollectionBuilder<TComponent>>? parameterBuilder)

src/bunit.core/Rendering/TestRenderer.cs

@@ -8,6 +8,8 @@ namespace Bunit.Rendering;
 /// </summary>
 public class TestRenderer : Renderer, ITestRenderer
 {
+	private readonly object renderTreeUpdateLock = new();
+	private readonly SynchronizationContext? usersSyncContext = SynchronizationContext.Current;
 	private readonly Dictionary<int, IRenderedFragmentBase> renderedComponents = new();
 	private readonly List<RootComponent> rootComponents = new();
 	private readonly ILogger<TestRenderer> logger;
@@ -79,34 +81,39 @@ public Task DispatchEventAsync(
 		if (fieldInfo is null)
 			throw new ArgumentNullException(nameof(fieldInfo));
 
-		var result = Dispatcher.InvokeAsync(() =>
+		// Calling base.DispatchEventAsync updates the render tree
+		// if the event contains associated data.
+		lock (renderTreeUpdateLock)
 		{
-			ResetUnhandledException();
-
-			try
-			{
-				return base.DispatchEventAsync(eventHandlerId, fieldInfo, eventArgs);
-			}
-			catch (ArgumentException ex) when (string.Equals(ex.Message, $"There is no event handler associated with this event. EventId: '{eventHandlerId}'. (Parameter 'eventHandlerId')", StringComparison.Ordinal))
+			var result = Dispatcher.InvokeAsync(() =>
 			{
-				if (ignoreUnknownEventHandlers)
+				ResetUnhandledException();
+
+				try
+				{
+					return base.DispatchEventAsync(eventHandlerId, fieldInfo, eventArgs);
+				}
+				catch (ArgumentException ex) when (string.Equals(ex.Message, $"There is no event handler associated with this event. EventId: '{eventHandlerId}'. (Parameter 'eventHandlerId')", StringComparison.Ordinal))
 				{
-					return Task.CompletedTask;
+					if (ignoreUnknownEventHandlers)
+					{
+						return Task.CompletedTask;
+					}
+
+					var betterExceptionMsg = new UnknownEventHandlerIdException(eventHandlerId, fieldInfo, ex);
+					return Task.FromException(betterExceptionMsg);
 				}
+			});
 
-				var betterExceptionMsg = new UnknownEventHandlerIdException(eventHandlerId, fieldInfo, ex);
-				return Task.FromException(betterExceptionMsg);
+			if (result.IsFaulted && result.Exception is not null)
+			{
+				HandleException(result.Exception);
 			}
-		});
 
-		if (result.IsFaulted && result.Exception is not null)
-		{
-			HandleException(result.Exception);
-		}
-
-		AssertNoUnhandledExceptions();
+			AssertNoUnhandledExceptions();
 
-		return result;
+			return result;
+		}
 	}
 
 	/// <inheritdoc/>
@@ -124,7 +131,6 @@ public IReadOnlyList<IRenderedComponentBase<TComponent>> FindComponents<TCompone
 		where TComponent : IComponent
 		=> FindComponents<TComponent>(parentComponent, int.MaxValue);
 
-
 	/// <inheritdoc />
 	public void DisposeComponents()
 	{
@@ -151,13 +157,54 @@ public void DisposeComponents()
 		AssertNoUnhandledExceptions();
 	}
 
+	/// <inheritdoc/>
+	protected override void ProcessPendingRender()
+	{
+		// Blocks updates to the renderers internal render tree
+		// while the render tree is being read elsewhere.
+		// base.ProcessPendingRender calls UpdateDisplayAsync,
+		// so there is no need to lock in that method.
+		lock (renderTreeUpdateLock)
+		{
+			base.ProcessPendingRender();
+		}
+	}
+
 	/// <inheritdoc/>
 	protected override Task UpdateDisplayAsync(in RenderBatch renderBatch)
 	{
-		logger.LogNewRenderBatchReceived();
+		if (usersSyncContext is not null && usersSyncContext != SynchronizationContext.Current)
+		{
+			// The users' sync context, typically one established by
+			// xUnit or another testing framework is used to update any
+			// rendered fragments/dom trees and trigger WaitForX handlers.
+			// This ensures that changes to DOM observed inside a WaitForX handler
+			// will also be visible outside a WaitForX handler, since
+			// they will be running in the same sync context. The theory is that 
+			// this should mitigate the issues where Blazor's dispatcher/thread is used 
+			// to verify an assertion inside a WaitForX handler, and another thread is 
+			// used again to access the DOM/repeat the assertion, where the change
+			// may not be visible yet (another theory about why that may happen is different 
+			// CPU cache updates not happening immediately).
+			//
+			// There is no guarantee a caller/test framework has set a sync context.
+			usersSyncContext.Send(static (state) =>
+			{
+				var (renderBatch, renderer) = ((RenderBatch, TestRenderer))state!;
+				renderer.UpdateDisplay(renderBatch);
+			}, (renderBatch, this));
+		}
+		else
+		{
+			UpdateDisplay(renderBatch);
+		}
+
+		return Task.CompletedTask;
+	}
 
+	private void UpdateDisplay(in RenderBatch renderBatch)
+	{
 		RenderCount++;
-		
 		var renderEvent = new RenderEvent(renderBatch, new RenderTreeFrameDictionary());
 
 		// removes disposed components
@@ -177,12 +224,12 @@ protected override Task UpdateDisplayAsync(in RenderBatch renderBatch)
 		// notify each rendered component about the render
 		foreach (var (key, rc) in renderedComponents.ToArray())
 		{
-			logger.LogComponentRendered(rc.ComponentId);
-
 			LoadRenderTreeFrames(rc.ComponentId, renderEvent.Frames);
 
 			rc.OnRender(renderEvent);
 
+			logger.LogComponentRendered(rc.ComponentId);
+
 			// RC can replace the instance of the component it is bound
 			// to while processing the update event.
 			if (key != rc.ComponentId)
@@ -191,10 +238,6 @@ protected override Task UpdateDisplayAsync(in RenderBatch renderBatch)
 				renderedComponents.Add(rc.ComponentId, rc);
 			}
 		}
-
-		logger.LogChangedComponentsMarkupUpdated();
-
-		return Task.CompletedTask;
 	}
 
 	/// <inheritdoc/>
@@ -255,63 +298,56 @@ private IReadOnlyList<IRenderedComponentBase<TComponent>> FindComponents<TCompon
 		if (parentComponent is null)
 			throw new ArgumentNullException(nameof(parentComponent));
 
-		// Ensure FindComponents runs on the same thread as the renderer,
-		// and that the renderer does not perform any renders while
-		// FindComponents is traversing the current render tree.
-		// Without this, the render tree could change while FindComponentsInternal
-		// is traversing down the render tree, with indeterministic as a results.
-		return Dispatcher.InvokeAsync(() =>
-		{
-			var result = new List<IRenderedComponentBase<TComponent>>();
-			var framesCollection = new RenderTreeFrameDictionary();
+		var result = new List<IRenderedComponentBase<TComponent>>();
+		var framesCollection = new RenderTreeFrameDictionary();
 
+		// Blocks the renderer from changing the render tree
+		// while this method searches through it.
+		lock (renderTreeUpdateLock)
+		{
 			FindComponentsInRenderTree(parentComponent.ComponentId);
+		}
 
-			return result;
+		return result;
 
-			void FindComponentsInRenderTree(int componentId)
-			{
-				var frames = GetOrLoadRenderTreeFrame(framesCollection, componentId);
+		void FindComponentsInRenderTree(int componentId)
+		{
+			var frames = GetOrLoadRenderTreeFrame(framesCollection, componentId);
 
-				for (var i = 0; i < frames.Count; i++)
+			for (var i = 0; i < frames.Count; i++)
+			{
+				ref var frame = ref frames.Array[i];
+				if (frame.FrameType == RenderTreeFrameType.Component)
 				{
-					ref var frame = ref frames.Array[i];
-					if (frame.FrameType == RenderTreeFrameType.Component)
+					if (frame.Component is TComponent component)
 					{
-						if (frame.Component is TComponent component)
-						{
-							result.Add(GetOrCreateRenderedComponent(framesCollection, frame.ComponentId, component));
-
-							if (result.Count == resultLimit)
-								return;
-						}
-
-						FindComponentsInRenderTree(frame.ComponentId);
+						result.Add(GetOrCreateRenderedComponent(framesCollection, frame.ComponentId, component));
 
 						if (result.Count == resultLimit)
 							return;
 					}
+
+					FindComponentsInRenderTree(frame.ComponentId);
+
+					if (result.Count == resultLimit)
+						return;
 				}
 			}
-		}).GetAwaiter().GetResult();
+		}
 	}
 
 	IRenderedComponentBase<TComponent> GetOrCreateRenderedComponent<TComponent>(RenderTreeFrameDictionary framesCollection, int componentId, TComponent component)
 		where TComponent : IComponent
 	{
-		IRenderedComponentBase<TComponent> result;
-
 		if (renderedComponents.TryGetValue(componentId, out var renderedComponent))
 		{
-			result = (IRenderedComponentBase<TComponent>)renderedComponent;
-		}
-		else
-		{
-			LoadRenderTreeFrames(componentId, framesCollection);
-			result = activator.CreateRenderedComponent(componentId, component, framesCollection);
-			renderedComponents.Add(result.ComponentId, result);
+			return (IRenderedComponentBase<TComponent>)renderedComponent;
 		}
 
+		LoadRenderTreeFrames(componentId, framesCollection);
+		var result = activator.CreateRenderedComponent(componentId, component, framesCollection);
+		renderedComponents.Add(result.ComponentId, result);
+
 		return result;
 	}
 

src/bunit.core/TestContextBase.cs

@@ -13,18 +13,7 @@ public abstract class TestContextBase : IDisposable
 	/// <summary>
 	/// Gets the renderer used by the test context.
 	/// </summary>
-	public ITestRenderer Renderer
-	{
-		get
-		{
-			if (testRenderer is null)
-			{
-				testRenderer = Services.GetRequiredService<ITestRenderer>();
-			}
-
-			return testRenderer;
-		}
-	}
+	public ITestRenderer Renderer => testRenderer ??= Services.GetRequiredService<ITestRenderer>();
 
 	/// <summary>
 	/// Gets the service collection and service provider that is used when a
@@ -90,7 +79,7 @@ protected virtual void Dispose(bool disposing)
 
 		// Ignore the async task as GetAwaiter().GetResult() can cause deadlock
 		// and implementing IAsyncDisposable in TestContext will be a breaking change.
-		// 
+		//
 		// NOTE: This has to be called before Services.Dispose().
 		// If there are IAsyncDisposable services registered, calling Dispose first
 		// causes the service provider to throw an exception.

src/bunit.core/TestServiceProvider.cs

@@ -1,5 +1,4 @@
 using System.Collections;
-using Microsoft.Extensions.DependencyInjection;
 
 namespace Bunit;