Fix blame parameter, warning, and add all testhosts to be ngend (#2579)

nohwnd · web-flow · commit 70a599d905c8 · 2020-09-18T16:37:20.000Z
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
@@ -348,6 +348,7 @@ private void ValidateAndAddHangBasedProcessDumpParameters(XmlElement collectDump
 
                         break;
 
+                    // allow HangDumpType attribute to be used on the hang dump this is the prefered way
                     case XmlAttribute attribute when string.Equals(attribute.Name, Constants.HangDumpTypeKey, StringComparison.OrdinalIgnoreCase):
 
                         if (string.Equals(attribute.Value, Constants.FullConfigurationValue, StringComparison.OrdinalIgnoreCase) || string.Equals(attribute.Value, Constants.MiniConfigurationValue, StringComparison.OrdinalIgnoreCase))
@@ -361,6 +362,20 @@ private void ValidateAndAddHangBasedProcessDumpParameters(XmlElement collectDump
 
                         break;
 
+                    // allow DumpType attribute to be used on the hang dump for backwards compatibility
+                    case XmlAttribute attribute when 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));
+                        }
+
+                        break;
+
                     default:
 
                         this.logger.LogWarning(this.context.SessionDataCollectionContext, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterKeyIncorrect, blameAttribute.Name));
@@ -454,7 +469,7 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args)
                 {
                     try
                     {
-                        var dumpFiles = this.processDumpUtility.GetDumpFiles();
+                        var dumpFiles = this.processDumpUtility.GetDumpFiles(warnOnNoDumpFiles: this.collectDumpAlways);
                         foreach (var dumpFile in dumpFiles)
                         {
                             if (!string.IsNullOrEmpty(dumpFile))
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs
@@ -11,10 +11,11 @@ public interface IProcessDumpUtility
         /// <summary>
         /// Get generated dump files
         /// </summary>
+        /// <param name="warnOnNoDumpFiles">Writes warning when no dump file is found.</param>
         /// <returns>
         /// Path of dump file
         /// </returns>
-        IEnumerable<string> GetDumpFiles();
+        IEnumerable<string> GetDumpFiles(bool warnOnNoDumpFiles = true);
 
         /// <summary>
         /// Launch proc dump process
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs
@@ -51,7 +51,7 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper,
         };
 
         /// <inheritdoc/>
-        public IEnumerable<string> GetDumpFiles()
+        public IEnumerable<string> GetDumpFiles(bool warnOnNoDumpFiles = true)
         {
             if (!this.wasHangDumped)
             {
@@ -82,7 +82,7 @@ public IEnumerable<string> GetDumpFiles()
                 }
             }
 
-            if (!foundDumps.Any())
+            if (warnOnNoDumpFiles && !foundDumps.Any())
             {
                 EqtTrace.Error($"ProcessDumpUtility.GetDumpFile: Could not find any dump file in {this.hangDumpDirectory}.");
                 throw new FileNotFoundException(Resources.Resources.DumpFileNotGeneratedErrorMessage);
diff --git a/src/package/VSIXProject/TestPlatform.csproj b/src/package/VSIXProject/TestPlatform.csproj
@@ -238,6 +238,22 @@
 
     <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.exe" /> 
     <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.x86.exe" />
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net452.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net452.exe" /> 
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net452.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net452.x86.exe" />
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net46.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net46.exe" /> 
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net46.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net46.x86.exe" />
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net461.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net461.exe" /> 
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net461.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net461.x86.exe" />
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net462.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net462.exe" /> 
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net462.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net462.x86.exe" />
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net47.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net47.exe" /> 
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net47.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net47.x86.exe" />
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net471.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net471.exe" /> 
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net471.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net471.x86.exe" />
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net472.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net472.exe" /> 
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net472.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net472.x86.exe" />
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net48.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net48.exe" /> 
+    <VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net48.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net48.x86.exe" />
   </ItemGroup>
   <ItemGroup>
     <Content Include="License.rtf">
diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs
@@ -172,7 +172,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout()
             this.mockFileHelper.Setup(x => x.Exists(It.Is<string>(y => y == "abc_hang.dmp"))).Returns(true);
             this.mockFileHelper.Setup(x => x.GetFullPath(It.Is<string>(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp");
             this.mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()));
-            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { dumpFile });
+            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Returns(new[] { dumpFile });
             this.mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny<FileTransferInformation>())).Callback(() => hangBasedDumpcollected.Set());
 
             this.blameDataCollector.Initialize(
@@ -184,7 +184,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout()
 
             hangBasedDumpcollected.Wait(1000);
             this.mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
-            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
+            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
             this.mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is<FileTransferInformation>(y => y.Path == dumpFile)), Times.Once);
         }
 
@@ -206,7 +206,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet
             this.mockFileHelper.Setup(x => x.Exists(It.Is<string>(y => y == "abc_hang.dmp"))).Returns(true);
             this.mockFileHelper.Setup(x => x.GetFullPath(It.Is<string>(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp");
             this.mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()));
-            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception"));
+            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception"));
 
             this.blameDataCollector.Initialize(
                 this.GetDumpConfigurationElement(false, false, true, 0),
@@ -217,7 +217,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet
 
             hangBasedDumpcollected.Wait(1000);
             this.mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
-            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
+            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
         }
 
         /// <summary>
@@ -239,7 +239,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt
             this.mockFileHelper.Setup(x => x.Exists(It.Is<string>(y => y == "abc_hang.dmp"))).Returns(true);
             this.mockFileHelper.Setup(x => x.GetFullPath(It.Is<string>(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp");
             this.mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()));
-            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { dumpFile });
+            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Returns(new[] { dumpFile });
             this.mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny<FileTransferInformation>())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some other exception"));
 
             this.blameDataCollector.Initialize(
@@ -251,7 +251,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt
 
             hangBasedDumpcollected.Wait(1000);
             this.mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
-            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
+            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
             this.mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is<FileTransferInformation>(y => y.Path == dumpFile)), Times.Once);
         }
 
@@ -366,7 +366,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled()
                 this.context);
 
             // Setup
-            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { this.filepath });
+            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny<bool>())).Returns(new[] { this.filepath });
             this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()))
                 .Returns(this.filepath);
 
@@ -376,7 +376,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled()
             this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));
 
             // Verify GetDumpFiles Call
-            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
+            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(It.IsAny<bool>()), Times.Once);
         }
 
         /// <summary>
@@ -418,7 +418,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab
                 this.context);
 
             // Setup
-            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { this.filepath });
+            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Returns(new[] { this.filepath });
             this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()))
                 .Returns(this.filepath);
 
@@ -427,7 +427,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab
             this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));
 
             // Verify GetDumpFiles Call
-            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
+            this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
         }
 
         /// <summary>
@@ -437,8 +437,10 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab
         public void TriggerSessionEndedHandlerShouldLogWarningIfGetDumpFileThrowsFileNotFound()
         {
             // Initializing Blame Data Collector
+            // force it to collect dump on exit, which won't happen and we should see a warning
+            // but we should not see warning if we tell it to create dump and there is no crash
             this.blameDataCollector.Initialize(
-                this.GetDumpConfigurationElement(),
+                this.GetDumpConfigurationElement(false, collectDumpOnExit: true),
                 this.mockDataColectionEvents.Object,
                 this.mockDataCollectionSink.Object,
                 this.mockLogger.Object,
@@ -447,7 +449,7 @@ public void TriggerSessionEndedHandlerShouldLogWarningIfGetDumpFileThrowsFileNot
             // Setup and raise events
             this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()))
                 .Returns(this.filepath);
-            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Throws(new FileNotFoundException());
+            this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Throws(new FileNotFoundException());
             this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
             this.mockDataColectionEvents.Raise(x => x.TestCaseStart += null, new TestCaseStartEventArgs(new TestCase()));
             this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));
@@ -605,6 +607,29 @@ public void TriggerTestHostLaunchedHandlerShouldLogWarningForNonBooleanCollectAl
             this.mockLogger.Verify(x => x.LogWarning(It.IsAny<DataCollectionContext>(), It.Is<string>(str => str == string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterValueIncorrect, "DumpType", BlameDataCollector.Constants.FullConfigurationValue, BlameDataCollector.Constants.MiniConfigurationValue))), Times.Once);
         }
 
+        /// <summary>
+        /// The trigger test host launched handler should start process dump utility for full dump if full dump was enabled
+        /// </summary>
+        [TestMethod]
+        public void TriggerTestHostLaunchedHandlerShouldLogNoWarningWhenDumpTypeIsUsedWithHangDumpBecauseEitherHangDumpTypeOrDumpTypeCanBeSpecified()
+        {
+            var dumpConfig = this.GetDumpConfigurationElement(isFullDump: true, false, colectDumpOnHang: true, 1800000);
+
+            // Initializing Blame Data Collector
+            this.blameDataCollector.Initialize(
+                dumpConfig,
+                this.mockDataColectionEvents.Object,
+                this.mockDataCollectionSink.Object,
+                this.mockLogger.Object,
+                this.context);
+
+            // Raise TestHostLaunched
+            this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
+
+            // Verify
+            this.mockLogger.Verify(x => x.LogWarning(It.IsAny<DataCollectionContext>(), It.IsAny<string>()), Times.Never);
+        }
+
         /// <summary>
         /// The trigger test host launched handler should not break if start process dump throws TestPlatFormExceptions and log error message
         /// </summary>
diff --git a/test/vstest.console.UnitTests/Processors/EnableBlameArgumentProcessorTests.cs b/test/vstest.console.UnitTests/Processors/EnableBlameArgumentProcessorTests.cs

Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper,`
`51`	`51`	`};`
`52`	`52`
`53`	`53`	`/// <inheritdoc/>`
`54`		`- public IEnumerable<string> GetDumpFiles()`
	`54`	`+ public IEnumerable<string> GetDumpFiles(bool warnOnNoDumpFiles = true)`
`55`	`55`	`{`
`56`	`56`	`if (!this.wasHangDumped)`
`57`	`57`	`{`
`@@ -82,7 +82,7 @@ public IEnumerable<string> GetDumpFiles()`
`82`	`82`	`}`
`83`	`83`	`}`
`84`	`84`
`85`		`- if (!foundDumps.Any())`
	`85`	`+ if (warnOnNoDumpFiles && !foundDumps.Any())`
`86`	`86`	`{`
`87`	`87`	`EqtTrace.Error($"ProcessDumpUtility.GetDumpFile: Could not find any dump file in {this.hangDumpDirectory}.");`
`88`	`88`	`throw new FileNotFoundException(Resources.Resources.DumpFileNotGeneratedErrorMessage);`