[Refactor] Use List<T> rather than Collection<T>

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

@@ -72,7 +72,7 @@ public AssemblyEnumerator(MSTestSettings settings) =>
     internal ICollection<UnitTestElement> EnumerateAssembly(
         string assemblyFileName,
         [StringSyntax(StringSyntaxAttribute.Xml, nameof(runSettingsXml))] string? runSettingsXml,
-        out ICollection<string> warnings)
+        out List<string> warnings)
     {
         DebugEx.Assert(!StringEx.IsNullOrWhiteSpace(assemblyFileName), "Invalid assembly file name.");
         var warningMessages = new List<string>();
@@ -228,8 +228,7 @@ private List<UnitTestElement> DiscoverTestsInType(
         {
             typeFullName = type.FullName;
             TypeEnumerator testTypeEnumerator = GetTypeEnumerator(type, assemblyFileName, discoverInternals, discoveryOption, testIdGenerationStrategy);
-            ICollection<UnitTestElement>? unitTestCases = testTypeEnumerator.Enumerate(out ICollection<string> warningsFromTypeEnumerator);
-            warningMessages.AddRange(warningsFromTypeEnumerator);
+            List<UnitTestElement>? unitTestCases = testTypeEnumerator.Enumerate(warningMessages);
 
             if (unitTestCases != null)
             {

src/Adapter/MSTest.TestAdapter/Discovery/TypeEnumerator.cs

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Collections.ObjectModel;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
@@ -54,10 +54,8 @@
     /// </summary>
     /// <param name="warnings"> Contains warnings if any, that need to be passed back to the caller. </param>
     /// <returns> list of test cases.</returns>
-    internal virtual ICollection<UnitTestElement>? Enumerate(out ICollection<string> warnings)
+    internal virtual List<UnitTestElement>? Enumerate(List<string> warnings)
     {
-        warnings = new Collection<string>();
-
         if (!_typeValidator.IsValidTestClass(_type, warnings))
         {
             return null;
@@ -72,11 +70,11 @@
     /// </summary>
     /// <param name="warnings"> Contains warnings if any, that need to be passed back to the caller. </param>
     /// <returns> List of Valid Tests. </returns>
-    internal Collection<UnitTestElement> GetTests(ICollection<string> warnings)
+    internal List<UnitTestElement> GetTests(List<string> warnings)
     {
         bool foundDuplicateTests = false;
         var foundTests = new HashSet<string>();
-        var tests = new Collection<UnitTestElement>();
+        var tests = new List<UnitTestElement>();
 
         // Test class is already valid. Verify methods.
         // PERF: GetRuntimeMethods is used here to get all methods, including non-public, and static methods.
@@ -119,12 +117,11 @@
             currentType = currentType.BaseType;
         }
 
-        return new Collection<UnitTestElement>(
-            tests.GroupBy(
-                t => t.TestMethod.Name,
-                (_, elements) =>
-                    elements.OrderBy(t => inheritanceDepths[t.TestMethod.DeclaringClassFullName ?? t.TestMethod.FullClassName]).First())
-                .ToList());
+        return tests.GroupBy(
+            t => t.TestMethod.Name,
+            (_, elements) =>
+                elements.OrderBy(t => inheritanceDepths[t.TestMethod.DeclaringClassFullName ?? t.TestMethod.FullClassName]).First())
+            .ToList();
     }
 
     /// <summary>

src/Adapter/MSTest.TestAdapter/Discovery/TypeValidator.cs

@@ -49,7 +49,7 @@ internal TypeValidator(ReflectHelper reflectHelper, bool discoverInternals)
     /// <param name="type">The reflected type.</param>
     /// <param name="warnings">Contains warnings if any, that need to be passed back to the caller.</param>
     /// <returns>Return true if it is a valid test class.</returns>
-    internal virtual bool IsValidTestClass(Type type, ICollection<string> warnings)
+    internal virtual bool IsValidTestClass(Type type, List<string> warnings)
     {
         // PERF: We are doing caching reflection here, meaning we will cache every class info in the
         // assembly, this is because when we discover and run we will repeatedly inspect all the types in the assembly, and this

test/UnitTests/MSTestAdapter.UnitTests/Discovery/AssemblyEnumeratorTests.cs

@@ -26,7 +26,7 @@ public class AssemblyEnumeratorTests : TestContainer
     private readonly AssemblyEnumerator _assemblyEnumerator;
     private readonly TestablePlatformServiceProvider _testablePlatformServiceProvider;
 
-    private ICollection<string> _warnings;
+    private List<string> _warnings;
 
     public AssemblyEnumeratorTests()
     {
@@ -240,7 +240,7 @@ public void EnumerateAssemblyShouldReturnEmptyListWhenNoTestElementsInAType()
             .Returns([typeof(DummyTestClass).GetTypeInfo()]);
         _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly("DummyAssembly", false))
             .Returns(mockAssembly.Object);
-        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(out _warnings))
+        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(_warnings))
             .Returns((ICollection<UnitTestElement>)null);
 
         Verify(_assemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings).Count == 0);
@@ -259,7 +259,7 @@ public void EnumerateAssemblyShouldReturnTestElementsForAType()
             .Returns([typeof(DummyTestClass).GetTypeInfo()]);
         _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly("DummyAssembly", false))
             .Returns(mockAssembly.Object);
-        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(out _warnings))
+        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(_warnings))
             .Returns(new Collection<UnitTestElement> { unitTestElement });
 
         ICollection<UnitTestElement> testElements = testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings);
@@ -281,7 +281,7 @@ public void EnumerateAssemblyShouldReturnMoreThanOneTestElementForAType()
             .Returns([typeof(DummyTestClass).GetTypeInfo()]);
         _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly("DummyAssembly", false))
             .Returns(mockAssembly.Object);
-        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(out _warnings))
+        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(_warnings))
             .Returns(expectedTestElements);
 
         ICollection<UnitTestElement> testElements = testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings);
@@ -303,7 +303,7 @@ public void EnumerateAssemblyShouldReturnMoreThanOneTestElementForMoreThanOneTyp
             .Returns([typeof(DummyTestClass).GetTypeInfo(), typeof(DummyTestClass).GetTypeInfo()]);
         _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly("DummyAssembly", false))
             .Returns(mockAssembly.Object);
-        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(out _warnings))
+        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(_warnings))
             .Returns(expectedTestElements);
 
         ICollection<UnitTestElement> testElements = testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings);
@@ -317,7 +317,7 @@ public void EnumerateAssemblyShouldNotLogWarningsIfNonePresent()
     {
         Mock<TestableAssembly> mockAssembly = CreateMockTestableAssembly();
         var testableAssemblyEnumerator = new TestableAssemblyEnumerator();
-        ICollection<string> warningsFromTypeEnumerator = [];
+        List<string> warningsFromTypeEnumerator = [];
 
         // Setup mocks
         mockAssembly.Setup(a => a.GetTypes())
@@ -326,7 +326,7 @@ public void EnumerateAssemblyShouldNotLogWarningsIfNonePresent()
             .Returns([typeof(InternalTestClass).GetTypeInfo()]);
         _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly("DummyAssembly", false))
             .Returns(mockAssembly.Object);
-        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(out warningsFromTypeEnumerator));
+        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(warningsFromTypeEnumerator));
 
         testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings);
         Verify(_warnings.Count == 0);
@@ -336,7 +336,7 @@ public void EnumerateAssemblyShouldLogWarningsIfPresent()
     {
         Mock<TestableAssembly> mockAssembly = CreateMockTestableAssembly();
         var testableAssemblyEnumerator = new TestableAssemblyEnumerator();
-        ICollection<string> warningsFromTypeEnumerator = new Collection<string>
+        var warningsFromTypeEnumerator = new List<string>
         {
             "DummyWarning",
         };
@@ -348,7 +348,7 @@ public void EnumerateAssemblyShouldLogWarningsIfPresent()
             .Returns([typeof(InternalTestClass).GetTypeInfo()]);
         _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly("DummyAssembly", false))
             .Returns(mockAssembly.Object);
-        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(out warningsFromTypeEnumerator));
+        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(warningsFromTypeEnumerator));
 
         testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings);
 
@@ -368,7 +368,7 @@ public void EnumerateAssemblyShouldHandleExceptionsWhileEnumeratingAType()
             .Returns([typeof(InternalTestClass).GetTypeInfo()]);
         _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly("DummyAssembly", false))
             .Returns(mockAssembly.Object);
-        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(out _warnings)).Throws(exception);
+        testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(_warnings)).Throws(exception);
 
         testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings);
 

test/UnitTests/MSTestAdapter.UnitTests/Discovery/TypeEnumeratorTests.cs

@@ -27,7 +27,7 @@ public class TypeEnumeratorTests : TestContainer
     private readonly TestablePlatformServiceProvider _testablePlatformServiceProvider;
     private readonly Mock<IMessageLogger> _mockMessageLogger;
 
-    private ICollection<string> _warnings;
+    private readonly List<string> _warnings;
 
     public TypeEnumeratorTests()
     {
@@ -55,15 +55,15 @@ protected override void Dispose(bool disposing)
     public void EnumerateShouldReturnNullIfTypeIsNotValid()
     {
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(IDummyInterface), string.Empty);
-        Verify(typeEnumerator.Enumerate(out _warnings) is null);
+        Verify(typeEnumerator.Enumerate(_warnings) is null);
     }
 
     public void EnumerateShouldReturnEmptyCollectionWhenNoValidTestMethodsExist()
     {
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: false, isMethodFromSameAssembly: true);
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummyTestClass), string.Empty);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
         Verify(tests.Count == 0);
@@ -78,7 +78,7 @@ public void GetTestsShouldReturnDeclaredTestMethods()
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: true, isMethodFromSameAssembly: true);
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummyBaseTestClass), Assembly.GetExecutingAssembly().FullName);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
 
@@ -91,7 +91,7 @@ public void GetTestsShouldReturnBaseTestMethodsInSameAssembly()
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: true, isMethodFromSameAssembly: true);
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummyDerivedTestClass), Assembly.GetExecutingAssembly().FullName);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
 
@@ -122,7 +122,7 @@ public void GetTestsShouldReturnBaseTestMethodsFromAnotherAssemblyByDefault()
 
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummyDerivedTestClass), Assembly.GetExecutingAssembly().FullName);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
 
@@ -152,7 +152,7 @@ public void GetTestsShouldReturnBaseTestMethodsFromAnotherAssemblyByConfiguratio
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: true, isMethodFromSameAssembly: true);
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummyDerivedTestClass), Assembly.GetExecutingAssembly().FullName);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
 
@@ -183,7 +183,7 @@ public void GetTestsShouldNotReturnBaseTestMethodsFromAnotherAssemblyByConfigura
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: true, isMethodFromSameAssembly: false);
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummyDerivedTestClass), Assembly.GetExecutingAssembly().FullName);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
 
@@ -197,7 +197,7 @@ public void GetTestsShouldNotReturnHiddenTestMethods()
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: true, isMethodFromSameAssembly: true);
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummyHidingTestClass), Assembly.GetExecutingAssembly().FullName);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
 
@@ -216,7 +216,7 @@ public void GetTestsShouldReturnOverriddenTestMethods()
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: true, isMethodFromSameAssembly: true);
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummyOverridingTestClass), Assembly.GetExecutingAssembly().FullName);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
 
@@ -239,7 +239,7 @@ public void GetTestsShouldNotReturnHiddenTestMethodsFromAnyLevel()
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: true, isMethodFromSameAssembly: true);
         TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(typeof(DummySecondHidingTestClass), Assembly.GetExecutingAssembly().FullName);
 
-        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(out _warnings);
+        ICollection<MSTest.TestAdapter.ObjectModel.UnitTestElement> tests = typeEnumerator.Enumerate(_warnings);
 
         Verify(tests is not null);
 
@@ -577,7 +577,7 @@ public void GetTestFromMethodShouldSetDisplayNameFromDataTestMethodAttribute()
 
     private void SetupTestClassAndTestMethods(bool isValidTestClass, bool isValidTestMethod, bool isMethodFromSameAssembly)
     {
-        _mockTypeValidator.Setup(tv => tv.IsValidTestClass(It.IsAny<Type>(), It.IsAny<ICollection<string>>()))
+        _mockTypeValidator.Setup(tv => tv.IsValidTestClass(It.IsAny<Type>(), It.IsAny<List<string>>()))
             .Returns(isValidTestClass);
         _mockTestMethodValidator.Setup(
             tmv => tmv.IsValidTestMethod(It.IsAny<MethodInfo>(), It.IsAny<Type>(), It.IsAny<ICollection<string>>())).Returns(isValidTestMethod);