From 8b865e4b1f983a4c95d72285132ee8b7d92a40ba Mon Sep 17 00:00:00 2001 From: samadala Date: Tue, 31 Jul 2018 16:27:23 +0530 Subject: [PATCH 1/6] Print start of testhost standard error message --- .../Constants.cs | 7 ++ .../Client/ProxyOperationManager.cs | 2 - .../DataCollection/DataCollectionLauncher.cs | 7 +- .../Hosting/DefaultTestHostManager.cs | 7 +- .../Hosting/DotnetTestHostManager.cs | 7 +- .../Hosting/TestHostManagerCallbacks.cs | 23 ++--- .../Hosting/DefaultTestHostManagerTests.cs | 3 +- .../Hosting/DotnetTestHostManagerTests.cs | 1 - .../Hosting/TestHostManagerCallbacksTests.cs | 88 +++++++++++++++++++ 9 files changed, 108 insertions(+), 37 deletions(-) create mode 100644 test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/TestHostManagerCallbacksTests.cs diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs b/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs index d9856906a0..345ee39469 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs +++ b/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs @@ -22,5 +22,12 @@ public class Constants /// Datacollector process name, without file extension(.exe/.dll) /// public const string DatacollectorProcessName = "datacollector"; + + + /// + /// Number of character should be logged on child process exited with + /// error message on stadard error. + /// + public const int StandardErrorMaxLength = 20480; // 20 KB } } diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyOperationManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyOperationManager.cs index 0a2a83b2f5..3bb0dd852b 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyOperationManager.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyOperationManager.cs @@ -70,8 +70,6 @@ protected ProxyOperationManager(IRequestData requestData, ITestRequestSender req #region Properties - protected int ErrorLength { get; set; } = 1000; - /// /// Gets or sets the server for communication. /// diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs index 1fb1745a19..84c42b15ed 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs @@ -36,17 +36,12 @@ public DataCollectionLauncher(IProcessHelper processHelper, IMessageLogger messa { this.processHelper = processHelper; this.messageLogger = messageLogger; - this.processStdError = new StringBuilder(this.ErrorLength, this.ErrorLength); + this.processStdError = new StringBuilder(0, CoreUtilities.Constants.StandardErrorMaxLength); } /// public int DataCollectorProcessId { get; protected set; } - /// - /// Gets or sets the error length for data collector error stream. - /// - protected int ErrorLength { get; set; } = 4096; - /// /// Gets callback on process exit /// diff --git a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DefaultTestHostManager.cs b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DefaultTestHostManager.cs index 83550821cd..d73a6739e1 100644 --- a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DefaultTestHostManager.cs +++ b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DefaultTestHostManager.cs @@ -96,11 +96,6 @@ internal DefaultTestHostManager(IProcessHelper processHelper, IFileHelper fileHe /// public IDictionary Properties => new Dictionary(); - /// - /// Gets or sets the error length for runtime error stream. - /// - protected int ErrorLength { get; set; } = 4096; - /// /// Gets callback on process exit /// @@ -367,7 +362,7 @@ private void OnHostExited(HostProviderEventArgs e) private bool LaunchHost(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken) { - this.testHostProcessStdError = new StringBuilder(this.ErrorLength, this.ErrorLength); + this.testHostProcessStdError = new StringBuilder(0, CoreUtilities.Constants.StandardErrorMaxLength); EqtTrace.Verbose("Launching default test Host Process {0} with arguments {1}", testHostStartInfo.FileName, testHostStartInfo.Arguments); if (this.customTestHostLauncher == null) diff --git a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DotnetTestHostManager.cs b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DotnetTestHostManager.cs index 2f7150244d..a962a82f91 100644 --- a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DotnetTestHostManager.cs +++ b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/DotnetTestHostManager.cs @@ -113,11 +113,6 @@ internal DotnetTestHostManager( /// internal bool MakeRunsettingsCompatible => this.hostPackageVersion.StartsWith("15.0.0-preview"); - /// - /// Gets or sets the error length for runtime error stream. - /// - protected int ErrorLength { get; set; } = 4096; - /// /// Gets callback on process exit /// @@ -326,7 +321,7 @@ private void OnHostExited(HostProviderEventArgs e) private bool LaunchHost(TestProcessStartInfo testHostStartInfo, CancellationToken cancellationToken) { - this.testHostProcessStdError = new StringBuilder(this.ErrorLength, this.ErrorLength); + this.testHostProcessStdError = new StringBuilder(0, CoreUtilities.Constants.StandardErrorMaxLength); if (this.customTestHostLauncher == null) { EqtTrace.Verbose("DotnetTestHostManager: Starting process '{0}' with command line '{1}'", testHostStartInfo.FileName, testHostStartInfo.Arguments); diff --git a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs index 5a4082efe6..a4543ce71e 100644 --- a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs +++ b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs @@ -20,24 +20,19 @@ public static void ErrorReceivedCallback(StringBuilder testHostProcessStdError, // This is helpful in abnormal failure of testhost. EqtTrace.Warning("TestHostManagerCallbacks.ErrorReceivedCallback Test host standard error line: {0}", data); - // Add newline for readbility. - data += Environment.NewLine; - - // if incoming data stream is huge empty entire testError stream, & limit data stream to MaxCapacity - if (data.Length > testHostProcessStdError.MaxCapacity) + // Don't append more data if already reached max length. + if (testHostProcessStdError.Length >= testHostProcessStdError.MaxCapacity) { - testHostProcessStdError.Clear(); - data = data.Substring(data.Length - testHostProcessStdError.MaxCapacity); + return; } - // remove only what is required, from beginning of error stream - else + // Add newline for readbility. + data += Environment.NewLine; + + // Append sub string of data if appending all the data exceeds max capacity. + if (testHostProcessStdError.Length + data.Length >= testHostProcessStdError.MaxCapacity) { - int required = data.Length + testHostProcessStdError.Length - testHostProcessStdError.MaxCapacity; - if (required > 0) - { - testHostProcessStdError.Remove(0, required); - } + data = data.Substring(0, testHostProcessStdError.MaxCapacity - testHostProcessStdError.Length); } testHostProcessStdError.Append(data); diff --git a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs index f3405e1bc0..2523761212 100644 --- a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs +++ b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -namespace TestPlatform.TestHostProvider.UnitTests.Hosting +namespace TestPlatform.TestHostProvider.Hosting.UnitTests { using System; using System.Collections.Generic; @@ -600,7 +600,6 @@ public TestableTestHostManager( IMessageLogger logger) : base(processHelper, new FileHelper(), new PlatformEnvironment(), new DotnetHostHelper()) { - this.ErrorLength = errorLength; this.Initialize(logger, $" {architecture} {framework} {!shared} "); } } diff --git a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs index 285dcda804..ac8b537f8b 100644 --- a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs +++ b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs @@ -781,7 +781,6 @@ internal class TestableDotnetTestHostManager : DotnetTestHostManager public TestableDotnetTestHostManager(IProcessHelper processHelper, IFileHelper fileHelper, IDotnetHostHelper dotnetTestHostHelper, int errorLength) : base(processHelper, fileHelper, dotnetTestHostHelper) { - this.ErrorLength = errorLength; } } } diff --git a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/TestHostManagerCallbacksTests.cs b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/TestHostManagerCallbacksTests.cs new file mode 100644 index 0000000000..74f7f76f54 --- /dev/null +++ b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/TestHostManagerCallbacksTests.cs @@ -0,0 +1,88 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace TestPlatform.TestHostProvider.Hosting.UnitTests +{ + using System; + using System.Text; + using Microsoft.TestPlatform.TestHostProvider.Hosting; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class TestHostManagerCallbacksTests + { + private StringBuilder testHostProcessStdError; + + public TestHostManagerCallbacksTests() + { + this.testHostProcessStdError = new StringBuilder(0, Microsoft.VisualStudio.TestPlatform.CoreUtilities.Constants.StandardErrorMaxLength); + } + + [TestMethod] + public void ErrorReceivedCallbackShouldAppendNoDataOnNullDataReceived() + { + this.testHostProcessStdError.Append("NoDataShouldAppend"); + TestHostManagerCallbacks.ErrorReceivedCallback(this.testHostProcessStdError, null); + + Assert.AreEqual("NoDataShouldAppend", this.testHostProcessStdError.ToString()); + } + + [TestMethod] + public void ErrorReceivedCallbackShouldAppendNoDataOnEmptyDataReceived() + { + this.testHostProcessStdError.Append("NoDataShouldAppend"); + TestHostManagerCallbacks.ErrorReceivedCallback(this.testHostProcessStdError, string.Empty); + + Assert.AreEqual("NoDataShouldAppend", this.testHostProcessStdError.ToString()); + } + + [TestMethod] + public void ErrorReceivedCallbackShouldAppendGivenData() + { + this.testHostProcessStdError.Append("NoDataShouldAppend"); + TestHostManagerCallbacks.ErrorReceivedCallback(this.testHostProcessStdError, "new data"); + + Assert.AreEqual("NoDataShouldAppendnew data" + Environment.NewLine, this.testHostProcessStdError.ToString()); + } + + [TestMethod] + public void ErrorReceivedCallbackShouldNotAppendNewDataIfErrorMessageAlreadyReachedMaxLength() + { + this.testHostProcessStdError = new StringBuilder(0, 5); + this.testHostProcessStdError.Append("12345"); + TestHostManagerCallbacks.ErrorReceivedCallback(this.testHostProcessStdError, "678"); + + Assert.AreEqual("12345", this.testHostProcessStdError.ToString()); + } + + [TestMethod] + public void ErrorReceivedCallbackShouldAppendSubStringOfDataIfErrorMessageReachedMaxLength() + { + this.testHostProcessStdError = new StringBuilder(0, 5); + this.testHostProcessStdError.Append("1234"); + TestHostManagerCallbacks.ErrorReceivedCallback(this.testHostProcessStdError, "5678"); + + Assert.AreEqual("12345", this.testHostProcessStdError.ToString()); + } + + [TestMethod] + public void ErrorReceivedCallbackShouldAppendEntireStringEvenItReachesToMaxLength() + { + this.testHostProcessStdError = new StringBuilder(0, 5); + this.testHostProcessStdError.Append("12"); + TestHostManagerCallbacks.ErrorReceivedCallback(this.testHostProcessStdError, "3"); + + Assert.AreEqual("123" + Environment.NewLine, this.testHostProcessStdError.ToString()); + } + + [TestMethod] + public void ErrorReceivedCallbackShouldAppendNewLineApproprioritlyWhenReachingMaxLength() + { + this.testHostProcessStdError = new StringBuilder(0, 5); + this.testHostProcessStdError.Append("123"); + TestHostManagerCallbacks.ErrorReceivedCallback(this.testHostProcessStdError, "4"); + + Assert.AreEqual("1234" + Environment.NewLine.Substring(0, 1), this.testHostProcessStdError.ToString()); + } + } +} From 268d35351b38c4296214e2a083c3033586b0f14f Mon Sep 17 00:00:00 2001 From: samadala Date: Wed, 1 Aug 2018 10:07:21 +0530 Subject: [PATCH 2/6] update unit tests --- .../Constants.cs | 1 - .../Hosting/DefaultTestHostManagerTests.cs | 18 --------------- .../Hosting/DotnetTestHostManagerTests.cs | 22 +++++-------------- 3 files changed, 5 insertions(+), 36 deletions(-) diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs b/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs index 345ee39469..2a67634dba 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs +++ b/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs @@ -23,7 +23,6 @@ public class Constants /// public const string DatacollectorProcessName = "datacollector"; - /// /// Number of character should be logged on child process exited with /// error message on stadard error. diff --git a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs index 2523761212..a1456f7010 100644 --- a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs +++ b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs @@ -40,7 +40,6 @@ public class DefaultTestHostManagerTests private DefaultTestHostManager testHostManager; private TestableTestHostManager testableTestHostManager; - private int maxStdErrStringLength = 22; private string errorMessage; private int exitCode; private int testHostId; @@ -344,7 +343,6 @@ public void LaunchTestHostAsyncShouldNotStartHostProcessIfCancellationTokenIsSet Framework.DefaultFramework, this.mockProcessHelper.Object, true, - this.maxStdErrStringLength, this.mockMessageLogger.Object); CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); @@ -423,19 +421,6 @@ public async Task ErrorMessageShouldBeReadAsynchronously() Assert.AreEqual(errorData, this.errorMessage); } - [TestMethod] - public async Task ErrorMessageShouldBeTruncatedToMatchErrorLength() - { - string errorData = "Long Custom Error Strings"; - this.ErrorCallBackTestHelper(errorData, -1); - - await this.testableTestHostManager.LaunchTestHostAsync(this.GetDefaultStartInfo(), CancellationToken.None); - - // Ignore new line chars - Assert.AreEqual(this.maxStdErrStringLength - Environment.NewLine.Length, this.errorMessage.Length); - Assert.AreEqual(errorData.Substring(5), this.errorMessage); - } - [TestMethod] public async Task NoErrorMessageIfExitCodeZero() { @@ -527,7 +512,6 @@ private void ErrorCallBackTestHelper(string errorMessage, int exitCode) Framework.DefaultFramework, this.mockProcessHelper.Object, true, - this.maxStdErrStringLength, this.mockMessageLogger.Object); this.testableTestHostManager.HostExited += this.TestHostManagerHostExited; @@ -560,7 +544,6 @@ private void ExitCallBackTestHelper(int exitCode) Framework.DefaultFramework, this.mockProcessHelper.Object, true, - this.maxStdErrStringLength, this.mockMessageLogger.Object); this.testableTestHostManager.HostExited += this.TestableTestHostManagerHostExited; @@ -596,7 +579,6 @@ public TestableTestHostManager( Framework framework, IProcessHelper processHelper, bool shared, - int errorLength, IMessageLogger logger) : base(processHelper, new FileHelper(), new PlatformEnvironment(), new DotnetHostHelper()) { diff --git a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs index ac8b537f8b..26d3055238 100644 --- a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs +++ b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs @@ -49,7 +49,6 @@ public class DotnetTestHostManagerTests private readonly TestableDotnetTestHostManager dotnetHostManager; private string errorMessage; - private int maxStdErrStringLength = 22; private int exitCode; @@ -69,8 +68,7 @@ public DotnetTestHostManagerTests() this.dotnetHostManager = new TestableDotnetTestHostManager( this.mockProcessHelper.Object, this.mockFileHelper.Object, - new DotnetHostHelper(this.mockFileHelper.Object, mockEnvironment.Object), - this.maxStdErrStringLength); + new DotnetHostHelper(this.mockFileHelper.Object, mockEnvironment.Object)); this.dotnetHostManager.Initialize(this.mockMessageLogger.Object, string.Empty); this.dotnetHostManager.HostExited += this.DotnetHostManagerHostExited; @@ -620,19 +618,6 @@ public async Task DotNetCoreErrorMessageShouldBeReadAsynchronouslyAsync() Assert.AreEqual(errorData, this.errorMessage); } - [TestMethod] - public async Task DotNetCoreErrorMessageShouldBeTruncatedToMatchErrorLength() - { - var errorData = "Long Custom Error Strings"; - this.ErrorCallBackTestHelper(errorData, -1); - - await this.dotnetHostManager.LaunchTestHostAsync(this.defaultTestProcessStartInfo, CancellationToken.None); - - // Ignore new line chars - Assert.AreEqual(this.maxStdErrStringLength - Environment.NewLine.Length, this.errorMessage.Length); - Assert.AreEqual(errorData.Substring(5), this.errorMessage); - } - [TestMethod] public async Task DotNetCoreNoErrorMessageIfExitCodeZero() { @@ -778,7 +763,10 @@ private TestProcessStartInfo GetDefaultStartInfo() internal class TestableDotnetTestHostManager : DotnetTestHostManager { - public TestableDotnetTestHostManager(IProcessHelper processHelper, IFileHelper fileHelper, IDotnetHostHelper dotnetTestHostHelper, int errorLength) + public TestableDotnetTestHostManager( + IProcessHelper processHelper, + IFileHelper fileHelper, + IDotnetHostHelper dotnetTestHostHelper) : base(processHelper, fileHelper, dotnetTestHostHelper) { } From a27b17de151c75117693aebdb8ee32d418ca4d3e Mon Sep 17 00:00:00 2001 From: samadala Date: Wed, 1 Aug 2018 10:49:40 +0530 Subject: [PATCH 3/6] Print datacollector stderr from the begining --- .../Utilities/StringExtensions.cs | 25 +++++++++++++ .../DataCollection/DataCollectionLauncher.cs | 36 ++++--------------- .../Hosting/TestHostManagerCallbacks.cs | 27 +++----------- .../Hosting/TestHostManagerCallbacksTests.cs | 9 +++++ 4 files changed, 46 insertions(+), 51 deletions(-) diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs index f7a618c5fe..933c0295e1 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs +++ b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs @@ -5,6 +5,8 @@ namespace Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions { using System; using System.Net; + using System.Text; + using ObjectModel; public static class StringExtensions { @@ -17,5 +19,28 @@ public static string AddDoubleQuote(this string value) { return "\"" + value + "\""; } + + public static void AppendToStringBuilderBasedOnMaxLength(this string data, StringBuilder result) + { + if (!string.IsNullOrEmpty(data)) + { + // Don't append more data if already reached max length. + if (result.Length >= result.MaxCapacity) + { + return; + } + + // Add newline for readbility. + data += Environment.NewLine; + + // Append sub string of data if appending all the data exceeds max capacity. + if (result.Length + data.Length >= result.MaxCapacity) + { + data = data.Substring(0, result.MaxCapacity - result.Length); + } + + result.Append(data); + } + } } } diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs index 84c42b15ed..ce4c7188d9 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs @@ -6,7 +6,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection using System; using System.Collections.Generic; using System.Text; - + using CoreUtilities.Extensions; using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection.Interfaces; using Microsoft.VisualStudio.TestPlatform.ObjectModel; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; @@ -71,35 +71,13 @@ public DataCollectionLauncher(IProcessHelper processHelper, IMessageLogger messa /// Gets callback to read from process error stream /// protected Action ErrorReceivedCallback => (process, data) => - { - if (!string.IsNullOrEmpty(data)) - { - // Log all standard error message because on too much data we ignore starting part. - // This is helpful in abnormal failure of process. - EqtTrace.Warning("DataCollectionLauncher.ErrorReceivedCallback: Data collector standard error line: {0}", data); - - // Add newline for readbility. - data += Environment.NewLine; - - // if incoming data stream is huge empty entire testError stream, & limit data stream to MaxCapacity - if (data.Length > this.processStdError.MaxCapacity) - { - this.processStdError.Clear(); - data = data.Substring(data.Length - this.processStdError.MaxCapacity); - } - else - { - // remove only what is required, from beginning of error stream - int required = data.Length + this.processStdError.Length - this.processStdError.MaxCapacity; - if (required > 0) - { - this.processStdError.Remove(0, required); - } - } + { + // Log all standard error message because on too much data we ignore starting part. + // This is helpful in abnormal failure of datacollector. + EqtTrace.Warning("DataCollectionLauncher.ErrorReceivedCallback datacollector standard error line: {0}", data); - this.processStdError.Append(data); - } - }; + data.AppendToStringBuilderBasedOnMaxLength(this.processStdError); + }; /// /// The launch data collector. diff --git a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs index a4543ce71e..dda10ee1b5 100644 --- a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs +++ b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs @@ -9,34 +9,17 @@ namespace Microsoft.TestPlatform.TestHostProvider.Hosting using Microsoft.VisualStudio.TestPlatform.ObjectModel; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Host; using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces; + using VisualStudio.TestPlatform.CoreUtilities.Extensions; internal class TestHostManagerCallbacks { public static void ErrorReceivedCallback(StringBuilder testHostProcessStdError, string data) { - if (!string.IsNullOrEmpty(data)) - { - // Log all standard error message because on too much data we ignore starting part. - // This is helpful in abnormal failure of testhost. - EqtTrace.Warning("TestHostManagerCallbacks.ErrorReceivedCallback Test host standard error line: {0}", data); - - // Don't append more data if already reached max length. - if (testHostProcessStdError.Length >= testHostProcessStdError.MaxCapacity) - { - return; - } - - // Add newline for readbility. - data += Environment.NewLine; + // Log all standard error message because on too much data we ignore starting part. + // This is helpful in abnormal failure of testhost. + EqtTrace.Warning("TestHostManagerCallbacks.ErrorReceivedCallback Test host standard error line: {0}", data); - // Append sub string of data if appending all the data exceeds max capacity. - if (testHostProcessStdError.Length + data.Length >= testHostProcessStdError.MaxCapacity) - { - data = data.Substring(0, testHostProcessStdError.MaxCapacity - testHostProcessStdError.Length); - } - - testHostProcessStdError.Append(data); - } + data.AppendToStringBuilderBasedOnMaxLength(testHostProcessStdError); } public static void ExitCallBack( diff --git a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/TestHostManagerCallbacksTests.cs b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/TestHostManagerCallbacksTests.cs index 74f7f76f54..98e8f2d9cf 100644 --- a/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/TestHostManagerCallbacksTests.cs +++ b/test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/TestHostManagerCallbacksTests.cs @@ -36,6 +36,15 @@ public void ErrorReceivedCallbackShouldAppendNoDataOnEmptyDataReceived() Assert.AreEqual("NoDataShouldAppend", this.testHostProcessStdError.ToString()); } + [TestMethod] + public void ErrorReceivedCallbackShouldAppendWhiteSpaceString() + { + this.testHostProcessStdError.Append("OldData"); + TestHostManagerCallbacks.ErrorReceivedCallback(this.testHostProcessStdError, " "); + + Assert.AreEqual("OldData " + Environment.NewLine, this.testHostProcessStdError.ToString()); + } + [TestMethod] public void ErrorReceivedCallbackShouldAppendGivenData() { From 55f879903917703a2fb34db7c9acfc15b74d2afd Mon Sep 17 00:00:00 2001 From: samadala Date: Wed, 1 Aug 2018 14:16:50 +0530 Subject: [PATCH 4/6] Create StringBuilderExtensions --- .../Constants.cs | 4 +- .../Utilities/StringBuilderExtensions.cs | 41 +++++++++++++++++++ .../Utilities/StringExtensions.cs | 2 - .../DataCollection/DataCollectionLauncher.cs | 2 +- .../Hosting/TestHostManagerCallbacks.cs | 2 +- 5 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringBuilderExtensions.cs diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs b/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs index 2a67634dba..1af6ef5371 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs +++ b/src/Microsoft.TestPlatform.CoreUtilities/Constants.cs @@ -25,8 +25,8 @@ public class Constants /// /// Number of character should be logged on child process exited with - /// error message on stadard error. + /// error message on standard error. /// - public const int StandardErrorMaxLength = 20480; // 20 KB + public const int StandardErrorMaxLength = 8192; // 8 KB } } diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringBuilderExtensions.cs b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringBuilderExtensions.cs new file mode 100644 index 0000000000..368b5a6757 --- /dev/null +++ b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringBuilderExtensions.cs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions +{ + using System; + using System.Text; + + public static class StringBuilderExtensions + { + /// + /// Add double quote around string. Useful in case of path which has white space in between. + /// + /// string builder + /// /// data to be appended. + /// + + public static void AppendSafeWithNewLine(this StringBuilder result, string data) + { + if (!string.IsNullOrEmpty(data)) + { + // Don't append more data if already reached max length. + if (result.Length >= result.MaxCapacity) + { + return; + } + + // Add newline for readbility. + data += Environment.NewLine; + + // Append sub string of data if appending all the data exceeds max capacity. + if (result.Length + data.Length >= result.MaxCapacity) + { + data = data.Substring(0, result.MaxCapacity - result.Length); + } + + result.Append(data); + } + } + } +} diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs index 933c0295e1..a80a65faa8 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs +++ b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs @@ -4,9 +4,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions { using System; - using System.Net; using System.Text; - using ObjectModel; public static class StringExtensions { diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs index ce4c7188d9..d2fd456915 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs @@ -76,7 +76,7 @@ public DataCollectionLauncher(IProcessHelper processHelper, IMessageLogger messa // This is helpful in abnormal failure of datacollector. EqtTrace.Warning("DataCollectionLauncher.ErrorReceivedCallback datacollector standard error line: {0}", data); - data.AppendToStringBuilderBasedOnMaxLength(this.processStdError); + this.processStdError.AppendSafeWithNewLine(data); }; /// diff --git a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs index dda10ee1b5..33c09bd210 100644 --- a/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs +++ b/src/Microsoft.TestPlatform.TestHostProvider/Hosting/TestHostManagerCallbacks.cs @@ -19,7 +19,7 @@ public static void ErrorReceivedCallback(StringBuilder testHostProcessStdError, // This is helpful in abnormal failure of testhost. EqtTrace.Warning("TestHostManagerCallbacks.ErrorReceivedCallback Test host standard error line: {0}", data); - data.AppendToStringBuilderBasedOnMaxLength(testHostProcessStdError); + testHostProcessStdError.AppendSafeWithNewLine(data); } public static void ExitCallBack( From 5a5097588471ae4d80cd2259e7a965fabed88c6f Mon Sep 17 00:00:00 2001 From: samadala Date: Wed, 1 Aug 2018 14:55:36 +0530 Subject: [PATCH 5/6] Fix nits --- .../Utilities/StringBuilderExtensions.cs | 7 +++--- .../Utilities/StringExtensions.cs | 23 ------------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringBuilderExtensions.cs b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringBuilderExtensions.cs index 368b5a6757..52e27472bf 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringBuilderExtensions.cs +++ b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringBuilderExtensions.cs @@ -9,12 +9,11 @@ namespace Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions public static class StringBuilderExtensions { /// - /// Add double quote around string. Useful in case of path which has white space in between. + /// Append given data from to string builder with new line. /// /// string builder - /// /// data to be appended. + /// data to be appended. /// - public static void AppendSafeWithNewLine(this StringBuilder result, string data) { if (!string.IsNullOrEmpty(data)) @@ -25,7 +24,7 @@ public static void AppendSafeWithNewLine(this StringBuilder result, string data) return; } - // Add newline for readbility. + // Add newline for readability. data += Environment.NewLine; // Append sub string of data if appending all the data exceeds max capacity. diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs index a80a65faa8..ffef131a7c 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs +++ b/src/Microsoft.TestPlatform.CoreUtilities/Utilities/StringExtensions.cs @@ -17,28 +17,5 @@ public static string AddDoubleQuote(this string value) { return "\"" + value + "\""; } - - public static void AppendToStringBuilderBasedOnMaxLength(this string data, StringBuilder result) - { - if (!string.IsNullOrEmpty(data)) - { - // Don't append more data if already reached max length. - if (result.Length >= result.MaxCapacity) - { - return; - } - - // Add newline for readbility. - data += Environment.NewLine; - - // Append sub string of data if appending all the data exceeds max capacity. - if (result.Length + data.Length >= result.MaxCapacity) - { - data = data.Substring(0, result.MaxCapacity - result.Length); - } - - result.Append(data); - } - } } } From 2fdda1fb2b5fe782be1e5fc54640eaca153072ff Mon Sep 17 00:00:00 2001 From: samadala Date: Wed, 1 Aug 2018 15:56:58 +0530 Subject: [PATCH 6/6] USe full namespace --- .../DataCollection/DataCollectionLauncher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs index d2fd456915..5496f46a51 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/DataCollection/DataCollectionLauncher.cs @@ -6,7 +6,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection using System; using System.Collections.Generic; using System.Text; - using CoreUtilities.Extensions; + using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions; using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection.Interfaces; using Microsoft.VisualStudio.TestPlatform.ObjectModel; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;