Do not crash data collector if a extension initialize fails. (microso…

…ft#1230) * Do not crash data collector if a extension initialize fails. * Add unit tests for the scenario.
codito · Oct 27, 2017 · 0dde783 · 0dde783
1 parent aec5eb4
commit 0dde783
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 6 deletions.
diff --git a/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionManager.cs b/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionManager.cs
@@ -537,9 +537,9 @@ private Dictionary<string, DataCollectionEnvironmentVariable> GetEnvironmentVari
             var dataCollectorEnvironmentVariable = new Dictionary<string, DataCollectionEnvironmentVariable>(StringComparer.OrdinalIgnoreCase);
             foreach (var dataCollectorInfo in this.RunDataCollectors.Values)
             {
-                dataCollectorInfo.SetTestExecutionEnvironmentVariables();
                 try
                 {
+                    dataCollectorInfo.SetTestExecutionEnvironmentVariables();
                     this.AddCollectorEnvironmentVariables(dataCollectorInfo, dataCollectorEnvironmentVariable);
                 }
                 catch (Exception ex)

diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyExecutionManagerWithDataCollection.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyExecutionManagerWithDataCollection.cs
@@ -26,7 +26,7 @@ internal class ProxyExecutionManagerWithDataCollection : ProxyExecutionManager
         /// <summary>
         /// Initializes a new instance of the <see cref="ProxyExecutionManagerWithDataCollection"/> class. 
         /// </summary>
-        /// <param name="testRequestSender">
+        /// <param name="requestSender">
         /// Test request sender instance.
         /// </param>
         /// <param name="testHostManager">
@@ -44,6 +44,7 @@ public ProxyExecutionManagerWithDataCollection(IRequestData requestData, ITestRe
             this.ProxyDataCollectionManager = proxyDataCollectionManager;
             this.DataCollectionRunEventsHandler = new DataCollectionRunEventsHandler();
             this.requestData = requestData;
+            this.dataCollectionEnvironmentVariables = new Dictionary<string, string>();
         }
 
         /// <summary>

diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/ProxyDataCollectionManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/ProxyDataCollectionManager.cs
@@ -149,7 +149,7 @@ public DataCollectionParameters BeforeTestRunStart(
             ITestMessageEventHandler runEventsHandler)
         {
             var areTestCaseLevelEventsRequired = false;
-            IDictionary<string, string> environmentVariables = null;
+            IDictionary<string, string> environmentVariables = new Dictionary<string, string>();
 
             var dataCollectionEventsPort = 0;
             this.InvokeDataCollectionServiceAction(

diff --git a/....TestPlatform.CrossPlatEngine.UnitTests/DataCollection/ProxyDataCollectionManagerTests.cs b/....TestPlatform.CrossPlatEngine.UnitTests/DataCollection/ProxyDataCollectionManagerTests.cs
@@ -136,9 +136,9 @@ public void BeforeTestRunStartsShouldInvokeRunEventsHandlerIfExceptionIsThrown()
             var result = this.proxyDataCollectionManager.BeforeTestRunStart(true, true, mockRunEventsHandler.Object);
 
             mockRunEventsHandler.Verify(eh => eh.HandleLogMessage(TestMessageLevel.Error, "Exception of type 'System.Exception' was thrown."), Times.Once);
-            Assert.AreEqual(result.EnvironmentVariables, null);
-            Assert.AreEqual(result.AreTestCaseLevelEventsRequired, false);
-            Assert.AreEqual(result.DataCollectionEventsPort, 0);
+            Assert.AreEqual(0, result.EnvironmentVariables.Count);
+            Assert.AreEqual(false, result.AreTestCaseLevelEventsRequired);
+            Assert.AreEqual(0, result.DataCollectionEventsPort);
         }
 
         [TestMethod]

diff --git a/test/datacollector.UnitTests/DataCollectionManagerTests.cs b/test/datacollector.UnitTests/DataCollectionManagerTests.cs
@@ -157,6 +157,17 @@ public void InitializeDataCollectorsShouldLogExceptionToMessageSinkIfInitializat
             this.mockMessageSink.Verify(x => x.SendMessage(It.IsAny<DataCollectionMessageEventArgs>()), Times.Once);
         }
 
+        [TestMethod]
+        public void InitializeDataCollectorsShouldLogExceptionToMessageSinkIfSetEnvironmentVariableFails()
+        {
+            this.mockDataCollector.As<ITestExecutionEnvironmentSpecifier>().Setup(x => x.GetTestExecutionEnvironmentVariables()).Throws<Exception>();
+
+            this.dataCollectionManager.InitializeDataCollectors(this.dataCollectorSettings);
+
+            Assert.AreEqual(0, this.dataCollectionManager.RunDataCollectors.Count);
+            this.mockMessageSink.Verify(x => x.SendMessage(It.IsAny<DataCollectionMessageEventArgs>()), Times.Once);
+        }
+
         [TestMethod]
         public void InitializeDataCollectorsShouldReturnFirstEnvironmentVariableIfMoreThanOneVariablesWithSameKeyIsSpecified()
         {