Skip to content

Commit 70a599d

Browse files
nohwndnohwnd
authored
Fix blame parameter, warning, and add all testhosts to be ngend (#2579)
1 parent 0c0fafa commit 70a599d

6 files changed

Lines changed: 325 additions & 26 deletions

File tree

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ private void ValidateAndAddHangBasedProcessDumpParameters(XmlElement collectDump
348348

349349
break;
350350

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

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

362363
break;
363364

365+
// allow DumpType attribute to be used on the hang dump for backwards compatibility
366+
case XmlAttribute attribute when string.Equals(attribute.Name, Constants.DumpTypeKey, StringComparison.OrdinalIgnoreCase):
367+
368+
if (string.Equals(attribute.Value, Constants.FullConfigurationValue, StringComparison.OrdinalIgnoreCase) || string.Equals(attribute.Value, Constants.MiniConfigurationValue, StringComparison.OrdinalIgnoreCase))
369+
{
370+
this.processFullDumpEnabled = string.Equals(attribute.Value, Constants.FullConfigurationValue, StringComparison.OrdinalIgnoreCase);
371+
}
372+
else
373+
{
374+
this.logger.LogWarning(this.context.SessionDataCollectionContext, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterValueIncorrect, attribute.Name, Constants.FullConfigurationValue, Constants.MiniConfigurationValue));
375+
}
376+
377+
break;
378+
364379
default:
365380

366381
this.logger.LogWarning(this.context.SessionDataCollectionContext, string.Format(CultureInfo.CurrentUICulture, Resources.Resources.BlameParameterKeyIncorrect, blameAttribute.Name));
@@ -454,7 +469,7 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args)
454469
{
455470
try
456471
{
457-
var dumpFiles = this.processDumpUtility.GetDumpFiles();
472+
var dumpFiles = this.processDumpUtility.GetDumpFiles(warnOnNoDumpFiles: this.collectDumpAlways);
458473
foreach (var dumpFile in dumpFiles)
459474
{
460475
if (!string.IsNullOrEmpty(dumpFile))

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ public interface IProcessDumpUtility
1111
/// <summary>
1212
/// Get generated dump files
1313
/// </summary>
14+
/// <param name="warnOnNoDumpFiles">Writes warning when no dump file is found.</param>
1415
/// <returns>
1516
/// Path of dump file
1617
/// </returns>
17-
IEnumerable<string> GetDumpFiles();
18+
IEnumerable<string> GetDumpFiles(bool warnOnNoDumpFiles = true);
1819

1920
/// <summary>
2021
/// Launch proc dump process

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper,
5151
};
5252

5353
/// <inheritdoc/>
54-
public IEnumerable<string> GetDumpFiles()
54+
public IEnumerable<string> GetDumpFiles(bool warnOnNoDumpFiles = true)
5555
{
5656
if (!this.wasHangDumped)
5757
{
@@ -82,7 +82,7 @@ public IEnumerable<string> GetDumpFiles()
8282
}
8383
}
8484

85-
if (!foundDumps.Any())
85+
if (warnOnNoDumpFiles && !foundDumps.Any())
8686
{
8787
EqtTrace.Error($"ProcessDumpUtility.GetDumpFile: Could not find any dump file in {this.hangDumpDirectory}.");
8888
throw new FileNotFoundException(Resources.Resources.DumpFileNotGeneratedErrorMessage);

‎src/package/VSIXProject/TestPlatform.csproj‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,22 @@
238238

239239
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.exe" />
240240
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.x86.exe" />
241+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net452.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net452.exe" />
242+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net452.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net452.x86.exe" />
243+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net46.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net46.exe" />
244+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net46.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net46.x86.exe" />
245+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net461.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net461.exe" />
246+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net461.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net461.x86.exe" />
247+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net462.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net462.exe" />
248+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net462.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net462.x86.exe" />
249+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net47.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net47.exe" />
250+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net47.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net47.x86.exe" />
251+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net471.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net471.exe" />
252+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net471.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net471.x86.exe" />
253+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net472.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net472.exe" />
254+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net472.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net472.x86.exe" />
255+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net48.exe" Ngen="true" NgenArchitecture="X64" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net48.exe" />
256+
<VsixSourceItem Update="$(VsixInputFileLocation)\testhost.net48.x86.exe" Ngen="true" NgenArchitecture="X86" NgenPriority="2" NgenApplication="$(ExtensionInstallationRelativeToVS)\testhost.net48.x86.exe" />
241257
</ItemGroup>
242258
<ItemGroup>
243259
<Content Include="License.rtf">

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

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout()
172172
this.mockFileHelper.Setup(x => x.Exists(It.Is<string>(y => y == "abc_hang.dmp"))).Returns(true);
173173
this.mockFileHelper.Setup(x => x.GetFullPath(It.Is<string>(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp");
174174
this.mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()));
175-
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { dumpFile });
175+
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Returns(new[] { dumpFile });
176176
this.mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny<FileTransferInformation>())).Callback(() => hangBasedDumpcollected.Set());
177177

178178
this.blameDataCollector.Initialize(
@@ -184,7 +184,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout()
184184

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

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

211211
this.blameDataCollector.Initialize(
212212
this.GetDumpConfigurationElement(false, false, true, 0),
@@ -217,7 +217,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet
217217

218218
hangBasedDumpcollected.Wait(1000);
219219
this.mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<Action<string>>()), Times.Once);
220-
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
220+
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
221221
}
222222

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

245245
this.blameDataCollector.Initialize(
@@ -251,7 +251,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt
251251

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

@@ -366,7 +366,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled()
366366
this.context);
367367

368368
// Setup
369-
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { this.filepath });
369+
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny<bool>())).Returns(new[] { this.filepath });
370370
this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()))
371371
.Returns(this.filepath);
372372

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

378378
// Verify GetDumpFiles Call
379-
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
379+
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(It.IsAny<bool>()), Times.Once);
380380
}
381381

382382
/// <summary>
@@ -418,7 +418,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab
418418
this.context);
419419

420420
// Setup
421-
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { this.filepath });
421+
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Returns(new[] { this.filepath });
422422
this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()))
423423
.Returns(this.filepath);
424424

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

429429
// Verify GetDumpFiles Call
430-
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Once);
430+
this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true), Times.Once);
431431
}
432432

433433
/// <summary>
@@ -437,8 +437,10 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab
437437
public void TriggerSessionEndedHandlerShouldLogWarningIfGetDumpFileThrowsFileNotFound()
438438
{
439439
// Initializing Blame Data Collector
440+
// force it to collect dump on exit, which won't happen and we should see a warning
441+
// but we should not see warning if we tell it to create dump and there is no crash
440442
this.blameDataCollector.Initialize(
441-
this.GetDumpConfigurationElement(),
443+
this.GetDumpConfigurationElement(false, collectDumpOnExit: true),
442444
this.mockDataColectionEvents.Object,
443445
this.mockDataCollectionSink.Object,
444446
this.mockLogger.Object,
@@ -447,7 +449,7 @@ public void TriggerSessionEndedHandlerShouldLogWarningIfGetDumpFileThrowsFileNot
447449
// Setup and raise events
448450
this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny<List<Guid>>(), It.IsAny<Dictionary<Guid, BlameTestObject>>(), It.IsAny<string>()))
449451
.Returns(this.filepath);
450-
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Throws(new FileNotFoundException());
452+
this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true)).Throws(new FileNotFoundException());
451453
this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
452454
this.mockDataColectionEvents.Raise(x => x.TestCaseStart += null, new TestCaseStartEventArgs(new TestCase()));
453455
this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));
@@ -605,6 +607,29 @@ public void TriggerTestHostLaunchedHandlerShouldLogWarningForNonBooleanCollectAl
605607
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);
606608
}
607609

610+
/// <summary>
611+
/// The trigger test host launched handler should start process dump utility for full dump if full dump was enabled
612+
/// </summary>
613+
[TestMethod]
614+
public void TriggerTestHostLaunchedHandlerShouldLogNoWarningWhenDumpTypeIsUsedWithHangDumpBecauseEitherHangDumpTypeOrDumpTypeCanBeSpecified()
615+
{
616+
var dumpConfig = this.GetDumpConfigurationElement(isFullDump: true, false, colectDumpOnHang: true, 1800000);
617+
618+
// Initializing Blame Data Collector
619+
this.blameDataCollector.Initialize(
620+
dumpConfig,
621+
this.mockDataColectionEvents.Object,
622+
this.mockDataCollectionSink.Object,
623+
this.mockLogger.Object,
624+
this.context);
625+
626+
// Raise TestHostLaunched
627+
this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234));
628+
629+
// Verify
630+
this.mockLogger.Verify(x => x.LogWarning(It.IsAny<DataCollectionContext>(), It.IsAny<string>()), Times.Never);
631+
}
632+
608633
/// <summary>
609634
/// The trigger test host launched handler should not break if start process dump throws TestPlatFormExceptions and log error message
610635
/// </summary>

0 commit comments

Comments
 (0)