From 94c512e3553c22553f23e2d8bcb7e1003fb7bb75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Mon, 18 May 2026 03:44:21 +0200 Subject: [PATCH 1/4] Fix TestCaseStart/Stop events dropped for data-driven tests with shared TestCase.Id When data-driven tests share the same TestCase.Id (because all rows have identical fully-qualified names), RecordStart for rows after the first would be silently dropped by the deduplication guard in TestExecutionRecorder. Changed _testCaseInProgressMap from HashSet to Dictionary and removed the deduplication guard in RecordStart. Now every RecordStart call forwards a TestCaseStart event, and reference counting ensures each Start has exactly one matching End (TestCaseEnd is not sent if there is no preceding TestCaseStart). Fixes #4997 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Adapter/TestExecutionRecorder.cs | 36 +++++++++++-------- .../Adapter/TestExecutionRecorderTests.cs | 23 ++++++++++-- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs index 6ae681e5d5..2e610b8a1f 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs @@ -23,10 +23,10 @@ internal class TestExecutionRecorder : TestSessionMessageLogger, ITestExecutionR private readonly ITestCaseEventsHandler? _testCaseEventsHandler; /// - /// Contains TestCase Ids for test cases that are in progress - /// Start has been recorded but End has not yet been recorded. + /// Tracks the number of in-progress starts per test case ID. + /// Multiple data-driven test executions sharing the same ID are each counted. /// - private readonly HashSet _testCaseInProgressMap; + private readonly Dictionary _testCaseInProgressMap; private readonly object _testCaseInProgressSyncObject = new(); @@ -47,7 +47,7 @@ public TestExecutionRecorder(ITestCaseEventsHandler? testCaseEventsHandler, ITes // 3. Test Case Result. // If that is not that case. // If Test Adapters don't send the events in the above order, Test Case Results are cached till the Test Case End event is received. - _testCaseInProgressMap = new HashSet(); + _testCaseInProgressMap = new Dictionary(); } /// @@ -75,12 +75,12 @@ public void RecordStart(TestCase testCase) { lock (_testCaseInProgressSyncObject) { - // Do not send TestCaseStart for a test in progress - if (!_testCaseInProgressMap.Contains(testCase.Id)) - { - _testCaseInProgressMap.Add(testCase.Id); - _testCaseEventsHandler.SendTestCaseStart(testCase); - } + // Track the number of in-progress starts for this test case ID. + // Data-driven tests may call RecordStart multiple times with the same ID + // (when rows share the same fully-qualified name), so we must forward + // every start rather than deduplicating by ID. + _testCaseInProgressMap[testCase.Id] = _testCaseInProgressMap.TryGetValue(testCase.Id, out int count) ? count + 1 : 1; + _testCaseEventsHandler.SendTestCaseStart(testCase); } } } @@ -129,14 +129,22 @@ private void SendTestCaseEnd(TestCase testCase, TestOutcome outcome) { lock (_testCaseInProgressSyncObject) { - // TestCaseEnd must always be preceded by TestCaseStart for a given test case id - if (_testCaseInProgressMap.Contains(testCase.Id)) + // TestCaseEnd must always be preceded by TestCaseStart for a given test case id. + // Use the reference count to ensure we send exactly one End for each Start. + if (_testCaseInProgressMap.TryGetValue(testCase.Id, out int count) && count > 0) { // Send test case end event to handler. _testCaseEventsHandler.SendTestCaseEnd(testCase, outcome); - // Remove it from map so that we send only one TestCaseEnd for every TestCaseStart. - _testCaseInProgressMap.Remove(testCase.Id); + // Decrement the count; remove the entry when there are no more in-progress starts. + if (count == 1) + { + _testCaseInProgressMap.Remove(testCase.Id); + } + else + { + _testCaseInProgressMap[testCase.Id] = count - 1; + } } } } diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/TestExecutionRecorderTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/TestExecutionRecorderTests.cs index cce3ab3ad1..4c0772a380 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/TestExecutionRecorderTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/TestExecutionRecorderTests.cs @@ -156,15 +156,32 @@ public void RecordEndShouldInvokeSendTestCaseEndMultipleTimesInDataDrivenScenari } [TestMethod] - public void RecordStartAndRecordEndShouldIgnoreRedundantTestCaseStartAndTestCaseEnd() + public void RecordStartAndRecordEndShouldForwardAllTestCaseStartAndEndEvents() { _testRecorderWithTestEventsHandler.RecordStart(_testCase); _testRecorderWithTestEventsHandler.RecordStart(_testCase); _testRecorderWithTestEventsHandler.RecordEnd(_testCase, TestOutcome.Passed); _testRecorderWithTestEventsHandler.RecordEnd(_testCase, TestOutcome.Passed); - _mockTestCaseEventsHandler.Verify(x => x.SendTestCaseStart(_testCase), Times.Exactly(1)); - _mockTestCaseEventsHandler.Verify(x => x.SendTestCaseEnd(_testCase, TestOutcome.Passed), Times.Exactly(1)); + _mockTestCaseEventsHandler.Verify(x => x.SendTestCaseStart(_testCase), Times.Exactly(2)); + _mockTestCaseEventsHandler.Verify(x => x.SendTestCaseEnd(_testCase, TestOutcome.Passed), Times.Exactly(2)); + } + + [TestMethod] + public void RecordStartAndRecordEndShouldSendEventsForNestedDataDrivenTestsWithSameId() + { + // Simulate a data-driven scenario where the parent test and its row executions + // share the same TestCase.Id (same fully-qualified name), with nested Start calls: + // Start(parent) → Start(row1) → End(row1) → Start(row2) → End(row2) → End(parent) + _testRecorderWithTestEventsHandler.RecordStart(_testCase); // parent start + _testRecorderWithTestEventsHandler.RecordStart(_testCase); // row1 start (same ID) + _testRecorderWithTestEventsHandler.RecordEnd(_testCase, TestOutcome.Passed); // row1 end + _testRecorderWithTestEventsHandler.RecordStart(_testCase); // row2 start (same ID) + _testRecorderWithTestEventsHandler.RecordEnd(_testCase, TestOutcome.Passed); // row2 end + _testRecorderWithTestEventsHandler.RecordEnd(_testCase, TestOutcome.Passed); // parent end + + _mockTestCaseEventsHandler.Verify(x => x.SendTestCaseStart(_testCase), Times.Exactly(3)); + _mockTestCaseEventsHandler.Verify(x => x.SendTestCaseEnd(_testCase, TestOutcome.Passed), Times.Exactly(3)); } [TestMethod] From 9a7b32d0b21e08050b39f362a33833a4878f2151 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Mon, 18 May 2026 04:02:32 +0200 Subject: [PATCH 2/4] fix: guard RecordResult safety-net against consuming parent count in nested data-driven scenario MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When data-driven test rows share the same TestCase.Id, RecordResult's safety-net was calling SendTestCaseEnd even after RecordEnd had already decremented the reference count for a row. This consumed the count slot belonging to the parent, causing the parent's RecordEnd to be silently dropped. Fix: track IDs for which RecordEnd was called while still in-progress (_testCaseEndCalledSet). The RecordResult safety-net now skips if an explicit RecordEnd was already sent for that ID. The set entry is cleared when the last in-progress count reaches zero. Add test RecordResultShouldNotSendSpuriousTestCaseEndForParentInNestedDataDrivenScenario covering: Start(parent) → Start(row1) → End(row1) → Result(row1) → End(parent) which was producing 3 Ends (2 expected) before this fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Adapter/TestExecutionRecorder.cs | 54 ++++++++++++++----- .../Adapter/TestExecutionRecorderTests.cs | 25 +++++++++ 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs index 2e610b8a1f..213840044d 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs @@ -28,6 +28,16 @@ internal class TestExecutionRecorder : TestSessionMessageLogger, ITestExecutionR /// private readonly Dictionary _testCaseInProgressMap; + /// + /// Tracks test case IDs for which has been called at least once + /// while the entry is still in progress (count > 0). Used to suppress the + /// safety-net in nested data-driven scenarios: once an + /// explicit RecordEnd fires for an ID, subsequent RecordResult calls must not send a + /// spurious extra TestCaseEnd that would consume the parent's pending count slot. + /// The ID is removed when the last in-progress count reaches zero. + /// + private readonly HashSet _testCaseEndCalledSet; + private readonly object _testCaseInProgressSyncObject = new(); /// @@ -48,6 +58,7 @@ public TestExecutionRecorder(ITestCaseEventsHandler? testCaseEventsHandler, ITes // If that is not that case. // If Test Adapters don't send the events in the above order, Test Case Results are cached till the Test Case End event is received. _testCaseInProgressMap = new Dictionary(); + _testCaseEndCalledSet = new HashSet(); } /// @@ -96,8 +107,29 @@ public void RecordResult(TestResult testResult) EqtTrace.Verbose("TestExecutionRecorder.RecordResult: Received result for test: {0}.", testResult.TestCase.FullyQualifiedName); if (_testCaseEventsHandler != null) { - // Send TestCaseEnd in case RecordEnd was not called. - SendTestCaseEnd(testResult.TestCase, testResult.Outcome); + lock (_testCaseInProgressSyncObject) + { + // Safety net: send TestCaseEnd in case RecordEnd was not called. + // Guard: skip if RecordEnd was already called for this ID (indicated by presence in + // _testCaseEndCalledSet) to avoid consuming a count slot belonging to a parent or + // sibling in a nested data-driven scenario where all rows share the same TestCase.Id. + if (_testCaseInProgressMap.TryGetValue(testResult.TestCase.Id, out int count) + && count > 0 + && !_testCaseEndCalledSet.Contains(testResult.TestCase.Id)) + { + _testCaseEventsHandler.SendTestCaseEnd(testResult.TestCase, testResult.Outcome); + + if (count == 1) + { + _testCaseInProgressMap.Remove(testResult.TestCase.Id); + } + else + { + _testCaseInProgressMap[testResult.TestCase.Id] = count - 1; + } + } + } + _testCaseEventsHandler.SendTestResult(testResult); } @@ -115,16 +147,7 @@ public void RecordEnd(TestCase testCase, TestOutcome outcome) { EqtTrace.Verbose("TestExecutionRecorder.RecordEnd: test: {0} execution completed.", testCase.FullyQualifiedName); _testRunCache.OnTestCompletion(testCase); - SendTestCaseEnd(testCase, outcome); - } - /// - /// Send TestCaseEnd event for given testCase if not sent already - /// - /// - /// - private void SendTestCaseEnd(TestCase testCase, TestOutcome outcome) - { if (_testCaseEventsHandler != null) { lock (_testCaseInProgressSyncObject) @@ -133,13 +156,18 @@ private void SendTestCaseEnd(TestCase testCase, TestOutcome outcome) // Use the reference count to ensure we send exactly one End for each Start. if (_testCaseInProgressMap.TryGetValue(testCase.Id, out int count) && count > 0) { - // Send test case end event to handler. + // Mark that RecordEnd was called for this ID while it is still in progress. + // This suppresses the RecordResult safety-net for any subsequent Result calls + // that share the same ID (e.g., row results in a nested data-driven test). + _testCaseEndCalledSet.Add(testCase.Id); + _testCaseEventsHandler.SendTestCaseEnd(testCase, outcome); - // Decrement the count; remove the entry when there are no more in-progress starts. + // Decrement the count; remove both tracking entries when there are no more in-progress starts. if (count == 1) { _testCaseInProgressMap.Remove(testCase.Id); + _testCaseEndCalledSet.Remove(testCase.Id); } else { diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/TestExecutionRecorderTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/TestExecutionRecorderTests.cs index 4c0772a380..b984f7f81b 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/TestExecutionRecorderTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/TestExecutionRecorderTests.cs @@ -236,5 +236,30 @@ public void RecordResultShouldSendTestCaseEndEventIfRecordEndWasNotCalled() Assert.Contains(_testResult, _testableTestRunCache.TestResultList); } + [TestMethod] + public void RecordResultShouldNotSendSpuriousTestCaseEndForParentInNestedDataDrivenScenario() + { + // Regression test for the nested data-driven scenario where the parent test and its row + // executions share the same TestCase.Id (same fully-qualified name). + // + // Pattern: Start(parent) → Start(row1) → End(row1) → Result(row1) → End(parent) + // + // Before the fix, RecordResult's safety-net would fire after End(row1) decremented + // the count to 1, consuming the count slot that belonged to the parent and causing + // the subsequent End(parent) to be silently dropped. + _testResult.Outcome = TestOutcome.Passed; + + _testRecorderWithTestEventsHandler.RecordStart(_testCase); // parent start → count=1 + _testRecorderWithTestEventsHandler.RecordStart(_testCase); // row1 start → count=2 + _testRecorderWithTestEventsHandler.RecordEnd(_testCase, TestOutcome.Passed); // row1 end → count=1, sends End + _testRecorderWithTestEventsHandler.RecordResult(_testResult); // row1 result → safety-net must NOT fire + _testRecorderWithTestEventsHandler.RecordEnd(_testCase, TestOutcome.Passed); // parent end → count=0, sends End + + // Exactly two Ends: one for row1 (from RecordEnd) and one for the parent (from RecordEnd). + // A third End from the RecordResult safety-net would indicate the regression is present. + _mockTestCaseEventsHandler.Verify(x => x.SendTestCaseEnd(_testCase, TestOutcome.Passed), Times.Exactly(2)); + _mockTestCaseEventsHandler.Verify(x => x.SendTestResult(_testResult), Times.Once); + } + #endregion } From 83d3cc08945d6ca4c670f186425245ae827042fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Mon, 18 May 2026 04:16:41 +0200 Subject: [PATCH 3/4] docs: document known ID-scoped suppression limitation in _testCaseEndCalledSet Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Adapter/TestExecutionRecorder.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs index 213840044d..4236570b77 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Adapter/TestExecutionRecorder.cs @@ -35,6 +35,13 @@ internal class TestExecutionRecorder : TestSessionMessageLogger, ITestExecutionR /// explicit RecordEnd fires for an ID, subsequent RecordResult calls must not send a /// spurious extra TestCaseEnd that would consume the parent's pending count slot. /// The ID is removed when the last in-progress count reaches zero. + /// + /// Known limitation: suppression is ID-scoped, not per-invocation. If an adapter + /// calls for some rows sharing the same + /// but relies on the safety-net for others, those latter rows + /// will have their safety-net suppressed. In practice this is not a concern because + /// real adapters apply uniformly across all rows with the same ID. + /// /// private readonly HashSet _testCaseEndCalledSet; From 606a6882d3caa2002c6e7d95ef5fd92b0aa2b5f6 Mon Sep 17 00:00:00 2001 From: Jakub Jares Date: Mon, 18 May 2026 11:40:03 +0200 Subject: [PATCH 4/4] test: add acceptance test for data-driven TestCaseStart events (#4997) Verifies that data collectors receive TestCaseStart for every DataRow execution, not just the first one. Uses the existing SampleDataCollector which creates one file per TestCaseStart event, and asserts the file count matches the number of test executions (3 DataRow rows + 1 simple test). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DataCollectionTests.cs | 38 +++++++++++++++++++ .../DataDrivenTestProject.csproj | 21 ++++++++++ .../DataDrivenTestProject/DataDrivenTests.cs | 25 ++++++++++++ test/TestAssets/TestAssets.slnx | 1 + 4 files changed, 85 insertions(+) create mode 100644 test/TestAssets/DataDrivenTestProject/DataDrivenTestProject.csproj create mode 100644 test/TestAssets/DataDrivenTestProject/DataDrivenTests.cs diff --git a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectionTests.cs b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectionTests.cs index 7beb391ba9..1b4fb75c0c 100644 --- a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectionTests.cs +++ b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectionTests.cs @@ -241,4 +241,42 @@ private static string GetRunsettingsFilePath(string resultsDir) CreateDataCollectionRunSettingsFile(runsettingsPath, dataCollectionAttributes); return runsettingsPath; } + + [TestMethod] + [NetFullTargetFrameworkDataSource] + [NetCoreTargetFrameworkDataSource] + public void DataCollectorReceivesTestCaseStartForEveryDataDrivenRow(RunnerInfo runnerInfo) + { + // Regression test for https://github.com/microsoft/vstest/issues/4997 + // When data-driven tests share the same TestCase.Id, TestCaseStart events + // must still fire for every row execution so data collectors can track each one. + SetTestEnvironment(_testEnvironment, runnerInfo); + + var assemblyPaths = GetAssetFullPath("DataDrivenTestProject.dll"); + string runSettings = GetRunsettingsFilePath(TempDirectory.Path); + string diagFileName = Path.Combine(TempDirectory.Path, "diaglog.txt"); + var extensionsPath = Path.GetDirectoryName(GetTestDllForFramework("OutOfProcDataCollector.dll", "netstandard2.0")); + var arguments = PrepareArguments(assemblyPaths, null, runSettings, FrameworkArgValue, runnerInfo.InIsolationValue, resultsDirectory: TempDirectory.Path); + arguments = string.Concat(arguments, $" /Diag:{diagFileName}", $" /TestAdapterPath:{extensionsPath}"); + + var env = new Dictionary + { + ["TEST_ASSET_SAMPLE_COLLECTOR_PATH"] = TempDirectory.Path, + }; + + InvokeVsTest(arguments, env); + + // DataDrivenTestProject has 4 test executions: 3 DataRow rows + 1 simple test. + ValidateSummaryStatus(4, 0, 0); + + // The SampleDataCollector creates one testcasefilename{i}.txt per TestCaseStart event. + // Before the fix, DataRow rows sharing the same TestCase.Id would get deduplicated, + // producing fewer files than actual test executions. + var resultFiles = Directory.GetFiles(TempDirectory.Path, "testcasefilename*", SearchOption.AllDirectories); + Assert.HasCount(4, resultFiles); + + // Verify the collector logged start/end for each execution. + StdOutputContains("TestCaseStarted"); + StdOutputContains("TestCaseEnded"); + } } diff --git a/test/TestAssets/DataDrivenTestProject/DataDrivenTestProject.csproj b/test/TestAssets/DataDrivenTestProject/DataDrivenTestProject.csproj new file mode 100644 index 0000000000..d44f992bf8 --- /dev/null +++ b/test/TestAssets/DataDrivenTestProject/DataDrivenTestProject.csproj @@ -0,0 +1,21 @@ + + + + + DataDrivenTestProject + $(TestProjectTargetFrameworks) + + + + + $(MSTestTestFrameworkVersion) + + + $(MSTestTestAdapterVersion) + + + $(PackageVersion) + + + + diff --git a/test/TestAssets/DataDrivenTestProject/DataDrivenTests.cs b/test/TestAssets/DataDrivenTestProject/DataDrivenTests.cs new file mode 100644 index 0000000000..46bd26f416 --- /dev/null +++ b/test/TestAssets/DataDrivenTestProject/DataDrivenTests.cs @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace DataDrivenTestProject; + +[TestClass] +public class DataDrivenTests +{ + [TestMethod] + [DataRow(1, "first")] + [DataRow(2, "second")] + [DataRow(3, "third")] + public void ParameterizedTest(int value, string name) + { + Assert.IsTrue(value > 0); + Assert.IsNotNull(name); + } + + [TestMethod] + public void SimpleTest() + { + } +} diff --git a/test/TestAssets/TestAssets.slnx b/test/TestAssets/TestAssets.slnx index 83551a9a06..0d5f0ccd3e 100644 --- a/test/TestAssets/TestAssets.slnx +++ b/test/TestAssets/TestAssets.slnx @@ -67,6 +67,7 @@ +