From 27c6275cf3b21d62c3c4f5939165e52d4ef8a29f Mon Sep 17 00:00:00 2001 From: Arun Mahapatra Date: Thu, 26 Oct 2017 14:07:48 +0530 Subject: [PATCH 1/2] Do not crash data collector if a extension initialize fails. --- .../DataCollection/DataCollectionManager.cs | 2 +- .../Client/ProxyExecutionManagerWithDataCollection.cs | 3 ++- .../DataCollection/ProxyDataCollectionManager.cs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionManager.cs b/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionManager.cs index d933f3aefe..395f937052 100644 --- a/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionManager.cs +++ b/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionManager.cs @@ -537,9 +537,9 @@ private Dictionary GetEnvironmentVari var dataCollectorEnvironmentVariable = new Dictionary(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 index baac2470d5..e4b5d7186a 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyExecutionManagerWithDataCollection.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyExecutionManagerWithDataCollection.cs @@ -26,7 +26,7 @@ internal class ProxyExecutionManagerWithDataCollection : ProxyExecutionManager /// /// Initializes a new instance of the class. /// - /// + /// /// Test request sender instance. /// /// @@ -44,6 +44,7 @@ public ProxyExecutionManagerWithDataCollection(IRequestData requestData, ITestRe this.ProxyDataCollectionManager = proxyDataCollectionManager; this.DataCollectionRunEventsHandler = new DataCollectionRunEventsHandler(); this.requestData = requestData; + this.dataCollectionEnvironmentVariables = new Dictionary(); } /// diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/ProxyDataCollectionManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/ProxyDataCollectionManager.cs index 9fc0ac15c0..687066c8ce 100644 --- 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 environmentVariables = null; + IDictionary environmentVariables = new Dictionary(); var dataCollectionEventsPort = 0; this.InvokeDataCollectionServiceAction( From 336698c6f1083db247532b2950d8580b9d014556 Mon Sep 17 00:00:00 2001 From: Arun Mahapatra Date: Thu, 26 Oct 2017 16:02:26 +0530 Subject: [PATCH 2/2] Add unit tests for the scenario. --- .../DataCollection/ProxyDataCollectionManagerTests.cs | 6 +++--- .../DataCollectionManagerTests.cs | 11 +++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/DataCollection/ProxyDataCollectionManagerTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/DataCollection/ProxyDataCollectionManagerTests.cs index a2526df5ca..ba778d8021 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/DataCollection/ProxyDataCollectionManagerTests.cs +++ b/test/Microsoft.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 index 9c9963323f..94d5bf01ee 100644 --- 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()), Times.Once); } + [TestMethod] + public void InitializeDataCollectorsShouldLogExceptionToMessageSinkIfSetEnvironmentVariableFails() + { + this.mockDataCollector.As().Setup(x => x.GetTestExecutionEnvironmentVariables()).Throws(); + + this.dataCollectionManager.InitializeDataCollectors(this.dataCollectorSettings); + + Assert.AreEqual(0, this.dataCollectionManager.RunDataCollectors.Count); + this.mockMessageSink.Verify(x => x.SendMessage(It.IsAny()), Times.Once); + } + [TestMethod] public void InitializeDataCollectorsShouldReturnFirstEnvironmentVariableIfMoreThanOneVariablesWithSameKeyIsSpecified() {