8000 [NUnitLite] don't use System.Reflection in `async` tests by jonathanpeppers · Pull Request #9904 · dotnet/android · GitHub
[go: up one dir, main page]

Skip to content

Conversation

jonathanpeppers
Copy link
Member

The current NUnit implementation can result in failures under NativeAOT:

Mono.Android.NET_Tests, Xamarin.Android.NetTests.AndroidMessageHandlerNegotiateAuthenticationTests.RequestWithCredentialsSucceeds / Release
System.NullReferenceException : Arg_NullReferenceException
   at NUnit.Framework.Internal.AsyncInvocationRegion.AsyncTaskInvocationRegion.WaitForPendingOperationsToComplete(Object) + 0x60
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunAsyncTestMethod(TestExecutionContext) + 0x45

The original code in NUnitLite uses System.Reflection, because it needed to run in very limited contexts like Silverlight, portable class libraries, etc.

To solve this, let's just stop using System.Reflection, and just do:

if (invocationResult is Task task)
{
    task.Wait();
}

Or in another case:

if (exception is AggregateException ae)
    return ae.InnerExceptions;

NUnitLite also called the Result property on any Task<T>. I simply removed this functionality, as we have no tests using this feature.

The current NUnit implementation can result in failures under
NativeAOT:

    Mono.Android.NET_Tests, Xamarin.Android.NetTests.AndroidMessageHandlerNegotiateAuthenticationTests.RequestWithCredentialsSucceeds / Release
    System.NullReferenceException : Arg_NullReferenceException
       at NUnit.Framework.Internal.AsyncInvocationRegion.AsyncTaskInvocationRegion.WaitForPendingOperationsToComplete(Object) + 0x60
       at NUnit.Framework.Internal.Commands.TestMethodCommand.RunAsyncTestMethod(TestExecutionContext) + 0x45

The original code in NUnitLite uses System.Reflection, because it
needed to run in very limited contexts like Silverlight, portable
class libraries, etc.

To solve this, let's just stop using System.Reflection, and just do:

    if (invocationResult is Task task)
    {
        task.Wait();
    }

Or in another case:

    if (exception is AggregateException ae)
        return ae.InnerExceptions;

NUnitLite also called the `Result` property on any `Task<T>`. I simply
removed this functionality, as we have no tests using this feature.
@jonathanpeppers jonathanpeppers marked this pull request as ready for review March 10, 2025 20:56
@jonathanpeppers jonathanpeppers merged commit 155709f into main Mar 11, 2025
58 checks passed
@jonathanpeppers jonathanpeppers deleted the dev/peppers/nunitlite1 branch March 11, 2025 13:37
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0