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.

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

Catch unhandled exception and avoid crash on test host exit #4291

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Text;

using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Extensions;
Expand All @@ -14,6 +15,8 @@ namespace Microsoft.TestPlatform.TestHostProvider.Hosting;

internal class TestHostManagerCallbacks
{
const int E_HANDLE = unchecked((int)0x80070006);
MarcoRossignoli marked this conversation as resolved.
Show resolved Hide resolved

public static void ErrorReceivedCallback(StringBuilder testHostProcessStdError, string? data)
{
// Log all standard error message because on too much data we ignore starting part.
Expand All @@ -32,7 +35,15 @@ public static void ErrorReceivedCallback(StringBuilder testHostProcessStdError,
EqtTrace.Verbose("TestHostProvider.ExitCallBack: Host exited starting callback.");
var testHostProcessStdErrorStr = testHostProcessStdError.ToString();

processHelper.TryGetExitCode(process, out int exitCode);
int exitCode = -1;
try
{
processHelper.TryGetExitCode(process, out exitCode);
}
catch (COMException ex) when (ex.HResult == E_HANDLE)
{
EqtTrace.Error("TestHostProvider.ExitCallBack: Invalid process handle we cannot get the error code, error {0}.", ex);
}

int procId = -1;
try
Expand Down
Expand Up @@ -2,9 +2,13 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Text;

using Microsoft.TestPlatform.TestHostProvider.Hosting;
using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions;
using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TestPlatform.TestHostProvider.Hosting.UnitTests;
Expand Down Expand Up @@ -94,4 +98,103 @@ public void ErrorReceivedCallbackShouldAppendNewLineApproprioritlyWhenReachingMa

Assert.AreEqual("1234" + Environment.NewLine.Substring(0, 1), _testHostProcessStdError.ToString());
}

[TestMethod]
public void ErrorReceivedCallbackShouldNotCrashIfInvalidProcessHandle()
{
var handleProcessHelper = new InvalidHandleProcessHelper();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you could simply use a mock here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah....I start to hate mocks...fakes are so simple will do.

bool onHostExitedCalled = false;
TestHostManagerCallbacks.ExitCallBack(handleProcessHelper, null, new StringBuilder(),
hostProviderEventArgs =>
{
onHostExitedCalled = true;
Assert.AreEqual(-1, hostProviderEventArgs.ErrroCode);
});
MarcoRossignoli marked this conversation as resolved.
Show resolved Hide resolved

Assert.IsTrue(handleProcessHelper.TryGetExitCodeCalled, "TryGetExitCodeCalled was not called");
Assert.IsTrue(onHostExitedCalled, "onHostExited was not called");
}

class InvalidHandleProcessHelper : IProcessHelper
MarcoRossignoli marked this conversation as resolved.
Show resolved Hide resolved
{
public bool TryGetExitCodeCalled { get; private set; }

public PlatformArchitecture GetCurrentProcessArchitecture()
{
throw new NotImplementedException();
}

public string? GetCurrentProcessFileName()
{
throw new NotImplementedException();
}

public int GetCurrentProcessId()
{
throw new NotImplementedException();
}

public string GetCurrentProcessLocation()
{
throw new NotImplementedException();
}

public string GetNativeDllDirectory()
{
throw new NotImplementedException();
}

public PlatformArchitecture GetProcessArchitecture(int processId)
{
throw new NotImplementedException();
}

public nint GetProcessHandle(int processId)
{
throw new NotImplementedException();
}

public int GetProcessId(object? process)
{
throw new NotImplementedException();
}

public string GetProcessName(int processId)
{
throw new NotImplementedException();
}

public string? GetTestEngineDirectory()
{
throw new NotImplementedException();
}

public object LaunchProcess(string processPath, string? arguments, string? workingDirectory, IDictionary<string, string?>? envVariables, Action<object?, string?>? errorCallback, Action<object?>? exitCallBack, Action<object?, string?>? outputCallBack)
{
throw new NotImplementedException();
}

public void SetExitCallback(int processId, Action<object?>? callbackAction)
{
throw new NotImplementedException();
}

public void TerminateProcess(object? process)
{
throw new NotImplementedException();
}

public bool TryGetExitCode(object? process, out int exitCode)
{
var err = new COMException("Invalid handle", unchecked((int)0x80070006));
typeof(COMException).GetProperty("HResult")!.SetValue(err, unchecked((int)0x80070006));
TryGetExitCodeCalled = true;
throw err;
}

public void WaitForProcessExit(object? process)
{
throw new NotImplementedException();
}
}
}