From d37b3e5048937020ce4c07f318d2ac0cfae2d010 Mon Sep 17 00:00:00 2001
From: nohwnd <me@jakubjares.com>
Date: Wed, 18 Nov 2020 13:49:25 +0100
Subject: [PATCH 1/2] Fix collect dump always

---
 scripts/build/TestPlatform.Dependencies.props   |  2 +-
 scripts/build/TestPlatform.Settings.targets     |  2 +-
 .../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 ++++----
 .../BlameCollectorTests.cs                      | 16 ++++++++--------
 .../ProcDumpArgsBuilderTests.cs                 | 16 +++++++++++++---
 .../ProcessDumpUtilityTests.cs                  |  2 +-
 13 files changed, 57 insertions(+), 29 deletions(-)

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/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.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 @@ 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)
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);

From 53b5ab6b9610fde9035ef7972b8610b66fa3c496 Mon Sep 17 00:00:00 2001
From: nohwnd <me@jakubjares.com>
Date: Thu, 19 Nov 2020 17:53:08 +0100
Subject: [PATCH 2/2] Fix acceptance tests

---
 scripts/build.ps1                             |  2 +-
 scripts/verify-nupkgs.ps1                     |  4 +-
 .../BlameDataCollectorTests.cs                | 48 +++++++++++++++++--
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/scripts/build.ps1 b/scripts/build.ps1
index 5379bc0284..e8cf3e74ad 100644
--- a/scripts/build.ps1
+++ b/scripts/build.ps1
@@ -734,7 +734,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/verify-nupkgs.ps1 b/scripts/verify-nupkgs.ps1
index ca4fe4fbf0..9a535f5a70 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" = 145;
                      "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/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);