diff --git a/CHANGELOG.md b/CHANGELOG.md index f289f6066..82adb1477 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ All notable changes to **bUnit** will be documented in this file. The project ad ## [Unreleased] +### Fixed + +- `Find` threw `TypeLoadException` with AngleSharp 1.8.6 because `HtmlSelectElementWrapper` did not implement the new `SelectedIndex` setter. Reported by [@poxet](https://github.com/poxet) in #1957. Fixed by [@linkdotnet](https://github.com/linkdotnet). +- A `WaitFor*` call whose first check passed left its timeout timer running, which kept the helper, the renderer and everything the renderer holds alive until the timeout elapsed. With a long `DefaultWaitTimeout`, a test suite could hold every rendered component in memory for its whole run. Fixed by [@xaviergxf](https://github.com/xaviergxf). + ## [2.12.5] - 2026-10-19 ### Fixed diff --git a/Directory.Packages.props b/Directory.Packages.props index 8a23376ef..95a3da0d9 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -8,7 +8,7 @@ - + @@ -18,8 +18,8 @@ - - + + diff --git a/src/bunit/Extensions/WaitForHelpers/WaitForFailedException.cs b/src/bunit/Extensions/WaitForHelpers/WaitForFailedException.cs index ec18da2ff..1033e6947 100644 --- a/src/bunit/Extensions/WaitForHelpers/WaitForFailedException.cs +++ b/src/bunit/Extensions/WaitForHelpers/WaitForFailedException.cs @@ -7,6 +7,33 @@ namespace Bunit.Extensions.WaitForHelpers; /// public sealed class WaitForFailedException : Exception, ITestTimeoutException { + internal const string CheckNotStartedMessage = """ + The wait condition was never checked, because the renderer's dispatcher did not run the check before the timeout. + This points to the test setup rather than the component under test: the thread pool may be starved, + e.g. by many tests blocking threads with synchronous WaitFor* or Click calls in parallel, or other work is blocking the dispatcher. + + Consider one of the following: + - Increase the timeout per call, e.g. WaitForElement("div", TimeSpan.FromSeconds(15)), + or globally via BunitContext.DefaultWaitTimeout. + - Prefer the async WaitFor*Async overloads, which do not block a thread pool thread while waiting. + - Raise the thread pool minimum once for the test assembly, e.g.: + + internal static class ThreadPoolSetup + { + [System.Runtime.CompilerServices.ModuleInitializer] + internal static void RaiseMinThreads() + { + ThreadPool.GetMinThreads(out var workers, out var io); + ThreadPool.SetMinThreads(Math.Max(workers, Environment.ProcessorCount * 4), io); + } + } + """; + + internal const string CheckNotCompletedMessage = + "The wait condition check started but did not complete before the timeout. " + + "The asynchronous check is most likely awaiting something that never completes, or that takes longer than the timeout. " + + "This usually indicates a problem in the test itself."; + /// /// Initializes a new instance of the class. /// diff --git a/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs b/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs index d8e5111e1..99790d681 100644 --- a/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs +++ b/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs @@ -20,6 +20,7 @@ internal abstract class WaitForHelper : IDisposable private bool isDisposed; private bool isChecking; private bool isDirty; + private bool isCheckStarted; private int checkCount; private Exception? capturedException; @@ -94,17 +95,22 @@ protected WaitForHelper( // If the initial check did not complete successfully, // start the timer and recheck after every render until the timer expires. - if (!WaitTask.IsCompleted) + // A check that completed synchronously has already disposed this helper + // before WaitTask, which completes on the thread pool, reflects it. A timer + // started after that would never be disposed, and would keep this helper, + // and through it the renderer, alive until the timeout elapsed. + if (!isDisposed && !WaitTask.IsCompleted) { timer = new Timer( static (state) => { var @this = (WaitForHelper)state!; + var checks = Volatile.Read(ref @this.checkCount); @this.logger.LogWaiterTimedOut(@this.renderedComponent.ComponentId); @this.checkPassedCompletionSource.TrySetException( new WaitForFailedException( - @this.TimeoutErrorMessage ?? string.Empty, - @this.checkCount, + @this.CreateTimeoutErrorMessage(checks), + checks, @this.renderedComponent.RenderCount, @this.renderer.RenderCount, @this.capturedException)); @@ -112,6 +118,12 @@ protected WaitForHelper( this, GetRuntimeTimeout(timeout), Timeout.InfiniteTimeSpan); + + // The check may have completed on the renderer's dispatcher, and disposed + // this helper while the timer field was still null, between the test above + // and the assignment. Dispose(bool) found no timer to stop in that case. + if (isDisposed) + timer.Dispose(); } } @@ -159,12 +171,27 @@ private void CheckAndInitializeWaiting() { // Before subscribing to renderedFragment.OnAfterRender, // we need to make sure that the desired state has not already been reached. + Volatile.Write(ref isCheckStarted, true); OnAfterRender(this, EventArgs.Empty); SubscribeToOnAfterRender(); }); } } + // Called from the timer on a thread pool thread, while the check fields are written on the dispatcher. + private string CreateTimeoutErrorMessage(int completedCheckCount) + { + var message = TimeoutErrorMessage ?? string.Empty; + if (completedCheckCount > 0) + return message; + + var reason = Volatile.Read(ref isCheckStarted) + ? WaitForFailedException.CheckNotCompletedMessage + : WaitForFailedException.CheckNotStartedMessage; + + return $"{message}{Environment.NewLine}{Environment.NewLine}{reason}"; + } + private Task CreateWaitTask() { // Two to failure conditions, that the renderer captures an unhandled diff --git a/tests/bunit.generators.tests/Web.AngleSharp/WrapperElementsGeneratorTest.Generator#HtmlSelectElementWrapper.g.verified.cs b/tests/bunit.generators.tests/Web.AngleSharp/WrapperElementsGeneratorTest.Generator#HtmlSelectElementWrapper.g.verified.cs index b4f0b77c5..32178d2a8 100644 --- a/tests/bunit.generators.tests/Web.AngleSharp/WrapperElementsGeneratorTest.Generator#HtmlSelectElementWrapper.g.verified.cs +++ b/tests/bunit.generators.tests/Web.AngleSharp/WrapperElementsGeneratorTest.Generator#HtmlSelectElementWrapper.g.verified.cs @@ -1572,6 +1572,10 @@ public int SelectedIndex [System.Diagnostics.DebuggerStepThrough] [System.Runtime.CompilerServices.MethodImpl(MethodImplOptions.AggressiveInlining)] get => WrappedElement.SelectedIndex; + [System.Diagnostics.DebuggerHidden] + [System.Diagnostics.DebuggerStepThrough] + [System.Runtime.CompilerServices.MethodImpl(MethodImplOptions.AggressiveInlining)] + set => WrappedElement.SelectedIndex = value; } /// diff --git a/tests/bunit.tests/Assembly.cs b/tests/bunit.tests/Assembly.cs index 786c37ff8..7ea1fae08 100644 --- a/tests/bunit.tests/Assembly.cs +++ b/tests/bunit.tests/Assembly.cs @@ -1 +1,17 @@ +using System.Runtime.CompilerServices; + [assembly: System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + +namespace Bunit; + +internal static class ThreadPoolSetup +{ + // Many tests block a pool thread synchronously (WaitFor*, Click) while xunit runs 2x processor count in parallel, + // so the default pool floor starves the continuations those tests wait on, causing sporadic WaitFor* timeouts on CI. + [ModuleInitializer] + internal static void RaiseMinThreads() + { + ThreadPool.GetMinThreads(out var workers, out var io); + ThreadPool.SetMinThreads(Math.Max(workers, Environment.ProcessorCount * 4), io); + } +} diff --git a/tests/bunit.tests/Extensions/WaitForHelpers/RenderedComponentWaitForHelperExtensions.cs b/tests/bunit.tests/Extensions/WaitForHelpers/RenderedComponentWaitForHelperExtensions.cs index 0d0044ea4..34482aff9 100644 --- a/tests/bunit.tests/Extensions/WaitForHelpers/RenderedComponentWaitForHelperExtensions.cs +++ b/tests/bunit.tests/Extensions/WaitForHelpers/RenderedComponentWaitForHelperExtensions.cs @@ -1,3 +1,4 @@ +using System.Runtime.CompilerServices; using AngleSharp.Dom; namespace Bunit.Extensions.WaitForHelpers; @@ -240,6 +241,110 @@ await cut.WaitForAssertionAsync(async () => expected.Message.ShouldStartWith(WaitForAssertionHelper.TimeoutMessage); } + [Fact(DisplayName = "WaitForStateAsync reports a setup problem when the dispatcher never runs the check before the timeout")] + public async Task Test500() + { + var cut = Render(); + var cancellationToken = Xunit.TestContext.Current.CancellationToken; + using var dispatcherEntered = new ManualResetEventSlim(); + using var releaseDispatcher = new ManualResetEventSlim(); + var blocker = Task.Run(() => cut.InvokeAsync(() => + { + dispatcherEntered.Set(); + releaseDispatcher.Wait(cancellationToken); + }), cancellationToken); + + try + { + dispatcherEntered.Wait(cancellationToken); + + var expected = await Should.ThrowAsync( + cut.WaitForStateAsync(() => true, TimeSpan.FromMilliseconds(50))); + + expected.Message.ShouldContain(WaitForFailedException.CheckNotStartedMessage); + expected.Message.ShouldContain("Check count: 0."); + } + finally + { + releaseDispatcher.Set(); + await blocker; + } + } + + [Fact(DisplayName = "WaitForStateAsync reports a test problem when the async check never completes before the timeout")] + public async Task Test501() + { + var cut = Render(); + var neverCompletes = new TaskCompletionSource(); + + var expected = await Should.ThrowAsync( + cut.WaitForStateAsync(() => neverCompletes.Task, TimeSpan.FromMilliseconds(50))); + + expected.Message.ShouldContain(WaitForFailedException.CheckNotCompletedMessage); + expected.Message.ShouldContain("Check count: 0."); + } + + [Fact(DisplayName = "WaitForState timeout after completed checks does not report a setup or test problem")] + public void Test502() + { + var cut = Render(); + + var expected = Should.Throw(() => + cut.WaitForState(() => false, TimeSpan.FromMilliseconds(50))); + + expected.Message.ShouldNotContain(WaitForFailedException.CheckNotStartedMessage); + expected.Message.ShouldNotContain(WaitForFailedException.CheckNotCompletedMessage); + } + + [Fact(DisplayName = "WaitForAssertion does not keep the renderer alive after the first check passes")] + public async Task Test503() + { + // The first call is slow while it is being JIT compiled, which is enough for the wait task + // to complete before the helper decides whether to start its timer, so it is not measured. + RenderAndWaitForAssertionThatPassesAtOnce(); + + var renderers = Enumerable.Range(0, 10) + .Select(_ => RenderAndWaitForAssertionThatPassesAtOnce()) + .ToList(); + + // Disposing a context queues work that still references the renderer: cancelling its + // unhandled exception task wakes the helper's failure continuation on the thread pool, and + // the renderer's own base disposal goes through the dispatcher. On a busy test host that + // can still be queued when the first collection runs, so the collection is retried for a + // short while. A timer left running roots the renderer for ten minutes, so no number of + // retries makes the leaking case pass. + var alive = renderers.Count; + for (var attempt = 0; attempt < 20 && alive > 0; attempt++) + { + if (attempt > 0) + await Task.Delay(50, Xunit.TestContext.Current.CancellationToken); + +#pragma warning disable S1215 // A full collection is what proves the renderers are no longer rooted. + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); +#pragma warning restore S1215 + + alive = renderers.Count(r => r.IsAlive); + } + + alive.ShouldBe(0, "a wait whose first check passed should leave no timer running"); + } + + // Kept out of the test method so that nothing on its stack frame keeps the context alive. + [MethodImpl(MethodImplOptions.NoInlining)] + private static WeakReference RenderAndWaitForAssertionThatPassesAtOnce() + { + using var context = new BunitContext(); + var cut = context.Render(); + + // A timeout far longer than the test: a timer left running roots the helper, + // and through it the renderer, until it fires. + cut.WaitForAssertion(() => { }, TimeSpan.FromMinutes(10)); + + return new WeakReference(context.Renderer); + } + private sealed class ThrowsAfterAsyncOperation : ComponentBase { protected override async Task OnInitializedAsync() diff --git a/version.json b/version.json index 20c1c480d..b94e03074 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "2.12", + "version": "2.13", "assemblyVersion": { "precision": "revision" },