Prevented race condition between disposing of WaitForHandler and starting timer. Tweaks to FixtureBase to provide better error messages

Author: egil

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

@@ -55,9 +55,21 @@ protected WaitForHelper(IRenderedFragmentBase renderedFragment, Func<bool> compl
 
 			_renderedFragment.OnAfterRender += OnAfterRender;
 			OnAfterRender();
+			StartTimer(timeout);
+		}
+
+		private void StartTimer(TimeSpan? timeout)
+		{
+			if (_isDisposed)
+				return;
+
+			lock (_completionSouce)
+			{
+				if (_isDisposed)
+					return;
 
-			if (!_isDisposed)
 				_timer.Change(GetRuntimeTimeout(timeout), Timeout.InfiniteTimeSpan);
+			}
 		}
 
 		private void OnAfterRender()
@@ -123,11 +135,17 @@ public void Dispose()
 			if (_isDisposed)
 				return;
 
-			_isDisposed = true;
-			_renderedFragment.OnAfterRender -= OnAfterRender;
-			_timer.Dispose();
-			_completionSouce.TrySetCanceled();
-			_logger.LogDebug(new EventId(6, nameof(Dispose)), $"The state wait helper for component {_renderedFragment.ComponentId} disposed");
+			lock (_completionSouce)
+			{
+				if (_isDisposed)
+					return;
+
+				_isDisposed = true;
+				_renderedFragment.OnAfterRender -= OnAfterRender;
+				_timer.Dispose();
+				_completionSouce.TrySetCanceled();
+				_logger.LogDebug(new EventId(6, nameof(Dispose)), $"The state wait helper for component {_renderedFragment.ComponentId} disposed");
+			}
 		}
 
 		private static TimeSpan GetRuntimeTimeout(TimeSpan? timeout)

src/bunit.core/RazorTesting/FixtureBase.cs

@@ -72,9 +72,9 @@ public override Task SetParametersAsync(ParameterView parameters)
 			TestAsync = parameters.GetValueOrDefault<Func<TFixture, Task>>(nameof(TestAsync));
 
 #pragma warning disable CS0618 // Type or member is obsolete
-			if (parameters.TryGetValue<IReadOnlyCollection<Action<TFixture>>>("Tests", out var tests))
+			if (parameters.TryGetValue<IReadOnlyCollection<Action<TFixture>>>(nameof(Tests), out var tests))
 				Tests = tests;
-			if (parameters.TryGetValue<IReadOnlyCollection<Func<TFixture, Task>>>("TestsAsync", out var asyncTests))
+			if (parameters.TryGetValue<IReadOnlyCollection<Func<TFixture, Task>>>(nameof(TestsAsync), out var asyncTests))
 				TestsAsync = asyncTests;
 #pragma warning restore CS0618 // Type or member is obsolete
 
@@ -86,16 +86,16 @@ public override void Validate()
 		{
 			base.Validate();
 			if (ChildContent is null)
-				throw new ArgumentException($"No {nameof(ChildContent)} specified in the {GetType().Name} component.");
+				throw new ArgumentException($"No '{nameof(ChildContent)}' specified in the {GetType().Name} component.");
 			if (Test is null && TestAsync is null)
-				throw new ArgumentException($"No test/assertions provided to the {GetType().Name} component.");
+				throw new ArgumentException($"No test action provided via the '{nameof(Test)}' or '{nameof(TestAsync)}' parameters to the {GetType().Name} component.");
 			if (Test is { } && TestAsync is { })
-				throw new ArgumentException($"Only a single test method can be provided to the {GetType().Name} component at the time.");
+				throw new ArgumentException($"Only one of the '{nameof(Test)}' or '{nameof(TestAsync)}' actions can be provided to the {GetType().Name} component at the same time.");
 #pragma warning disable CS0618 // Type or member is obsolete
 			if (Tests is { })
-				throw new ArgumentException($"The user of the Tests parameter has been obsoleted, and any methods assigned to it will not longer be invoked.");
+				throw new ArgumentException($"The use of the '{nameof(Tests)}' parameter has been obsoleted, and any methods assigned to it will not longer be invoked.");
 			if (TestsAsync is { })
-				throw new ArgumentException($"The user of the TestsAsync parameter has been obsoleted, and any methods assigned to it will not longer be invoked.");
+				throw new ArgumentException($"The use of the '{nameof(TestsAsync)}' parameter has been obsoleted, and any methods assigned to it will not longer be invoked.");
 #pragma warning restore CS0618 // Type or member is obsolete
 		}