Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Blame upload on crash even if hang dump started #2553

Merged
4 commits merged into from Sep 2, 2020
Merged

Blame upload on crash even if hang dump started #2553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5dd8e9e
Upload on session end when HangDump started
nohwnd Sep 1, 2020
bae96ef
Fix procdump condition
nohwnd Sep 1, 2020
ce719b9
Merge branch 'master' into blame-upload-on-crash-even-if-hang-dump-st…
nohwnd Sep 1, 2020
a7293ff
Remove test, because we want to upload all dumps on session end when …
nohwnd Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 24 additions & 36 deletions src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
View file
Open in desktop
Expand Up @@ -46,7 +46,6 @@ public class BlameCollector : DataCollector, ITestExecutionEnvironmentSpecifier
private IInactivityTimer inactivityTimer;
private TimeSpan inactivityTimespan = TimeSpan.FromMinutes(DefaultInactivityTimeInMinutes);
private int testHostProcessId;
private bool dumpWasCollectedByHangDumper;
private string targetFramework;
private List<KeyValuePair<string, string>> environmentVariables = new List<KeyValuePair<string, string>>();
private bool uploadDumpFiles;
Expand Down Expand Up @@ -223,7 +222,7 @@ private void CollectDumpAndAbortTesthost()
}
catch (Exception ex)
{
ConsoleOutput.Instance.Error(true, $"Blame: Creating hang dump failed with error {ex}.");
this.logger.LogError(this.context.SessionDataCollectionContext, $"Blame: Creating hang dump failed with error.", ex);
}

if (this.collectProcessDumpOnTrigger)
Expand All @@ -244,7 +243,6 @@ private void CollectDumpAndAbortTesthost()
{
if (!string.IsNullOrEmpty(dumpFile))
{
this.dumpWasCollectedByHangDumper = true;
var fileTransferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true, this.fileHelper);
this.dataCollectionSink.SendFileAsync(fileTransferInformation);
}
Expand All @@ -263,7 +261,7 @@ private void CollectDumpAndAbortTesthost()
}
catch (Exception ex)
{
ConsoleOutput.Instance.Error(true, $"Blame: Collecting hang dump failed with error {ex}.");
this.logger.LogError(this.context.SessionDataCollectionContext, $"Blame: Collecting hang dump failed with error.", ex);
}
}
else
Expand Down Expand Up @@ -452,47 +450,37 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args)
this.logger.LogWarning(this.context.SessionDataCollectionContext, Resources.Resources.NotGeneratingSequenceFile);
}

if (this.collectProcessDumpOnTrigger)
if (this.uploadDumpFiles)
{
// If there was a test case crash or if we need to collect dump on process exit.
//
// Do not try to collect dump when we already collected one from the hang dump
// we won't dump the killed process again and that would just show a warning on the command line
if ((this.testStartCount > this.testEndCount || this.collectDumpAlways) && !this.dumpWasCollectedByHangDumper)
try
{
if (this.uploadDumpFiles)
var dumpFiles = this.processDumpUtility.GetDumpFiles();
foreach (var dumpFile in dumpFiles)
{
try
if (!string.IsNullOrEmpty(dumpFile))
{
var dumpFiles = this.processDumpUtility.GetDumpFiles();
foreach (var dumpFile in dumpFiles)
try
nohwnd marked this conversation as resolved.
Show resolved Hide resolved
{
if (!string.IsNullOrEmpty(dumpFile))
{
try
{
var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
this.dataCollectionSink.SendFileAsync(fileTranferInformation);
}
catch (FileNotFoundException ex)
{
EqtTrace.Warning(ex.ToString());
this.logger.LogWarning(args.Context, ex.ToString());
}
}
var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
this.dataCollectionSink.SendFileAsync(fileTranferInformation);
}
catch (FileNotFoundException ex)
{
EqtTrace.Warning(ex.ToString());
this.logger.LogWarning(args.Context, ex.ToString());
}
}
catch (FileNotFoundException ex)
{
EqtTrace.Warning(ex.ToString());
this.logger.LogWarning(args.Context, ex.ToString());
}
}
else
{
EqtTrace.Info("BlameCollector.CollectDumpAndAbortTesthost: Custom path to dump directory was provided via VSTEST_DUMP_PATH. Skipping attachment upload, the caller is responsible for collecting and uploading the dumps themselves.");
}
}
catch (FileNotFoundException ex)
{
EqtTrace.Warning(ex.ToString());
this.logger.LogWarning(args.Context, ex.ToString());
}
}
else
{
EqtTrace.Info("BlameCollector.CollectDumpAndAbortTesthost: Custom path to dump directory was provided via VSTEST_DUMP_PATH. Skipping attachment upload, the caller is responsible for collecting and uploading the dumps themselves.");
}
}
finally
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/CrashDumperFactory.cs
View file
Open in desktop
Expand Up @@ -44,7 +44,7 @@ public ICrashDumper Create(string targetFramework)
// This proven to be working okay while net5.0 could not create dumps from Task.Run, and I was using this same technique
// to get dump of testhost. This needs PROCDUMP_PATH set to directory with procdump.exe, or having it in path.
var procdumpOverride = Environment.GetEnvironmentVariable("VSTEST_DUMP_FORCEPROCDUMP")?.Trim();
var forceUsingProcdump = string.IsNullOrWhiteSpace(procdumpOverride) && procdumpOverride != "0";
var forceUsingProcdump = !string.IsNullOrWhiteSpace(procdumpOverride) && procdumpOverride != "0";
if (forceUsingProcdump)
{
EqtTrace.Info($"CrashDumperFactory: This is Windows on {targetFramework}. Forcing the use of ProcDumpCrashDumper that uses ProcDump utility, via VSTEST_DUMP_FORCEPROCDUMP={procdumpOverride}.");
Expand Down
27 changes: 0 additions & 27 deletions test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs
View file
Open in desktop
Expand Up @@ -403,33 +403,6 @@ public void TriggerSessionEndedHandlerShouldEnsureProcDumpProcessIsTerminated()
this.mockProcessDumpUtility.Verify(x => x.DetachFromTargetProcess(It.IsAny<int>()), Times.Once);
}

/// <summary>
/// The trigger session ended handler should not dump files if proc dump was enabled and test host did not crash
/// </summary>
[TestMethod]
public void TriggerSessionEndedHandlerShouldNotGetDumpFileIfNoCrash()
{
// Initializing Blame Data Collector
this.blameDataCollector.Initialize(
this.GetDumpConfigurationElement(),
this.mockDataColectionEvents.Object,
this.mockDataCollectionSink.Object,
this.mockLogger.Object,
this.context);

// Setup
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { this.filepath });
this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()))
.Returns(this.filepath);

// Raise
this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));

// Verify GetDumpFiles Call
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Never);
}

/// <summary>
/// The trigger session ended handler should get dump files if collect dump on exit was enabled irrespective of completed test case count
/// </summary>
Expand Down