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

Fix blame parameter, warning, and add all testhosts to be ngend #2579

Merged
1 commit merged into from Sep 18, 2020
Merged

Fix blame parameter, warning, and add all testhosts to be ngend #2579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 16 additions & 1 deletion src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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