Fix folded data-driven tests sharing TestContextImplementation across iterations

[Evangelink]

Fixes #7933.

Problem

The folded data-driven test execution path in TestMethodRunner (TryExecuteFoldedDataDrivenTestsAsync and the legacy ExecuteTestFromDataSourceAttributeAsync) reused the same TestContextImplementation across all DataRow iterations, while the unfolded path (DataType == ITestDataSource) creates a fresh TestMethodRunner -> own TestContextImplementation per row.

Any per-test state on TestContextImplementation that accumulates (captured stdout/stderr/trace, diagnostic messages, result files, outcome, exception, property bag mutations, ...) was therefore visible to subsequent iterations in the folded path. PR #7926 fixed the immediate O(n^2) TRX output symptom by clearing the three string builders, but any new accumulated field would be a repeat of the same bug.

Fix

Each iteration now runs against a fresh TestContextImplementation produced by CloneForDataDrivenIteration:

• Shares the message logger, the TestRunCancellationToken registration, TestRunCount, and a shallow snapshot of the property bag.
• Starts with no accumulated per-iteration state (output buffers, diagnostic messages, result files, outcome, exception, TestData, DataRow).

The clone is disposed at the end of each iteration. ExecuteTestWithDataSourceAsync / ExecuteTestWithDataRowAsync / ExecuteTestAsync now take the iteration context explicitly rather than reading the field _testContext directly, so the same code path serves both single-test and per-iteration execution.

This makes the folded path structurally equivalent to the unfolded path - mutations made by row N can no longer be observed by row N+1, and any new accumulated state field added to TestContextImplementation in the future is automatically isolated.

Tests added

• TestContextImplementationTests: 7 unit tests for CloneForDataDrivenIteration covering property-bag isolation, fresh output buffers, fresh outcome/exception, fresh result files, TestRunCount preservation, and message logger sharing.
• TestMethodRunnerTests: 3 regression tests for TryExecuteFoldedDataDrivenTestsAsync:
  • Each folded iteration receives a unique TestContext instance distinct from the outer one.
  • Console-out buffer is empty at the start of each iteration (no leak between rows).
  • Property bag is the same size at the start of each iteration even when each row adds a key (no leak between rows).

All 814 unit tests in MSTestAdapter.PlatformServices.UnitTests pass on net9.0, net8.0, and net462. Full build.cmd succeeds with 0 errors / 0 warnings.

[Copilot]

Pull request overview

This PR fixes state leakage between iterations in the folded data-driven execution path by ensuring each iteration runs with a fresh TestContextImplementation, aligning folded execution with the unfolded-per-row behavior and preventing accumulated per-test state from being observed by subsequent rows.

Changes:

• Added TestContextImplementation.CloneForDataDrivenIteration() to create per-iteration sibling contexts with isolated per-test state while preserving shared configuration (e.g., property bag snapshot, message logger, test-run cancellation token).
• Updated TestMethodRunner to pass an explicit execution context through the data-driven execution helpers and to create/dispose a fresh context per folded iteration (and per legacy DataSourceAttribute row).
• Added unit/regression tests covering cloning semantics and verifying no context/output/property-bag leakage across folded iterations.

Show a summary per file

File	Description
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/TestContextImplementationTests.cs	Adds unit tests validating cloning behavior (property bag snapshot/isolation, output/result-file isolation, logger/run count behavior).
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestMethodRunnerTests.cs	Adds regressions asserting folded iterations get distinct TestContext instances and don’t leak output/property mutations across rows.
src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs	Introduces CloneForDataDrivenIteration() and stores the test-run cancellation token for reuse when cloning.
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs	Refactors execution helpers to accept an explicit context and creates/disposes per-iteration contexts for folded + legacy data-driven paths.

Copilot's findings

• Files reviewed: 4/4 changed files
• Comments generated: 2

[Evangelink]

Expert Review — PR #8439: Fix folded data-driven tests sharing TestContextImplementation

Verdict: REQUEST_CHANGES (one blocking defect in tests; production code is sound)

---

Production Code — LGTM

Dimension	File	Verdict
Algorithmic Correctness	TestMethodRunner.cs, TestContextImplementation.cs	✅ LGTM
Threading & Concurrency	Both	✅ LGTM — iterations are sequential (each await serialises them); only one clone is alive at any time; Dispose() is always in a finally block
Resource & IDisposable Mgmt	TestMethodRunner.cs	✅ LGTM — iterationContext.Dispose() is unconditional in finally; the clone's CancellationTokenRegistration is cleaned up correctly
Security	N/A	✅ N/A
Public API	TestContextImplementation.cs	✅ LGTM — CloneForDataDrivenIteration is internal; no new public API surface
Performance	Both	✅ Acceptable — one Dictionary copy per data-row; not on the reflection/attribute caching hot-path
Cross-TFM	Both	✅ No TFM-guarded APIs added; #if NETFRAMEWORK regions in the clone path are handled by delegating to existing SetDataRow/SetDataConnection
Defensive Coding	TestMethodRunner.cs	✅ LGTM — clone is created before the try, no risk of null reference; SetArguments(null) cleanup preserved
Localization	N/A	✅ No new user-facing strings
Mock wiring	Test infra	✅ MockableReflectionOperations.GetCustomAttributesCached correctly delegates to mock.Object.GetCustomAttributes(memberInfo), so the GetCustomAttributes mock setups in the new tests are wired correctly
UnitTestOutcome.Failed default	TestContextImplementationTests.cs	✅ Failed = 0 is the first (default) enum value — the assertion is correct

---

Blocking Defect

All 10 new test methods are missing [TestMethod] (3 in TestMethodRunnerTests.cs, 7 in TestContextImplementationTests.cs).

MSTest will not discover or execute any of them. The PR description itself confirms this: "All 814 unit tests pass" — the count did not increase to 824, which means every new test is silently skipped. CloneForDataDrivenIteration has zero executed test coverage despite seven tests being written for it.

Fix: add [TestMethod] to each of the 10 methods. Inline comments are on the first affected method in each file.

Generated by Expert Code Review (on open) for issue #8439 · ● 15.1M

[Copilot]

Copilot's findings

• Files reviewed: 4/4 changed files
• Comments generated: 2