microsoft / testfx

MSTest framework and adapter
MIT License
684 stars 248 forks source link

FailedTestNodeStateProperty should include the ability to report without an exception #2536

Open bradwilson opened 4 months ago

bradwilson commented 4 months ago

The constructors for FailedTestNodeStateProperty do not include the ability to provide extended error information outside of an Exception object.

xUnit.net's error reporting APIs do not include Exception objects. Given our split between runners and execution environment, it may not even be possible for us to re-create instances of the original Exception classes since the types in question may not be available inside the runner infrastructure.

Please provide an alternative way to report errors that allow capturing the essence of Exception without depending on specific instances.

MarcoRossignoli commented 4 months ago

Do you mean something similar to

public class ExceptionInformation(string exceptionMessage, string? stackTrace = null);

Feel free to propose something.

bradwilson commented 4 months ago

Based on https://github.com/microsoft/testfx/blob/main/docs/mstest-runner-protocol/001-protocol-intro.md#test-node I would say an overloaded constructor for FailedTestNodeStateProperty that would accept a message and stackTrace parameter makes the most sense.

Evangelink commented 4 months ago

In case it helps, this is the custom exception I have created to map some failed tests report from VSTest object model to the new platform:

internal sealed class VSTestException : Exception
{
    public VSTestException(string? message, string? stackTrace)
        : base(message)
    {
        StackTrace = stackTrace;
    }

    public override string? StackTrace { get; }
}

used as

case TestOutcome.Failed:
    testNode.Properties.Add(new FailedTestNodeStateProperty(new VSTestException(testResult.ErrorMessage, testResult.ErrorStackTrace)));
    break;
bradwilson commented 4 months ago

Since there doesn't appear to be any type reporting on the exceptions, this seems reasonable, albeit hard to discover for general usage. 😄

bradwilson commented 4 months ago

Also, it does not appear that the system can handle inner exceptions, I presume for simplicity's sake. How would expect inner exceptions to be reported in this system? We have a "format" that we use to report messages and stack traces when there are inner exceptions, which of course fit into "strings" just fine, but in the case of stack traces they aren't necessarily guaranteed to be "one stack frame per line" since some of the lines describe the relationships. Is that acceptable in the new model?