[master] Fix collect dump always

scripts/build.ps1

@@ -818,7 +818,7 @@ function Create-NugetPackages
 
     # Verifies that expected number of files gets shipped in nuget packages.
     # Few nuspec uses wildcard characters.
-    Verify-Nuget-Packages $packageOutputDir
+    Verify-Nuget-Packages $packageOutputDir $TPB_Version
 
     Write-Log "Create-NugetPackages: Complete. {$(Get-ElapsedTime($timer))}"
 }
@@ -1026,7 +1026,7 @@ function Generate-Manifest
     Write-Log "Generate-Manifest: Started."
 
     $sdkTaskPath = Join-Path $env:TP_ROOT_DIR "eng\common\sdk-task.ps1"
-    & $sdkTaskPath -restore -task GenerateBuildManifest /p:PackagesToPublishPattern=$TPB_PackageOutDir\*.nupkg /p:AssetManifestFilePath=$TPB_PackageOutDir\manifest\manifest.xml /p:ManifestBuildData="Location=https://dotnetfeed.blob.core.windows.net/dotnet-core/index.json" /p:BUILD_BUILDNUMBER=$BuildNumber
+    & $sdkTaskPath -restore -task GenerateBuildManifest /p:PackagesToPublishPattern=$TPB_PackageOutDir\*.nupkg /p:AssetManifestFilePath=$TPB_PackageOutDir\manifest\manifest.xml /p:ManifestBuildData="Location=https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json" /p:BUILD_BUILDNUMBER=$BuildNumber
 
     Write-Log "Generate-Manifest: Completed."
 }

scripts/verify-nupkgs.ps1

@@ -8,7 +8,8 @@ function Unzip
     [System.IO.Compression.ZipFile]::ExtractToDirectory($zipfile, $outpath)
 }
 
-function Verify-Nuget-Packages($packageDirectory)
+
+function Verify-Nuget-Packages($packageDirectory, $version)
 {
     Write-Log "Starting Verify-Nuget-Packages."
     $expectedNumOfFiles = @{
@@ -23,7 +24,7 @@ function Verify-Nuget-Packages($packageDirectory)
                      "Microsoft.TestPlatform.TestHost" = 212;
                      "Microsoft.TestPlatform.TranslationLayer" = 121}
 
-    $nugetPackages = Get-ChildItem -Filter "*.nupkg" $packageDirectory | % { $_.FullName}
+    $nugetPackages = Get-ChildItem -Filter "*$version*.nupkg" $packageDirectory | % { $_.FullName }
 
     Write-VerboseLog "Unzip NuGet packages."
     $unzipNugetPackageDirs =  New-Object System.Collections.Generic.List[System.Object]

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs

@@ -462,7 +462,10 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args)
                 }
                 else
                 {
-                    this.logger.LogWarning(this.context.SessionDataCollectionContext, Resources.Resources.NotGeneratingSequenceFile);
+                    if (this.collectProcessDumpOnTestHostHang)
+                    {
+                        this.logger.LogWarning(this.context.SessionDataCollectionContext, Resources.Resources.NotGeneratingSequenceFile);
+                    }
                 }
 
                 if (this.uploadDumpFiles)
@@ -528,7 +531,7 @@ private void TestHostLaunchedHandler(object sender, TestHostLaunchedEventArgs ar
             try
             {
                 var dumpDirectory = this.GetDumpDirectory();
-                this.processDumpUtility.StartTriggerBasedProcessDump(args.TestHostProcessId, dumpDirectory, this.processFullDumpEnabled, this.targetFramework);
+                this.processDumpUtility.StartTriggerBasedProcessDump(args.TestHostProcessId, dumpDirectory, this.processFullDumpEnabled, this.targetFramework, this.collectDumpAlways);
             }
             catch (TestPlatformException e)
             {
@@ -585,7 +588,15 @@ private string GetTempDirectory()
         {
             if (string.IsNullOrWhiteSpace(this.tempDirectory))
             {
-                this.tempDirectory = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+                // DUMP_TEMP_PATH will be used as temporary storage location
+                // for the dumps, this won't affect the dump uploads. Just the place where
+                // we store them before moving them to the final folder.
+
+                // AGENT_TEMPDIRECTORY is AzureDevops variable, which is set to path
+                // that is cleaned up after every job. This is preferable to use over
+                // just the normal temp.
+                var temp = Environment.GetEnvironmentVariable("VSTEST_DUMP_TEMP_PATH") ?? Environment.GetEnvironmentVariable("AGENT_TEMPDIRECTORY") ?? Path.GetTempPath();
+                this.tempDirectory = Path.Combine(temp, Guid.NewGuid().ToString());
                 Directory.CreateDirectory(this.tempDirectory);
                 return this.tempDirectory;
             }

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs

@@ -5,7 +5,7 @@ namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
 {
     public interface ICrashDumper
     {
-        void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType);
+        void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType, bool collectAlways);
 
         void WaitForDumpToFinish();
 

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcDumpArgsBuilder.cs

@@ -22,8 +22,11 @@ public interface IProcDumpArgsBuilder
         /// <param name="isFullDump">
         /// Is full dump enabled
         /// </param>
+        /// <param name="collectAlways">
+        /// Collects the dump on process exit even when there is no exception
+        /// </param>
         /// <returns>Arguments</returns>
-        string BuildTriggerBasedProcDumpArgs(int processId, string filename, IEnumerable<string> procDumpExceptionsList, bool isFullDump);
+        string BuildTriggerBasedProcDumpArgs(int processId, string filename, IEnumerable<string> procDumpExceptionsList, bool isFullDump, bool collectAlways);
 
         /// <summary>
         /// Arguments for procdump.exe for getting a dump in case of a testhost hang

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs

@@ -32,7 +32,10 @@ public interface IProcessDumpUtility
         /// <param name="targetFramework">
         /// The target framework of the process
         /// </param>
-        void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework);
+        /// <param name="collectAlways">
+        /// Collect the dump on process exit even if there is no exception
+        /// </param>
+        void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework, bool collectAlways);
 
         /// <summary>
         /// Launch proc dump process to capture dump in case of a testhost hang and wait for it to exit

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs

@@ -5,7 +5,7 @@ namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
 {
     internal class NetClientCrashDumper : ICrashDumper
     {
-        public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType)
+        public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType, bool collectAlways)
         {
             // we don't need to do anything directly here, we setup the env variables
             // in the dumper configuration, including the path

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpArgsBuilder.cs

@@ -9,15 +9,15 @@ namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
     public class ProcDumpArgsBuilder : IProcDumpArgsBuilder
     {
         /// <inheritdoc />
-        public string BuildTriggerBasedProcDumpArgs(int processId, string filename, IEnumerable<string> procDumpExceptionsList, bool isFullDump)
+        public string BuildTriggerBasedProcDumpArgs(int processId, string filename, IEnumerable<string> procDumpExceptionsList, bool isFullDump, bool collectAlways)
         {
             // -accepteula: Auto accept end-user license agreement
             // -e: Write a dump when the process encounters an unhandled exception. Include the 1 to create dump on first chance exceptions.
             // -g: Run as a native debugger in a managed process (no interop).
             // -t: Write a dump when the process terminates.
             // -ma: Full dump argument.
             // -f: Filter the exceptions.
-            StringBuilder procDumpArgument = new StringBuilder("-accepteula -e 1 -g -t ");
+            StringBuilder procDumpArgument = new StringBuilder($"-accepteula -e 1 -g {(collectAlways ? "-t " : string.Empty)}");
             if (isFullDump)
             {
                 procDumpArgument.Append("-ma ");

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpCrashDumper.cs

@@ -69,7 +69,7 @@ public void WaitForDumpToFinish()
         }
 
         /// <inheritdoc/>
-        public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType)
+        public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType, bool collectAlways)
         {
             var process = Process.GetProcessById(processId);
             var outputFile = Path.Combine(outputDirectory, $"{process.ProcessName}_{process.Id}_{DateTime.Now:yyyyMMddTHHmmss}_crashdump.dmp");
@@ -96,7 +96,8 @@ public void AttachToTargetProcess(int processId, string outputDirectory, DumpTyp
                 processId,
                 this.dumpFileName,
                 ProcDumpExceptionsList,
-                isFullDump: dumpType == DumpTypeOption.Full);
+                isFullDump: dumpType == DumpTypeOption.Full,
+                collectAlways: collectAlways);
 
             EqtTrace.Info($"ProcDumpCrashDumper.AttachToTargetProcess: Running ProcDump with arguments: '{procDumpArgs}'.");
             this.procDumpProcess = this.processHelper.LaunchProcess(

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs

@@ -98,9 +98,9 @@ public void StartHangBasedProcessDump(int processId, string tempDirectory, bool
         }
 
         /// <inheritdoc/>
-        public void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework)
+        public void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework, bool collectAlways)
         {
-            this.CrashDump(processId, testResultsDirectory, isFullDump ? DumpTypeOption.Full : DumpTypeOption.Mini, targetFramework);
+            this.CrashDump(processId, testResultsDirectory, isFullDump ? DumpTypeOption.Full : DumpTypeOption.Mini, targetFramework, collectAlways);
         }
 
         /// <inheritdoc/>
@@ -109,15 +109,15 @@ public void DetachFromTargetProcess(int targetProcessId)
             this.crashDumper?.DetachFromTargetProcess(targetProcessId);
         }
 
-        private void CrashDump(int processId, string tempDirectory, DumpTypeOption dumpType, string targetFramework)
+        private void CrashDump(int processId, string tempDirectory, DumpTypeOption dumpType, string targetFramework, bool collectAlways)
         {
             var processName = this.processHelper.GetProcessName(processId);
             EqtTrace.Info($"ProcessDumpUtility.CrashDump: Creating {dumpType.ToString().ToLowerInvariant()} dump of process {processName} ({processId}) into temporary path '{tempDirectory}'.");
             this.crashDumpDirectory = tempDirectory;
 
             this.crashDumper = this.crashDumperFactory.Create(targetFramework);
             ConsoleOutput.Instance.Information(false, $"Blame: Attaching crash dump utility to process {processName} ({processId}).");
-            this.crashDumper.AttachToTargetProcess(processId, tempDirectory, dumpType);
+            this.crashDumper.AttachToTargetProcess(processId, tempDirectory, dumpType, collectAlways);
         }
 
         private void HangDump(int processId, string tempDirectory, DumpTypeOption dumpType, string targetFramework, Action<string> logWarning = null)

test/Microsoft.TestPlatform.AcceptanceTests/BlameDataCollectorTests.cs

@@ -47,7 +47,7 @@ public void BlameDataCollectorShouldGiveCorrectTestCaseName(RunnerInfo runnerInf
             arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
             this.InvokeVsTest(arguments);
 
-            this.VaildateOutput();
+            this.VaildateOutput("BlameUnitTestProject.UnitTest1.TestMethod2");
         }
 
         [TestMethod]
@@ -57,14 +57,51 @@ public void BlameDataCollectorShouldOutputDumpFile(RunnerInfo runnerInfo)
         {
             Environment.SetEnvironmentVariable("PROCDUMP_PATH", Path.Combine(this.testEnvironment.PackageDirectory, @"procdump\0.0.1\bin"));
 
+            AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo);            
+            var assemblyPaths = this.BuildMultipleAssemblyPath("SimpleTestProject3.dll").Trim('\"');
+            var arguments = PrepareArguments(assemblyPaths, this.GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue);
+            arguments = string.Concat(arguments, $" /Blame:CollectDump");
+            arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
+            arguments = string.Concat(arguments, " /testcasefilter:ExitWithStackoverFlow");
+            this.InvokeVsTest(arguments);
+
+            this.VaildateOutput("SampleUnitTestProject3.UnitTest1.ExitWithStackoverFlow", validateDumpFile: true);
+        }
+
+        [TestMethod]
+        [NetFullTargetFrameworkDataSource]
+        [NetCoreTargetFrameworkDataSource]
+        public void BlameDataCollectorShouldNotOutputDumpFileWhenNoCrashOccurs(RunnerInfo runnerInfo)
+        {
+            Environment.SetEnvironmentVariable("PROCDUMP_PATH", Path.Combine(this.testEnvironment.PackageDirectory, @"procdump\0.0.1\bin"));
+
             AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo);
-            var assemblyPaths = this.GetAssetFullPath("BlameUnitTestProject.dll");
+            var assemblyPaths = this.BuildMultipleAssemblyPath("SimpleTestProject.dll").Trim('\"');
             var arguments = PrepareArguments(assemblyPaths, this.GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue);
             arguments = string.Concat(arguments, $" /Blame:CollectDump");
             arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
+            arguments = string.Concat(arguments, " /testcasefilter:PassingTest");
+            this.InvokeVsTest(arguments);
+
+            Assert.IsFalse(this.StdOut.Contains(".dmp"), "it should not collect a dump, because nothing crashed");
+        }
+
+        [TestMethod]
+        [NetFullTargetFrameworkDataSource]
+        [NetCoreTargetFrameworkDataSource]
+        public void BlameDataCollectorShouldOutputDumpFileWhenNoCrashOccursButCollectAlwaysIsEnabled(RunnerInfo runnerInfo)
+        {
+            Environment.SetEnvironmentVariable("PROCDUMP_PATH", Path.Combine(this.testEnvironment.PackageDirectory, @"procdump\0.0.1\bin"));
+
+            AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo);
+            var assemblyPaths = this.BuildMultipleAssemblyPath("SimpleTestProject.dll").Trim('\"');
+            var arguments = PrepareArguments(assemblyPaths, this.GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue);
+            arguments = string.Concat(arguments, $" /Blame:CollectDump;CollectAlways=True");
+            arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
+            arguments = string.Concat(arguments, " /testcasefilter:PassingTest");
             this.InvokeVsTest(arguments);
 
-            this.VaildateOutput(true);
+            Assert.IsTrue(this.StdOut.Contains(".dmp"), "it should collect dump, even if nothing crashed");
         }
 
         [TestMethod]
@@ -243,12 +280,12 @@ private void ValidateDump(int expectedDumpCount = 1)
             }
         }
 
-        private void VaildateOutput(bool validateDumpFile = false)
+        private void VaildateOutput(string testName, bool validateDumpFile = false)
         {
             bool isSequenceAttachmentReceived = false;
             bool isDumpAttachmentReceived = false;
             bool isValid = false;
-            this.StdErrorContains("BlameUnitTestProject.UnitTest1.TestMethod2");
+            this.StdErrorContains(testName);
             this.StdOutputContains("Sequence_");
             var resultFiles = Directory.GetFiles(this.resultsDir, "*", SearchOption.AllDirectories);
 

test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs

@@ -477,7 +477,7 @@ public void TriggerTestHostLaunchedHandlerShouldStartProcDumpUtilityIfProcDumpEn
             this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
 
             // Verify StartProcessDumpCall
-            this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>()));
+            this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>(), false));
         }
 
         /// <summary>
@@ -498,7 +498,7 @@ public void TriggerTestHostLaunchedHandlerShouldStartProcDumpUtilityForFullDumpI
             this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
 
             // Verify StartProcessDumpCall
-            this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), true, It.IsAny<string>()));
+            this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), true, It.IsAny<string>(), false));
         }
 
         /// <summary>
@@ -527,7 +527,7 @@ public void TriggerTestHostLaunchedHandlerShouldStartProcDumpUtilityForFullDumpI
             this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
 
             // Verify StartProcessDumpCall
-            this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), true, It.IsAny<string>()));
+            this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), true, It.IsAny<string>(), false));
         }
 
         /// <summary>
@@ -647,7 +647,7 @@ public void TriggerTestHostLaunchedHandlerShouldCatchTestPlatFormExceptionsAndRe
 
             // Make StartProcessDump throw exception
             var tpex = new TestPlatformException("env var exception");
-            this.mockProcessDumpUtility.Setup(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>()))
+            this.mockProcessDumpUtility.Setup(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>(), false))
                                        .Throws(tpex);
 
             // Raise TestHostLaunched
@@ -673,7 +673,7 @@ public void TriggerTestHostLaunchedHandlerShouldCatchAllUnexpectedExceptionsAndR
 
             // Make StartProcessDump throw exception
             var ex = new Exception("start process failed");
-            this.mockProcessDumpUtility.Setup(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>()))
+            this.mockProcessDumpUtility.Setup(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>(), false))
                                        .Throws(ex);
 
             // Raise TestHostLaunched
@@ -710,9 +710,9 @@ private XmlElement GetDumpConfigurationElement(
 
             if (collectDumpOnExit)
             {
-                var fulldumpAttribute = xmldoc.CreateAttribute(BlameDataCollector.Constants.CollectDumpAlwaysKey);
-                fulldumpAttribute.Value = "true";
-                node.Attributes.Append(fulldumpAttribute);
+                var collectDumpOnExitAttribute = xmldoc.CreateAttribute(BlameDataCollector.Constants.CollectDumpAlwaysKey);
+                collectDumpOnExitAttribute.Value = "true";
+                node.Attributes.Append(collectDumpOnExitAttribute);
             }
 
             if (colectDumpOnHang)