Skip to content

Commit

Permalink
Fix blame parameter, warning, and add all testhosts to be ngend (#2579)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nohwnd
nohwnd committed Sep 18, 2020
1 parent 0c0fafa commit 70a599d
Show file tree
Hide file tree
Showing 6 changed files with 325 additions and 26 deletions.
17 changes: 16 additions & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
View file
Expand Up @@ -348,6 +348,7 @@ private void ValidateAndAddHangBasedProcessDumpParameters(XmlElement collectDump

break;

// allow HangDumpType attribute to be used on the hang dump this is the prefered way
case XmlAttribute attribute when string.Equals(attribute.Name, Constants.HangDumpTypeKey, StringComparison.OrdinalIgnoreCase):

if (string.Equals(attribute.Value, Constants.FullConfigurationValue, StringComparison.OrdinalIgnoreCase) || string.Equals(attribute.Value, Constants.MiniConfigurationValue, StringComparison.OrdinalIgnoreCase))
Expand All @@ -361,6 +362,20 @@ private void ValidateAndAddHangBasedProcessDumpParameters(XmlElement collectDump

break;

// allow DumpType attribute to be used on the hang dump for backwards compatibility
case XmlAttribute attribute when string.Equals(attribute.Name, Constants.DumpTypeKey, StringComparison.OrdinalIgnoreCase):

if (string.Equals(attribute.Value, Constants.FullConfigurationValue, StringComparison.OrdinalIgnoreCase) || string.Equals(attribute.Value, Constants.MiniConfigurationValue, StringComparison.OrdinalIgnoreCase))
{
this.processFullDumpEnabled = string.Equals(attribute.Value, Constants.FullConfigurationValue, StringComparison.OrdinalIgnoreCase);
}
else
{
this.logger.LogWarning(this.context.SessionDataCollectionContext, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterValueIncorrect, attribute.Name, Constants.FullConfigurationValue, Constants.MiniConfigurationValue));
}

break;

default:

this.logger.LogWarning(this.context.SessionDataCollectionContext, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterKeyIncorrect, blameAttribute.Name));
Expand Down Expand Up @@ -454,7 +469,7 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args)
{
try
{
var dumpFiles = this.processDumpUtility.GetDumpFiles();
var dumpFiles = this.processDumpUtility.GetDumpFiles(warnOnNoDumpFiles: this.collectDumpAlways);
foreach (var dumpFile in dumpFiles)
{
if (!string.IsNullOrEmpty(dumpFile))
Expand Down
3 changes: 2 additions & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs
View file
Expand Up @@ -11,10 +11,11 @@ public interface IProcessDumpUtility
/// <summary>
/// Get generated dump files
/// </summary>
/// <param name="warnOnNoDumpFiles">Writes warning when no dump file is found.</param>
/// <returns>
/// Path of dump file
/// </returns>
IEnumerable<string> GetDumpFiles();
IEnumerable<string> GetDumpFiles(bool warnOnNoDumpFiles = true);

/// <summary>
/// Launch proc dump process
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs
View file
Expand Up @@ -51,7 +51,7 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper,
};

/// <inheritdoc/>
public IEnumerable<string> GetDumpFiles()
public IEnumerable<string> GetDumpFiles(bool warnOnNoDumpFiles = true)
{
if (!this.wasHangDumped)
{
Expand Down Expand Up @@ -82,7 +82,7 @@ public IEnumerable<string> GetDumpFiles()
}
}

if (!foundDumps.Any())
if (warnOnNoDumpFiles && !foundDumps.Any())
{
EqtTrace.Error($"ProcessDumpUtility.GetDumpFile: Could not find any dump file in {this.hangDumpDirectory}.");
throw new FileNotFoundException(Resources.Resources.DumpFileNotGeneratedErrorMessage);
Expand Down
16 changes: 16 additions & 0 deletions src/package/VSIXProject/TestPlatform.csproj
View file
Expand Up @@ -238,6 +238,22 @@

<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.x86.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net452.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net452.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net452.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net452.x86.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net46.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net46.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net46.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net46.x86.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net461.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net461.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net461.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net461.x86.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net462.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net462.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net462.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net462.x86.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net47.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net47.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net47.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net47.x86.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net471.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net471.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net471.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net471.x86.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net472.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net472.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net472.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net472.x86.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net48.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net48.exe" />
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net48.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net48.x86.exe" />
</ItemGroup>
<ItemGroup>
<Content Include="License.rtf">
Expand Down
49 changes: 37 additions & 12 deletions test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs
View file
Expand Up @@ -172,7 +172,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout()
this.mockFileHelper.Setup(x => x.Exists(It.Is<string>(y => y == "abc_hang.dmp"))).Returns(true);
this.mockFileHelper.Setup(x => x.GetFullPath(It.Is<string>(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp");
this.mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()));
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { dumpFile });
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Returns(new[] { dumpFile });
this.mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny<FileTransferInformation>())).Callback(() => hangBasedDumpcollected.Set());

this.blameDataCollector.Initialize(
Expand All @@ -184,7 +184,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout()

hangBasedDumpcollected.Wait(1000);
this.mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
this.mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is<FileTransferInformation>(y => y.Path == dumpFile)), Times.Once);
}

Expand All @@ -206,7 +206,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet
this.mockFileHelper.Setup(x => x.Exists(It.Is<string>(y => y == "abc_hang.dmp"))).Returns(true);
this.mockFileHelper.Setup(x => x.GetFullPath(It.Is<string>(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp");
this.mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()));
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception"));
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception"));

this.blameDataCollector.Initialize(
this.GetDumpConfigurationElement(false, false, true, 0),
Expand All @@ -217,7 +217,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet

hangBasedDumpcollected.Wait(1000);
this.mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
}

/// <summary>
Expand All @@ -239,7 +239,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt
this.mockFileHelper.Setup(x => x.Exists(It.Is<string>(y => y == "abc_hang.dmp"))).Returns(true);
this.mockFileHelper.Setup(x => x.GetFullPath(It.Is<string>(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp");
this.mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()));
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { dumpFile });
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Returns(new[] { dumpFile });
this.mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny<FileTransferInformation>())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some other exception"));

this.blameDataCollector.Initialize(
Expand All @@ -251,7 +251,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt

hangBasedDumpcollected.Wait(1000);
this.mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
this.mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is<FileTransferInformation>(y => y.Path == dumpFile)), Times.Once);
}

Expand Down Expand Up @@ -366,7 +366,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled()
this.context);

// Setup
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { this.filepath });
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny<bool>())).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);

Expand All @@ -376,7 +376,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled()
this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));

// Verify GetDumpFiles Call
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(It.IsAny<bool>()), Times.Once);
}

/// <summary>
Expand Down Expand Up @@ -418,7 +418,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab
this.context);

// Setup
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { this.filepath });
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).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);

Expand All @@ -427,7 +427,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab
this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));

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

/// <summary>
Expand All @@ -437,8 +437,10 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab
public void TriggerSessionEndedHandlerShouldLogWarningIfGetDumpFileThrowsFileNotFound()
{
// Initializing Blame Data Collector
// force it to collect dump on exit, which won't happen and we should see a warning
// but we should not see warning if we tell it to create dump and there is no crash
this.blameDataCollector.Initialize(
this.GetDumpConfigurationElement(),
this.GetDumpConfigurationElement(false, collectDumpOnExit: true),
this.mockDataColectionEvents.Object,
this.mockDataCollectionSink.Object,
this.mockLogger.Object,
Expand All @@ -447,7 +449,7 @@ public void TriggerSessionEndedHandlerShouldLogWarningIfGetDumpFileThrowsFileNot
// Setup and raise events
this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()))
.Returns(this.filepath);
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Throws(new FileNotFoundException());
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Throws(new FileNotFoundException());
this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
this.mockDataColectionEvents.Raise(x => x.TestCaseStart += null, new TestCaseStartEventArgs(new TestCase()));
this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));
Expand Down Expand Up @@ -605,6 +607,29 @@ public void TriggerTestHostLaunchedHandlerShouldLogWarningForNonBooleanCollectAl
this.mockLogger.Verify(x => x.LogWarning(It.IsAny<DataCollectionContext>(), It.Is<string>(str => str == string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterValueIncorrect, "DumpType", BlameDataCollector.Constants.FullConfigurationValue, BlameDataCollector.Constants.MiniConfigurationValue))), Times.Once);
}

/// <summary>
/// The trigger test host launched handler should start process dump utility for full dump if full dump was enabled
/// </summary>
[TestMethod]
public void TriggerTestHostLaunchedHandlerShouldLogNoWarningWhenDumpTypeIsUsedWithHangDumpBecauseEitherHangDumpTypeOrDumpTypeCanBeSpecified()
{
var dumpConfig = this.GetDumpConfigurationElement(isFullDump: true, false, colectDumpOnHang: true, 1800000);

// Initializing Blame Data Collector
this.blameDataCollector.Initialize(
dumpConfig,
this.mockDataColectionEvents.Object,
this.mockDataCollectionSink.Object,
this.mockLogger.Object,
this.context);

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

// Verify
this.mockLogger.Verify(x => x.LogWarning(It.IsAny<DataCollectionContext>(), It.IsAny<string>()), Times.Never);
}

/// <summary>
/// The trigger test host launched handler should not break if start process dump throws TestPlatFormExceptions and log error message
/// </summary>
Expand Down

0 comments on commit 70a599d

Please sign in to comment.