Skip to content

fix nullability of AssemblyEnumerator.GetTypes #5401

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix nullability of AssemblyEnumerator.GetTypes #5401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
91dcbf5
fix nullability of AssemblyEnumerator.GetTypes
SimonCropp Apr 6, 2025
04148da
Merge branch 'main' into fix-nullability-of-AssemblyEnumerator.GetTypes
SimonCropp Apr 6, 2025
e9a7a08
Merge branch 'main' into fix-nullability-of-AssemblyEnumerator.GetTypes
SimonCropp Apr 7, 2025
b34eb51
Merge branch 'main' into fix-nullability-of-AssemblyEnumerator.GetTypes
SimonCropp Apr 8, 2025
2ebfc48
Update TypeCache.cs
SimonCropp Apr 10, 2025
b6e3637
Update AssemblyEnumerator.cs
SimonCropp Apr 10, 2025
f6d41e3
Merge branch 'main' into fix-nullability-of-AssemblyEnumerator.GetTypes
SimonCropp Apr 20, 2025
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
8 changes: 4 additions & 4 deletions src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@

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

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

Expand Down Expand Up @@ -103,7 +103,7 @@
},
};

foreach (Type type in types)
foreach (Type? type in types)
{
if (type == null)
{
Expand All @@ -125,7 +125,7 @@
/// <param name="assemblyFileName">The file name of the assembly.</param>
/// <param name="warningMessages">Contains warnings if any, that need to be passed back to the caller.</param>
/// <returns>Gets the types defined in the provided assembly.</returns>
internal static Type[] GetTypes(Assembly assembly, string assemblyFileName, ICollection<string>? warningMessages)
internal static Type?[] GetTypes(Assembly assembly, string assemblyFileName, ICollection<string>? warningMessages)
{
try
{
Expand All @@ -136,14 +136,14 @@
PlatformServiceProvider.Instance.AdapterTraceLogger.LogWarning($"MSTestExecutor.TryGetTests: {Resource.TestAssembly_AssemblyDiscoveryFailure}", assemblyFileName, ex);
PlatformServiceProvider.Instance.AdapterTraceLogger.LogWarning(Resource.ExceptionsThrown);

if (ex.LoaderExceptions != null)
if (ex.LoaderExceptions.Any())
{
// If not able to load all type, log a warning and continue with loaded types.
string message = string.Format(CultureInfo.CurrentCulture, Resource.TypeLoadFailed, assemblyFileName, GetLoadExceptionDetails(ex));

warningMessages?.Add(message);

foreach (Exception? loaderEx in ex.LoaderExceptions)

Check failure on line 146 in src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build MacOS Debug)

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

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs(146,49): error CS8602: (NETCORE_ENGINEERING_TELEMETRY=Build) Dereference of a possibly null reference.

Check failure on line 146 in src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build MacOS Release)

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

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs(146,49): error CS8602: (NETCORE_ENGINEERING_TELEMETRY=Build) Dereference of a possibly null reference.

Check failure on line 146 in src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Release)

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

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs(146,49): error CS8602: (NETCORE_ENGINEERING_TELEMETRY=Build) Dereference of a possibly null reference.

Check failure on line 146 in src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Debug)

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

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs(146,49): error CS8602: (NETCORE_ENGINEERING_TELEMETRY=Build) Dereference of a possibly null reference.

Check failure on line 146 in src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx

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

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs(146,49): error CS8602: (NETCORE_ENGINEERING_TELEMETRY=Build) Dereference of a possibly null reference.

Check failure on line 146 in src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx

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

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs(146,49): error CS8602: (NETCORE_ENGINEERING_TELEMETRY=Build) Dereference of a possibly null reference.
{
PlatformServiceProvider.Instance.AdapterTraceLogger.LogWarning("{0}", loaderEx);
}
Expand Down
15 changes: 10 additions & 5 deletions src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,14 +384,19 @@ private TestAssemblyInfo GetAssemblyInfo(Assembly assembly)
{
var assemblyInfo = new TestAssemblyInfo(assembly);

Type[] types = AssemblyEnumerator.GetTypes(assembly, assembly.FullName!, null);
Type?[] types = AssemblyEnumerator.GetTypes(assembly, assembly.FullName!, null);

foreach (Type t in types)
foreach (Type? type in types)
{
if (type == null)
{
continue;
}
Comment on lines +391 to +394
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change in behavior.

Previously if type was null, we will go through the try, which will fail, then we get into the catch that will fail due to t.FullName (it will throw NullReferenceException).

The original intent is to probably ignore types that failed to load, but that as well wasn't a good design decision.

I think it's best in v4 if we just not catch any ReflectionTypeLoadException and let everything fail.


try
{
// Only examine classes which are TestClass or derives from TestClass attribute
if (!@this._reflectionHelper.IsDerivedAttributeDefined<TestClassAttribute>(t, inherit: false))
if (!@this._reflectionHelper.IsDerivedAttributeDefined<TestClassAttribute>(type, inherit: false))
{
continue;
}
Expand All @@ -401,14 +406,14 @@ private TestAssemblyInfo GetAssemblyInfo(Assembly assembly)
// If we fail to discover type from an assembly, then do not abort. Pick the next type.
PlatformServiceProvider.Instance.AdapterTraceLogger.LogWarning(
"TypeCache: Exception occurred while checking whether type {0} is a test class or not. {1}",
t.FullName,
type.FullName,
ex);

continue;
}

// Enumerate through all methods and identify the Assembly Init and cleanup methods.
foreach (MethodInfo methodInfo in PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethods(t))
foreach (MethodInfo methodInfo in PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethods(type))
{
if (@this.IsAssemblyOrClassInitializeMethod<AssemblyInitializeAttribute>(methodInfo))
{
Expand Down
6 changes: 3 additions & 3 deletions test/UnitTests/MSTestAdapter.UnitTests/Discovery/AssemblyEnumeratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void GetTypesShouldReturnSetOfDefinedTypes()
// Setup mocks
mockAssembly.Setup(a => a.GetTypes()).Returns(expectedTypes);

IReadOnlyList<Type> types = AssemblyEnumerator.GetTypes(mockAssembly.Object, string.Empty, _warnings);
IReadOnlyList<Type?> types = AssemblyEnumerator.GetTypes(mockAssembly.Object, string.Empty, _warnings);
Verify(expectedTypes.SequenceEqual(types));
}

Expand All @@ -116,7 +116,7 @@ public void GetTypesShouldReturnReflectionTypeLoadExceptionTypesOnException()
// Setup mocks
mockAssembly.Setup(a => a.GetTypes()).Throws(new ReflectionTypeLoadException(reflectedTypes, null));

IReadOnlyList<Type> types = AssemblyEnumerator.GetTypes(mockAssembly.Object, string.Empty, _warnings);
IReadOnlyList<Type?> types = AssemblyEnumerator.GetTypes(mockAssembly.Object, string.Empty, _warnings);

Verify(types is not null);
Verify(reflectedTypes.Equals(types));
Expand All @@ -131,7 +131,7 @@ public void GetTypesShouldLogWarningsWhenReflectionFailsWithLoaderExceptions()
mockAssembly.Setup(a => a.GetTypes()).Throws(new ReflectionTypeLoadException(null, exceptions));
mockAssembly.Setup(a => a.GetTypes()).Throws(new ReflectionTypeLoadException(null, exceptions));

IReadOnlyList<Type> types = AssemblyEnumerator.GetTypes(mockAssembly.Object, "DummyAssembly", _warnings);
IReadOnlyList<Type?> types = AssemblyEnumerator.GetTypes(mockAssembly.Object, "DummyAssembly", _warnings);

Verify(_warnings.Count == 1);
Verify(_warnings.ToList().Contains(
Expand Down
Loading