Fix log accumulation across DynamicData test invocations by @Evangelink in #7925 (backport to rel/4.2)#7931
Merged
Evangelink merged 3 commits intomicrosoft:rel/4.2from Apr 29, 2026
Conversation
GetOut(), GetErr(), and GetTrace() on TestContextImplementation returned accumulated buffer contents but never cleared them. When DynamicData tests reuse the same TestContext across multiple invocations, each subsequent result contained all logs from all previous invocations, causing exponential memory growth (>20 GB for simple projects). Replace these with GetAndClearOutput(), GetAndClearError(), and GetAndClearTrace() that clear the StringBuilder after extracting the value, matching the existing GetAndClearDiagnosticMessages() pattern. Fixes microsoft#7846
Address review feedback: ToString() + Clear() as two separate synchronized calls could lose appended text if another thread writes between them. Add a single synchronized GetAndClear() method to SynchronizedStringBuilder that performs both operations under one lock.
- Add focused GetAndClear{Output,Error,Trace} unit tests in
TestContextImplementationTests that verify the buffer is cleared after
the first call (returns empty on second call).
- Add GetAndClearTrace() call to WritesFromBackgroundThreadShouldNotThrow
to complete thread-safety coverage for all three buffer types.
- Consolidate regression test comments: single block comment noting that
TestClassInfo and UnitTestRunner call sites use the same GetAndClear*
methods tested in isolation.
Youssef1313
reviewed
Apr 29, 2026
Member
Youssef1313
left a comment
Youssef1313
left a comment
There was a problem hiding this comment.
- I strongly suggest dogfooding this internally before backporting.
- I don't know if @nohwnd is currently looking at a different approach that should be considered instead.
Member
|
Validated the memory reduction. I wasn't able to reproduce the concurrency issue on the texts with the provided example. |
Evangelink
approved these changes
Apr 29, 2026
This was referenced Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #7925 to rel/4.2
/cc @Evangelink