Skip to content

CSHARP-5930: Stop throwing in test discovery when running locally#1917

Open
ajcvickers wants to merge 1 commit intomongodb:mainfrom
ajcvickers:RunTests
Open

CSHARP-5930: Stop throwing in test discovery when running locally#1917
ajcvickers wants to merge 1 commit intomongodb:mainfrom
ajcvickers:RunTests

Conversation

@ajcvickers
Copy link
Contributor

Both EnsureNoUnobservedTaskException and EnsureNoUnobservedTaskException_Integration have the "UnobservedExceptionTracking" category. This means that the SingleOrDefault throws in test dicovery.

I don't know that this is the correct fix, but it's a starting point.

@ajcvickers ajcvickers requested a review from sanych-sun March 17, 2026 14:48
@ajcvickers ajcvickers requested a review from a team as a code owner March 17, 2026 14:48
Copilot AI review requested due to automatic review settings March 17, 2026 14:48
var baseSummary = await base.RunTestCollectionsAsync(messageBus, cancellationTokenSource);

if (_unobservedExceptionTrackingTestCase == null)
if (_unobservedExceptionTrackingTestCase.Any())
Copy link
Contributor Author

@ajcvickers ajcvickers Mar 17, 2026

Choose a reason for hiding this comment

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

It's possible/probable that the intention is only to run one of these tests at a time, in which case we need to figure out how to prevent both from being returned by default when running locally.

Copy link
Member

Choose a reason for hiding this comment

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

You are right, the idea was that on CI we run either tests without category assigned (aka unit-tests) or we run integration tests only (by category), but we want to track for unobserved exceptions in both cases. I was trying to generate that test dynamically, but did not found a nice way to do that, so instead I've simply created 2 tests, but forget to improve runner to support a case when there are 2 of them.

@ajcvickers ajcvickers added the chore Non–user-facing code changes (tests, build scripts, etc.). label Mar 17, 2026
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

Updates the custom xUnit assembly runner used by the test helpers to avoid SingleOrDefault throwing during test discovery when multiple tests are tagged with the UnobservedExceptionTracking category.

Changes:

  • Collects all UnobservedExceptionTracking test cases (instead of requiring a single one).
  • Runs the collected unobserved-exception test cases after the main test run and aggregates their RunSummary.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +64 to +69
foreach (var testCase in _unobservedExceptionTrackingTestCase)
{
var unobservedExceptionTestCaseRunSummary = await RunTestCollectionAsync(
messageBus,
testCase.TestMethod.TestClass.TestCollection,
[testCase],
Copy link
Member

Choose a reason for hiding this comment

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

The idea of this code was to make sure the _unobservedExceptionTrackingTestCase is being executed as the very last test case in the collection. We do not need to wrap with foreach, we should simply run all of them after the rest of tests are done.

Both EnsureNoUnobservedTaskException and EnsureNoUnobservedTaskException_Integration have the "UnobservedExceptionTracking" category. This means that the SingleOrDefault throws in test dicovery.

I don't know that this is the correct fix, but it's a starting point.
@ajcvickers
Copy link
Contributor Author

Updated to run only one if more than one test exists.

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.

3 participants