Skip to content

Commit 6e11010

Browse files
nohwndnohwnd
authored
[16.8] Fix collect dump always (#2641)
* Fix collect dump always * Fix acceptance tests
1 parent b074e2c commit 6e11010

File tree

16 files changed

+103
-37
lines changed

16 files changed

+103
-37
lines changed

‎scripts/build.ps1

+1-1
Original file line numberDiff line numberDiff line change
@@ -737,7 +737,7 @@ function Create-NugetPackages
737737

738738
# Verifies that expected number of files gets shipped in nuget packages.
739739
# Few nuspec uses wildcard characters.
740-
Verify-Nuget-Packages $packageOutputDir
740+
Verify-Nuget-Packages $packageOutputDir $TPB_Version
741741

742742
Write-Log "Create-NugetPackages: Complete. {$(Get-ElapsedTime($timer))}"
743743
}

‎scripts/build/TestPlatform.Dependencies.props

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
<!-- this version also needs to be "statically" readable because the test fixture will inspect this file for the version
1212
and because during the test `dotnet test` will run and re-build some of the test projects and at that time the version
1313
from a build parameter would not be available, so I am writing this version from the build.ps1 script to keep it in sync -->
14-
<NETTestSdkVersion>16.8.0-dev</NETTestSdkVersion>
14+
<NETTestSdkVersion>16.8.1-dev</NETTestSdkVersion>
1515

1616
<MSTestFrameworkVersion>2.1.0</MSTestFrameworkVersion>
1717
<MSTestAdapterVersion>2.1.0</MSTestAdapterVersion>

‎scripts/build/TestPlatform.Settings.targets

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<!-- This version is read by vsts-prebuild.ps1 and is a base for the current version, this should be updated
66
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
77
readable when we read the file as xml, don't move it to a .props file, unless you change the build server process -->
8-
<TPVersionPrefix>16.8.0</TPVersionPrefix>
8+
<TPVersionPrefix>16.8.1</TPVersionPrefix>
99
</PropertyGroup>
1010
<PropertyGroup>
1111
<!-- Versioning is defined from the build script. Use a default dev build if it's not defined.

‎scripts/verify-nupkgs.ps1

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ function Unzip
99
}
1010

1111

12-
function Verify-Nuget-Packages($packageDirectory)
12+
function Verify-Nuget-Packages($packageDirectory, $version)
1313
{
1414
Write-Log "Starting Verify-Nuget-Packages."
1515
$expectedNumOfFiles = @{
@@ -24,7 +24,7 @@ function Verify-Nuget-Packages($packageDirectory)
2424
"Microsoft.TestPlatform.TestHost" = 154;
2525
"Microsoft.TestPlatform.TranslationLayer" = 121}
2626

27-
$nugetPackages = Get-ChildItem -Filter "*.nupkg" $packageDirectory | % { $_.FullName}
27+
$nugetPackages = Get-ChildItem -Filter "*$version*.nupkg" $packageDirectory | % { $_.FullName }
2828

2929
Write-VerboseLog "Unzip NuGet packages."
3030
$unzipNugetPackageDirs = New-Object System.Collections.Generic.List[System.Object]

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

+14-3
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,10 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args)
462462
}
463463
else
464464
{
465-
this.logger.LogWarning(this.context.SessionDataCollectionContext, Resources.Resources.NotGeneratingSequenceFile);
465+
if (this.collectProcessDumpOnTestHostHang)
466+
{
467+
this.logger.LogWarning(this.context.SessionDataCollectionContext, Resources.Resources.NotGeneratingSequenceFile);
468+
}
466469
}
467470

468471
if (this.uploadDumpFiles)
@@ -528,7 +531,7 @@ private void TestHostLaunchedHandler(object sender, TestHostLaunchedEventArgs ar
528531
try
529532
{
530533
var dumpDirectory = this.GetDumpDirectory();
531-
this.processDumpUtility.StartTriggerBasedProcessDump(args.TestHostProcessId, dumpDirectory, this.processFullDumpEnabled, this.targetFramework);
534+
this.processDumpUtility.StartTriggerBasedProcessDump(args.TestHostProcessId, dumpDirectory, this.processFullDumpEnabled, this.targetFramework, this.collectDumpAlways);
532535
}
533536
catch (TestPlatformException e)
534537
{
@@ -585,7 +588,15 @@ private string GetTempDirectory()
585588
{
586589
if (string.IsNullOrWhiteSpace(this.tempDirectory))
587590
{
588-
this.tempDirectory = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
591+
// DUMP_TEMP_PATH will be used as temporary storage location
592+
// for the dumps, this won't affect the dump uploads. Just the place where
593+
// we store them before moving them to the final folder.
594+
595+
// AGENT_TEMPDIRECTORY is AzureDevops variable, which is set to path
596+
// that is cleaned up after every job. This is preferable to use over
597+
// just the normal temp.
598+
var temp = Environment.GetEnvironmentVariable("VSTEST_DUMP_TEMP_PATH") ?? Environment.GetEnvironmentVariable("AGENT_TEMPDIRECTORY") ?? Path.GetTempPath();
599+
this.tempDirectory = Path.Combine(temp, Guid.NewGuid().ToString());
589600
Directory.CreateDirectory(this.tempDirectory);
590601
return this.tempDirectory;
591602
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
55
{
66
public interface ICrashDumper
77
{
8-
void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType);
8+
void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType, bool collectAlways);
99

1010
void WaitForDumpToFinish();
1111

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,11 @@ public interface IProcDumpArgsBuilder
2222
/// <param name="isFullDump">
2323
/// Is full dump enabled
2424
/// </param>
25+
/// <param name="collectAlways">
26+
/// Collects the dump on process exit even when there is no exception
27+
/// </param>
2528
/// <returns>Arguments</returns>
26-
string BuildTriggerBasedProcDumpArgs(int processId, string filename, IEnumerable<string> procDumpExceptionsList, bool isFullDump);
29+
string BuildTriggerBasedProcDumpArgs(int processId, string filename, IEnumerable<string> procDumpExceptionsList, bool isFullDump, bool collectAlways);
2730

2831
/// <summary>
2932
/// Arguments for procdump.exe for getting a dump in case of a testhost hang

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ public interface IProcessDumpUtility
3232
/// <param name="targetFramework">
3333
/// The target framework of the process
3434
/// </param>
35-
void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework);
35+
/// <param name="collectAlways">
36+
/// Collect the dump on process exit even if there is no exception
37+
/// </param>
38+
void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework, bool collectAlways);
3639

3740
/// <summary>
3841
/// 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

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
55
{
66
internal class NetClientCrashDumper : ICrashDumper
77
{
8-
public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType)
8+
public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType, bool collectAlways)
99
{
1010
// we don't need to do anything directly here, we setup the env variables
1111
// in the dumper configuration, including the path

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

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

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public void WaitForDumpToFinish()
6969
}
7070

7171
/// <inheritdoc/>
72-
public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType)
72+
public void AttachToTargetProcess(int processId, string outputDirectory, DumpTypeOption dumpType, bool collectAlways)
7373
{
7474
var process = Process.GetProcessById(processId);
7575
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
9696
processId,
9797
this.dumpFileName,
9898
ProcDumpExceptionsList,
99-
isFullDump: dumpType == DumpTypeOption.Full);
99+
isFullDump: dumpType == DumpTypeOption.Full,
100+
collectAlways: collectAlways);
100101

101102
EqtTrace.Info($"ProcDumpCrashDumper.AttachToTargetProcess: Running ProcDump with arguments: '{procDumpArgs}'.");
102103
this.procDumpProcess = this.processHelper.LaunchProcess(

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ public void StartHangBasedProcessDump(int processId, string tempDirectory, bool
9898
}
9999

100100
/// <inheritdoc/>
101-
public void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework)
101+
public void StartTriggerBasedProcessDump(int processId, string testResultsDirectory, bool isFullDump, string targetFramework, bool collectAlways)
102102
{
103-
this.CrashDump(processId, testResultsDirectory, isFullDump ? DumpTypeOption.Full : DumpTypeOption.Mini, targetFramework);
103+
this.CrashDump(processId, testResultsDirectory, isFullDump ? DumpTypeOption.Full : DumpTypeOption.Mini, targetFramework, collectAlways);
104104
}
105105

106106
/// <inheritdoc/>
@@ -109,15 +109,15 @@ public void DetachFromTargetProcess(int targetProcessId)
109109
this.crashDumper?.DetachFromTargetProcess(targetProcessId);
110110
}
111111

112-
private void CrashDump(int processId, string tempDirectory, DumpTypeOption dumpType, string targetFramework)
112+
private void CrashDump(int processId, string tempDirectory, DumpTypeOption dumpType, string targetFramework, bool collectAlways)
113113
{
114114
var processName = this.processHelper.GetProcessName(processId);
115115
EqtTrace.Info($"ProcessDumpUtility.CrashDump: Creating {dumpType.ToString().ToLowerInvariant()} dump of process {processName} ({processId}) into temporary path '{tempDirectory}'.");
116116
this.crashDumpDirectory = tempDirectory;
117117

118118
this.crashDumper = this.crashDumperFactory.Create(targetFramework);
119119
ConsoleOutput.Instance.Information(false, $"Blame: Attaching crash dump utility to process {processName} ({processId}).");
120-
this.crashDumper.AttachToTargetProcess(processId, tempDirectory, dumpType);
120+
this.crashDumper.AttachToTargetProcess(processId, tempDirectory, dumpType, collectAlways);
121121
}
122122

123123
private void HangDump(int processId, string tempDirectory, DumpTypeOption dumpType, string targetFramework, Action<string> logWarning = null)

‎test/Microsoft.TestPlatform.AcceptanceTests/BlameDataCollectorTests.cs

+43-5
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ namespace Microsoft.TestPlatform.AcceptanceTests
66
using Microsoft.VisualStudio.TestTools.UnitTesting;
77
using System;
88
using System.IO;
9+
using System.Text.RegularExpressions;
910
using System.Xml;
1011

1112
[TestClass]
@@ -41,7 +42,7 @@ public void BlameDataCollectorShouldGiveCorrectTestCaseName(RunnerInfo runnerInf
4142
arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
4243
this.InvokeVsTest(arguments);
4344

44-
this.VaildateOutput();
45+
this.VaildateOutput("BlameUnitTestProject.UnitTest1.TestMethod2");
4546
}
4647

4748
[TestMethod]
@@ -51,22 +52,59 @@ public void BlameDataCollectorShouldOutputDumpFile(RunnerInfo runnerInfo)
5152
{
5253
Environment.SetEnvironmentVariable("PROCDUMP_PATH", Path.Combine(this.testEnvironment.PackageDirectory, @"procdump\0.0.1\bin"));
5354

55+
AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo);
56+
var assemblyPaths = this.BuildMultipleAssemblyPath("SimpleTestProject3.dll").Trim('\"');
57+
var arguments = PrepareArguments(assemblyPaths, this.GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue);
58+
arguments = string.Concat(arguments, $" /Blame:CollectDump");
59+
arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
60+
arguments = string.Concat(arguments, " /testcasefilter:ExitWithStackoverFlow");
61+
this.InvokeVsTest(arguments);
62+
63+
this.VaildateOutput("SampleUnitTestProject3.UnitTest1.ExitWithStackoverFlow", validateDumpFile: true);
64+
}
65+
66+
[TestMethod]
67+
[NetFullTargetFrameworkDataSource]
68+
[NetCoreTargetFrameworkDataSource]
69+
public void BlameDataCollectorShouldNotOutputDumpFileWhenNoCrashOccurs(RunnerInfo runnerInfo)
70+
{
71+
Environment.SetEnvironmentVariable("PROCDUMP_PATH", Path.Combine(this.testEnvironment.PackageDirectory, @"procdump\0.0.1\bin"));
72+
5473
AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo);
55-
var assemblyPaths = this.GetAssetFullPath("BlameUnitTestProject.dll");
74+
var assemblyPaths = this.BuildMultipleAssemblyPath("SimpleTestProject.dll").Trim('\"');
5675
var arguments = PrepareArguments(assemblyPaths, this.GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue);
5776
arguments = string.Concat(arguments, $" /Blame:CollectDump");
5877
arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
78+
arguments = string.Concat(arguments, " /testcasefilter:PassingTest");
79+
this.InvokeVsTest(arguments);
80+
81+
StringAssert.DoesNotMatch(this.StdOut, new Regex( @"\.dmp"), "it should not collect a dump, because nothing crashed");
82+
}
83+
84+
[TestMethod]
85+
[NetFullTargetFrameworkDataSource]
86+
[NetCoreTargetFrameworkDataSource]
87+
public void BlameDataCollectorShouldOutputDumpFileWhenNoCrashOccursButCollectAlwaysIsEnabled(RunnerInfo runnerInfo)
88+
{
89+
Environment.SetEnvironmentVariable("PROCDUMP_PATH", Path.Combine(this.testEnvironment.PackageDirectory, @"procdump\0.0.1\bin"));
90+
91+
AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerInfo);
92+
var assemblyPaths = this.BuildMultipleAssemblyPath("SimpleTestProject.dll").Trim('\"');
93+
var arguments = PrepareArguments(assemblyPaths, this.GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue);
94+
arguments = string.Concat(arguments, $" /Blame:CollectDump;CollectAlways=True");
95+
arguments = string.Concat(arguments, $" /ResultsDirectory:{resultsDir}");
96+
arguments = string.Concat(arguments, " /testcasefilter:PassingTest");
5997
this.InvokeVsTest(arguments);
6098

61-
this.VaildateOutput(true);
99+
StringAssert.Matches(this.StdOut, new Regex(@"\.dmp"), "it should collect dump, even if nothing crashed");
62100
}
63101

64-
private void VaildateOutput(bool validateDumpFile = false)
102+
private void VaildateOutput(string testName, bool validateDumpFile = false)
65103
{
66104
bool isSequenceAttachmentReceived = false;
67105
bool isDumpAttachmentReceived = false;
68106
bool isValid = false;
69-
this.StdErrorContains("BlameUnitTestProject.UnitTest1.TestMethod2");
107+
this.StdErrorContains(testName);
70108
this.StdOutputContains("Sequence_");
71109
var resultFiles = Directory.GetFiles(this.resultsDir, "*", SearchOption.AllDirectories);
72110

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ public void TriggerTestHostLaunchedHandlerShouldStartProcDumpUtilityIfProcDumpEn
476476
this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
477477

478478
// Verify StartProcessDumpCall
479-
this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>()));
479+
this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>(), false));
480480
}
481481

482482
/// <summary>
@@ -497,7 +497,7 @@ public void TriggerTestHostLaunchedHandlerShouldStartProcDumpUtilityForFullDumpI
497497
this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
498498

499499
// Verify StartProcessDumpCall
500-
this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), true, It.IsAny<string>()));
500+
this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), true, It.IsAny<string>(), false));
501501
}
502502

503503
/// <summary>
@@ -526,7 +526,7 @@ public void TriggerTestHostLaunchedHandlerShouldStartProcDumpUtilityForFullDumpI
526526
this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
527527

528528
// Verify StartProcessDumpCall
529-
this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), true, It.IsAny<string>()));
529+
this.mockProcessDumpUtility.Verify(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), true, It.IsAny<string>(), false));
530530
}
531531

532532
/// <summary>
@@ -646,7 +646,7 @@ public void TriggerTestHostLaunchedHandlerShouldCatchTestPlatFormExceptionsAndRe
646646

647647
// Make StartProcessDump throw exception
648648
var tpex = new TestPlatformException("env var exception");
649-
this.mockProcessDumpUtility.Setup(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>()))
649+
this.mockProcessDumpUtility.Setup(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>(), false))
650650
.Throws(tpex);
651651

652652
// Raise TestHostLaunched
@@ -672,7 +672,7 @@ public void TriggerTestHostLaunchedHandlerShouldCatchAllUnexpectedExceptionsAndR
672672

673673
// Make StartProcessDump throw exception
674674
var ex = new Exception("start process failed");
675-
this.mockProcessDumpUtility.Setup(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>()))
675+
this.mockProcessDumpUtility.Setup(x => x.StartTriggerBasedProcessDump(1234, It.IsAny<string>(), false, It.IsAny<string>(), false))
676676
.Throws(ex);
677677

678678
// Raise TestHostLaunched
@@ -709,9 +709,9 @@ private XmlElement GetDumpConfigurationElement(
709709

710710
if (collectDumpOnExit)
711711
{
712-
var fulldumpAttribute = xmldoc.CreateAttribute(BlameDataCollector.Constants.CollectDumpAlwaysKey);
713-
fulldumpAttribute.Value = "true";
714-
node.Attributes.Append(fulldumpAttribute);
712+
var collectDumpOnExitAttribute = xmldoc.CreateAttribute(BlameDataCollector.Constants.CollectDumpAlwaysKey);
713+
collectDumpOnExitAttribute.Value = "true";
714+
node.Attributes.Append(collectDumpOnExitAttribute);
715715
}
716716

717717
if (colectDumpOnHang)

0 commit comments

Comments
 (0)