Users/kavipriya/procdumpchanges (#1682)

* Proc dump changes * PR comments * PR comments * Fixing help text * Help text * Making read on binary reader thread safe * Refactoring * Renaming parameter * Minor fixes * Fixing tests
microsoft · Jul 27, 2018 · 9f8fb1e · 9f8fb1e
1 parent 9bb6969
commit 9f8fb1e
Show file tree

Hide file tree

Showing 41 changed files with 1,190 additions and 100 deletions.
diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/SocketCommunicationManager.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/SocketCommunicationManager.cs
@@ -70,6 +70,11 @@ public class SocketCommunicationManager : ICommunicationManager
         /// </summary>
         private object sendSyncObject = new object();
 
+        /// <summary>
+        /// Sync object for receiving messages
+        /// </summary>
+        private object receiveSyncObject = new object();
+
         private Socket socket;
 
         /// <summary>
@@ -311,7 +316,11 @@ public async Task<Message> ReceiveMessageAsync(CancellationToken cancellationTok
         /// <returns> Raw message string </returns>
         public string ReceiveRawMessage()
         {
-            return this.binaryReader.ReadString();
+            lock (this.receiveSyncObject)
+            {
+                // Reading message on binaryreader is not thread-safe
+                return this.binaryReader.ReadString();
+            }
         }
 
         /// <summary>

diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
@@ -30,6 +30,8 @@ public class BlameCollector : DataCollector, ITestExecutionEnvironmentSpecifier
         private int testStartCount;
         private int testEndCount;
         private bool processDumpEnabled;
+        private bool collectDumpAlways;
+        private bool processFullDumpEnabled;
         private string attachmentGuid;
 
         /// <summary>
@@ -97,12 +99,50 @@ public override void Initialize(
 
             if (this.configurationElement != null)
             {
-                this.processDumpEnabled = this.configurationElement[Constants.DumpModeKey] != null;
+                var collectDumpNode = this.configurationElement[Constants.DumpModeKey];
+                this.processDumpEnabled = collectDumpNode != null;
+                if (this.processDumpEnabled)
+                {
+                    this.ValidateAndAddProcessDumpParameters(collectDumpNode);
+                }
             }
 
             this.attachmentGuid = Guid.NewGuid().ToString().Replace("-", string.Empty);
         }
 
+        private void ValidateAndAddProcessDumpParameters(XmlElement collectDumpNode)
+        {
+            foreach (XmlAttribute attribute in collectDumpNode.Attributes)
+            {
+                if (string.Equals(attribute.Name, Constants.CollectDumpAlwaysKey, StringComparison.OrdinalIgnoreCase))
+                {
+                    if (string.Equals(attribute.Value, Constants.TrueConfigurationValue, StringComparison.OrdinalIgnoreCase) || string.Equals(attribute.Value, Constants.FalseConfigurationValue, StringComparison.OrdinalIgnoreCase))
+                    {
+                        bool.TryParse(attribute.Value, out this.collectDumpAlways);
+                    }
+                    else
+                    {
+                        this.logger.LogWarning(this.context.SessionDataCollectionContext, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterValueIncorrect, attribute.Name, Constants.TrueConfigurationValue, Constants.FalseConfigurationValue));
+                    }
+                }
+                else if (string.Equals(attribute.Name, Constants.DumpTypeKey, StringComparison.OrdinalIgnoreCase))
+                {
+                    if (string.Equals(attribute.Value, Constants.FullConfigurationValue, StringComparison.OrdinalIgnoreCase) || string.Equals(attribute.Value, Constants.MiniConfigurationValue, StringComparison.OrdinalIgnoreCase))
+                    {
+                        this.processFullDumpEnabled = string.Equals(attribute.Value, Constants.FullConfigurationValue, StringComparison.OrdinalIgnoreCase);
+                    }
+                    else
+                    {
+                        this.logger.LogWarning(this.context.SessionDataCollectionContext, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterValueIncorrect, attribute.Name, Constants.FullConfigurationValue, Constants.MiniConfigurationValue));
+                    }
+                }
+                else
+                {
+                    this.logger.LogWarning(this.context.SessionDataCollectionContext, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterKeyIncorrect, attribute.Name));
+                }
+            }
+        }
+
         /// <summary>
         /// Called when Test Case Start event is invoked
         /// </summary>
@@ -159,23 +199,29 @@ private void SessionEnded_Handler(object sender, SessionEndEventArgs args)
 
             if (this.processDumpEnabled)
             {
-                try
+                // If there was a test case crash or if we need to collect dump on process exit.
+                if (this.testStartCount > this.testEndCount || this.collectDumpAlways)
                 {
-                    var dumpFile = this.processDumpUtility.GetDumpFile();
-                    if (!string.IsNullOrEmpty(dumpFile))
+                    try
                     {
-                        var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
-                        this.dataCollectionSink.SendFileAsync(fileTranferInformation);
+                        var dumpFile = this.processDumpUtility.GetDumpFile();
+                        if (!string.IsNullOrEmpty(dumpFile))
+                        {
+                            var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
+                            this.dataCollectionSink.SendFileAsync(fileTranferInformation);
+                        }
+                        else
+                        {
+                            EqtTrace.Warning("BlameCollector.SessionEnded_Handler: blame:CollectDump was enabled but dump file was not generated.");
+                            this.logger.LogWarning(args.Context, Resources.Resources.ProcDumpNotGenerated);
+                        }
                     }
-                    else
+                    catch (FileNotFoundException ex)
                     {
-                        EqtTrace.Warning("BlameCollector.SessionEnded_Handler: blame:CollectDump was enabled but dump file was not generated.");
+                        EqtTrace.Warning(ex.Message);
+                        this.logger.LogWarning(args.Context, ex.Message);
                     }
                 }
-                catch (FileNotFoundException ex)
-                {
-                    this.logger.LogWarning(args.Context, ex.Message);
-                }
             }
 
             this.DeregisterEvents();
@@ -195,7 +241,7 @@ private void TestHostLaunched_Handler(object sender, TestHostLaunchedEventArgs a
 
             try
             {
-                this.processDumpUtility.StartProcessDump(args.TestHostProcessId, this.attachmentGuid, this.GetResultsDirectory());
+                this.processDumpUtility.StartProcessDump(args.TestHostProcessId, this.attachmentGuid, this.GetResultsDirectory(), this.processFullDumpEnabled);
             }
             catch (TestPlatformException e)
             {
@@ -204,7 +250,7 @@ private void TestHostLaunched_Handler(object sender, TestHostLaunchedEventArgs a
                     EqtTrace.Warning("BlameCollector.TestHostLaunched_Handler: Could not start process dump. {0}", e);
                 }
 
-                this.logger.LogWarning(args.Context, e.Message);
+                this.logger.LogWarning(args.Context, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.ProcDumpCouldNotStart, e.Message));
             }
             catch (Exception e)
             {
@@ -213,7 +259,7 @@ private void TestHostLaunched_Handler(object sender, TestHostLaunchedEventArgs a
                     EqtTrace.Warning("BlameCollector.TestHostLaunched_Handler: Could not start process dump. {0}", e);
                 }
 
-                this.logger.LogWarning(args.Context, e.ToString());
+                this.logger.LogWarning(args.Context, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.ProcDumpCouldNotStart, e.ToString()));
             }
         }
 

diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Constants.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Constants.cs
@@ -42,5 +42,35 @@ internal static class Constants
         /// Configuration key name for dump mode
         /// </summary>
         public const string DumpModeKey = "CollectDump";
+
+        /// <summary>
+        /// Configuration key name for collect dump always
+        /// </summary>
+        public const string CollectDumpAlwaysKey = "CollectAlways";
+
+        /// <summary>
+        /// Configuration key name for dump type
+        /// </summary>
+        public const string DumpTypeKey = "DumpType";
+
+        /// <summary>
+        /// Configuration value for true
+        /// </summary>
+        public const string TrueConfigurationValue = "True";
+
+        /// <summary>
+        /// Configuration value for false
+        /// </summary>
+        public const string FalseConfigurationValue = "False";
+
+        /// <summary>
+        /// Configuration value for full
+        /// </summary>
+        public const string FullConfigurationValue = "Full";
+
+        /// <summary>
+        /// Configuration value for mini
+        /// </summary>
+        public const string MiniConfigurationValue = "Mini";
     }
 }
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs
@@ -27,6 +27,9 @@ public interface IProcessDumpUtility
         /// <param name="testResultsDirectory">
         /// Path to TestResults directory
         /// </param>
-        void StartProcessDump(int processId, string dumpFileGuid, string testResultsDirectory);
+        /// <param name="isFullDump">
+        /// Is full dump enabled
+        /// </param>
+        void StartProcessDump(int processId, string dumpFileGuid, string testResultsDirectory, bool isFullDump = false);
     }
 }
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs
@@ -71,14 +71,14 @@ public string GetDumpFile()
         }
 
         /// <inheritdoc/>
-        public void StartProcessDump(int processId, string dumpFileGuid, string testResultsDirectory)
+        public void StartProcessDump(int processId, string dumpFileGuid, string testResultsDirectory, bool isFullDump = false)
         {
             this.dumpFileName = $"{this.processHelper.GetProcessName(processId)}_{processId}_{dumpFileGuid}";
             this.testResultsDirectory = testResultsDirectory;
 
             this.procDumpProcess = this.processHelper.LaunchProcess(
                                             this.GetProcDumpExecutable(),
-                                            ProcessDumpUtility.BuildProcDumpArgs(processId, this.dumpFileName),
+                                            ProcessDumpUtility.BuildProcDumpArgs(processId, this.dumpFileName, isFullDump),
                                             testResultsDirectory,
                                             null,
                                             null,
@@ -94,13 +94,24 @@ public void StartProcessDump(int processId, string dumpFileGuid, string testResu
         /// <param name="filename">
         /// Filename for dump file
         /// </param>
+        /// <param name="isFullDump">
+        /// Is full dump enabled
+        /// </param>
         /// <returns>Arguments</returns>
-        private static string BuildProcDumpArgs(int processId, string filename)
+        private static string BuildProcDumpArgs(int processId, string filename, bool isFullDump = false)
         {
             // -accepteula: Auto accept end-user license agreement
             // -t: Write a dump when the process terminates.
-            // This will create a minidump of the process with specified filename
-            return "-accepteula -t " + processId + " " + filename + ".dmp";
+            if (isFullDump)
+            {
+                // This will create a fulldump of the process with specified filename
+                return "-accepteula -t -ma " + processId + " " + filename + ".dmp";
+            }
+            else
+            {
+                // This will create a minidump of the process with specified filename
+                return "-accepteula -t " + processId + " " + filename + ".dmp";
+            }
         }
 
         /// <summary>

diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/Resources.Designer.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/Resources.Designer.cs
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/Resources.resx b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/Resources.resx
@@ -120,10 +120,22 @@
   <data name="AbortedTestRun" xml:space="preserve">
     <value>The active Test Run was aborted because the host process exited unexpectedly while executing following test(s):</value>
   </data>
+  <data name="BlameParameterKeyIncorrect" xml:space="preserve">
+    <value>The blame parameter key specified {0} is not valid. Ignoring this key.</value>
+  </data>
+  <data name="BlameParameterValueIncorrect" xml:space="preserve">
+    <value>The blame parameter key  {0} can only support values {1}/{2}. Ignoring this key.</value>
+  </data>
   <data name="DumpFileNotGeneratedErrorMessage" xml:space="preserve">
     <value>Collect dump was enabled but no dump file was generated.</value>
   </data>
+  <data name="ProcDumpCouldNotStart" xml:space="preserve">
+    <value>Could not start process dump: {0}</value>
+  </data>
   <data name="ProcDumpEnvVarEmpty" xml:space="preserve">
     <value>Required environment variable PROCDUMP_PATH was null or empty. Set PROCDUMP_PATH to path of folder containing appropriate procdump executable.</value>
   </data>
+  <data name="ProcDumpNotGenerated" xml:space="preserve">
+    <value>CollectDump was enabled but dump file was not generated.</value>
+  </data>
 </root>
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.cs.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.cs.xlf
@@ -35,6 +35,26 @@
         <target state="translated">Bylo povoleno shromáždění výpisu, ale nebyl vygenerován žádný soubor výpisu.</target>
         <note />
       </trans-unit>
+      <trans-unit id="BlameParameterKeyIncorrect">
+        <source>The blame parameter key specified {0} is not valid. Ignoring this key.</source>
+        <target state="new">The blame parameter key {0} is not valid. Ignoring this parameter.</target>
+        <note></note>
+      </trans-unit>
+      <trans-unit id="BlameParameterValueIncorrect">
+        <source>The blame parameter key  {0} can only support values {1}/{2}. Ignoring this key.</source>
+        <target state="new">The blame parameter key  {0} can only support values {1}/{2}. Ignoring this parameter.</target>
+        <note></note>
+      </trans-unit>
+      <trans-unit id="ProcDumpCouldNotStart">
+        <source>Could not start process dump: {0}</source>
+        <target state="new">Could not start process dump: {0}</target>
+        <note></note>
+      </trans-unit>
+      <trans-unit id="ProcDumpNotGenerated">
+        <source>CollectDump was enabled but dump file was not generated.</source>
+        <target state="new">CollectDump was enabled but dump file was not generated.</target>
+        <note></note>
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.de.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.de.xlf
@@ -35,6 +35,26 @@
         <target state="translated">Die Abbilderfassung war aktiviert, aber es wurde keine Abbilddatei generiert.</target>
         <note />
       </trans-unit>
+      <trans-unit id="BlameParameterKeyIncorrect">
+        <source>The blame parameter key specified {0} is not valid. Ignoring this key.</source>
+        <target state="new">The blame parameter key {0} is not valid. Ignoring this parameter.</target>
+        <note></note>
+      </trans-unit>
+      <trans-unit id="BlameParameterValueIncorrect">
+        <source>The blame parameter key  {0} can only support values {1}/{2}. Ignoring this key.</source>
+        <target state="new">The blame parameter key  {0} can only support values {1}/{2}. Ignoring this parameter.</target>
+        <note></note>
+      </trans-unit>
+      <trans-unit id="ProcDumpCouldNotStart">
+        <source>Could not start process dump: {0}</source>
+        <target state="new">Could not start process dump: {0}</target>
+        <note></note>
+      </trans-unit>
+      <trans-unit id="ProcDumpNotGenerated">
+        <source>CollectDump was enabled but dump file was not generated.</source>
+        <target state="new">CollectDump was enabled but dump file was not generated.</target>
+        <note></note>
+      </trans-unit>
     </body>
   </file>
 </xliff>