Skip to content

Commit 6b1b2e5

Browse files
EvangelinkCopilot
andauthored
Fix folded data-driven tests sharing TestContextImplementation across iterations (#8439)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 26d3839 commit 6b1b2e5

4 files changed

Lines changed: 318 additions & 16 deletions

File tree

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ internal async Task<TestResult[]> RunTestMethodAsync()
123123
}
124124

125125
object?[]? data = _test.ActualData ?? DataSerializationHelper.Deserialize(_test.SerializedData);
126-
TestResult[] testResults = await ExecuteTestWithDataSourceAsync(null, data, actualDataAlreadyHandledDuringDiscovery: true).ConfigureAwait(false);
126+
TestResult[] testResults = await ExecuteTestWithDataSourceAsync(_testContext, null, data, actualDataAlreadyHandledDuringDiscovery: true).ConfigureAwait(false);
127127
results.AddRange(testResults);
128128
}
129129
else if (await TryExecuteDataSourceBasedTestsAsync(results).ConfigureAwait(false))
@@ -137,7 +137,7 @@ internal async Task<TestResult[]> RunTestMethodAsync()
137137
else
138138
{
139139
_testContext.SetDisplayName(_test.DisplayName);
140-
TestResult[] testResults = await ExecuteTestAsync(_testMethodInfo).ConfigureAwait(false);
140+
TestResult[] testResults = await ExecuteTestAsync(_testContext, _testMethodInfo).ConfigureAwait(false);
141141

142142
foreach (TestResult testResult in testResults)
143143
{
@@ -207,6 +207,8 @@ private async Task<bool> TryExecuteDataSourceBasedTestsAsync(List<TestResult> re
207207
private async Task<bool> TryExecuteFoldedDataDrivenTestsAsync(List<TestResult> results)
208208
{
209209
bool hasTestDataSource = false;
210+
var outerContext = (TestContextImplementation)_testContext.Context;
211+
210212
foreach (Attribute attribute in PlatformServiceProvider.Instance.ReflectionOperations.GetCustomAttributesCached(_testMethodInfo.MethodInfo))
211213
{
212214
if (attribute is not UTF.ITestDataSource testDataSource)
@@ -227,15 +229,24 @@ private async Task<bool> TryExecuteFoldedDataDrivenTestsAsync(List<TestResult> r
227229
foreach (object?[] data in testDataSource.GetData(_testMethodInfo.MethodInfo))
228230
{
229231
dataSourceHasData = true;
232+
233+
// Create a fresh TestContextImplementation per iteration so the folded path is
234+
// structurally equivalent to the unfolded path (where each row gets its own
235+
// context). This isolates per-row state (captured output, diagnostic messages,
236+
// result files, outcome, exception, property bag mutations, ...) so a leak in
237+
// any current or future TestContextImplementation field cannot accumulate across
238+
// rows. See https://github.com/microsoft/testfx/issues/7933.
239+
TestContextImplementation iterationContext = outerContext.CloneForDataDrivenIteration();
230240
try
231241
{
232-
TestResult[] testResults = await ExecuteTestWithDataSourceAsync(testDataSource, data, actualDataAlreadyHandledDuringDiscovery: false).ConfigureAwait(false);
242+
TestResult[] testResults = await ExecuteTestWithDataSourceAsync(iterationContext, testDataSource, data, actualDataAlreadyHandledDuringDiscovery: false).ConfigureAwait(false);
233243

234244
results.AddRange(testResults);
235245
}
236246
finally
237247
{
238248
_testMethodInfo.SetArguments(null);
249+
iterationContext.Dispose();
239250
}
240251
}
241252

@@ -278,11 +289,23 @@ private async Task ExecuteTestFromDataSourceAttributeAsync(List<TestResult> resu
278289
try
279290
{
280291
int rowIndex = 0;
292+
var outerContext = (TestContextImplementation)_testContext.Context;
281293

282294
foreach (object dataRow in dataRows)
283295
{
284-
TestResult[] testResults = await ExecuteTestWithDataRowAsync(dataRow, rowIndex++).ConfigureAwait(false);
285-
results.AddRange(testResults);
296+
// Create a fresh TestContextImplementation per row for the same structural
297+
// reason as in TryExecuteFoldedDataDrivenTestsAsync — each row should
298+
// start with no accumulated per-test state.
299+
TestContextImplementation iterationContext = outerContext.CloneForDataDrivenIteration();
300+
try
301+
{
302+
TestResult[] testResults = await ExecuteTestWithDataRowAsync(iterationContext, dataRow, rowIndex++).ConfigureAwait(false);
303+
results.AddRange(testResults);
304+
}
305+
finally
306+
{
307+
iterationContext.Dispose();
308+
}
286309
}
287310
}
288311
finally
@@ -303,7 +326,7 @@ private async Task ExecuteTestFromDataSourceAttributeAsync(List<TestResult> resu
303326
}
304327
}
305328

306-
private async Task<TestResult[]> ExecuteTestWithDataSourceAsync(UTF.ITestDataSource? testDataSource, object?[]? data, bool actualDataAlreadyHandledDuringDiscovery)
329+
private async Task<TestResult[]> ExecuteTestWithDataSourceAsync(ITestContext executionContext, UTF.ITestDataSource? testDataSource, object?[]? data, bool actualDataAlreadyHandledDuringDiscovery)
307330
{
308331
string? displayName = StringEx.IsNullOrWhiteSpace(_test.DisplayName)
309332
? _test.Name
@@ -353,12 +376,12 @@ private async Task<TestResult[]> ExecuteTestWithDataSourceAsync(UTF.ITestDataSou
353376

354377
var stopwatch = Stopwatch.StartNew();
355378
_testMethodInfo.SetArguments(data);
356-
_testContext.SetTestData(data);
357-
_testContext.SetDisplayName(displayName);
379+
executionContext.SetTestData(data);
380+
executionContext.SetDisplayName(displayName);
358381

359382
TestResult[] testResults = ignoreFromTestDataRow is not null
360383
? [TestResult.CreateIgnoredResult(ignoreFromTestDataRow)]
361-
: await ExecuteTestAsync(_testMethodInfo).ConfigureAwait(false);
384+
: await ExecuteTestAsync(executionContext, _testMethodInfo).ConfigureAwait(false);
362385

363386
stopwatch.Stop();
364387

@@ -375,7 +398,7 @@ private async Task<TestResult[]> ExecuteTestWithDataSourceAsync(UTF.ITestDataSou
375398
return testResults;
376399
}
377400

378-
private async Task<TestResult[]> ExecuteTestWithDataRowAsync(object dataRow, int rowIndex)
401+
private async Task<TestResult[]> ExecuteTestWithDataRowAsync(ITestContext executionContext, object dataRow, int rowIndex)
379402
{
380403
string displayName = string.Format(CultureInfo.CurrentCulture, Resource.DataDrivenResultDisplayName, _test.DisplayName, rowIndex);
381404
Stopwatch? stopwatch = null;
@@ -384,13 +407,13 @@ private async Task<TestResult[]> ExecuteTestWithDataRowAsync(object dataRow, int
384407
try
385408
{
386409
stopwatch = Stopwatch.StartNew();
387-
_testContext.SetDataRow(dataRow);
388-
testResults = await ExecuteTestAsync(_testMethodInfo).ConfigureAwait(false);
410+
executionContext.SetDataRow(dataRow);
411+
testResults = await ExecuteTestAsync(executionContext, _testMethodInfo).ConfigureAwait(false);
389412
}
390413
finally
391414
{
392415
stopwatch?.Stop();
393-
_testContext.SetDataRow(null);
416+
executionContext.SetDataRow(null);
394417
}
395418

396419
foreach (TestResult testResult in testResults)
@@ -402,7 +425,7 @@ private async Task<TestResult[]> ExecuteTestWithDataRowAsync(object dataRow, int
402425
return testResults;
403426
}
404427

405-
private async Task<TestResult[]> ExecuteTestAsync(TestMethodInfo testMethodInfo)
428+
private async Task<TestResult[]> ExecuteTestAsync(ITestContext executionContext, TestMethodInfo testMethodInfo)
406429
{
407430
try
408431
{
@@ -415,9 +438,9 @@ private async Task<TestResult[]> ExecuteTestAsync(TestMethodInfo testMethodInfo)
415438
{
416439
try
417440
{
418-
using (TestContextImplementation.SetCurrentTestContext(_testContext as TestContext))
441+
using (TestContextImplementation.SetCurrentTestContext(executionContext as TestContext))
419442
{
420-
testMethodInfo.TestContext = _testContext;
443+
testMethodInfo.TestContext = executionContext;
421444
tcs.SetResult(await _testMethodInfo.Executor.ExecuteAsync(testMethodInfo).ConfigureAwait(false));
422445
}
423446
}

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public override string ToString()
6767
/// </summary>
6868
private readonly Dictionary<string, object?> _properties;
6969
private readonly IMessageLogger? _messageLogger;
70+
private readonly TestRunCancellationToken? _testRunCancellationToken;
7071

7172
private CancellationTokenRegistration? _cancellationTokenRegistration;
7273

@@ -140,6 +141,7 @@ internal TestContextImplementation(ITestMethod? testMethod, string? testClassFul
140141
}
141142

142143
_messageLogger = messageLogger;
144+
_testRunCancellationToken = testRunCancellationToken;
143145
_cancellationTokenRegistration = testRunCancellationToken?.Register(CancelDelegate, this);
144146
}
145147

@@ -460,4 +462,45 @@ private SynchronizedStringBuilder GetTestContextMessagesStringBuilder()
460462

461463
internal string? GetAndClearTrace()
462464
=> _traceStringBuilder?.GetAndClear();
465+
466+
/// <summary>
467+
/// Creates a sibling <see cref="TestContextImplementation"/> for use by a single iteration
468+
/// of the folded data-driven test execution path.
469+
/// <para>
470+
/// The clone inherits the same configuration as this context (a shallow snapshot of the
471+
/// property bag, the message logger, the same test-run cancellation token, and on .NET
472+
/// Framework the current data connection), but registers its own cancellation callback and
473+
/// starts with no accumulated per-test state (no captured stdout/stderr/trace,
474+
/// no diagnostic messages, no result files, no exception, no data row, and the
475+
/// default <see cref="UnitTestOutcome"/> value rather than the original's current outcome).
476+
/// This keeps the folded path structurally equivalent to the unfolded path, where each
477+
/// row gets its own <see cref="TestContextImplementation"/>.
478+
/// </para>
479+
/// </summary>
480+
/// <returns>A fresh context suitable for one folded data-driven iteration.</returns>
481+
internal TestContextImplementation CloneForDataDrivenIteration()
482+
{
483+
// Take a shallow snapshot of the current property bag so that the clone starts with
484+
// the same properties (including TestNameLabel / FullyQualifiedTestClassNameLabel and
485+
// anything merged from AssemblyInitialize / ClassInitialize) but is otherwise isolated.
486+
// Per-iteration mutations to the clone's property bag won't leak back to this instance
487+
// nor to subsequent iterations.
488+
var snapshot = new Dictionary<string, object?>(_properties);
489+
490+
// Pass testMethod: null and testClassFullName: null because the relevant labels are
491+
// already in the snapshot. The constructor will copy the snapshot as-is.
492+
var clone = new TestContextImplementation(testMethod: null, testClassFullName: null, snapshot, _messageLogger, _testRunCancellationToken);
493+
494+
// Preserve TestRunCount so user code that observes it (e.g. retry-aware tests) sees
495+
// the same value it would see in the unfolded path. TestRunCount represents the
496+
// execution-attempt count of this test, not per-row state, so it must flow into
497+
// each iteration's context.
498+
clone.Context.TestRunCount = Context.TestRunCount;
499+
500+
#if NETFRAMEWORK
501+
clone.SetDataConnection(_dbConnection);
502+
#endif
503+
504+
return clone;
505+
}
463506
}

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodRunnerTests.cs

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,110 @@ public async Task RunTestMethodShouldPassWhenAttributeInvokesTestMethodOnExecuti
437437
results[0].Outcome.Should().Be(UnitTestOutcome.Passed);
438438
}
439439

440+
public async Task RunTestMethodShouldUseFreshTestContextPerIterationForFoldedDataDrivenTests()
441+
{
442+
// Capture TestContext.Current per iteration so we can verify each row gets a distinct
443+
// TestContextImplementation instance — see https://github.com/microsoft/testfx/issues/7933.
444+
#pragma warning disable MSTESTEXP // TestContext.Current is experimental.
445+
var observedContexts = new List<TestContext?>();
446+
DataRowAttribute dataRowAttribute1 = new(1);
447+
DataRowAttribute dataRowAttribute2 = new(2);
448+
DataRowAttribute dataRowAttribute3 = new(3);
449+
var attributes = new Attribute[] { dataRowAttribute1, dataRowAttribute2, dataRowAttribute3 };
450+
451+
_testablePlatformServiceProvider.MockReflectionOperations.Setup(ro => ro.GetCustomAttributes(_methodInfo)).Returns(attributes);
452+
453+
var testMethodInfo = new TestableTestMethodInfo(_methodInfo, _testClassInfo, _testMethodOptions, () =>
454+
{
455+
observedContexts.Add(TestContext.Current);
456+
return new TestResult { Outcome = UnitTestOutcome.Passed };
457+
});
458+
var testMethodRunner = new TestMethodRunner(testMethodInfo, _testMethod, _testContextImplementation);
459+
460+
_ = await testMethodRunner.RunTestMethodAsync();
461+
462+
observedContexts.Should().HaveCount(3);
463+
observedContexts.Should().AllSatisfy(c => c.Should().NotBeNull());
464+
465+
// Each iteration must observe its own fresh context instance — none should be the
466+
// outer shared context, and no two iterations should share an instance.
467+
observedContexts.Should().OnlyHaveUniqueItems();
468+
observedContexts.Should().NotContain(_testContextImplementation);
469+
#pragma warning restore MSTESTEXP
470+
}
471+
472+
public async Task RunTestMethodShouldNotLeakCapturedOutputAcrossFoldedDataDrivenIterations()
473+
{
474+
// Each iteration writes to its own context's console-out buffer. With a fresh
475+
// per-iteration context, the iteration that fires write N must observe an empty buffer
476+
// before writing — without the fix, row 2 would observe row 1's content.
477+
var observedLengthsBeforeWrite = new List<int>();
478+
DataRowAttribute dataRowAttribute1 = new(1);
479+
DataRowAttribute dataRowAttribute2 = new(2);
480+
DataRowAttribute dataRowAttribute3 = new(3);
481+
var attributes = new Attribute[] { dataRowAttribute1, dataRowAttribute2, dataRowAttribute3 };
482+
483+
_testablePlatformServiceProvider.MockReflectionOperations.Setup(ro => ro.GetCustomAttributes(_methodInfo)).Returns(attributes);
484+
485+
var testMethodInfo = new TestableTestMethodInfo(_methodInfo, _testClassInfo, _testMethodOptions, () =>
486+
{
487+
#pragma warning disable MSTESTEXP // TestContext.Current is experimental.
488+
var current = (TestContextImplementation?)TestContext.Current;
489+
#pragma warning restore MSTESTEXP
490+
current.Should().NotBeNull();
491+
string? existing = current!.GetAndClearOutput();
492+
observedLengthsBeforeWrite.Add(existing?.Length ?? 0);
493+
494+
// Write a fairly large chunk of console output. If contexts were shared, the next
495+
// iteration would observe a non-zero existing length.
496+
current.WriteConsoleOut(new string('x', 1024));
497+
return new TestResult { Outcome = UnitTestOutcome.Passed };
498+
});
499+
var testMethodRunner = new TestMethodRunner(testMethodInfo, _testMethod, _testContextImplementation);
500+
501+
_ = await testMethodRunner.RunTestMethodAsync();
502+
503+
observedLengthsBeforeWrite.Should().HaveCount(3);
504+
observedLengthsBeforeWrite.Should().AllSatisfy(len => len.Should().Be(0));
505+
}
506+
507+
public async Task RunTestMethodShouldNotLeakPropertyBagMutationsAcrossFoldedDataDrivenIterations()
508+
{
509+
// Each iteration adds a unique property key. If contexts were shared, row N would see
510+
// the keys added by rows 1..N-1. With a fresh per-iteration context, every row starts
511+
// with the same baseline property set.
512+
var observedKeyCounts = new List<int>();
513+
DataRowAttribute dataRowAttribute1 = new(1);
514+
DataRowAttribute dataRowAttribute2 = new(2);
515+
DataRowAttribute dataRowAttribute3 = new(3);
516+
var attributes = new Attribute[] { dataRowAttribute1, dataRowAttribute2, dataRowAttribute3 };
517+
518+
_testablePlatformServiceProvider.MockReflectionOperations.Setup(ro => ro.GetCustomAttributes(_methodInfo)).Returns(attributes);
519+
520+
int iteration = 0;
521+
var testMethodInfo = new TestableTestMethodInfo(_methodInfo, _testClassInfo, _testMethodOptions, () =>
522+
{
523+
#pragma warning disable MSTESTEXP // TestContext.Current is experimental.
524+
TestContext? current = TestContext.Current;
525+
#pragma warning restore MSTESTEXP
526+
current.Should().NotBeNull();
527+
observedKeyCounts.Add(current!.Properties.Count);
528+
current.Properties[$"AddedByIteration_{iteration++}"] = "value";
529+
return new TestResult { Outcome = UnitTestOutcome.Passed };
530+
});
531+
var testMethodRunner = new TestMethodRunner(testMethodInfo, _testMethod, _testContextImplementation);
532+
533+
_ = await testMethodRunner.RunTestMethodAsync();
534+
535+
// All three iterations observe the same key count (no leak from prior iterations) and
536+
// the outer context is unchanged.
537+
observedKeyCounts.Should().HaveCount(3);
538+
observedKeyCounts.Should().AllSatisfy(c => c.Should().Be(observedKeyCounts[0]));
539+
_testContextImplementation.Properties.Should().NotContainKey("AddedByIteration_0");
540+
_testContextImplementation.Properties.Should().NotContainKey("AddedByIteration_1");
541+
_testContextImplementation.Properties.Should().NotContainKey("AddedByIteration_2");
542+
}
543+
440544
#region Test data
441545

442546
private sealed class ExecutionContextUnsafeThreadTestMethodAttribute : TestMethodAttribute

0 commit comments

Comments
 (0)