thomhurst / TUnit

A modern, fast and flexible .NET testing framework
MIT License
2.16k stars 37 forks source link

Test running spins indefinitely when exception thrown from resilience pipeline #1018

Open Vijay-Braidable opened 3 hours ago

Vijay-Braidable commented 3 hours ago

In VS I'm in debug mode, when I debug the test it seems to execute just fine, 100% of the time, when I run the test and the resiliancepipeline throws a couple of exceptions (for the multiple failures) it just spins forever (I left a 1 second test for many minutes to check this) unwinding a stacktrace. This did not happen when I was using the non-preview version of VS and XUnit, only when I ported to Preview with TUnit.

Running via dotnet test shows the stacktrace being written to the console for over 5 minutes.

I've speculatively included information I think may be helpful, let me know if I can provide something else.

TUnit Version: 0.2.26 Operating System: Windows 10 Pro Visual Studio Version: 2022 (640bit) - Preview (17.12.0 Preview 4.0) .NET Version: net8.0 Extensions: The only installed extension is ReSharper, I disabled this and the test still failed in the same way.

The Test:

    [Test]
    [MethodDataSource(typeof(TestBase), nameof(TestData))]
    public async Task TestSomething(SomeType someType)
    {
...

TestData in base class

    public static IEnumerable<SomeType > TestData()
    {
        yield return ManyTypes.SomeType;
    }

Generated Test Hooks:

// <auto-generated/>
#pragma warning disable
using global::System.Linq;
using global::System.Reflection;
using global::System.Runtime.CompilerServices;
using global::TUnit.Core;
using global::TUnit.Core.Interfaces;

namespace TUnit.SourceGenerated;

[global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
file partial class Hooks_REDACTED_Tests : TUnit.Core.Interfaces.SourceGenerator.ITestHookSource
{
    [global::System.Runtime.CompilerServices.ModuleInitializer]
    public static void Initialise()
    {
        var instance = new Hooks_REDACTED_Tests();
        SourceRegistrar.RegisterTestHookSource(instance);
    }
    public global::System.Collections.Generic.IReadOnlyList<StaticHookMethod<TestContext>> CollectBeforeEveryTestHooks()
    {
        return
        [
        ];
    }
    public global::System.Collections.Generic.IReadOnlyList<StaticHookMethod<TestContext>> CollectAfterEveryTestHooks()
    {
        return
        [
        ];
    }
    public global::System.Collections.Generic.IReadOnlyList<InstanceHookMethod> CollectBeforeTestHooks()
    {
        return
        [
        ];
    }
    public global::System.Collections.Generic.IReadOnlyList<InstanceHookMethod> CollectAfterTestHooks()
    {
        return
        [
            new InstanceHookMethod<global::REDACTED.Tests>
            {
                 MethodInfo = typeof(global::REDACTED.Tests).GetMethod("REDACTED", 0, []),
                 Body = (classInstance, context, cancellationToken) => AsyncConvert.Convert(() => classInstance.REDACTED()),
                 HookExecutor = DefaultExecutor.Instance,
                 Order = 0,
            },
        ];
    }
}

Generated Tests:

// <auto-generated/>
#pragma warning disable
using global::TUnit.Core;
using global::System.Reflection;
using global::System.Linq;

namespace TUnit.SourceGenerated;

[global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
file partial class REDACTED_Tests : TUnit.Core.Interfaces.SourceGenerator.ITestSource
{
    [global::System.Runtime.CompilerServices.ModuleInitializer]
    public static void Initialise()
    {
        SourceRegistrar.Register(new REDACTED_Tests());
    }
    public global::System.Collections.Generic.IReadOnlyList<SourceGeneratedTestNode> CollectTests()
    {
        return
        [
            ..Tests0(),
        ];
    }
    private global::System.Collections.Generic.List<SourceGeneratedTestNode> Tests0()
    {
        global::System.Collections.Generic.List<SourceGeneratedTestNode> nodes = [];
        var classDataIndex = 0;
        var testMethodDataIndex = 0;
        try
        {
            var testClassType = typeof(global::REDACTED.Tests);
            var methodInfo = typeof(global::REDACTED.Tests).GetMethod("REDACTED_MethodName", 0, [typeof(global::REDACTED.TestData)]);

            var objectBag = new global::System.Collections.Generic.Dictionary<string, object>();

            var resettableClassFactoryDelegate = () => new ResettableLazy<global::REDACTED.Tests>(() => 
                new global::REDACTED.Tests()
            );

            var resettableClassFactory = resettableClassFactoryDelegate();

            foreach (var methodData in global::REDACTED.Tests.SomethingTest())
            {
                testMethodDataIndex++;

                nodes.Add(new TestMetadata<global::REDACTED.Tests>
                {
                    TestId = $"global::TUnit.Core.MethodDataSourceAttribute:{testMethodDataIndex}:TL-EMDS0:{testMethodDataIndex}:REDACTED.Tests.REDACTED_MethodName(REDACTED.TestData):0",
                    TestClassArguments = [],
                    TestMethodArguments = [methodData],
                    TestClassProperties = [],
                    CurrentRepeatAttempt = 0,
                    RepeatLimit = 0,
                    MethodInfo = methodInfo,
                    ResettableClassFactory = resettableClassFactory,
                    TestMethodFactory = (classInstance, cancellationToken) => AsyncConvert.Convert(() => classInstance.REDACTED_MethodName(methodData)),
                    TestExecutor = DefaultExecutor.Instance,
                    ParallelLimit = null,
                    TestFilePath = @"REDACTED_PATH",
                    TestLineNumber = 165,
                    AttributeTypes = [ typeof(global::TUnit.Core.TestAttribute), typeof(global::System.Runtime.CompilerServices.NullableContextAttribute), typeof(global::System.Runtime.CompilerServices.NullableAttribute), typeof(global::System.Runtime.Versioning.TargetFrameworkAttribute), typeof(global::System.Reflection.AssemblyCompanyAttribute), typeof(global::System.Reflection.AssemblyConfigurationAttribute), typeof(global::System.Reflection.AssemblyFileVersionAttribute), typeof(global::System.Reflection.AssemblyInformationalVersionAttribute), typeof(global::System.Reflection.AssemblyProductAttribute), typeof(global::System.Reflection.AssemblyTitleAttribute), typeof(global::System.Reflection.AssemblyVersionAttribute), typeof(global::System.Reflection.AssemblyMetadataAttribute) ],
                    DataAttributes = [  ],
                    ObjectBag = objectBag,
                });
                resettableClassFactory = resettableClassFactoryDelegate();
            }
        }
        catch (global::System.Exception exception)
        {
            nodes.Add(new FailedInitializationTest
            {
                TestId = $"global::TUnit.Core.MethodDataSourceAttribute:{testMethodDataIndex}:TL-EMDS0:{testMethodDataIndex}:REDACTED.Tests.REDACTED_MethodName(REDACTED.TestData):0",
                TestClass = typeof(global::REDACTED.Tests),
                ReturnType = typeof(global::REDACTED.Tests).GetMethod("REDACTED_MethodName", 0, [typeof(global::REDACTED.TestData)]).ReturnType,
                ParameterTypeFullNames = [typeof(global::REDACTED.TestData)],
                TestName = "REDACTED_MethodName",
                TestFilePath = @"REDACTED_PATH",
                TestLineNumber = 165,
                Exception = exception,
            });
        }
        return nodes;
    }
}
thomhurst commented 3 hours ago

What do you mean by resiliancepipeline?

Vijay-Braidable commented 3 hours ago

It's a retry method encouraged by MS: https://learn.microsoft.com/en-us/dotnet/core/resilience/?tabs=dotnet-cli

The short version is that it creates a cyclic stack trace when it throws multiple exceptions (the pipeline retries fail).

Vijay-Braidable commented 3 hours ago

I span up a PR to try to resolve the issue, I think I found the source (json serialization), but I'm not certain is I'm very new to this project: https://github.com/thomhurst/TUnit/pull/1020

thomhurst commented 3 hours ago

The change you've made is only for the rpc tests. If you're experiencing an issue with standard test execution then the rpc test project is completely separate to that so wouldn't resolve it

Vijay-Braidable commented 3 hours ago

Ah, you're right, my test isn't an RPC test. I'll cancel the PR, though you might want to consider adding that in later to ensure correct serialization when types are cyclical.

Is there anything else I can grab from my test runs that might help locate the issue?

thomhurst commented 3 hours ago

You say it works normally - can you provide source code with this resilience pipeline? I'm unfamiliar with it

Vijay-Braidable commented 2 hours ago

The smallest toy example I can make quickly is without a pipeline:

    [Test]
    public void WhenThrowing_CyclicalException_TestShouldFinish2()
    {
        // Arrange
        var exceptionA = new Exception("Exception A");
        var exceptionB = new Exception("Exception B", exceptionA);

        // Use reflection to set exceptionA's InnerException to exceptionB, creating a cycle
        FieldInfo innerExceptionField = typeof(Exception).GetField("_innerException", BindingFlags.Instance | BindingFlags.NonPublic);
        innerExceptionField.SetValue(exceptionA, exceptionB);

        // Act: Throw exceptionA, which now has a cyclic InnerException reference
        throw exceptionA;
    }

When running this via TUnit it never finishes running the test for me.

Vijay-Braidable commented 2 hours ago

Just in case it's relevant, here is the csproj file used when I made that test:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <IsPackable>false</IsPackable>
    <IsTestProject>true</IsTestProject>

    <PackageId>REDACTED.$(AssemblyName)</PackageId>
    <RootNamespace>REDACTED.$(MSBuildProjectName.Replace(" ", "_"))</RootNamespace>
    <AssemblyName>$(MSBuildProjectName.Replace(" ", "_"))</AssemblyName>

  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="TUnit" />
    <PackageReference Include="FluentAssertions" />
    <PackageReference Include="Moq" />
  </ItemGroup>

</Project>
thomhurst commented 2 hours ago

The smallest toy example I can make quickly is without a pipeline:

    [Test]
    public void WhenThrowing_CyclicalException_TestShouldFinish2()
    {
        // Arrange
        var exceptionA = new Exception("Exception A");
        var exceptionB = new Exception("Exception B", exceptionA);

        // Use reflection to set exceptionA's InnerException to exceptionB, creating a cycle
        FieldInfo innerExceptionField = typeof(Exception).GetField("_innerException", BindingFlags.Instance | BindingFlags.NonPublic);
        innerExceptionField.SetValue(exceptionA, exceptionB);

        // Act: Throw exceptionA, which now has a cyclic InnerException reference
        throw exceptionA;
    }

When running this via TUnit it never finishes running the test for me.

Thanks! I'm out right now so it might be a couple of days before I can look but I'll get on it when I have a chance

Vijay-Braidable commented 1 hour ago

I embedded the test in TUnit.TestProject, I can break on all the pre test running lines, but as soon as the exception is thrown it vanishes into the ether. I'm starting to wonder is this is a deeper problem than the library, but in the new testing platform. If you think of a way for me to easily verify this while you're away please let me know!

thomhurst commented 1 hour ago

It very well could be the new testing platform. Since you think it's related to the json rpc, TUnit doesn't deal with that specifically. And since I might not be available for a couple of days, would you be able to also raise it with Microsoft regarding the new platform?

Their repo is https://github.com/microsoft/testfx

Vijay-Braidable commented 40 minutes ago

Confirmed to exist in the latest version of MSTest, so it seems as though it's an underlying issue.

thomhurst commented 35 minutes ago

Glad to know it's not TUnit haha. But also hoping it gets solved soon!

Vijay-Braidable commented 33 minutes ago

Fingers crossed! It's killing my commit tests atm, I just swapped to TUnit and the new platform. Now do decorate all of my tests with timeouts...

Vijay-Braidable commented 22 minutes ago

So it turns out that the Timeout(TIME_IN_MS) attribute doesn't have a force stop, it's literally just the CancellationToken. I think this means I have to leave my test swap over in a branch and hope it doesn't get too stale while we wait for the fix, unless there is another "Force stop" option I'm missing?

thomhurst commented 13 minutes ago

So it turns out that the Timeout(TIME_IN_MS) attribute doesn't have a force stop, it's literally just the CancellationToken. I think this means I have to leave my test swap over in a branch and hope it doesn't get too stale while we wait for the fix, unless there is another "Force stop" option I'm missing?

This is by design. This isn't really isolated to TUnit either. There isn't really a way to abort threads other than requesting cancellation and then honouring it.

TUnit attempts to shut down early but it's entirely possible the original test method could still be executing in the background if it didn't honour any cancellations.

So I don't know what to suggest here other than use the cancellation token given to you. Aborting threads is a no-no (also TUnit doesn't start threads manually, it leverages the standard thread pool)

Vijay-Braidable commented 7 minutes ago

Sadly, as the thread lives forever once the exception is thrown, using the cancellation token appears to have no impact. Guess I’ll dig into the MSTest code before bed!On 27 Oct 2024, at 00:14, Tom Longhurst @.***> wrote:

So it turns out that the Timeout(TIME_IN_MS) attribute doesn't have a force stop, it's literally just the CancellationToken. I think this means I have to leave my test swap over in a branch and hope it doesn't get too stale while we wait for the fix, unless there is another "Force stop" option I'm missing?

This is by design. This isn't really isolated to TUnit either. There isn't really a way to abort threads other than requesting cancellation and then honouring it. TUnit attempts to shut down early but it's entirely possible the original test method could still be executing in the background if it didn't honour any cancellations. So I don't know what to suggest here other than use the cancellation token given to you. Aborting threads is a no-no (also TUnit doesn't start threads manually, it leverages the standard thread pool)

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>