From 6e11010cfed1efcfd81e491180f7543da5b1ead5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Jare=C5=A1?= <me@jakubjares.com>
Date: Thu, 19 Nov 2020 18:49:58 +0100
Subject: [PATCH] [16.8] Fix collect dump always (#2641)

* Fix collect dump always

* Fix acceptance tests
---
 scripts/build.ps1                             |  2 +-
 scripts/build/TestPlatform.Dependencies.props |  2 +-
 scripts/build/TestPlatform.Settings.targets   |  2 +-
 scripts/verify-nupkgs.ps1                     |  4 +-
 .../BlameCollector.cs                         | 17 +++++--
 .../ICrashDumper.cs                           |  2 +-
 .../Interfaces/IProcDumpArgsBuilder.cs        |  5 +-
 .../Interfaces/IProcessDumpUtility.cs         |  5 +-
 .../NetClientCrashDumper.cs                   |  2 +-
 .../ProcDumpArgsBuilder.cs                    |  4 +-
 .../ProcDumpCrashDumper.cs                    |  5 +-
 .../ProcessDumpUtility.cs                     |  8 ++--
 .../BlameDataCollectorTests.cs                | 48 +++++++++++++++++--
 .../BlameCollectorTests.cs                    | 16 +++----
 .../ProcDumpArgsBuilderTests.cs               | 16 +++++--
 .../ProcessDumpUtilityTests.cs                |  2 +-
 16 files changed, 103 insertions(+), 37 deletions(-)

diff --git a/scripts/build.ps1 b/scripts/build.ps1
index ed67487c38..3a5170b5a7 100644
--- a/scripts/build.ps1
+++ b/scripts/build.ps1
@@ -737,7 +737,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))}"
 }
diff --git a/scripts/build/TestPlatform.Dependencies.props b/scripts/build/TestPlatform.Dependencies.props
index e324c3fe1d..8bc57e156a 100644
--- a/scripts/build/TestPlatform.Dependencies.props
+++ b/scripts/build/TestPlatform.Dependencies.props
@@ -11,7 +11,7 @@
     <!-- this version also needs to be "statically" readable because the test fixture will inspect this file for the version 
     and because during the test `dotnet test` will run and re-build some of the test projects and at that time the version 
     from a build parameter would not be available, so I am writing this version from the build.ps1 script to keep it in sync --> 
-    <NETTestSdkVersion>16.8.0-dev</NETTestSdkVersion>
+    <NETTestSdkVersion>16.8.1-dev</NETTestSdkVersion>
 
     <MSTestFrameworkVersion>2.1.0</MSTestFrameworkVersion>
     <MSTestAdapterVersion>2.1.0</MSTestAdapterVersion>
diff --git a/scripts/build/TestPlatform.Settings.targets b/scripts/build/TestPlatform.Settings.targets
index fc061bafd8..aa95afd229 100644
--- a/scripts/build/TestPlatform.Settings.targets
+++ b/scripts/build/TestPlatform.Settings.targets
@@ -5,7 +5,7 @@
     <!-- This version is read by vsts-prebuild.ps1 and is a base for the current version, this should be updated
     at the start of new iteration to the goal number. This is also used to version the local packages. This version needs to be statically
     readable when we read the file as xml, don't move it to a .props file, unless you change the build server process -->
-    <TPVersionPrefix>16.8.0</TPVersionPrefix>
+    <TPVersionPrefix>16.8.1</TPVersionPrefix>
   </PropertyGroup>
   <PropertyGroup>
     <!-- Versioning is defined from the build script. Use a default dev build if it's not defined.
diff --git a/scripts/verify-nupkgs.ps1 b/scripts/verify-nupkgs.ps1
index 9735d4e3f8..cc62a1d064 100644
--- a/scripts/verify-nupkgs.ps1
+++ b/scripts/verify-nupkgs.ps1
@@ -9,7 +9,7 @@ function Unzip
 }
 
 
-function Verify-Nuget-Packages($packageDirectory)
+function Verify-Nuget-Packages($packageDirectory, $version)
 {
     Write-Log "Starting Verify-Nuget-Packages."
     $expectedNumOfFiles = @{
@@ -24,7 +24,7 @@ function Verify-Nuget-Packages($packageDirectory)
                      "Microsoft.TestPlatform.TestHost" = 154;
                      "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]
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
index 57b4cd624f..0ac20b7df3 100644
--- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
+++ b/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;
             }
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs
index 8f05092fc8..dc6b0c402c 100644
--- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs
+++ b/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();
 
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcDumpArgsBuilder.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcDumpArgsBuilder.cs
index 67cf836dd2..29e03c0013 100644
--- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcDumpArgsBuilder.cs
+++ b/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
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs
index aca460dc0d..b646a3bbbf 100644
--- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs
+++ b/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
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs
index 6dbdc3242c..96dce6fc1e 100644
--- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs
+++ b/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
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpArgsBuilder.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpArgsBuilder.cs
index ade08f80e1..034fbbcf9a 100644
--- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpArgsBuilder.cs
+++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpArgsBuilder.cs
@@ -9,7 +9,7 @@ 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.
@@ -17,7 +17,7 @@ public string BuildTriggerBasedProcDumpArgs(int processId, string filename, IEnu
             // -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 ");
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpCrashDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpCrashDumper.cs
index 008e194a6a..fb7b07c38c 100644
--- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpCrashDumper.cs
+++ b/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(
diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs
index 8c7166e815..2a6b6445a3 100644
--- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs
+++ b/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,7 +109,7 @@ 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}'.");
@@ -117,7 +117,7 @@ private void CrashDump(int processId, string tempDirectory, DumpTypeOption dumpT
 
             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)
diff --git a/test/Microsoft.TestPlatform.AcceptanceTests/BlameDataCollectorTests.cs b/test/Microsoft.TestPlatform.AcceptanceTests/BlameDataCollectorTests.cs
index f3939f2e6b..fd101aa92b 100644
--- a/test/Microsoft.TestPlatform.AcceptanceTests/BlameDataCollectorTests.cs
+++ b/test/Microsoft.TestPlatform.AcceptanceTests/BlameDataCollectorTests.cs
@@ -6,6 +6,7 @@ namespace Microsoft.TestPlatform.AcceptanceTests
     using Microsoft.VisualStudio.TestTools.UnitTesting;
     using System;
     using System.IO;
+    using System.Text.RegularExpressions;
     using System.Xml;
 
     [TestClass]
@@ -41,7 +42,7 @@ public void BlameDataCollectorShouldGiveCorrectTestCaseName(RunnerInfo runnerInf
             arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
             this.InvokeVsTest(arguments);
 
-            this.VaildateOutput();
+            this.VaildateOutput("BlameUnitTestProject.UnitTest1.TestMethod2");
         }
 
         [TestMethod]
@@ -51,22 +52,59 @@ 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);
+
+            StringAssert.DoesNotMatch(this.StdOut, new Regex( @"\.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);
+            StringAssert.Matches(this.StdOut, new Regex(@"\.dmp"), "it should collect dump, even if nothing crashed");
         }
 
-        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);
 
diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs
index 7a036fdda3..4df97f01e3 100644
--- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs
+++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs
@@ -476,7 +476,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>
@@ -497,7 +497,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>
@@ -526,7 +526,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>
@@ -646,7 +646,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
@@ -672,7 +672,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
@@ -709,9 +709,9 @@ public void CleanUp()
 
             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)
diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcDumpArgsBuilderTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcDumpArgsBuilderTests.cs
index 5ea0ade9a7..07f331761d 100644
--- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcDumpArgsBuilderTests.cs
+++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcDumpArgsBuilderTests.cs
@@ -32,15 +32,25 @@ public void BuildHangBasedProcDumpArgsWithFullDumpEnabledShouldCreateCorrectArgS
         public void BuildTriggerBasedProcDumpArgsShouldCreateCorrectArgString()
         {
             var procDumpArgsBuilder = new ProcDumpArgsBuilder();
-            var argString = procDumpArgsBuilder.BuildTriggerBasedProcDumpArgs(this.defaultProcId, this.defaultDumpFileName, new List<string> { "a", "b" }, false);
-            Assert.AreEqual("-accepteula -e 1 -g -t -f a -f b 1234 dump.dmp", argString);
+            var argString = procDumpArgsBuilder.BuildTriggerBasedProcDumpArgs(this.defaultProcId, this.defaultDumpFileName, new List<string> { "a", "b" }, false, false);
+            Assert.AreEqual("-accepteula -e 1 -g -f a -f b 1234 dump.dmp", argString);
         }
 
         [TestMethod]
         public void BuildTriggerProcDumpArgsWithFullDumpEnabledShouldCreateCorrectArgString()
         {
             var procDumpArgsBuilder = new ProcDumpArgsBuilder();
-            var argString = procDumpArgsBuilder.BuildTriggerBasedProcDumpArgs(this.defaultProcId, this.defaultDumpFileName, new List<string> { "a", "b" }, true);
+            var argString = procDumpArgsBuilder.BuildTriggerBasedProcDumpArgs(this.defaultProcId, this.defaultDumpFileName, new List<string> { "a", "b" }, true, false);
+            Assert.AreEqual("-accepteula -e 1 -g -ma -f a -f b 1234 dump.dmp", argString);
+        }
+
+        [TestMethod]
+        public void BuildTriggerProcDumpArgsWithAlwaysCollectShouldCreateCorrectArgString()
+        {
+            var procDumpArgsBuilder = new ProcDumpArgsBuilder();
+            var argString = procDumpArgsBuilder.BuildTriggerBasedProcDumpArgs(this.defaultProcId, this.defaultDumpFileName, new List<string> { "a", "b" }, true, collectAlways: true);
+
+            // adds -t for collect on every process exit
             Assert.AreEqual("-accepteula -e 1 -g -t -ma -f a -f b 1234 dump.dmp", argString);
         }
     }
diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs
index 1c6bcee8d6..6b1d71c063 100644
--- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs
+++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs
@@ -61,7 +61,7 @@ public void GetDumpFileWillThrowExceptionIfNoDumpfile()
                 this.mockHangDumperFactory.Object,
                 this.mockCrashDumperFactory.Object);
 
-            processDumpUtility.StartTriggerBasedProcessDump(processId, testResultsDirectory, false, ".NETCoreApp,Version=v5.0");
+            processDumpUtility.StartTriggerBasedProcessDump(processId, testResultsDirectory, false, ".NETCoreApp,Version=v5.0", false);
 
             var ex = Assert.ThrowsException<FileNotFoundException>(() => processDumpUtility.GetDumpFiles());
             Assert.AreEqual(ex.Message, Resources.Resources.DumpFileNotGeneratedErrorMessage);