Cleanup and simplify ObjectModelConverters (#5525)

Author: Youssef1313

src/Platform/Microsoft.Testing.Extensions.VSTestBridge/ObjectModel/ObjectModelConverters.cs

@@ -9,7 +9,6 @@
 using Microsoft.Testing.Platform.CommandLine;
 using Microsoft.Testing.Platform.Extensions.Messages;
 using Microsoft.Testing.Platform.ServerMode;
-using Microsoft.TestPlatform.AdapterUtilities;
 using Microsoft.TestPlatform.AdapterUtilities.ManagedNameUtilities;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel;
 
@@ -71,7 +70,7 @@ public static TestNode ToTestNode(this TestCase testCase, bool isTrxEnabled, INa
 
         if (ShouldAddVSTestProviderProperties(namedFeatureCapability, commandLineOptions))
         {
-            CopyVSTestProviderProperties(testCase.Properties, testNode, testCase.GetPropertyValue);
+            CopyVSTestProviderProperties(testNode, testCase);
         }
 
         if (testCase.CodeFilePath is not null)
@@ -109,34 +108,21 @@ private static void CopyCategoryAndTraits(TestObject testCaseOrResult, TestNode
         }
     }
 
-    private static void CopyVSTestProviderProperties(IEnumerable<TestProperty> testProperties, TestNode testNode, Func<TestProperty, object?> getPropertyValue)
+    private static void CopyVSTestProviderProperties(TestNode testNode, TestCase testCase)
     {
-        foreach (TestProperty property in testProperties)
+        if (testCase.Id is Guid testCaseId)
         {
-            // If vstestProvider is enabled (only known to be true for NUnit and Expecto so far), and we are running server mode in IDE (not dotnet test),
-            // we add these stuff.
-            // Once NUnit and Expecto allow us to move forward and remove vstestProvider, we can remove this logic and get rid of the whole vstestProvider capability.
-            if (property.Id == TestCaseProperties.Id.Id
-                    && getPropertyValue(property) is Guid testCaseId)
-            {
-                testNode.Properties.Add(new SerializableKeyValuePairStringProperty("vstest.TestCase.Id", testCaseId.ToString()));
-            }
-            else if (property.Id == TestCaseProperties.FullyQualifiedName.Id
-                && getPropertyValue(property) is string testCaseFqn)
-            {
-                testNode.Properties.Add(new SerializableKeyValuePairStringProperty("vstest.TestCase.FullyQualifiedName", testCaseFqn));
-            }
-            else if (property.Id == OriginalExecutorUriProperty.Id
-                && getPropertyValue(property) is Uri originalExecutorUri)
-            {
-                testNode.Properties.Add(new SerializableKeyValuePairStringProperty("vstest.original-executor-uri", originalExecutorUri.AbsoluteUri));
-            }
-            else if (property.Id == HierarchyConstants.HierarchyPropertyId
-                && getPropertyValue(property) is string[] testCaseHierarchy
-                && testCaseHierarchy.Length == 4)
-            {
-                testNode.Properties.Add(new SerializableNamedArrayStringProperty("vstest.TestCase.Hierarchy", testCaseHierarchy));
-            }
+            testNode.Properties.Add(new SerializableKeyValuePairStringProperty("vstest.TestCase.Id", testCaseId.ToString()));
+        }
+
+        if (testCase.FullyQualifiedName is string testCaseFqn)
+        {
+            testNode.Properties.Add(new SerializableKeyValuePairStringProperty("vstest.TestCase.FullyQualifiedName", testCaseFqn));
+        }
+
+        if (testCase.GetPropertyValue(OriginalExecutorUriProperty) is Uri originalExecutorUri)
+        {
+            testNode.Properties.Add(new SerializableKeyValuePairStringProperty("vstest.original-executor-uri", originalExecutorUri.AbsoluteUri));
         }
     }
 
@@ -149,15 +135,6 @@ public static TestNode ToTestNode(this TestResult testResult, bool isTrxEnabled,
 
         CopyCategoryAndTraits(testResult, testNode, isTrxEnabled);
 
-        bool addVSTestProviderProperties = ShouldAddVSTestProviderProperties(namedFeatureCapability, commandLineOptions);
-        if (addVSTestProviderProperties)
-        {
-            // TODO: This call might be unnecessary.
-            // All the relevant properties should be on TestCase, not TestResult.
-            // And properties on TestCase were already copied as part of ToTestNode call above.
-            CopyVSTestProviderProperties(testResult.Properties, testNode, testResult.GetPropertyValue);
-        }
-
         testNode.AddOutcome(testResult);
 
         if (isTrxEnabled)
@@ -189,6 +166,7 @@ public static TestNode ToTestNode(this TestResult testResult, bool isTrxEnabled,
 
         var standardErrorMessages = new List<string>();
         var standardOutputMessages = new List<string>();
+        bool addVSTestProviderProperties = ShouldAddVSTestProviderProperties(namedFeatureCapability, commandLineOptions);
         foreach (TestResultMessage testResultMessage in testResult.Messages)
         {
             if (testResultMessage.Category == TestResultMessage.StandardErrorCategory)

src/Platform/Microsoft.Testing.Platform/Messages/TestNodeProperties.cs

@@ -381,17 +381,3 @@ public record StandardErrorProperty(string StandardError) : IProperty;
 public record FileArtifactProperty(FileInfo FileInfo, string DisplayName, string? Description = null) : IProperty;
 
 internal sealed record SerializableKeyValuePairStringProperty(string Key, string Value) : KeyValuePairStringProperty(Key, Value);
-
-internal sealed record SerializableNamedArrayStringProperty(string Name, string[] Values) : IProperty
-{
-    [SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "https://github.com/dotnet/roslyn/issues/52421")]
-    private bool PrintMembers(StringBuilder builder)
-    {
-        builder.Append("Name = ");
-        builder.Append(Name);
-        builder.Append(", Values = [");
-        builder.AppendJoin(", ", Values);
-        builder.Append(']');
-        return true;
-    }
-}

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.cs

@@ -147,12 +147,6 @@ public Json(Dictionary<Type, JsonSerializer>? serializers = null, Dictionary<Typ
                     continue;
                 }
 
-                if (property is SerializableNamedArrayStringProperty namedArrayStringProperty)
-                {
-                    properties.Add((namedArrayStringProperty.Name, namedArrayStringProperty.Values));
-                    continue;
-                }
-
                 if (property is TestFileLocationProperty fileLocationProperty)
                 {
                     properties.Add(("location.file", fileLocationProperty.FilePath));

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/SerializerUtilities.cs

@@ -209,12 +209,6 @@ static SerializerUtilities()
                         continue;
                     }
 
-                    if (property is SerializableNamedArrayStringProperty namedArrayStringProperty)
-                    {
-                        properties[namedArrayStringProperty.Name] = namedArrayStringProperty.Values;
-                        continue;
-                    }
-
                     if (property is TestFileLocationProperty fileLocationProperty)
                     {
                         properties["location.file"] = fileLocationProperty.FilePath;

test/UnitTests/Microsoft.Testing.Extensions.VSTestBridge.UnitTests/ObjectModel/ObjectModelConvertersTests.cs

@@ -99,32 +99,15 @@ public void ToTestNode_WhenTestResultHasMSTestDiscovererTestCategoryTestProperty
     }
 
     [TestMethod]
-    public void ToTestNode_WhenTestResultHasTestCaseHierarchyTestProperty_TestNodePropertiesContainItInSerializableNamedArrayStringProperty()
+    public void ToTestNode_WhenTestCaseHasOriginalExecutorUriProperty_TestNodePropertiesContainItInSerializableKeyValuePairStringProperty()
     {
-        TestResult testResult = new(new TestCase("SomeFqn", new("executor://uri", UriKind.Absolute), "source.cs"));
-        var testCaseHierarchy = TestProperty.Register("TestCase.Hierarchy", "Label", typeof(string[]), TestPropertyAttributes.None, typeof(TestCase));
-        testResult.SetPropertyValue<string[]>(testCaseHierarchy, ["assembly", "class", "category", "test"]);
-
-        var testNode = testResult.ToTestNode(false, new NamedFeatureCapabilityWithVSTestProvider(), new ServerModeCommandLineOptions());
-
-        SerializableNamedArrayStringProperty[] trxCategoriesProperty = testNode.Properties.OfType<SerializableNamedArrayStringProperty>().ToArray();
-        Assert.AreEqual(1, trxCategoriesProperty.Length);
-        Assert.AreEqual("assembly", trxCategoriesProperty[0].Values[0]);
-        Assert.AreEqual("class", trxCategoriesProperty[0].Values[1]);
-        Assert.AreEqual("category", trxCategoriesProperty[0].Values[2]);
-        Assert.AreEqual("test", trxCategoriesProperty[0].Values[3]);
-    }
-
-    [TestMethod]
-    public void ToTestNode_WhenTestResultHasOriginalExecutorUriProperty_TestNodePropertiesContainItInSerializableKeyValuePairStringProperty()
-    {
-        TestResult testResult = new(new TestCase("SomeFqn", new("executor://uri", UriKind.Absolute), "source.cs"));
+        var testCase = new TestCase("SomeFqn", new("executor://uri", UriKind.Absolute), "source.cs");
         var originalExecutorUriProperty = TestProperty.Register(
         VSTestTestNodeProperties.OriginalExecutorUriPropertyName, VSTestTestNodeProperties.OriginalExecutorUriPropertyName, typeof(Uri), typeof(TestCase));
 
-        testResult.SetPropertyValue<Uri>(originalExecutorUriProperty, new Uri("https://vs.com/"));
+        testCase.SetPropertyValue<Uri>(originalExecutorUriProperty, new Uri("https://vs.com/"));
 
-        var testNode = testResult.ToTestNode(false, new NamedFeatureCapabilityWithVSTestProvider(), new ServerModeCommandLineOptions());
+        var testNode = testCase.ToTestNode(false, new NamedFeatureCapabilityWithVSTestProvider(), new ServerModeCommandLineOptions());
 
         SerializableKeyValuePairStringProperty[] serializableKeyValuePairStringProperty = testNode.Properties.OfType<SerializableKeyValuePairStringProperty>().ToArray();
         Assert.AreEqual(3, serializableKeyValuePairStringProperty.Length);
@@ -229,20 +212,17 @@ public void ToTestNode_WhenTestResultOutcomeIsPassed_TestNodePropertiesContainPa
     }
 
     [TestMethod]
-    public void ToTestNode_WhenTestResultHasUidAndDisplayNameWithWellKnownClient_TestNodePropertiesContainSerializableKeyValuePairStringPropertyTwice()
+    public void ToTestNode_WhenTestCaseHasUidAndDisplayNameWithWellKnownClient_TestNodePropertiesContainSerializableKeyValuePairStringPropertyTwice()
     {
-        TestResult testResult = new(new TestCase("SomeFqn", new("executor://uri", UriKind.Absolute), "source.cs"))
-        {
-            DisplayName = "TestName",
-        };
+        var testCase = new TestCase("SomeFqn", new("executor://uri", UriKind.Absolute), "source.cs");
 
-        var testNode = testResult.ToTestNode(false, new NamedFeatureCapabilityWithVSTestProvider(), new ServerModeCommandLineOptions());
+        var testNode = testCase.ToTestNode(false, new NamedFeatureCapabilityWithVSTestProvider(), new ServerModeCommandLineOptions());
 
         SerializableKeyValuePairStringProperty[] errorTestNodeStateProperties = testNode.Properties.OfType<SerializableKeyValuePairStringProperty>().ToArray();
         Assert.AreEqual(2, errorTestNodeStateProperties.Length, "Expected 2 SerializableKeyValuePairStringProperty");
-        Assert.AreEqual("vstest.TestCase.Id", errorTestNodeStateProperties[0].Key);
-        Assert.AreEqual("vstest.TestCase.FullyQualifiedName", errorTestNodeStateProperties[1].Key);
-        Assert.AreEqual("SomeFqn", errorTestNodeStateProperties[1].Value);
+        Assert.AreEqual("vstest.TestCase.FullyQualifiedName", errorTestNodeStateProperties[0].Key);
+        Assert.AreEqual("SomeFqn", errorTestNodeStateProperties[0].Value);
+        Assert.AreEqual("vstest.TestCase.Id", errorTestNodeStateProperties[1].Key);
     }
 
     [TestMethod]

test/UnitTests/Microsoft.Testing.Platform.UnitTests/Messages/TestNodePropertiesTests.cs

@@ -213,10 +213,4 @@ public void TestMetadataProperty_ToStringIsCorrect()
         => Assert.AreEqual(
             "TestMetadataProperty { Key = some name, Value = some value }",
             new TestMetadataProperty("some name", "some value").ToString());
-
-    [TestMethod]
-    public void SerializableNamedArrayStringProperty_ToStringIsCorrect()
-        => Assert.AreEqual(
-            "SerializableNamedArrayStringProperty { Name = some name, Values = [value] }",
-            new SerializableNamedArrayStringProperty("some name", ["value"]).ToString());
 }

test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/FormatterUtilitiesTests.cs

@@ -176,7 +176,7 @@ private static void AssertSerialize(Type type, string instanceSerialized)
 
         if (type == typeof(TestNode))
         {
-            Assert.AreEqual("""{"uid":"uid","display-name":"DisplayName","traits":[{"testmetadata-key":"testmetadata-value"}],"array-key":["1","2"],"standardError":"textProperty2","standardOutput":"textProperty","time.start-utc":"2023-01-01T01:01:01.0000000+00:00","time.stop-utc":"2023-01-01T01:01:01.0000000+00:00","time.duration-ms":0,"location.type":"namespace.typeName","location.method":"methodName(param1,param2)","location.file":"filePath","location.line-start":1,"location.line-end":2,"key":"value","node-type":"action","execution-state":"failed","error.message":"sample","error.stacktrace":"","assert.actual":"","assert.expected":""}""".Replace(" ", string.Empty), instanceSerialized, because);
+            Assert.AreEqual("""{"uid":"uid","display-name":"DisplayName","traits":[{"testmetadata-key":"testmetadata-value"}],"standardError":"textProperty2","standardOutput":"textProperty","time.start-utc":"2023-01-01T01:01:01.0000000+00:00","time.stop-utc":"2023-01-01T01:01:01.0000000+00:00","time.duration-ms":0,"location.type":"namespace.typeName","location.method":"methodName(param1,param2)","location.file":"filePath","location.line-start":1,"location.line-end":2,"key":"value","node-type":"action","execution-state":"failed","error.message":"sample","error.stacktrace":"","assert.actual":"","assert.expected":""}""".Replace(" ", string.Empty), instanceSerialized, because);
             return;
         }
 
@@ -188,7 +188,7 @@ private static void AssertSerialize(Type type, string instanceSerialized)
 
         if (type == typeof(TestNodeUpdateMessage))
         {
-            Assert.AreEqual("""{"node":{"uid":"uid","display-name":"DisplayName","traits":[{"testmetadata-key":"testmetadata-value"}],"array-key":["1","2"],"standardError":"textProperty2","standardOutput":"textProperty","time.start-utc":"2023-01-01T01:01:01.0000000+00:00","time.stop-utc":"2023-01-01T01:01:01.0000000+00:00","time.duration-ms":0,"location.type":"namespace.typeName","location.method":"methodName(param1,param2)","location.file":"filePath","location.line-start":1,"location.line-end":2,"key":"value","node-type":"action","execution-state":"failed","error.message":"sample","error.stacktrace":"","assert.actual":"","assert.expected":""},"parent":"parent-uid"}""".Replace(" ", string.Empty), instanceSerialized, because);
+            Assert.AreEqual("""{"node":{"uid":"uid","display-name":"DisplayName","traits":[{"testmetadata-key":"testmetadata-value"}],"standardError":"textProperty2","standardOutput":"textProperty","time.start-utc":"2023-01-01T01:01:01.0000000+00:00","time.stop-utc":"2023-01-01T01:01:01.0000000+00:00","time.duration-ms":0,"location.type":"namespace.typeName","location.method":"methodName(param1,param2)","location.file":"filePath","location.line-start":1,"location.line-end":2,"key":"value","node-type":"action","execution-state":"failed","error.message":"sample","error.stacktrace":"","assert.actual":"","assert.expected":""},"parent":"parent-uid"}""".Replace(" ", string.Empty), instanceSerialized, because);
             return;
         }
 
@@ -462,7 +462,6 @@ static TestNode GetSampleTestNode()
             testNode.Properties.Add(new FailedTestNodeStateProperty(new InvalidOperationException("sample")));
             testNode.Properties.Add(new StandardOutputProperty("textProperty"));
             testNode.Properties.Add(new StandardErrorProperty("textProperty2"));
-            testNode.Properties.Add(new SerializableNamedArrayStringProperty("array-key", ["1", "2"]));
             testNode.Properties.Add(new TestMetadataProperty("testmetadata-key", "testmetadata-value"));
 
             return testNode;