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

ThrowsAsync reports OperationCanceledException as TaskCanceledException #2168

Closed
rouacke opened this issue May 15, 2017 · 9 comments · Fixed by #3095
Closed

ThrowsAsync reports OperationCanceledException as TaskCanceledException #2168

rouacke opened this issue May 15, 2017 · 9 comments · Fixed by #3095
Assignees
Milestone

Comments

@rouacke
Copy link

rouacke commented May 15, 2017

The ThrowsAsync method reports exceptions of type OperationCanceledException as TaskCanceledException instead.

Repro:

Assert.ThrowsAsync<OperationCanceledException>(async () =>
{
    await Task.Yield();
    throw new OperationCanceledException();
});

Result:

Failed : NUnitRepro.Tests.Repro
  Expected: <System.OperationCanceledException>
  But was:  <System.Threading.Tasks.TaskCanceledException: A task was canceled.
   at NUnit.Framework.Internal.AsyncInvocationRegion.AsyncTaskInvocationRegion.WaitForPendingOperationsToComplete(Object invocationResult)
   at NUnit.Framework.Assert.ThrowsAsync(IResolveConstraint expression, AsyncTestDelegate code, String message, Object[] args)>

Other exception types are reported correctly. The following test passes.

Assert.ThrowsAsync<TimeoutException>(async () =>
{
    await Task.Yield();
    throw new TimeoutException();
});

I believe it's possible to workaround this issue using the "constraint model". The following test passes.

Assert.That(async () =>
{
    await Task.Yield();
    throw new OperationCanceledException();
}, Throws.InstanceOf<OperationCanceledException>());

I observed the issue in version 3.5.0. I updated to 3.6.1 and confirmed that the issue is still present in the latest version.

I also installed the console runner and tested from the command line as per the contributing guidelines. Test output below.

C:\...\NUnitRepro\NUnitRepro\bin\Debug>..\..\..\packages\NUnit.ConsoleRunner.3.6.1\tools\nunit3-console.exe NUnitRepro.exe
NUnit Console Runner 3.6.1
Copyright (C) 2017 Charlie Poole

Runtime Environment
   OS Version: Microsoft Windows NT 10.0.14393.0
  CLR Version: 4.0.30319.42000

Test Files
    NUnitRepro.exe


Errors, Failures and Warnings

1) Failed : NUnitRepro.Tests.Repro
  Expected: <System.OperationCanceledException>
  But was:  <System.Threading.Tasks.TaskCanceledException: A task was canceled.
   at NUnit.Framework.Internal.AsyncInvocationRegion.AsyncTaskInvocationRegion.WaitForPendingOperationsToComplete(Object invocationResult)
   at NUnit.Framework.Assert.ThrowsAsync(IResolveConstraint expression, AsyncTestDelegate code, String message, Object[] args)>
at NUnitRepro.Tests.Repro() in c:\...\NUnitRepro\NUnitRepro\Tests.cs:line 16

Run Settings
    DisposeRunners: True
    WorkDirectory: C:\...\NUnitRepro\NUnitRepro\bin\Debug
    ImageRuntimeVersion: 4.0.30319
    ImageTargetFrameworkName: .NETFramework,Version=v4.6
    ImageRequiresX86: True
    RunAsX86: True
    ImageRequiresDefaultAppDomainAssemblyResolver: False
    NumberOfTestWorkers: 4

Test Run Summary
  Overall result: Failed
  Test Count: 3, Passed: 2, Failed: 1, Warnings: 0, Inconclusive: 0, Skipped: 0
    Failed Tests - Failures: 1, Errors: 0, Invalid: 0
  Start time: 2017-05-15 15:43:50Z
    End time: 2017-05-15 15:43:50Z
    Duration: 0.564 seconds

Results (nunit3) saved as TestResult.xml
@appel1
Copy link

appel1 commented May 16, 2017

NUnit waits for the Task by calling its Wait() method and it is that method that discards your OperationCanceledException and instead throws an AggreateException with an inner TaskCanceledException.

[Test]
public void OperationCanceledExceptionIsReplacedByTaskCanceledException()
{
    Assert.That(() => Foo().Wait(), 
        Throws.TypeOf<AggregateException>().With.InnerException.TypeOf<TaskCanceledException>());
}

private static async Task Foo()
{
    await Task.Yield();
    throw new OperationCanceledException();
}

Not sure NUnit can do anything about that.

@jnm2
Copy link
Contributor

jnm2 commented May 17, 2017

I'd much rather NUnit called .GetAwaiter().GetResult() instead of .Wait(). That does not have this (and other) problems.

@CharliePoole
Copy link
Member

@jnm2 Feel free to assign this to yourself. I'm thinking it may not be quite so simple, but let's see.

@jnm2
Copy link
Contributor

jnm2 commented May 20, 2017

@rouacke

I believe it's possible to workaround this issue using the "constraint model". The following test passes.

Assert.That(async () =>
{
    await Task.Yield();
    throw new OperationCanceledException();
}, Throws.InstanceOf<OperationCanceledException>());

That's because TaskCanceledException derives from OperationCanceledException; the equivalent test would actually be Throws.TypeOf<OperationCanceledException>() and that does fail just like Assert.ThrowsAsync<OperationCanceledException>.

@jnm2
Copy link
Contributor

jnm2 commented May 20, 2017

@CharliePoole Implementing this fix will be half a step away from supporting all C# awaitable types.
https://github.com/dotnet/csharplang/blob/master/spec/expressions.md#awaitable-expressions
https://github.com/dotnet/csharplang/blob/master/spec/expressions.md#runtime-evaluation-of-await-expressions

Since C# now supports arbitrary task-like return types on async methods, such as ValueTask and ValueTask<> in the BCL or any other custom async method builder, this has future-proofing value as well.

@jnm2
Copy link
Contributor

jnm2 commented May 21, 2017

Oh, btw. The net40 async polyfill will exhibit the same bad behavior with .GetAwaiter.GetResult() as .Wait() because... it's a polyfill.

If you target net45+ or netstandard, you're fine.

@Tyrrrz
Copy link

Tyrrrz commented Oct 17, 2017

I'm having the same issue. Also, the MSTest framework doesn't have this problem.

@jnm2
Copy link
Contributor

jnm2 commented Oct 17, 2017

Eh... the @nunit/framework-team is waiting on me for this. The work is basically done, but I haven't written all the tests I want us to carry forward. I would like comments on #2181.

@jnm2 jnm2 added this to the 3.12 milestone Nov 24, 2018
@jnm2
Copy link
Contributor

jnm2 commented May 13, 2019

Fixed in the prerelease feed and included in 3.12 which is just around the corner!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants