TypeDescriptor Fix (#530)

- Instead of register the custom converters with TypeDescriptor, use them as static fields. - Removing the custom guid converter - Fixed the InProcDataCollector Test
microsoft · Feb 22, 2017 · b78eafa · b78eafa
1 parent f168241
commit b78eafa
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 40 deletions.
diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs
@@ -18,17 +18,11 @@ namespace Microsoft.VisualStudio.TestPlatform.ObjectModel
     [DataContract]
     public abstract class TestObject
     {
-        static TestObject()
-        {
-            // Do TypeConverter registrations in static constructor only to avoid multiple registrations
-            // If we register too much, deserialization of large number of testobjects will cause StackOverflow exception
-            TypeDescriptor.AddAttributes(typeof(Guid), new TypeConverterAttribute(typeof(CustomGuidConverter)));
-            TypeDescriptor.AddAttributes(typeof(KeyValuePair<string, string>[]), new TypeConverterAttribute(typeof(CustomKeyValueConverter)));
-            TypeDescriptor.AddAttributes(typeof(string[]), new TypeConverterAttribute(typeof(CustomStringArrayConverter)));
-        }
-
         #region Fields
 
+        private static CustomKeyValueConverter keyValueConverter = new CustomKeyValueConverter();
+        private static CustomStringArrayConverter stringArrayConverter = new CustomStringArrayConverter();
+
         /// <summary>
         /// The store for all the properties registered.
         /// </summary>
@@ -62,6 +56,9 @@ static TestObject()
                         null,
                         property.Key.Attributes,
                         typeof(TestObject));
+
+                    // Do not call SetPropertyValue(TestProperty property, object value) as it does not
+                    // invoke ConvertPropertyFrom and does not store the properties in correct types.
                     this.SetPropertyValue(property.Key, property.Value, CultureInfo.InvariantCulture);
                 }
             }
@@ -287,6 +284,18 @@ private static object ConvertPropertyFrom<T>(TestProperty property, CultureInfo
                 return value;
             }
 
+            // Traits are KeyValuePair based. Use the custom converter in that case.
+            if (valueType == typeof(KeyValuePair<string, string>[]))
+            {
+                return keyValueConverter.ConvertFrom(null, culture, (string)value);
+            }
+
+            // Use a custom string array converter for string[] types.
+            if (valueType == typeof(string[]))
+            {
+                return stringArrayConverter.ConvertFrom(null, culture, (string)value);
+            }
+
             TypeConverter converter = TypeDescriptor.GetConverter(valueType);
             if (converter == null)
             {

diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomGuidConverter.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomGuidConverter.cs
diff --git a/...TestPlatform.CommunicationUtilities.UnitTests/Serialization/TestCaseSerializationTests.cs b/...TestPlatform.CommunicationUtilities.UnitTests/Serialization/TestCaseSerializationTests.cs
@@ -23,7 +23,7 @@ public class TestCaseSerializationTests
                                                {
                                                    CodeFilePath = "/user/src/testFile.cs",
                                                    DisplayName = "sampleTestCase",
-                                                   Id = Guid.Empty,
+                                                   Id = new Guid("be78d6fc-61b0-4882-9d07-40d796fd96ce"),
                                                    LineNumber = 999,
                                                    Traits = { new Trait("Priority", "0"), new Trait("Category", "unit") }
                                                };
@@ -47,7 +47,7 @@ public void TestCaseJsonShouldContainAllPropertiesOnSerialization()
             Assert.AreEqual("TestCase.DisplayName", properties[4]["Key"]["Id"].Value);
             Assert.AreEqual("sampleTestCase", properties[4]["Value"].Value);
             Assert.AreEqual("TestCase.Id", properties[5]["Key"]["Id"].Value);
-            Assert.AreEqual("00000000-0000-0000-0000-000000000000", properties[5]["Value"].Value);
+            Assert.AreEqual("be78d6fc-61b0-4882-9d07-40d796fd96ce", properties[5]["Value"].Value);
             Assert.AreEqual("TestCase.LineNumber", properties[6]["Key"]["Id"].Value);
             Assert.AreEqual(999, properties[6]["Value"].Value);
 
@@ -64,7 +64,7 @@ public void TestCaseObjectShouldContainAllPropertiesOnDeserialization()
                 + "{\"Key\":{\"Id\":\"TestCase.Source\",\"Label\":\"Source\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"sampleTest.dll\"},"
                 + "{\"Key\":{\"Id\":\"TestCase.CodeFilePath\",\"Label\":\"File Path\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"/user/src/testFile.cs\"},"
                 + "{\"Key\":{\"Id\":\"TestCase.DisplayName\",\"Label\":\"Name\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"sampleTestCase\"},"
-                + "{\"Key\":{\"Id\":\"TestCase.Id\",\"Label\":\"Id\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":1,\"ValueType\":\"System.Guid\"},\"Value\":\"00000000-0000-0000-0000-000000000000\"},"
+                + "{\"Key\":{\"Id\":\"TestCase.Id\",\"Label\":\"Id\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":1,\"ValueType\":\"System.Guid\"},\"Value\":\"be78d6fc-61b0-4882-9d07-40d796fd96ce\"},"
                 + "{\"Key\":{\"Id\":\"TestCase.LineNumber\",\"Label\":\"Line Number\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":1,\"ValueType\":\"System.Int32\"},\"Value\":999},"
                 + "{\"Key\":{\"Id\":\"TestObject.Traits\",\"Label\":\"Traits\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":5,\"ValueType\":\"System.Collections.Generic.KeyValuePair`2[[System.String],[System.String]][]\"},\"Value\":[{\"Key\":\"Priority\",\"Value\":\"0\"},{\"Key\":\"Category\",\"Value\":\"unit\"}]}]}";
             var test = Deserialize<TestCase>(json);
@@ -76,6 +76,7 @@ public void TestCaseObjectShouldContainAllPropertiesOnDeserialization()
             Assert.AreEqual(testCase.LineNumber, test.LineNumber);
             Assert.AreEqual(testCase.Source, test.Source);
             Assert.AreEqual(testCase.Traits.First().Name, test.Traits.First().Name);
+            Assert.AreEqual(testCase.Id, test.Id);
         }
 
         [TestMethod]

diff --git a/test/Microsoft.TestPlatform.SmokeTests/DataCollectorTests.cs b/test/Microsoft.TestPlatform.SmokeTests/DataCollectorTests.cs
@@ -13,7 +13,6 @@ namespace Microsoft.TestPlatform.SmokeTests
     public class DataCollectorTests : IntegrationTestBase
     {
         [TestMethod]
-        [Ignore]
         public void RunAllWithInProcDataCollectorSettings()
         {
             var runSettings = this.GetInProcDataCollectionRunsettingsFile();
@@ -46,7 +45,7 @@ private string GetInProcDataCollectionRunsettingsFile()
             var fileContents = @"<RunSettings>
                                     <InProcDataCollectionRunSettings>
                                         <InProcDataCollectors>
-                                            <InProcDataCollector friendlyName='Test Impact' uri='InProcDataCollector://Microsoft/TestImpact/1.0' assemblyQualifiedName='SimpleDataCollector.SimpleDataCollector, SimpleDataCollector, Version=1.0.0.0, Culture=neutral, PublicKeyToken=7ccb7239ffde675a'  codebase={0}>
+                                            <InProcDataCollector friendlyName='Test Impact' uri='InProcDataCollector://Microsoft/TestImpact/1.0' assemblyQualifiedName='SimpleDataCollector.SimpleDataCollector, SimpleDataCollector, Version=15.0.0.0, Culture=neutral, PublicKeyToken=7ccb7239ffde675a'  codebase={0}>
                                                 <Configuration>
                                                     <Port>4312</Port>
                                                 </Configuration>