diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs index 97774c907e..1756e5ed13 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs @@ -200,7 +200,8 @@ public override void Initialize( if (_collectProcessDumpOnHang) { _inactivityTimer ??= new InactivityTimer(CollectDumpAndAbortTesthost); - ResetInactivityTimer(); + // Don't start the timer here — wait until TestHostLaunched so we know + // which process to dump. ResetInactivityTimer is called from TestHostLaunchedHandler. } } @@ -244,6 +245,10 @@ private void CollectDumpAndAbortTesthost() EqtTrace.Verbose("Inactivity timer is already disposed."); } + // If testhost has not launched yet, the timer should not have been started, + // so this callback should not fire. Assert to catch unexpected paths. + TPDebug.Assert(_testHostProcessId != 0, "CollectDumpAndAbortTesthost called but testhost has not launched yet."); + if (_collectProcessDumpOnCrash) { // Detach the dumper from the testhost process to prevent crashing testhost process. When the dumper is procdump.exe @@ -630,9 +635,9 @@ private void SessionEndedHandler(object? sender, SessionEndEventArgs args) /// TestHostLaunchedEventArgs private void TestHostLaunchedHandler(object? sender, TestHostLaunchedEventArgs args) { - ResetInactivityTimer(); - _testHostProcessId = args.TestHostProcessId; + Interlocked.Exchange(ref _testHostProcessId, args.TestHostProcessId); _testHostProcessName = _processHelper.GetProcessName(args.TestHostProcessId); + ResetInactivityTimer(); if (!_collectProcessDumpOnCrash) { diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.cs.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.cs.xlf index dafafdf58d..05cf1e4042 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.cs.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.cs.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.de.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.de.xlf index 5c21b44514..cb707603fa 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.de.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.de.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.es.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.es.xlf index de67d8af2c..082bc817e7 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.es.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.es.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.fr.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.fr.xlf index ae354a94d3..95bc78f7cd 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.fr.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.fr.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.it.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.it.xlf index 6fb6917ae9..b8343d82a1 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.it.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.it.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ja.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ja.xlf index 6342208092..6c00d0a78c 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ja.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ja.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ko.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ko.xlf index 17e587362a..d48e2ac432 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ko.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ko.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pl.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pl.xlf index bc99526529..176832f588 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pl.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pl.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pt-BR.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pt-BR.xlf index 5b39153cf7..70917a127d 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pt-BR.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.pt-BR.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ru.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ru.xlf index fc5cb4b297..5ac9e2d81c 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ru.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.ru.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.tr.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.tr.xlf index 1ef6ca4a85..dcadc8956c 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.tr.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.tr.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.zh-Hans.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.zh-Hans.xlf index 38702a9594..e50417ea86 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.zh-Hans.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.zh-Hans.xlf @@ -1,4 +1,4 @@ - + diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.zh-Hant.xlf b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.zh-Hant.xlf index d28661032e..28e001dac8 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.zh-Hant.xlf +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Resources/xlf/Resources.zh-Hant.xlf @@ -1,4 +1,4 @@ - + diff --git a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs index cdec3a8de8..7b0b2db2f9 100644 --- a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs @@ -238,6 +238,46 @@ public void HangDumpChildProcesses(RunnerInfo runnerInfo) ValidateDump(2); } + [TestMethod] + [DoNotParallelize] // Modifies the test asset's runtimeconfig.json on disk. + [NetCoreTargetFrameworkDataSource] + public void HangDumpShouldNotHangWhenTestHostFailsToStart(RunnerInfo runnerInfo) + { + // When testhost can't start (e.g. wrong runtime version), the inactivity timer + // must not try to dump PID 0. It should exit cleanly because the timer only starts + // after TestHostLaunched, which never fires here. + SetTestEnvironment(_testEnvironment, runnerInfo); + const string testAssetProjectName = "SimpleTestProjectMessedUpTargetFramework"; + var assemblyPath = GetTestDllForFramework(testAssetProjectName + ".dll", Core11TargetFramework); + + // Make testhost fail immediately by targeting a non-existent runtime. + var runtimeConfigJson = Path.Combine(Path.GetDirectoryName(assemblyPath)!, testAssetProjectName + ".runtimeconfig.json"); + var originalContent = File.ReadAllText(runtimeConfigJson); + try + { + var updatedContent = Regex.Replace(originalContent, @"""version""\s*:\s*""[\d.]+""", @"""version"": ""9999.0.0"""); + File.WriteAllText(runtimeConfigJson, updatedContent); + + var arguments = PrepareArguments(assemblyPath, GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue); + arguments = string.Concat(arguments, $" /ResultsDirectory:{TempDirectory.Path}"); + arguments = string.Concat(arguments, $@" /Blame:""CollectHangDump;HangDumpType=mini;TestTimeout=30s"""); + InvokeVsTest(arguments); + + // vstest should exit with failure (testhost didn't start), but not hang and not crash. + ExitCodeEquals(1); + // Verify the failure was specifically because the runtime wasn't found, not some other error. + // The .NET runtime error can appear in stderr or stdout depending on the platform/host. + Assert.IsTrue( + Regex.IsMatch(StdErr, "9999\\.0\\.0") || Regex.IsMatch(StdOut, "9999\\.0\\.0"), + $"Expected '9999.0.0' in output.\nStdErr: {StdErr}\nStdOut: {StdOut}"); + Assert.DoesNotContain(".dmp", StdOut, "no dump should be collected when testhost never launched"); + } + finally + { + File.WriteAllText(runtimeConfigJson, originalContent); + } + } + [TestMethod] [TestCategory("Windows-Review")] [DoNotParallelize] // Installs/uninstalls procdump as machine-wide postmortem debugger via HKLM registry. diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs index b48dd14294..f6f1eaba86 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs @@ -111,7 +111,7 @@ public void InitializeWithDumpForHangDisabledShouldNotInitializeInactivityTimerO /// /// Initializing with collect dump for hang should configure the timer with the right values and should - /// not call the reset method if no events are received. + /// start the timer when testhost launches (not during Initialize). /// [TestMethod] public void InitializeWithDumpForHangShouldInitializeInactivityTimerAndCallResetOnce() @@ -127,12 +127,17 @@ public void InitializeWithDumpForHangShouldInitializeInactivityTimerAndCallReset _mockLogger.Object, _context); - Assert.AreEqual(1, resetCalledCount, "Should have called InactivityTimer.Reset exactly once since no events were received"); + Assert.AreEqual(0, resetCalledCount, "Should not have called InactivityTimer.Reset during Initialize — timer starts on TestHostLaunched"); + + // Simulate testhost launching — this should start the timer. + _mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234)); + + Assert.AreEqual(1, resetCalledCount, "Should have called InactivityTimer.Reset exactly once after TestHostLaunched"); } /// /// Initializing with collect dump for hang should configure the timer with the right values and should - /// reset for each event received + /// reset for each event received (including TestHostLaunched which starts the timer). /// [TestMethod] public void InitializeWithDumpForHangShouldInitializeInactivityTimerAndResetForEachEventReceived() @@ -151,6 +156,9 @@ public void InitializeWithDumpForHangShouldInitializeInactivityTimerAndResetForE _mockLogger.Object, _context); + // Simulate testhost launching — this starts the timer (1st reset). + _mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234)); + TestCase testcase = new("TestProject.UnitTest.TestMethod", new Uri("test:/abc"), "abc.dll"); _mockDataColectionEvents.Raise(x => x.TestCaseStart += null, new TestCaseStartEventArgs(testcase)); @@ -182,13 +190,16 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout() _mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny())).Callback(() => hangBasedDumpcollected.Set()); _blameDataCollector.Initialize( - GetDumpConfigurationElement(false, false, true, 0), + GetDumpConfigurationElement(false, false, true, 50), _mockDataColectionEvents.Object, _mockDataCollectionSink.Object, _mockLogger.Object, _context); - hangBasedDumpcollected.Wait(1000, TestContext.CancellationToken); + // Simulate testhost launching before the timer fires. + _mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234)); + + hangBasedDumpcollected.Wait(2000, TestContext.CancellationToken); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny()), Times.Once); _mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is(y => y.Path == dumpFile)), Times.Once); @@ -216,13 +227,16 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception")); _blameDataCollector.Initialize( - GetDumpConfigurationElement(false, false, true, 0), + GetDumpConfigurationElement(false, false, true, 50), _mockDataColectionEvents.Object, _mockDataCollectionSink.Object, _mockLogger.Object, _context); - hangBasedDumpcollected.Wait(1000, TestContext.CancellationToken); + // Simulate testhost launching before the timer fires. + _mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234)); + + hangBasedDumpcollected.Wait(2000, TestContext.CancellationToken); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny()), Times.Once); } @@ -251,18 +265,52 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt _mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some other exception")); _blameDataCollector.Initialize( - GetDumpConfigurationElement(false, false, true, 0), + GetDumpConfigurationElement(false, false, true, 50), _mockDataColectionEvents.Object, _mockDataCollectionSink.Object, _mockLogger.Object, _context); - hangBasedDumpcollected.Wait(1000, TestContext.CancellationToken); + // Simulate testhost launching before the timer fires. + _mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234)); + + hangBasedDumpcollected.Wait(2000, TestContext.CancellationToken); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny()), Times.Once); _mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is(y => y.Path == dumpFile)), Times.Once); } + /// + /// If testhost has not launched, the inactivity timer should not be started, so no hang dump + /// should be attempted even after the configured timeout elapses. + /// + [TestMethod] + public void InitializeWithDumpForHangShouldNotStartTimerIfTestHostHasNotLaunchedYet() + { + _blameDataCollector = new TestableBlameCollector( + _mockBlameReaderWriter.Object, + _mockProcessDumpUtility.Object, + null, + _mockFileHelper.Object, + _mockProcessHelper.Object); + + _blameDataCollector.Initialize( + GetDumpConfigurationElement(false, false, true, 0), + _mockDataColectionEvents.Object, + _mockDataCollectionSink.Object, + _mockLogger.Object, + _context); + + // Do NOT raise TestHostLaunched — timer should never start. + // Wait long enough that a started timer with timeout 0 would have fired. + Thread.Sleep(100); + + // The hang dump must not have been attempted. + _mockProcessDumpUtility.Verify( + x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), + Times.Never); + } + /// /// The trigger session ended handler should write to file if test start count is greater. ///