Skip to content

Commit

Permalink
[Refactor] Use List<T> rather than Collection<T> (#4315)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Youssef1313
Youssef1313 authored Dec 11, 2024
1 parent e1051b6 commit 23321d7
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 61 deletions.
11 changes: 4 additions & 7 deletions src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,16 @@ public AssemblyEnumerator(MSTestSettings settings) =>
internal ICollection<UnitTestElement> EnumerateAssembly(
string assemblyFileName,
[StringSyntax(StringSyntaxAttribute.Xml, nameof(runSettingsXml))] string? runSettingsXml,
out ICollection<string> warnings)
List<string> warnings)
{
DebugEx.Assert(!StringEx.IsNullOrWhiteSpace(assemblyFileName), "Invalid assembly file name.");
var warningMessages = new List<string>();
var tests = new List<UnitTestElement>();
// Contains list of assembly/class names for which we have already added fixture tests.
var fixturesTests = new HashSet<string>();

Assembly assembly = PlatformServiceProvider.Instance.FileOperations.LoadAssembly(assemblyFileName, isReflectionOnly: false);

Type[] types = GetTypes(assembly, assemblyFileName, warningMessages);
Type[] types = GetTypes(assembly, assemblyFileName, warnings);
bool discoverInternals = ReflectHelper.GetDiscoverInternalsAttribute(assembly) != null;
TestIdGenerationStrategy testIdGenerationStrategy = ReflectHelper.GetTestIdGenerationStrategy(assembly);

Expand All @@ -109,12 +108,11 @@ internal ICollection<UnitTestElement> EnumerateAssembly(
continue;
}

List<UnitTestElement> testsInType = DiscoverTestsInType(assemblyFileName, testRunParametersFromRunSettings, type, warningMessages, discoverInternals,
List<UnitTestElement> testsInType = DiscoverTestsInType(assemblyFileName, testRunParametersFromRunSettings, type, warnings, discoverInternals,
testDataSourceDiscovery, testIdGenerationStrategy, fixturesTests);
tests.AddRange(testsInType);
}

warnings = warningMessages;
return tests;
}

Expand Down Expand Up @@ -228,8 +226,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)
{
Expand Down
8 changes: 4 additions & 4 deletions src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumeratorWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ internal sealed class AssemblyEnumeratorWrapper
/// <param name="runSettings"> The run Settings. </param>
/// <param name="warnings"> Contains warnings if any, that need to be passed back to the caller. </param>
/// <returns> A collection of test elements. </returns>
internal ICollection<UnitTestElement>? GetTests(string? assemblyFileName, IRunSettings? runSettings, out ICollection<string> warnings)
internal ICollection<UnitTestElement>? GetTests(string? assemblyFileName, IRunSettings? runSettings, out List<string> warnings)
{
warnings = new List<string>();

Expand All @@ -52,7 +52,7 @@ internal sealed class AssemblyEnumeratorWrapper
}

// Load the assembly in isolation if required.
return GetTestsInIsolation(fullFilePath, runSettings, out warnings);
return GetTestsInIsolation(fullFilePath, runSettings, warnings);
}
catch (FileNotFoundException ex)
{
Expand Down Expand Up @@ -98,7 +98,7 @@ internal sealed class AssemblyEnumeratorWrapper
}
}

private static ICollection<UnitTestElement> GetTestsInIsolation(string fullFilePath, IRunSettings? runSettings, out ICollection<string> warnings)
private static ICollection<UnitTestElement> GetTestsInIsolation(string fullFilePath, IRunSettings? runSettings, List<string> warnings)
{
using MSTestAdapter.PlatformServices.Interface.ITestSourceHost isolationHost = PlatformServiceProvider.Instance.CreateTestSourceHost(fullFilePath, runSettings, frameworkHandle: null);

Expand All @@ -117,6 +117,6 @@ private static ICollection<UnitTestElement> GetTestsInIsolation(string fullFileP
PlatformServiceProvider.Instance.AdapterTraceLogger.LogWarning(Resource.OlderTFMVersionFound);
}

return assemblyEnumerator.EnumerateAssembly(fullFilePath, xml, out warnings);
return assemblyEnumerator.EnumerateAssembly(fullFilePath, xml, warnings);
}
}
20 changes: 8 additions & 12 deletions src/Adapter/MSTest.TestAdapter/Discovery/TypeEnumerator.cs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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;
Expand Down Expand Up @@ -54,10 +53,8 @@ internal TypeEnumerator(Type type, string assemblyFilePath, ReflectHelper reflec
/// </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;
Expand All @@ -72,11 +69,11 @@ internal TypeEnumerator(Type type, string assemblyFilePath, ReflectHelper reflec
/// </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.
Expand Down Expand Up @@ -119,12 +116,11 @@ internal Collection<UnitTestElement> GetTests(ICollection<string> warnings)
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>
Expand Down
2 changes: 1 addition & 1 deletion src/Adapter/MSTest.TestAdapter/Discovery/TypeValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Adapter/MSTest.TestAdapter/Discovery/UnitTestDiscoverer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ internal virtual void DiscoverTestsInSource(
ITestCaseDiscoverySink discoverySink,
IDiscoveryContext? discoveryContext)
{
ICollection<UnitTestElement>? testElements = _assemblyEnumeratorWrapper.GetTests(source, discoveryContext?.RunSettings, out ICollection<string>? warnings);
ICollection<UnitTestElement>? testElements = _assemblyEnumeratorWrapper.GetTests(source, discoveryContext?.RunSettings, out List<string> warnings);

bool treatDiscoveryWarningsAsErrors = MSTestSettings.CurrentSettings.TreatDiscoveryWarningsAsErrors;

Expand Down
47 changes: 24 additions & 23 deletions test/UnitTests/MSTestAdapter.UnitTests/Discovery/AssemblyEnumeratorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class AssemblyEnumeratorTests : TestContainer
private readonly AssemblyEnumerator _assemblyEnumerator;
private readonly TestablePlatformServiceProvider _testablePlatformServiceProvider;

private ICollection<string> _warnings;
private readonly List<string> _warnings;

public AssemblyEnumeratorTests()
{
Expand Down Expand Up @@ -225,7 +225,7 @@ public void EnumerateAssemblyShouldReturnEmptyListWhenNoDeclaredTypes()
_testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly("DummyAssembly", false))
.Returns(mockAssembly.Object);

Verify(_assemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings).Count == 0);
Verify(_assemblyEnumerator.EnumerateAssembly("DummyAssembly", null, _warnings).Count == 0);
}

public void EnumerateAssemblyShouldReturnEmptyListWhenNoTestElementsInAType()
Expand All @@ -240,10 +240,10 @@ 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))
.Returns((ICollection<UnitTestElement>)null);
testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(_warnings))
.Returns((List<UnitTestElement>)null);

Verify(_assemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings).Count == 0);
Verify(_assemblyEnumerator.EnumerateAssembly("DummyAssembly", null, _warnings).Count == 0);
}

public void EnumerateAssemblyShouldReturnTestElementsForAType()
Expand All @@ -259,10 +259,10 @@ 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))
.Returns(new Collection<UnitTestElement> { unitTestElement });
testableAssemblyEnumerator.MockTypeEnumerator.Setup(te => te.Enumerate(_warnings))
.Returns(new List<UnitTestElement> { unitTestElement });

ICollection<UnitTestElement> testElements = testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings);
ICollection<UnitTestElement> testElements = testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, _warnings);

Verify(new Collection<UnitTestElement> { unitTestElement }.SequenceEqual(testElements));
}
Expand All @@ -272,7 +272,7 @@ public void EnumerateAssemblyShouldReturnMoreThanOneTestElementForAType()
Mock<TestableAssembly> mockAssembly = CreateMockTestableAssembly();
var testableAssemblyEnumerator = new TestableAssemblyEnumerator();
var unitTestElement = new UnitTestElement(new TestMethod("DummyMethod", "DummyClass", "DummyAssembly", false));
var expectedTestElements = new Collection<UnitTestElement> { unitTestElement, unitTestElement };
var expectedTestElements = new List<UnitTestElement> { unitTestElement, unitTestElement };

// Setup mocks
mockAssembly.Setup(a => a.GetTypes())
Expand All @@ -281,10 +281,10 @@ 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);
ICollection<UnitTestElement> testElements = testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, _warnings);

Verify(expectedTestElements.SequenceEqual(testElements));
}
Expand All @@ -294,7 +294,7 @@ public void EnumerateAssemblyShouldReturnMoreThanOneTestElementForMoreThanOneTyp
Mock<TestableAssembly> mockAssembly = CreateMockTestableAssembly();
var testableAssemblyEnumerator = new TestableAssemblyEnumerator();
var unitTestElement = new UnitTestElement(new TestMethod("DummyMethod", "DummyClass", "DummyAssembly", false));
var expectedTestElements = new Collection<UnitTestElement> { unitTestElement, unitTestElement };
var expectedTestElements = new List<UnitTestElement> { unitTestElement, unitTestElement };

// Setup mocks
mockAssembly.Setup(a => a.GetTypes())
Expand All @@ -303,10 +303,10 @@ 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);
ICollection<UnitTestElement> testElements = testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, _warnings);

expectedTestElements.Add(unitTestElement);
expectedTestElements.Add(unitTestElement);
Expand All @@ -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())
Expand All @@ -326,17 +326,17 @@ 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);
testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, _warnings);
Verify(_warnings.Count == 0);
}

public void EnumerateAssemblyShouldLogWarningsIfPresent()
{
Mock<TestableAssembly> mockAssembly = CreateMockTestableAssembly();
var testableAssemblyEnumerator = new TestableAssemblyEnumerator();
ICollection<string> warningsFromTypeEnumerator = new Collection<string>
var warningsFromTypeEnumerator = new List<string>
{
"DummyWarning",
};
Expand All @@ -348,11 +348,12 @@ 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(_warnings))
.Callback(() => _warnings.AddRange(warningsFromTypeEnumerator));

testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, out _warnings);
testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, _warnings);

Verify(warningsFromTypeEnumerator.ToList().SequenceEqual(_warnings));
Verify(warningsFromTypeEnumerator.SequenceEqual(_warnings));
}

public void EnumerateAssemblyShouldHandleExceptionsWhileEnumeratingAType()
Expand All @@ -368,9 +369,9 @@ 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);
testableAssemblyEnumerator.EnumerateAssembly("DummyAssembly", null, _warnings);

Verify(_warnings.ToList().Contains(
string.Format(
Expand Down
2 changes: 1 addition & 1 deletion test/UnitTests/MSTestAdapter.UnitTests/Discovery/AssemblyEnumeratorWrapperTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class AssemblyEnumeratorWrapperTests : TestContainer
private readonly AssemblyEnumeratorWrapper _testableAssemblyEnumeratorWrapper;
private readonly TestablePlatformServiceProvider _testablePlatformServiceProvider;

private ICollection<string> _warnings;
private List<string> _warnings;

public AssemblyEnumeratorWrapperTests()
{
Expand Down
Loading

0 comments on commit 23321d7

Please sign in to comment.