From 684bc553ddecb5d5607af7e715c04ac2958b8c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 12 Mar 2026 22:08:31 +0100 Subject: [PATCH 1/4] Optimize TestMethodRunner: fix double enumeration, reduce allocations - Fix double enumeration in TryExecuteFoldedDataDrivenTestsAsync by materializing GetData() into an IReadOnlyList once instead of calling .Any() then re-enumerating - Remove unnecessary List allocation in UpdateResultsWithParentInfo by mutating in place and returning void - Cache result[result.Length - 1] into a local in ExecuteAsync catch block - Skip redundant first-element self-comparison in GetAggregateOutcome - Use Stopwatch.StartNew() in ExecuteTestFromDataSourceAttributeAsync --- .../Execution/TestMethodRunner.cs | 44 +++++++------------ 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs b/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs index 6f02c9a404..676c451e71 100644 --- a/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs +++ b/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs @@ -84,14 +84,15 @@ internal async Task ExecuteAsync(string? initializationLogs, strin } #pragma warning disable IDE0056 // Use index operator + TestResult lastResult = result[result.Length - 1]; result[result.Length - 1] = new TestResult { TestFailureException = new TestFailedException(UnitTestOutcome.Error, ex.TryGetMessage(), ex.TryGetStackTraceInformation()), - LogOutput = result[result.Length - 1].LogOutput, - LogError = result[result.Length - 1].LogError, - DebugTrace = result[result.Length - 1].DebugTrace, - TestContextMessages = result[result.Length - 1].TestContextMessages, - Duration = result[result.Length - 1].Duration, + LogOutput = lastResult.LogOutput, + LogError = lastResult.LogError, + DebugTrace = lastResult.DebugTrace, + TestContextMessages = lastResult.TestContextMessages, + Duration = lastResult.Duration, }; #pragma warning restore IDE0056 // Use index operator } @@ -167,7 +168,7 @@ internal async Task RunTestMethodAsync() // In case of data driven, set parent info in results. if (isDataDriven) { - results = UpdateResultsWithParentInfo(results); + UpdateResultsWithParentInfo(results); } // Set a result in case no result is present. @@ -214,13 +215,13 @@ private async Task TryExecuteFoldedDataDrivenTestsAsync(List r continue; } - IEnumerable? dataSource; - // This code is to execute tests. To discover the tests code is in AssemblyEnumerator.ProcessTestDataSourceTests. // Any change made here should be reflected in AssemblyEnumerator.ProcessTestDataSourceTests as well. - dataSource = testDataSource.GetData(_testMethodInfo.MethodInfo); + IReadOnlyList dataSource = testDataSource.GetData(_testMethodInfo.MethodInfo) is IReadOnlyList dataList + ? dataList + : testDataSource.GetData(_testMethodInfo.MethodInfo).ToList(); - if (!dataSource.Any()) + if (dataSource.Count == 0) { if (!MSTestSettings.CurrentSettings.ConsiderEmptyDataSourceAsInconclusive) { @@ -255,8 +256,7 @@ private async Task TryExecuteFoldedDataDrivenTestsAsync(List r private async Task ExecuteTestFromDataSourceAttributeAsync(List results) { - Stopwatch watch = new(); - watch.Start(); + var watch = Stopwatch.StartNew(); try { @@ -449,9 +449,9 @@ private static UnitTestOutcome GetAggregateOutcome(List results) // Get aggregate outcome. UnitTestOutcome aggregateOutcome = results[0].Outcome; - foreach (TestResult result in results) + for (int i = 1; i < results.Count; i++) { - aggregateOutcome = aggregateOutcome.GetMoreImportantOutcome(result.Outcome); + aggregateOutcome = aggregateOutcome.GetMoreImportantOutcome(results[i].Outcome); } return aggregateOutcome; @@ -462,26 +462,12 @@ private static UnitTestOutcome GetAggregateOutcome(List results) /// Add parent results as first result in updated result. /// /// Results. - /// Updated results which contains parent result as first result. All other results contains parent result info. - private static List UpdateResultsWithParentInfo(List results) + private static void UpdateResultsWithParentInfo(List results) { - // Return results in case there are no results. - if (results.Count == 0) - { - return results; - } - - // UpdatedResults contain parent result at first position and remaining results has parent info updated. - var updatedResults = new List(); - foreach (TestResult result in results) { result.ExecutionId = Guid.NewGuid(); result.ParentExecId = Guid.NewGuid(); - - updatedResults.Add(result); } - - return updatedResults; } } From 6fd6dfed32299e1814e9c8fbfa7567f54e82e058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 12 Mar 2026 22:10:15 +0100 Subject: [PATCH 2/4] Optimize UnitTestRunner: hoist cast, clean up code - Hoist loop-invariant 'testContext as TestContextImplementation' cast above the foreach loop in RunAssemblyCleanupAsync - Remove unnecessary nested scope in IsTestMethodRunnable - Use 'is null' pattern instead of '== null' - Simplify ignore message logic by inlining into the conditional block --- .../Execution/UnitTestRunner.cs | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs b/src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs index 6608e4f9d2..292e498e1f 100644 --- a/src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs +++ b/src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs @@ -273,12 +273,12 @@ private static async Task RunAssemblyInitializeIfNeededAsync(TestMet private static async Task RunAssemblyCleanupAsync(ITestContext testContext, TypeCache typeCache, TestResult[] results) { + var testContextImpl = testContext as TestContextImplementation; IEnumerable assemblyInfoCache = typeCache.AssemblyInfoListWithExecutableCleanupMethods; foreach (TestAssemblyInfo assemblyInfo in assemblyInfoCache) { TestFailedException? ex = await assemblyInfo.ExecuteAssemblyCleanupAsync(testContext.Context).ConfigureAwait(false); - var testContextImpl = testContext as TestContextImplementation; if (ex is not null) { return new TestResult() @@ -318,32 +318,25 @@ private static bool IsTestMethodRunnable( [NotNullWhen(false)] out TestResult[]? notRunnableResult) { // If the specified TestMethod could not be found, return a NotFound result. - if (testMethodInfo == null) + if (testMethodInfo is null) { - { - notRunnableResult = - [ - new TestResult - { - Outcome = UnitTestOutcome.NotFound, - IgnoreReason = string.Format(CultureInfo.CurrentCulture, Resource.TestNotFound, testMethod.Name), - }, - ]; - return false; - } + notRunnableResult = + [ + new TestResult + { + Outcome = UnitTestOutcome.NotFound, + IgnoreReason = string.Format(CultureInfo.CurrentCulture, Resource.TestNotFound, testMethod.Name), + }, + ]; + return false; } bool shouldIgnoreClass = testMethodInfo.Parent.ClassType.IsIgnored(out string? ignoreMessageOnClass); bool shouldIgnoreMethod = testMethodInfo.MethodInfo.IsIgnored(out string? ignoreMessageOnMethod); - string? ignoreMessage = ignoreMessageOnClass; - if (StringEx.IsNullOrEmpty(ignoreMessage) && shouldIgnoreMethod) - { - ignoreMessage = ignoreMessageOnMethod; - } - if (shouldIgnoreClass || shouldIgnoreMethod) { + string? ignoreMessage = StringEx.IsNullOrEmpty(ignoreMessageOnClass) ? ignoreMessageOnMethod : ignoreMessageOnClass; notRunnableResult = [TestResult.CreateIgnoredResult(ignoreMessage)]; return false; From d8532185b9ef56d0413ae761b6493a62dd8b78ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Sat, 14 Mar 2026 11:39:39 +0100 Subject: [PATCH 3/4] Fix --- .../MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs b/src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs index dcee91b395..13c45e5665 100644 --- a/src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs +++ b/src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs @@ -332,7 +332,7 @@ private static bool IsTestMethodRunnable( if (shouldIgnoreClass || shouldIgnoreMethod) { - string? ignoreMessage = StringEx.IsNullOrEmpty(ignoreMessageOnClass) ? ignoreMessageOnMethod : ignoreMessageOnClass; + string? ignoreMessage = shouldIgnoreMethod && StringEx.IsNullOrEmpty(ignoreMessageOnClass) ? ignoreMessageOnMethod : ignoreMessageOnClass; notRunnableResult = [TestResult.CreateIgnoredResult(ignoreMessage)]; return false; From e22a2ade180d1224659c7a52be72d71aedc0e279 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Sat, 14 Mar 2026 11:59:07 +0100 Subject: [PATCH 4/4] Address copilot review: fix stale comments and single-enum discovery path - Update comment in TestMethodRunner to reference AssemblyEnumerator.TryUnfoldITestDataSource - Update comment in AssemblyEnumerator to reference TestMethodRunner.TryExecuteFoldedDataDrivenTestsAsync - Apply same single-enumeration pattern in AssemblyEnumerator.TryUnfoldITestDataSource to avoid double-enumerating GetData() --- .../Discovery/AssemblyEnumerator.cs | 45 ++++++++++--------- .../Execution/TestMethodRunner.cs | 4 +- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs b/src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs index 84630c264a..b3ea7a2df6 100644 --- a/src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs +++ b/src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs @@ -269,34 +269,18 @@ private static bool TryUnfoldITestDataSources(UnitTestElement test, DiscoveryTes private static bool TryUnfoldITestDataSource(ITestDataSource dataSource, UnitTestElement test, ReflectionTestMethodInfo methodInfo, List tests, ref int globalTestCaseIndex) { // Otherwise, unfold the data source and verify it can be serialized. - IEnumerable? data; - // This code is to discover tests. To run the tests code is in TestMethodRunner.ExecuteDataSourceBasedTests. - // Any change made here should be reflected in TestMethodRunner.ExecuteDataSourceBasedTests as well. - data = dataSource.GetData(methodInfo); + // This code is to discover tests. To run the tests code is in TestMethodRunner.TryExecuteFoldedDataDrivenTestsAsync. + // Any change made here should be reflected in TestMethodRunner.TryExecuteFoldedDataDrivenTestsAsync as well. + IEnumerable dataEnumerable = dataSource.GetData(methodInfo); string? testDataSourceIgnoreMessage = (dataSource as ITestDataSourceIgnoreCapability)?.IgnoreMessage; - if (!data.Any()) - { - if (!MSTestSettings.CurrentSettings.ConsiderEmptyDataSourceAsInconclusive) - { - throw dataSource.GetExceptionForEmptyDataSource(methodInfo); - } - - UnitTestElement discoveredTest = test.Clone(); - // Make the test not data driven, because it had no data. - discoveredTest.TestMethod.DataType = DynamicDataType.None; - discoveredTest.TestMethod.TestDataSourceIgnoreMessage = testDataSourceIgnoreMessage; - discoveredTest.TestMethod.DisplayName = dataSource.GetDisplayName(methodInfo, null) ?? discoveredTest.TestMethod.DisplayName; - tests.Add(discoveredTest); - - return true; - } - var discoveredTests = new List(); + bool dataSourceHasData = false; - foreach (object?[] dataOrTestDataRow in data) + foreach (object?[] dataOrTestDataRow in dataEnumerable) { + dataSourceHasData = true; object?[] d = dataOrTestDataRow; ParameterInfo[] parameters = methodInfo.GetParameters(); if (TestDataSourceHelpers.TryHandleITestDataRow(d, parameters, out d, out string? ignoreMessageFromTestDataRow, out string? displayNameFromTestDataRow, out IList? testCategoriesFromTestDataRow)) @@ -365,6 +349,23 @@ private static bool TryUnfoldITestDataSource(ITestDataSource dataSource, UnitTes globalTestCaseIndex++; } + if (!dataSourceHasData) + { + if (!MSTestSettings.CurrentSettings.ConsiderEmptyDataSourceAsInconclusive) + { + throw dataSource.GetExceptionForEmptyDataSource(methodInfo); + } + + UnitTestElement discoveredTest = test.Clone(); + // Make the test not data driven, because it had no data. + discoveredTest.TestMethod.DataType = DynamicDataType.None; + discoveredTest.TestMethod.TestDataSourceIgnoreMessage = testDataSourceIgnoreMessage; + discoveredTest.TestMethod.DisplayName = dataSource.GetDisplayName(methodInfo, null) ?? discoveredTest.TestMethod.DisplayName; + tests.Add(discoveredTest); + + return true; + } + tests.AddRange(discoveredTests); return true; diff --git a/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs b/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs index f690bda3ac..3f0332fa95 100644 --- a/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs +++ b/src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodRunner.cs @@ -205,8 +205,8 @@ private async Task TryExecuteFoldedDataDrivenTestsAsync(List r continue; } - // This code is to execute tests. To discover the tests code is in AssemblyEnumerator.ProcessTestDataSourceTests. - // Any change made here should be reflected in AssemblyEnumerator.ProcessTestDataSourceTests as well. + // This code is to execute tests. To discover the tests code is in AssemblyEnumerator.TryUnfoldITestDataSource. + // Any change made here should be reflected in AssemblyEnumerator.TryUnfoldITestDataSource as well. bool dataSourceHasData = false; foreach (object?[] data in testDataSource.GetData(_testMethodInfo.MethodInfo)) {