Skip to content

Conversation

@Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Jan 8, 2026

Fixes #7120

After every test method completes, we marked it as completed.
Then after every class cleanup, we called RunAssemblyCleanupIfNeededAsync, which will check if all tests have been executed, and runs assembly cleanup. This is wrong because all tests could have been executed, but there are remaining class cleanups running in parallel.

This PR updates the logic so we have the concept of "marking test as completed" and "marking test class as completed". Previously we only had the former concept.

Copy link
Member

@nohwnd nohwnd left a comment

Choose a reason for hiding this comment

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

comments in code.

@Youssef1313 Youssef1313 enabled auto-merge (squash) January 12, 2026 14:56
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 fixes issue #7120 where AssemblyCleanup could run prematurely before all class cleanups had completed in parallel test execution scenarios. The fix introduces a two-stage completion tracking mechanism: marking individual tests as complete, and separately marking entire test classes as complete only after their cleanup methods finish.

Changes:

  • Introduced MarkClassComplete method to track when a test class has finished all cleanups
  • Modified MarkTestComplete to only indicate when the last test in a class completes, not trigger cleanup
  • Updated RunClassCleanupAsync to remove cleanup manager logic and focus on executing cleanup
  • Ensured assembly cleanup runs only after all classes are marked complete

Reviewed changes

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

Show a summary per file
File Description
ClassCleanupManager.cs Added MarkClassComplete method and refactored MarkTestComplete to return isLastTestInClass flag; removed cleanup execution logic
TestClassInfo.cs Simplified RunClassCleanupAsync signature by removing ClassCleanupManager and TestMethodInfo parameters and associated logic
UnitTestRunner.cs Updated to call MarkClassComplete after class cleanup finishes; checks ShouldRunEndOfAssemblyCleanup before running assembly cleanup
UnitTestRunnerTests.cs Refactored tests to create UnitTestRunner instances per test instead of sharing a field; added helper method CreateUnitTestRunner
ClassCleanupManagerTests.cs Updated test to use new MarkClassComplete API and removed dependency on TestMethodInfo
AssemblyCleanupTests.cs Added new integration test verifying assembly cleanup waits for parallel class cleanups with sleep delays

@Youssef1313 Youssef1313 merged commit 6f82b88 into main Jan 12, 2026
15 of 16 checks passed
@Youssef1313 Youssef1313 deleted the dev/ygerges/fix-asm-cleanup branch January 12, 2026 22:32
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.

AssemblyCleanup runs before all ClassCleanups finish

3 participants