Refactor TestMethod type metadata to a single semantic class identity (#8213)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Co-authored-by: Amaury Levé <amaury@nerdyduck.dev>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: 4 people

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

@@ -125,9 +125,13 @@ internal UnitTestElement GetTestFromMethod(MethodInfo method, bool classDisables
         // null if the current instance represents a generic type parameter.
         DebugEx.Assert(_type.AssemblyQualifiedName != null, "AssemblyQualifiedName for method is null.");
 
-        ManagedNameHelper.GetManagedNameAndHierarchy(method, out string managedType, out string managedMethod, out string?[] hierarchyValues);
+        // Note: We pass _type.FullName (the closed-generic CLR name) as FullClassName because TypeCache.LoadType
+        // calls assembly.GetType(FullClassName) which requires the closed-generic form to instantiate the type.
+        // The managed type name (open-generic form, used for VSTest ManagedType property) is derived from
+        // FullClassName by TestMethod.ManagedTypeName via stripping the generic argument list.
+        ManagedNameHelper.GetManagedNameAndHierarchy(method, out _, out string managedMethod, out string?[] hierarchyValues);
         ParameterInfo[] parameters = method.GetParameters();
-        var testMethod = new TestMethod(managedType, managedMethod, hierarchyValues, method.Name, _type.FullName!, _assemblyFilePath, null,
+        var testMethod = new TestMethod(managedMethod, hierarchyValues, method.Name, _type.FullName!, _assemblyFilePath, null,
             parameters.Length == 0 ? string.Empty : string.Join(",", Array.ConvertAll(parameters, static p => p.ParameterType.ToString())))
         {
             MethodInfo = method,

src/Adapter/MSTestAdapter.PlatformServices/Extensions/TestCaseExtensions.cs

@@ -88,10 +88,15 @@ internal static UnitTestElement ToUnitTestElementWithUpdatedSource(this TestCase
                 : unitTestElement.CloneWithUpdatedSource(source);
         }
 
-        string? testClassName = testCase.GetPropertyValue(EngineConstants.TestClassNameProperty) as string;
+        string? managedTypeName = testCase.GetManagedType();
+        string? legacyTestClassName = testCase.GetPropertyValue(EngineConstants.TestClassNameProperty) as string;
+        string? testClassName = testCase.GetClassNameWhenFullyQualifiedNameStartsWith(managedTypeName)
+            ?? testCase.GetClassNameWhenFullyQualifiedNameStartsWith(legacyTestClassName)
+            ?? managedTypeName
+            ?? legacyTestClassName;
         string name = testCase.GetTestName(testClassName);
 
-        var testMethod = new TestMethod(testCase.GetManagedType(), testCase.GetManagedMethod(), testCase.GetHierarchy(), name, testClassName!, source, testCase.DisplayName, testCase.GetPropertyValue<string>(EngineConstants.ParameterTypesProperty, null));
+        var testMethod = new TestMethod(testCase.GetManagedMethod(), testCase.GetHierarchy(), name, testClassName!, source, testCase.DisplayName, testCase.GetPropertyValue<string>(EngineConstants.ParameterTypesProperty, null));
 
         var dataType = (DynamicDataType)testCase.GetPropertyValue(EngineConstants.TestDynamicDataTypeProperty, (int)DynamicDataType.None);
         if (dataType != DynamicDataType.None)
@@ -139,6 +144,11 @@ internal static UnitTestElement ToUnitTestElementWithUpdatedSource(this TestCase
 
     internal static string? GetManagedMethod(this TestCase testCase) => testCase.GetPropertyValue<string>(ManagedMethodProperty, null);
 
+    private static string? GetClassNameWhenFullyQualifiedNameStartsWith(this TestCase testCase, string? testClassName)
+        => testClassName is not null && testCase.FullyQualifiedName.StartsWith($"{testClassName}.", StringComparison.Ordinal)
+            ? testClassName
+            : null;
+
     internal static string[]? GetHierarchy(this TestCase testCase) => testCase.GetPropertyValue<string[]>(HierarchyProperty, null);
 
     internal static void SetHierarchy(this TestCase testCase, string?[] value) => testCase.SetPropertyValue(HierarchyProperty, value);

src/Adapter/MSTestAdapter.PlatformServices/Interfaces/ObjectModel/ITestMethod.cs

@@ -16,7 +16,7 @@ internal interface ITestMethod
     string Name { get; }
 
     /// <summary>
-    /// Gets the full class name of the test method.
+    /// Gets the semantic full class name of the test method.
     /// </summary>
     string FullClassName { get; }
 
@@ -31,6 +31,10 @@ internal interface ITestMethod
     /// <example>
     ///     <c>NamespaceA.NamespaceB.ClassName`1+InnerClass`2</c>.
     /// </example>
+    /// <remarks>
+    /// This value is derived from <see cref="FullClassName"/>. Closed generic type arguments are omitted because
+    /// managed type metadata uses the open generic type definition.
+    /// </remarks>
     string? ManagedTypeName { get; }
 
     /// <summary>
@@ -42,7 +46,8 @@ internal interface ITestMethod
     string? ManagedMethodName { get; }
 
     /// <summary>
-    /// Gets a value indicating whether both <see cref="ManagedTypeName"/> and <see cref="ManagedMethodName"/> are not null or whitespace.
+    /// Gets a value indicating whether the managed method metadata is available.
+    /// <see cref="ManagedTypeName"/> is derived from <see cref="FullClassName"/>.
     /// </summary>
     bool HasManagedMethodAndTypeProperties { get; }
 

src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/TestMethod.cs

@@ -25,17 +25,16 @@ internal sealed class TestMethod : ITestMethod
     /// Initializes a new instance of the <see cref="TestMethod"/> class.
     /// </summary>
     /// <param name="name">The name of the method.</param>
-    /// <param name="fullClassName">The full name of the class declaring the method.</param>
+    /// <param name="fullClassName">The full name of the class containing the method in execution context.</param>
     /// <param name="assemblyName">The full assembly name.</param>
     /// <param name="displayName">The display name of the test method.</param>
     // This constructor is used in testing only, and we should remove it in the future and update the tests.
     internal TestMethod(string name, string fullClassName, string assemblyName, string? displayName)
-        : this(fullClassName, null, null, name, fullClassName, assemblyName, displayName, null)
+        : this(null, null, name, fullClassName, assemblyName, displayName, null)
     {
     }
 
     internal TestMethod(
-        string? managedTypeName,
         string? managedMethodName,
         string?[]? hierarchyValues,
         string name,
@@ -58,7 +57,6 @@ internal TestMethod(
             Hierarchy = new ReadOnlyCollection<string?>(hierarchyValues);
         }
 
-        ManagedTypeName = managedTypeName;
         ManagedMethodName = managedMethodName;
     }
 
@@ -74,14 +72,15 @@ internal TestMethod(
     public string AssemblyName { get; private set; }
 
     /// <inheritdoc />
-    public string? ManagedTypeName { get; }
+    public string? ManagedTypeName => GetManagedTypeName(FullClassName);
 
     /// <inheritdoc />
     public string? ManagedMethodName { get; }
 
     /// <inheritdoc />
-    [MemberNotNullWhen(true, nameof(ManagedTypeName), nameof(ManagedMethodName))]
-    public bool HasManagedMethodAndTypeProperties => !StringEx.IsNullOrWhiteSpace(ManagedTypeName) && !StringEx.IsNullOrWhiteSpace(ManagedMethodName);
+    [MemberNotNullWhen(true, nameof(ManagedMethodName))]
+    // ManagedTypeName is derived from FullClassName, so this only gates the managed method metadata.
+    public bool HasManagedMethodAndTypeProperties => !StringEx.IsNullOrWhiteSpace(ManagedMethodName);
 
     /// <inheritdoc />
     public IReadOnlyCollection<string?>? Hierarchy { get; }
@@ -107,6 +106,14 @@ internal TestMethod(
     [field: NonSerialized]
     internal MethodInfo? MethodInfo { get; set; }
 
+    private static string GetManagedTypeName(string fullClassName)
+    {
+        int genericArgumentsStart = fullClassName.IndexOf('[');
+        return genericArgumentsStart >= 0
+            ? fullClassName[..genericArgumentsStart]
+            : fullClassName;
+    }
+
     /// <summary>
     /// Gets or sets the test data source ignore message.
     /// </summary>

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

@@ -5,10 +5,13 @@
 
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
+using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
+using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
 using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.TestableImplementations;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 using Moq;
 
@@ -225,6 +228,30 @@ public void GetTestFromMethodShouldInitiateTestMethodWithCorrectParameters()
         testElement.TestMethod.AssemblyName.Should().Be("DummyAssemblyName");
     }
 
+    public void GetTestFromMethodShouldUseClosedFullClassNameAndOpenManagedTypeNameForGenericTypes()
+    {
+        Type closedType = typeof(DummyGenericTestClass<int>);
+        string assemblyName = Assembly.GetExecutingAssembly().FullName!;
+        TypeEnumerator typeEnumerator = GetTypeEnumeratorInstance(closedType, assemblyName);
+
+        MSTest.TestAdapter.ObjectModel.UnitTestElement testElement = typeEnumerator.GetTestFromMethod(closedType.GetMethod(nameof(DummyGenericTestClass<>.GenericTestMethod))!, classDisablesParallelization: false, _warnings);
+
+        testElement.TestMethod.FullClassName.Should().Be(closedType.FullName);
+        testElement.TestMethod.ManagedTypeName.Should().Be(typeof(DummyGenericTestClass<>).FullName);
+
+        var testCase = testElement.ToTestCase();
+        (testCase.GetPropertyValue(TestCaseExtensions.ManagedTypeProperty) as string).Should().Be(typeof(DummyGenericTestClass<>).FullName);
+
+        testCase.LocalExtensionData = null;
+        MSTest.TestAdapter.ObjectModel.UnitTestElement roundTrippedTestElement = testCase.ToUnitTestElementWithUpdatedSource(testCase.Source);
+        // Simulate execution after serialization, where MethodInfo is not available and managed metadata must resolve the method.
+        roundTrippedTestElement.TestMethod.MethodInfo = null;
+
+        _testablePlatformServiceProvider.MockFileOperations.Setup(fo => fo.LoadAssembly(assemblyName))
+            .Returns(Assembly.GetExecutingAssembly());
+        new TypeCache().GetTestMethodInfo(roundTrippedTestElement.TestMethod).Should().NotBeNull();
+    }
+
     public void GetTestFromMethodShouldInitializeAsyncTypeNameCorrectly()
     {
         SetupTestClassAndTestMethods(isValidTestClass: true, isValidTestMethod: true);
@@ -517,4 +544,13 @@ public class DummySecondHidingTestClass : DummyOverridingTestClass
     }
 }
 
+[TestClass]
+internal class DummyGenericTestClass<T>
+{
+    [TestMethod]
+    public void GenericTestMethod()
+    {
+    }
+}
+
 #endregion

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

@@ -54,7 +54,7 @@ public UnitTestDiscovererTests()
     }
 
     private TestMethod CreateTestMethod(string methodName, string typeFullName, string assemblyName, string? displayName)
-        => new(typeFullName, methodName, null, methodName, typeFullName, assemblyName, displayName, null);
+        => new(methodName, null, methodName, typeFullName, assemblyName, displayName, null);
 
     protected override void Dispose(bool disposing)
     {

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TestExecutionManagerTests.cs

@@ -810,7 +810,7 @@ private async Task RunTestsForTestShouldRunTestsInTheParentDomainsApartmentState
     private static TestCase GetTestCase(Type typeOfClass, string testName)
     {
         MethodInfo methodInfo = typeOfClass.GetMethod(testName)!;
-        var testMethod = new TestMethod(typeOfClass.FullName!, methodInfo.Name, hierarchyValues: null, methodInfo.Name, typeOfClass.FullName!, Assembly.GetExecutingAssembly().Location, displayName: null, null);
+        var testMethod = new TestMethod(methodInfo.Name, hierarchyValues: null, methodInfo.Name, typeOfClass.FullName!, Assembly.GetExecutingAssembly().Location, displayName: null, null);
         UnitTestElement element = new(testMethod);
         return element.ToTestCase();
     }

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/TypeCacheTests.cs

@@ -54,7 +54,7 @@ protected override void Dispose(bool disposing)
     #region GetTestMethodInfo tests
 
     private static TestMethod CreateTestMethod(string methodName, string className, string assemblyName, string? displayName)
-        => new(className, methodName, null, methodName, className, assemblyName, displayName, null);
+        => new(methodName, null, methodName, className, assemblyName, displayName, null);
 
     public void GetTestMethodInfoShouldReturnNullIfClassInfoForTheMethodIsNull()
     {

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Execution/UnitTestRunnerTests.cs

@@ -31,7 +31,7 @@ public UnitTestRunnerTests()
     }
 
     private TestMethod CreateTestMethod(string methodName, string typeFullName, string assemblyName, string? displayName)
-        => new(typeFullName, methodName, null, methodName, typeFullName, assemblyName, displayName, null);
+        => new(methodName, null, methodName, typeFullName, assemblyName, displayName, null);
 
     private UnitTestRunner CreateUnitTestRunner(UnitTestElement[] testsToRun)
         => new(GetSettingsWithDebugTrace(false), testsToRun);

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Extensions/TestCaseExtensionsTests.cs

@@ -55,4 +55,40 @@ public void ToUnitTestElementShouldAddDeclaringClassNameToTestElementWhenAvailab
 
         resultUnitTestElement.TestMethod.FullClassName.Should().Be("DummyClassName");
     }
+
+    public void ToUnitTestElementShouldPreferManagedTypeOverTestClassNameWhenAvailable()
+    {
+        TestCase testCase = new("SemanticClassName.DummyMethod", new("DummyUri", UriKind.Relative), Assembly.GetCallingAssembly().FullName!);
+        testCase.SetPropertyValue(EngineConstants.TestClassNameProperty, "SyntacticClassName");
+        testCase.SetPropertyValue(TestCaseExtensions.ManagedTypeProperty, "SemanticClassName");
+        testCase.SetPropertyValue(TestCaseExtensions.ManagedMethodProperty, "DummyMethod");
+
+        UnitTestElement resultUnitTestElement = testCase.ToUnitTestElementWithUpdatedSource(testCase.Source);
+
+        resultUnitTestElement.TestMethod.FullClassName.Should().Be("SemanticClassName");
+        resultUnitTestElement.TestMethod.ManagedTypeName.Should().Be("SemanticClassName");
+    }
+
+    public void ToUnitTestElementShouldParseLegacyClosedGenericFullyQualifiedNameWhenManagedTypeIsOpenGeneric()
+    {
+        Type closedType = typeof(DummyGenericTestClass<int>);
+        string methodName = nameof(DummyGenericTestClass<>.GenericTestMethod);
+        TestCase testCase = new($"{closedType.FullName}.{methodName}", new("DummyUri", UriKind.Relative), Assembly.GetCallingAssembly().FullName!);
+        testCase.SetPropertyValue(EngineConstants.TestClassNameProperty, closedType.FullName);
+        testCase.SetPropertyValue(TestCaseExtensions.ManagedTypeProperty, typeof(DummyGenericTestClass<>).FullName);
+        testCase.SetPropertyValue(TestCaseExtensions.ManagedMethodProperty, methodName);
+
+        UnitTestElement resultUnitTestElement = testCase.ToUnitTestElementWithUpdatedSource(testCase.Source);
+
+        resultUnitTestElement.TestMethod.Name.Should().Be(methodName);
+        resultUnitTestElement.TestMethod.FullClassName.Should().Be(closedType.FullName);
+        resultUnitTestElement.TestMethod.ManagedTypeName.Should().Be(typeof(DummyGenericTestClass<>).FullName);
+    }
+
+    private class DummyGenericTestClass<T>
+    {
+        public void GenericTestMethod()
+        {
+        }
+    }
 }