Register property attributes during deserialization (#1644)

When registering properties as part of deserialization, we were registering with the wrong attributes. This means, for example, that the registered MSTestDiscoverer.TestCategory property in devenv.exe would end up without the TestPropertyAttributes.Trait attribute.

The effects of this bug are unobservable for tests discovered via container/reflection based discovery (CBD) (since testcase.SetPropertyValues() is called with the deserialized testPropery objects directly as opposed to the property that was registered). However, this leads to problems for tests reported via live unit testing (LUT) and source based discovery (SBD). Both LUT and SBD need to apply MSTestDiscoverer.TestCategory property on tests and they call TestProperty.Find() to find existing an property with that id. If CBD has already happened LUT and SBD will find the property registered by the below code with missing TestPropertyAttributes.Trait attribute. Consequently new tests reported via LUT and SBD end up under the No Traits group in the test window if user has grouped their tests by traits. Changes in traits for existing tests updated via LUT are also not reflected in this view. Worse still, the problem only repros if CBD has happened before SBD or LUT (which can be somewhat racy because both LUT and CBD can be triggered parallelly after solution load).

This fix ensures that we when we register properties we also apply the correct attributes so that any other consumers who call TestProperty.Find() within the same process (or rather AppDomain) will find the correct property.

src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestCaseConverter.cs

@@ -71,7 +71,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
                             testCase.LineNumber = int.Parse(propertyData); break;
                         default:
                             // No need to register member properties as they get registered as part of TestCaseProperties class.
-                            TestProperty.Register(testProperty.Id, testProperty.Label, testProperty.GetValueType(), typeof(TestObject));
+                            testProperty = TestProperty.Register(testProperty.Id, testProperty.Label, testProperty.GetValueType(), testProperty.Attributes, typeof(TestObject));
                             testCase.SetPropertyValue(testProperty, propertyData);
                             break;
                     }

src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestResultConverter.cs

@@ -100,7 +100,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
                             testResult.ErrorStackTrace = propertyData; break;
                         default:
                             // No need to register member properties as they get registered as part of TestResultProperties class.
-                            TestProperty.Register(testProperty.Id, testProperty.Label, testProperty.GetValueType(), typeof(TestObject));
+                            testProperty = TestProperty.Register(testProperty.Id, testProperty.Label, testProperty.GetValueType(), testProperty.Attributes, typeof(TestObject));
                             testResult.SetPropertyValue(testProperty, propertyData);
                             break;
                     }

test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/Serialization/TestCaseSerializationTests.cs

@@ -130,7 +130,7 @@ public void TestCasePropertiesShouldGetRegisteredAsPartOfDeserialization()
             var json = "{\"Properties\":[{\"Key\":{\"Id\":\"TestCase.FullyQualifiedName\",\"Label\":\"FullyQualifiedName\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":1,\"ValueType\":\"System.String\"},\"Value\":\"a.b\"},"
                 + "{\"Key\":{\"Id\":\"TestCase.ExecutorUri\",\"Label\":\"Executor Uri\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":1,\"ValueType\":\"System.Uri\"},\"Value\":\"uri://x\"},"
                 + "{\"Key\":{\"Id\":\"TestCase.Source\",\"Label\":\"Source\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"/tmp/a.b.dll\"},"
-                + "{\"Key\":{\"Id\":\"DummyProperty\",\"Label\":\"DummyPropertyLabel\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"dummyString\"},"
+                + "{\"Key\":{\"Id\":\"DummyProperty\",\"Label\":\"DummyPropertyLabel\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":5,\"ValueType\":\"System.String\"},\"Value\":\"dummyString\"},"
                 + "{\"Key\":{\"Id\":\"TestObject.Traits\",\"Label\":\"Traits\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":5,\"ValueType\":\"System.Collections.Generic.KeyValuePair`2[[System.String],[System.String]][]\"},\"Value\":[{\"Key\":\"t\",\"Value\":\"SDJDDHW>,:&^%//\\\\\\\\\\\\\\\\\"}]}]}";
 
             var test = Deserialize<TestCase>(json);
@@ -224,7 +224,7 @@ public void TestCasePropertiesShouldGetRegisteredAsPartOfDeserializationV2()
         {
             TestProperty.TryUnregister("DummyProperty", out var property);
             var json = "{\"Id\": \"be78d6fc-61b0-4882-9d07-40d796fd96ce\",\"FullyQualifiedName\": \"sampleTestClass.sampleTestCase\",\"DisplayName\": \"sampleTestCase\",\"ExecutorUri\": \"executor://sampleTestExecutor\",\"Source\": \"sampleTest.dll\",\"CodeFilePath\": \"/user/src/testFile.cs\", \"LineNumber\": 999,"
-                + "\"Properties\": [{\"Key\":{\"Id\":\"DummyProperty\",\"Label\":\"DummyPropertyLabel\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"dummyString\"},"
+                + "\"Properties\": [{\"Key\":{\"Id\":\"DummyProperty\",\"Label\":\"DummyPropertyLabel\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":5,\"ValueType\":\"System.String\"},\"Value\":\"dummyString\"},"
                 + "{ \"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, 2);
@@ -271,6 +271,7 @@ private void VerifyDummyPropertyIsRegistered()
             Assert.IsNotNull(dummyProperty);
             Assert.AreEqual("DummyPropertyLabel", dummyProperty.Label);
             Assert.AreEqual("System.String", dummyProperty.ValueType);
+            Assert.AreEqual(5, (int)dummyProperty.Attributes);
         }
     }
 }

test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/Serialization/TestResultSerializationTests.cs

@@ -159,9 +159,9 @@ public void TestResultPropertiesShouldGetRegisteredAsPartOfDeserialization()
                 "{\"Key\":{\"Id\":\"TestResult.ErrorMessage\",\"Label\":\"Error Message\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":null},{\"Key\":{\"Id\":\"TestResult.ErrorStackTrace\",\"Label\":\"Error Stack Trace\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":null}," +
                 "{\"Key\":{\"Id\":\"TestResult.DisplayName\",\"Label\":\"TestResult Display Name\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":1,\"ValueType\":\"System.String\"},\"Value\":null},{\"Key\":{\"Id\":\"TestResult.ComputerName\",\"Label\":\"Computer Name\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"\"}," +
                 "{\"Key\":{\"Id\":\"TestResult.Duration\",\"Label\":\"Duration\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.TimeSpan\"},\"Value\":\"00:00:00\"},{\"Key\":{\"Id\":\"TestResult.StartTime\",\"Label\":\"Start Time\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.DateTimeOffset\"},\"Value\":\"0001-01-01T00:00:00+00:00\"}," +
-                "{\"Key\":{\"Id\":\"DummyProperty\",\"Label\":\"DummyPropertyLabel\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"dummyString\"}," +
+                "{\"Key\":{\"Id\":\"DummyProperty\",\"Label\":\"DummyPropertyLabel\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":5,\"ValueType\":\"System.String\"},\"Value\":\"dummyString\"}," +
                 "{\"Key\":{\"Id\":\"TestResult.EndTime\",\"Label\":\"End Time\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.DateTimeOffset\"},\"Value\":\"0001-01-01T00:00:00+00:00\"}]}";
-            var test = Deserialize<TestCase>(json);
+            var test = Deserialize<TestResult>(json);
 
             this.VerifyDummyPropertyIsRegistered();
         }
@@ -242,9 +242,9 @@ public void TestResultPropertiesShouldGetRegisteredAsPartOfDeserializationV2()
         {
             TestProperty.TryUnregister("DummyProperty", out var property);
             var json = "{\"TestCase\":{\"Id\":\"28e7a7ed-8fb9-05b7-5e90-4a8c52f32b5b\",\"FullyQualifiedName\":\"sampleTestClass.sampleTestCase\",\"DisplayName\":\"sampleTestClass.sampleTestCase\",\"ExecutorUri\":\"executor://sampleTestExecutor\",\"Source\":\"sampleTest.dll\",\"CodeFilePath\":null,\"LineNumber\":-1,\"Properties\":[]},\"Attachments\":[],\"Outcome\":1,\"ErrorMessage\":\"sampleError\",\"ErrorStackTrace\":\"sampleStackTrace\",\"DisplayName\":\"sampleTestResult\",\"Messages\":[],\"ComputerName\":\"sampleComputerName\",\"Duration\":\"10675199.02:48:05.4775807\",\"StartTime\":\"2007-03-10T00:00:00+00:00\",\"EndTime\":\"9999-12-31T23:59:59.9999999+00:00\"," +
-                "\"Properties\":[{\"Key\":{\"Id\":\"DummyProperty\",\"Label\":\"DummyPropertyLabel\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.String\"},\"Value\":\"dummyString\"},]}";
+                "\"Properties\":[{\"Key\":{\"Id\":\"DummyProperty\",\"Label\":\"DummyPropertyLabel\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":5,\"ValueType\":\"System.String\"},\"Value\":\"dummyString\"},]}";
 
-            var test = Deserialize<TestCase>(json, 2);
+            var test = Deserialize<TestResult>(json, 2);
 
             this.VerifyDummyPropertyIsRegistered();
         }
@@ -267,6 +267,7 @@ private void VerifyDummyPropertyIsRegistered()
             Assert.IsNotNull(dummyProperty);
             Assert.AreEqual("DummyPropertyLabel", dummyProperty.Label);
             Assert.AreEqual("System.String", dummyProperty.ValueType);
+            Assert.AreEqual(5, (int)dummyProperty.Attributes);
         }
     }
 }