Don't return null types from AssemblyEnumerator when ReflectionTypeLoadException is encountered by @Youssef1313 in #6276 (backport to rel/3.10) (#6278)

Co-authored-by: Youssef1313 <youssefvictor00@gmail.com>

Author: youssef-backport-bot

src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs

@@ -106,11 +106,6 @@ internal AssemblyEnumerationResult EnumerateAssembly(string assemblyFileName)
 
         foreach (Type type in types)
         {
-            if (type == null)
-            {
-                continue;
-            }
-
             List<UnitTestElement> testsInType = DiscoverTestsInType(assemblyFileName, type, warnings, discoverInternals,
                 dataSourcesUnfoldingStrategy, testIdGenerationStrategy, fixturesTests);
             tests.AddRange(testsInType);
@@ -150,7 +145,10 @@ internal static Type[] GetTypes(Assembly assembly, string assemblyFileName, ICol
                 }
             }
 
-            return ex.Types!;
+            // We already logged a warning or error.
+            // So don't return types that failed to load.
+            // The intent of the catch is to gracefully run the types that we could load.
+            return ex.Types is null ? [] : [.. ex.Types.Where(t => t is not null)!];
         }
     }
 

src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.cs

@@ -422,18 +422,25 @@ private TestAssemblyInfo GetAssemblyInfo(Assembly assembly)
                             assemblyInfo.AssemblyCleanupMethodTimeoutMilliseconds = @this.TryGetTimeoutInfo(methodInfo, FixtureKind.AssemblyCleanup);
                         }
 
-                        if (methodInfo is { IsPublic: true, IsStatic: true, IsGenericMethod: false, DeclaringType.IsGenericType: false, DeclaringType.IsPublic: true } &&
-                            methodInfo.GetParameters() is { } parameters && parameters.Length == 1 && parameters[0].ParameterType == typeof(TestContext) &&
-                            methodInfo.IsValidReturnType())
+                        bool isGlobalTestInitialize = @this._reflectionHelper.IsAttributeDefined<GlobalTestInitializeAttribute>(methodInfo, inherit: true);
+                        bool isGlobalTestCleanup = @this._reflectionHelper.IsAttributeDefined<GlobalTestCleanupAttribute>(methodInfo, inherit: true);
+
+                        if (isGlobalTestInitialize || isGlobalTestCleanup)
                         {
-                            bool isGlobalTestInitialize = @this._reflectionHelper.IsAttributeDefined<GlobalTestInitializeAttribute>(methodInfo, inherit: true);
-                            bool isGlobalTestCleanup = @this._reflectionHelper.IsAttributeDefined<GlobalTestCleanupAttribute>(methodInfo, inherit: true);
-                            if (isGlobalTestInitialize)
+                            // Only try to validate the method if it already has the needed attribute.
+                            // This avoids potential type load exceptions when the return type cannot be resolved.
+                            // NOTE: Users tend to load assemblies in AssemblyInitialize after finishing the discovery.
+                            // We want to avoid loading types early as much as we can.
+                            bool isValid = methodInfo is { IsSpecialName: false, IsPublic: true, IsStatic: true, IsGenericMethod: false, DeclaringType.IsGenericType: false, DeclaringType.IsPublic: true } &&
+                                methodInfo.GetParameters() is { } parameters && parameters.Length == 1 && parameters[0].ParameterType == typeof(TestContext) &&
+                                methodInfo.IsValidReturnType();
+
+                            if (isValid && isGlobalTestInitialize)
                             {
                                 assemblyInfo.GlobalTestInitializations.Add((methodInfo, @this.TryGetTimeoutInfo(methodInfo, FixtureKind.TestInitialize)));
                             }
 
-                            if (isGlobalTestCleanup)
+                            if (isValid && isGlobalTestCleanup)
                             {
                                 assemblyInfo.GlobalTestCleanups.Add((methodInfo, @this.TryGetTimeoutInfo(methodInfo, FixtureKind.TestCleanup)));
                             }

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TestDiscoveryWarningsTests.cs

@@ -35,7 +35,7 @@ public async Task DiscoverTests_ShowsWarningsForTestsThatFailedToDiscover(string
 
         TestHostResult testHostResult = await testHost.ExecuteAsync("--list-tests");
 
-        testHostResult.AssertExitCodeIsNot(ExitCodes.Success);
+        testHostResult.AssertExitCodeIs(ExitCodes.Success);
         testHostResult.AssertOutputContains("System.IO.FileNotFoundException: Could not load file or assembly 'TestDiscoveryWarningsBaseClass");
     }
 

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

@@ -120,7 +120,7 @@ public void GetTypesShouldReturnReflectionTypeLoadExceptionTypesOnException()
         Type[] types = AssemblyEnumerator.GetTypes(mockAssembly.Object, string.Empty, _warnings);
 
         Verify(types is not null);
-        Verify(reflectedTypes.Equals(types));
+        Verify(reflectedTypes.SequenceEqual(types));
     }
 
     public void GetTypesShouldLogWarningsWhenReflectionFailsWithLoaderExceptions()