Skip to content

Conversation

@sanych-sun
Copy link
Member

No description provided.

@sanych-sun sanych-sun requested a review from a team as a code owner November 27, 2025 23:42
@sanych-sun sanych-sun added the chore Non–user-facing code changes (tests, build scripts, etc.). label Nov 27, 2025
Copy link
Contributor

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 implements tracking for unobserved task exceptions during test execution to help identify tasks that complete with exceptions without proper handling. The implementation uses xUnit's extensibility to register a global handler that captures unobserved exceptions and verifies none occurred after all tests complete.

Key Changes

  • Added custom xUnit test case and discoverer infrastructure to track UnobservedTaskException events globally
  • Implemented a test that runs after all other tests to verify no unobserved exceptions were raised
  • Fixed a race condition in CSharp3302Tests where an unhandled task was causing unobserved exceptions

Reviewed changes

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

Show a summary per file
File Description
UnobservedExceptionTrackingTestCase.cs New test case class that registers/unregisters the UnobservedTaskException handler and stores exceptions in a static list
UnobservedExceptionTestDiscoverer.cs New xUnit test discoverer and fact attribute to enable unobserved exception tracking
TimeoutEnforcingXunitTestAssemblyRunner.cs Modified to run the unobserved exception tracking test after all other tests complete
UnobservedTaskExceptionTracking.cs New test class that verifies no unobserved exceptions were captured during test execution
CSharp3302Tests.cs Fixed to properly handle task cancellation to prevent unobserved exceptions
Cluster.cs Added disposal check to prevent ObjectDisposedException from timer callback

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +47 to +54
{
return new TimeoutEnforcingXunitTestCollectionRunner(testCollection, testCases, DiagnosticMessageSink, messageBus, TestCaseOrderer, new ExceptionAggregator(Aggregator), cancellationTokenSource).RunAsync();
}
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

[nitpick] The formatting change converts this to a multi-line method but the actual logic is still a single return statement. Consider keeping the expression-bodied member syntax => for consistency with the codebase style, or add meaningful logic to justify the block body.

Suggested change
{
return new TimeoutEnforcingXunitTestCollectionRunner(testCollection, testCases, DiagnosticMessageSink, messageBus, TestCaseOrderer, new ExceptionAggregator(Aggregator), cancellationTokenSource).RunAsync();
}
=> new TimeoutEnforcingXunitTestCollectionRunner(testCollection, testCases, DiagnosticMessageSink, messageBus, TestCaseOrderer, new ExceptionAggregator(Aggregator), cancellationTokenSource).RunAsync();

Copilot uses AI. Check for mistakes.
@sanych-sun sanych-sun requested review from BorisDog and removed request for adelinowona November 27, 2025 23:44
@sanych-sun sanych-sun requested a review from BorisDog December 3, 2025 19:01
public class UnobservedTaskExceptionTracking
{
[UnobservedExceptionTrackingFact]
public void EnsureNoUnobservedTaskException()
Copy link
Contributor

Choose a reason for hiding this comment

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

So why don't we add this to BSON a well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.

Copy link
Contributor

Choose a reason for hiding this comment

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

No issues with BSON?

@sanych-sun sanych-sun requested a review from BorisDog December 4, 2025 23:41
public class UnobservedTaskExceptionTracking
{
[UnobservedExceptionTrackingFact]
public void EnsureNoUnobservedTaskException()
Copy link
Contributor

Choose a reason for hiding this comment

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

No issues with BSON?

return result;
}

private class UnobservedExceptionDebugger : IDisposable
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

Copy link
Contributor

@BorisDog BorisDog left a comment

Choose a reason for hiding this comment

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

LGTM

@sanych-sun sanych-sun merged commit 4c9b66e into mongodb:main Dec 5, 2025
23 of 24 checks passed
@sanych-sun sanych-sun deleted the csharp5798 branch December 5, 2025 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Non–user-facing code changes (tests, build scripts, etc.).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants