Skip to content

Conversation

@linkdotnet
Copy link
Collaborator

@linkdotnet linkdotnet commented Oct 7, 2025

DisposeComponents did check immediately for AssertNoUnhandledExceptions which can creates race conditions when DisposeAsync/Dispose throws fast enough (aka in the sync part) but may behave oddly in the async part. Test201 was super flaky due this race condition.

So this "take" consolidates a central way of getting an exception out of the "DisposeComponents" method

@linkdotnet linkdotnet requested a review from egil October 7, 2025 06:29
@linkdotnet linkdotnet changed the title fix: Don't rethrow in DiposeComponents directly fix: Don't rethrow in DisposeComponents directly Oct 7, 2025
@egil
Copy link
Member

egil commented Oct 7, 2025

Horray, great catch!

@egil egil requested a review from Copilot October 7, 2025 08:58
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a race condition in the DisposeComponents method by removing the immediate exception rethrowing behavior and consolidating exception handling through the Renderer.UnhandledException task. The change addresses flakiness in Test201 by ensuring consistent behavior for both synchronous and asynchronous disposal exceptions.

  • Removed immediate AssertNoUnhandledExceptions() call from DisposeComponents method
  • Updated test expectations to check exceptions via Renderer.UnhandledException instead of expecting direct throws
  • Updated documentation to reflect the new consistent exception handling pattern

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/bunit/Rendering/BunitRenderer.cs Removed immediate exception assertion to prevent race conditions
tests/bunit.tests/BunitContextTest.cs Updated test to use new exception handling pattern via Renderer.UnhandledException
docs/site/docs/interaction/dispose-components.md Updated documentation to explain new consistent exception handling approach
docs/samples/tests/xunit/DisposeComponentsTest.cs Updated sample code to demonstrate proper exception handling patterns

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@linkdotnet linkdotnet merged commit 7f99c27 into main Oct 7, 2025
18 of 19 checks passed
@linkdotnet linkdotnet deleted the dispose-async-fix branch October 7, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants