Skip to content

Commit

Permalink
Fixed issue where proc dump was not getting terminated on no crash (#…
Browse files Browse the repository at this point in the history 
…1849)

* Fixed issue where proc dump was not getting terminated on no crash

* resolved pr comments

* resolved pr comments

* resolved pr comments

* incorporated sarab's rename comment
  • Loading branch information
@ShreyasRmsft
ShreyasRmsft authored Nov 22, 2018
1 parent b010901 commit e382908
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 31 deletions.
5 changes: 5 additions & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,8 @@ src/package/sign/sign.nuget.targets
*.tr.resx
*.zh-Hans.resx
*.zh-Hant.resx

# ===========================
# Vscode files
# ===========================
.vscode/
65 changes: 38 additions & 27 deletions src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,45 +202,56 @@ private void SessionEnded_Handler(object sender, SessionEndEventArgs args)
EqtTrace.Info("Blame Collector : Session End");
}

// If the last test crashes, it will not invoke a test case end and therefore
// In case of crash testStartCount will be greater than testEndCount and we need to write the sequence
// And send the attachment
if (this.testStartCount > this.testEndCount)
try
{
var filepath = Path.Combine(this.GetResultsDirectory(), Constants.AttachmentFileName + "_" + this.attachmentGuid);
filepath = this.blameReaderWriter.WriteTestSequence(this.testSequence, this.testObjectDictionary, filepath);
var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, filepath, true);
this.dataCollectionSink.SendFileAsync(fileTranferInformation);
}
// If the last test crashes, it will not invoke a test case end and therefore
// In case of crash testStartCount will be greater than testEndCount and we need to write the sequence
// And send the attachment
if (this.testStartCount > this.testEndCount)
{
var filepath = Path.Combine(this.GetResultsDirectory(), Constants.AttachmentFileName + "_" + this.attachmentGuid);
filepath = this.blameReaderWriter.WriteTestSequence(this.testSequence, this.testObjectDictionary, filepath);
var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, filepath, true);
this.dataCollectionSink.SendFileAsync(fileTranferInformation);
}

if (this.processDumpEnabled)
{
// If there was a test case crash or if we need to collect dump on process exit.
if (this.testStartCount > this.testEndCount || this.collectDumpAlways)
if (this.processDumpEnabled)
{
try
// If there was a test case crash or if we need to collect dump on process exit.
if (this.testStartCount > this.testEndCount || this.collectDumpAlways)
{
var dumpFile = this.processDumpUtility.GetDumpFile();
if (!string.IsNullOrEmpty(dumpFile))
try
{
var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
this.dataCollectionSink.SendFileAsync(fileTranferInformation);
var dumpFile = this.processDumpUtility.GetDumpFile();
if (!string.IsNullOrEmpty(dumpFile))
{
var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
this.dataCollectionSink.SendFileAsync(fileTranferInformation);
}
else
{
EqtTrace.Warning("BlameCollector.SessionEnded_Handler: blame:CollectDump was enabled but dump file was not generated.");
this.logger.LogWarning(args.Context, Resources.Resources.ProcDumpNotGenerated);
}
}
else
catch (FileNotFoundException ex)
{
EqtTrace.Warning("BlameCollector.SessionEnded_Handler: blame:CollectDump was enabled but dump file was not generated.");
this.logger.LogWarning(args.Context, Resources.Resources.ProcDumpNotGenerated);
EqtTrace.Warning(ex.Message);
this.logger.LogWarning(args.Context, ex.Message);
}
}
catch (FileNotFoundException ex)
{
EqtTrace.Warning(ex.Message);
this.logger.LogWarning(args.Context, ex.Message);
}
}
}
finally
{
// Attempt to terminate the proc dump process if proc dump was enabled
if (this.processDumpEnabled)
{
this.processDumpUtility.TerminateProcess();
}

this.DeregisterEvents();
this.DeregisterEvents();
}
}

/// <summary>
Expand Down
7 changes: 5 additions & 2 deletions src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
{
using System.Collections.Generic;

public interface IProcessDumpUtility
{
/// <summary>
Expand All @@ -31,5 +29,10 @@ public interface IProcessDumpUtility
/// Is full dump enabled
/// </param>
void StartProcessDump(int processId, string dumpFileGuid, string testResultsDirectory, bool isFullDump = false);

/// <summary>
/// Terminate the proc dump process
/// </summary>
void TerminateProcess();
}
}
18 changes: 16 additions & 2 deletions src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper,
};
}

/// <inheritdoc/>
public string GetDumpFile()
/// <inheritdoc/>
public string GetDumpFile()
{
if (this.procDumpProcess == null)
{
Expand Down Expand Up @@ -95,6 +95,20 @@ public void StartProcessDump(int processId, string dumpFileGuid, string testResu
null) as Process;
}

/// <inheritdoc/>
public void TerminateProcess()
{
try
{
EqtTrace.Info("ProcessDumpUtility : Attempting to kill proc dump process.");
this.processHelper.TerminateProcess(this.procDumpProcess);
}
catch (Exception e)
{
EqtTrace.Warning($"ProcessDumpUtility : Failed to kill proc dump process with exception {e}");
}
}

/// <summary>
/// Arguments for procdump.exe
/// </summary>
Expand Down
24 changes: 24 additions & 0 deletions test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,30 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled()
this.mockProcessDumpUtility.Verify(x => x.GetDumpFile(), Times.Once);
}

/// <summary>
/// The trigger session ended handler should ensure proc dump process is terminated when no crash is detected
/// </summary>
[TestMethod]
public void TriggerSessionEndedHandlerShouldEnsureProcDumpProcessIsTerminated()
{
// Initializing Blame Data Collector
this.blameDataCollector.Initialize(
this.GetDumpConfigurationElement(),
this.mockDataColectionEvents.Object,
this.mockDataCollectionSink.Object,
this.mockLogger.Object,
this.context);

// Mock proc dump utility terminate process call
this.mockProcessDumpUtility.Setup(x => x.TerminateProcess());

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

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

/// <summary>
/// The trigger session ended handler should not dump files if proc dump was enabled and test host did not crash
/// </summary>
Expand Down
22 changes: 22 additions & 0 deletions ...Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,28 @@ public void StartProcessDumpWillStartExeCorrespondingTo32BitTestHostProcessIn64B
this.mockProcessHelper.Verify(x => x.LaunchProcess(Path.Combine("D:\\procdump", "procdump.exe"), It.IsAny<string>(), It.IsAny<string>(), null, null, null));
}

/// <summary>
/// Ensure terminate process calls terminate on proc dump process
/// </summary>
[TestMethod]
public void TerminateProcessDumpShouldCallTerminateOnProcDumpProcess()
{
var processDumpUtility = new ProcessDumpUtility(
this.mockProcessHelper.Object,
this.mockFileHelper.Object,
this.mockPlatformEnvironment.Object,
this.mockNativeMethodsHelper.Object);

// Mock process helper
this.mockProcessHelper.Setup(x => x.TerminateProcess(It.IsAny<object>()));

// Raise
processDumpUtility.TerminateProcess();

// Verify
this.mockProcessHelper.Verify(x => x.TerminateProcess(It.IsAny<object>()), Times.Once);
}

[TestCleanup]
public void TestCleanup()
{
Expand Down

0 comments on commit e382908

Please sign in to comment.