Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs
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);
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

if (unitTestCases != null)
{
Expand Down
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
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(
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
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
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