Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ internal async Task<TestResult[]> RunTestMethodAsync()
}

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

foreach (TestResult testResult in testResults)
{
Expand Down Expand Up @@ -207,6 +207,8 @@ private async Task<bool> TryExecuteDataSourceBasedTestsAsync(List<TestResult> re
private async Task<bool> TryExecuteFoldedDataDrivenTestsAsync(List<TestResult> results)
{
bool hasTestDataSource = false;
var outerContext = (TestContextImplementation)_testContext.Context;

foreach (Attribute attribute in PlatformServiceProvider.Instance.ReflectionOperations.GetCustomAttributesCached(_testMethodInfo.MethodInfo))
{
if (attribute is not UTF.ITestDataSource testDataSource)
Expand All @@ -227,15 +229,24 @@ private async Task<bool> TryExecuteFoldedDataDrivenTestsAsync(List<TestResult> r
foreach (object?[] data in testDataSource.GetData(_testMethodInfo.MethodInfo))
{
dataSourceHasData = true;

// Create a fresh TestContextImplementation per iteration so the folded path is
// structurally equivalent to the unfolded path (where each row gets its own
// context). This isolates per-row state (captured output, diagnostic messages,
// result files, outcome, exception, property bag mutations, ...) so a leak in
// any current or future TestContextImplementation field cannot accumulate across
// rows. See https://github.com/microsoft/testfx/issues/7933.
TestContextImplementation iterationContext = outerContext.CloneForDataDrivenIteration();
try
{
TestResult[] testResults = await ExecuteTestWithDataSourceAsync(testDataSource, data, actualDataAlreadyHandledDuringDiscovery: false).ConfigureAwait(false);
TestResult[] testResults = await ExecuteTestWithDataSourceAsync(iterationContext, testDataSource, data, actualDataAlreadyHandledDuringDiscovery: false).ConfigureAwait(false);

results.AddRange(testResults);
}
finally
{
_testMethodInfo.SetArguments(null);
iterationContext.Dispose();
}
}

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

foreach (object dataRow in dataRows)
{
TestResult[] testResults = await ExecuteTestWithDataRowAsync(dataRow, rowIndex++).ConfigureAwait(false);
results.AddRange(testResults);
// Create a fresh TestContextImplementation per row for the same structural
// reason as in TryExecuteFoldedDataDrivenTestsAsync — each row should
// start with no accumulated per-test state.
TestContextImplementation iterationContext = outerContext.CloneForDataDrivenIteration();
try
{
TestResult[] testResults = await ExecuteTestWithDataRowAsync(iterationContext, dataRow, rowIndex++).ConfigureAwait(false);
results.AddRange(testResults);
}
finally
{
iterationContext.Dispose();
}
}
Comment thread
Evangelink marked this conversation as resolved.
}
finally
Expand All @@ -303,7 +326,7 @@ private async Task ExecuteTestFromDataSourceAttributeAsync(List<TestResult> resu
}
}

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

var stopwatch = Stopwatch.StartNew();
_testMethodInfo.SetArguments(data);
_testContext.SetTestData(data);
_testContext.SetDisplayName(displayName);
executionContext.SetTestData(data);
executionContext.SetDisplayName(displayName);

TestResult[] testResults = ignoreFromTestDataRow is not null
? [TestResult.CreateIgnoredResult(ignoreFromTestDataRow)]
: await ExecuteTestAsync(_testMethodInfo).ConfigureAwait(false);
: await ExecuteTestAsync(executionContext, _testMethodInfo).ConfigureAwait(false);

stopwatch.Stop();

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

private async Task<TestResult[]> ExecuteTestWithDataRowAsync(object dataRow, int rowIndex)
private async Task<TestResult[]> ExecuteTestWithDataRowAsync(ITestContext executionContext, object dataRow, int rowIndex)
{
string displayName = string.Format(CultureInfo.CurrentCulture, Resource.DataDrivenResultDisplayName, _test.DisplayName, rowIndex);
Stopwatch? stopwatch = null;
Expand All @@ -384,13 +407,13 @@ private async Task<TestResult[]> ExecuteTestWithDataRowAsync(object dataRow, int
try
{
stopwatch = Stopwatch.StartNew();
_testContext.SetDataRow(dataRow);
testResults = await ExecuteTestAsync(_testMethodInfo).ConfigureAwait(false);
executionContext.SetDataRow(dataRow);
testResults = await ExecuteTestAsync(executionContext, _testMethodInfo).ConfigureAwait(false);
}
finally
{
stopwatch?.Stop();
_testContext.SetDataRow(null);
executionContext.SetDataRow(null);
}

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

private async Task<TestResult[]> ExecuteTestAsync(TestMethodInfo testMethodInfo)
private async Task<TestResult[]> ExecuteTestAsync(ITestContext executionContext, TestMethodInfo testMethodInfo)
{
try
{
Expand All @@ -415,9 +438,9 @@ private async Task<TestResult[]> ExecuteTestAsync(TestMethodInfo testMethodInfo)
{
try
{
using (TestContextImplementation.SetCurrentTestContext(_testContext as TestContext))
using (TestContextImplementation.SetCurrentTestContext(executionContext as TestContext))
{
testMethodInfo.TestContext = _testContext;
testMethodInfo.TestContext = executionContext;
tcs.SetResult(await _testMethodInfo.Executor.ExecuteAsync(testMethodInfo).ConfigureAwait(false));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public override string ToString()
/// </summary>
private readonly Dictionary<string, object?> _properties;
private readonly IMessageLogger? _messageLogger;
private readonly TestRunCancellationToken? _testRunCancellationToken;

private CancellationTokenRegistration? _cancellationTokenRegistration;

Expand Down Expand Up @@ -140,6 +141,7 @@ internal TestContextImplementation(ITestMethod? testMethod, string? testClassFul
}

_messageLogger = messageLogger;
_testRunCancellationToken = testRunCancellationToken;
_cancellationTokenRegistration = testRunCancellationToken?.Register(CancelDelegate, this);
}

Expand Down Expand Up @@ -460,4 +462,45 @@ private SynchronizedStringBuilder GetTestContextMessagesStringBuilder()

internal string? GetAndClearTrace()
=> _traceStringBuilder?.GetAndClear();

/// <summary>
/// Creates a sibling <see cref="TestContextImplementation"/> for use by a single iteration
/// of the folded data-driven test execution path.
/// <para>
/// The clone inherits the same configuration as this context (a shallow snapshot of the
/// property bag, the message logger, the same test-run cancellation token, and on .NET
/// Framework the current data connection), but registers its own cancellation callback and
/// starts with no accumulated per-test state (no captured stdout/stderr/trace,
/// no diagnostic messages, no result files, no exception, no data row, and the
/// default <see cref="UnitTestOutcome"/> value rather than the original's current outcome).
/// This keeps the folded path structurally equivalent to the unfolded path, where each
/// row gets its own <see cref="TestContextImplementation"/>.
/// </para>
/// </summary>
/// <returns>A fresh context suitable for one folded data-driven iteration.</returns>
internal TestContextImplementation CloneForDataDrivenIteration()
{
// Take a shallow snapshot of the current property bag so that the clone starts with
// the same properties (including TestNameLabel / FullyQualifiedTestClassNameLabel and
// anything merged from AssemblyInitialize / ClassInitialize) but is otherwise isolated.
// Per-iteration mutations to the clone's property bag won't leak back to this instance
// nor to subsequent iterations.
var snapshot = new Dictionary<string, object?>(_properties);

// Pass testMethod: null and testClassFullName: null because the relevant labels are
// already in the snapshot. The constructor will copy the snapshot as-is.
var clone = new TestContextImplementation(testMethod: null, testClassFullName: null, snapshot, _messageLogger, _testRunCancellationToken);

// Preserve TestRunCount so user code that observes it (e.g. retry-aware tests) sees
// the same value it would see in the unfolded path. TestRunCount represents the
// execution-attempt count of this test, not per-row state, so it must flow into
// each iteration's context.
clone.Context.TestRunCount = Context.TestRunCount;

#if NETFRAMEWORK
clone.SetDataConnection(_dbConnection);
#endif

return clone;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,110 @@ public async Task RunTestMethodShouldPassWhenAttributeInvokesTestMethodOnExecuti
results[0].Outcome.Should().Be(UnitTestOutcome.Passed);
}

public async Task RunTestMethodShouldUseFreshTestContextPerIterationForFoldedDataDrivenTests()
Comment thread
Evangelink marked this conversation as resolved.
{
// Capture TestContext.Current per iteration so we can verify each row gets a distinct
// TestContextImplementation instance — see https://github.com/microsoft/testfx/issues/7933.
#pragma warning disable MSTESTEXP // TestContext.Current is experimental.
var observedContexts = new List<TestContext?>();
DataRowAttribute dataRowAttribute1 = new(1);
DataRowAttribute dataRowAttribute2 = new(2);
DataRowAttribute dataRowAttribute3 = new(3);
var attributes = new Attribute[] { dataRowAttribute1, dataRowAttribute2, dataRowAttribute3 };

_testablePlatformServiceProvider.MockReflectionOperations.Setup(ro => ro.GetCustomAttributes(_methodInfo)).Returns(attributes);

var testMethodInfo = new TestableTestMethodInfo(_methodInfo, _testClassInfo, _testMethodOptions, () =>
{
observedContexts.Add(TestContext.Current);
return new TestResult { Outcome = UnitTestOutcome.Passed };
});
var testMethodRunner = new TestMethodRunner(testMethodInfo, _testMethod, _testContextImplementation);

_ = await testMethodRunner.RunTestMethodAsync();

observedContexts.Should().HaveCount(3);
observedContexts.Should().AllSatisfy(c => c.Should().NotBeNull());

// Each iteration must observe its own fresh context instance — none should be the
// outer shared context, and no two iterations should share an instance.
observedContexts.Should().OnlyHaveUniqueItems();
observedContexts.Should().NotContain(_testContextImplementation);
#pragma warning restore MSTESTEXP
}

public async Task RunTestMethodShouldNotLeakCapturedOutputAcrossFoldedDataDrivenIterations()
{
// Each iteration writes to its own context's console-out buffer. With a fresh
// per-iteration context, the iteration that fires write N must observe an empty buffer
// before writing — without the fix, row 2 would observe row 1's content.
var observedLengthsBeforeWrite = new List<int>();
DataRowAttribute dataRowAttribute1 = new(1);
DataRowAttribute dataRowAttribute2 = new(2);
DataRowAttribute dataRowAttribute3 = new(3);
var attributes = new Attribute[] { dataRowAttribute1, dataRowAttribute2, dataRowAttribute3 };

_testablePlatformServiceProvider.MockReflectionOperations.Setup(ro => ro.GetCustomAttributes(_methodInfo)).Returns(attributes);

var testMethodInfo = new TestableTestMethodInfo(_methodInfo, _testClassInfo, _testMethodOptions, () =>
{
#pragma warning disable MSTESTEXP // TestContext.Current is experimental.
var current = (TestContextImplementation?)TestContext.Current;
#pragma warning restore MSTESTEXP
current.Should().NotBeNull();
string? existing = current!.GetAndClearOutput();
observedLengthsBeforeWrite.Add(existing?.Length ?? 0);

// Write a fairly large chunk of console output. If contexts were shared, the next
// iteration would observe a non-zero existing length.
current.WriteConsoleOut(new string('x', 1024));
return new TestResult { Outcome = UnitTestOutcome.Passed };
});
var testMethodRunner = new TestMethodRunner(testMethodInfo, _testMethod, _testContextImplementation);

_ = await testMethodRunner.RunTestMethodAsync();

observedLengthsBeforeWrite.Should().HaveCount(3);
observedLengthsBeforeWrite.Should().AllSatisfy(len => len.Should().Be(0));
}

public async Task RunTestMethodShouldNotLeakPropertyBagMutationsAcrossFoldedDataDrivenIterations()
{
// Each iteration adds a unique property key. If contexts were shared, row N would see
// the keys added by rows 1..N-1. With a fresh per-iteration context, every row starts
// with the same baseline property set.
var observedKeyCounts = new List<int>();
DataRowAttribute dataRowAttribute1 = new(1);
DataRowAttribute dataRowAttribute2 = new(2);
DataRowAttribute dataRowAttribute3 = new(3);
var attributes = new Attribute[] { dataRowAttribute1, dataRowAttribute2, dataRowAttribute3 };

_testablePlatformServiceProvider.MockReflectionOperations.Setup(ro => ro.GetCustomAttributes(_methodInfo)).Returns(attributes);

int iteration = 0;
var testMethodInfo = new TestableTestMethodInfo(_methodInfo, _testClassInfo, _testMethodOptions, () =>
{
#pragma warning disable MSTESTEXP // TestContext.Current is experimental.
TestContext? current = TestContext.Current;
#pragma warning restore MSTESTEXP
current.Should().NotBeNull();
observedKeyCounts.Add(current!.Properties.Count);
current.Properties[$"AddedByIteration_{iteration++}"] = "value";
return new TestResult { Outcome = UnitTestOutcome.Passed };
});
var testMethodRunner = new TestMethodRunner(testMethodInfo, _testMethod, _testContextImplementation);

_ = await testMethodRunner.RunTestMethodAsync();

// All three iterations observe the same key count (no leak from prior iterations) and
// the outer context is unchanged.
observedKeyCounts.Should().HaveCount(3);
observedKeyCounts.Should().AllSatisfy(c => c.Should().Be(observedKeyCounts[0]));
_testContextImplementation.Properties.Should().NotContainKey("AddedByIteration_0");
_testContextImplementation.Properties.Should().NotContainKey("AddedByIteration_1");
_testContextImplementation.Properties.Should().NotContainKey("AddedByIteration_2");
}

#region Test data

private sealed class ExecutionContextUnsafeThreadTestMethodAttribute : TestMethodAttribute
Expand Down
Loading
Loading