fix: TestObject use ConcurrentDictionary instead of Dictionary (#4450)

microsoft · May 16, 2023 · 23b2bb5 · 23b2bb5
1 parent aa01c7f
commit 23b2bb5
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 16 deletions.
diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs
@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.Diagnostics.CodeAnalysis;
@@ -26,7 +27,7 @@ public abstract class TestObject
     /// <summary>
     /// The store for all the properties registered.
     /// </summary>
-    private readonly Dictionary<TestProperty, object?> _store;
+    private readonly ConcurrentDictionary<TestProperty, object?> _store = new();
 
     /// <summary>
     /// Property used for Json (de)serialization of store dictionary. Serialization of dictionaries
@@ -66,11 +67,6 @@ public abstract class TestObject
         return _store;
     }
 
-    protected TestObject()
-    {
-        _store = new Dictionary<TestProperty, object?>();
-    }
-
     [OnSerializing]
 #if FullCLR
         private void CacheLazyValuesOnSerializing(StreamingContext context)
@@ -84,11 +80,11 @@ public void CacheLazyValuesOnSerializing(StreamingContext context)
         {
             var lazyValue = (ILazyPropertyValue?)kvp.Value;
             var value = lazyValue?.Value;
-            _store.Remove(kvp.Key);
+            _store.TryRemove(kvp.Key, out _);
 
             if (value != null)
             {
-                _store.Add(kvp.Key, value);
+                _store.TryAdd(kvp.Key, value);
             }
         }
     }
@@ -172,10 +168,7 @@ public void SetPropertyValue(TestProperty property, object? value)
     public void RemovePropertyValue(TestProperty property)
     {
         ValidateArg.NotNull(property, nameof(property));
-        if (_store.TryGetValue(property, out _))
-        {
-            _store.Remove(property);
-        }
+        _store.TryRemove(property, out _);
     }
 
     /// <summary>

diff --git a/...t.TestPlatform.CommunicationUtilities.UnitTests/Serialization/TestObjectConverterTests.cs b/...t.TestPlatform.CommunicationUtilities.UnitTests/Serialization/TestObjectConverterTests.cs
@@ -45,8 +45,14 @@ public void TestCaseObjectShouldSerializeCustomProperties()
         var json = Serialize(test);
 
         // Use raw deserialization to validate basic properties
-        var expectedJson = "{\"Properties\":[{\"Key\":{\"Id\":\"1\",\"Label\":\"label1\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.Guid\"},\"Value\":\"02048dfd-3da7-475d-a011-8dd1121855ec\"},{\"Key\":{\"Id\":\"2\",\"Label\":\"label2\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.Int32\"},\"Value\":29}]}";
-        Assert.AreEqual(expectedJson, json);
+        // Because properties are backed up by a ConcurrentDictionary we don't have control over the order of serialization
+        var expectedJsonWithKey1First = "{\"Properties\":[{\"Key\":{\"Id\":\"1\",\"Label\":\"label1\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.Guid\"},\"Value\":\"02048dfd-3da7-475d-a011-8dd1121855ec\"},{\"Key\":{\"Id\":\"2\",\"Label\":\"label2\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.Int32\"},\"Value\":29}]}";
+        var expectedJsonWithKey2First = "{\"Properties\":[{\"Key\":{\"Id\":\"2\",\"Label\":\"label2\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.Int32\"},\"Value\":29},{\"Key\":{\"Id\":\"1\",\"Label\":\"label1\",\"Category\":\"\",\"Description\":\"\",\"Attributes\":0,\"ValueType\":\"System.Guid\"},\"Value\":\"02048dfd-3da7-475d-a011-8dd1121855ec\"}]}";
+
+        if (json != expectedJsonWithKey1First && json != expectedJsonWithKey2First)
+        {
+            Assert.Fail($"Was expecting <{json}> to be either <{expectedJsonWithKey1First}> or <{expectedJsonWithKey2First}>.");
+        }
     }
 
     [TestMethod]
@@ -86,8 +92,8 @@ public void TestObjectShouldDeserializeCustomProperties()
 
         var properties = test.Properties.ToArray();
         Assert.AreEqual(2, properties.Length);
-        Assert.AreEqual(Guid.Parse("02048dfd-3da7-475d-a011-8dd1121855ec"), test.GetPropertyValue(properties[0]));
-        Assert.AreEqual(29, test.GetPropertyValue(properties[1]));
+        Assert.AreEqual(Guid.Parse("02048dfd-3da7-475d-a011-8dd1121855ec"), test.GetPropertyValue(properties.First(x => x.Label == "label1")));
+        Assert.AreEqual(29, test.GetPropertyValue(properties.First(x => x.Label == "label2")));
     }
 
     [TestMethod]