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

NUnit2023 fires on variables of type Task #258

Closed
manfred-brands opened this issue Jul 10, 2020 · 4 comments · Fixed by #262
Closed

NUnit2023 fires on variables of type Task #258

manfred-brands opened this issue Jul 10, 2020 · 4 comments · Fixed by #262
Milestone

Comments

@manfred-brands
Copy link
Member

The following code results in an 'The type of the actual argument - 'void' - can never be null.

Task task = Task.Run(Do.Nothing);
Assert.That(task, Is.Not.Null);

The source code does an Unwrap of an awaitable type regardless if it is actually awaited:

            if (actualType.IsAwaitable(out var awaitReturnType))
                actualType = awaitReturnType;

Roslyn should already do that for us when called like:

                Task task = Task.Run(Do.Nothing);
                Assert.That(await task, Is.Not.Null);
@manfred-brands
Copy link
Member Author

Related to this:
Raising NUnit2020 when comparing task for Reference:

Task task = Task.Run(Do.Nothing);
Task same = task;
Assert.That(same, Is.SameAs(task));

Raising NUnit2021 when comparing tasks for Equals:

Task task = Task.Run(Do.Nothing);
Task same = task;
Assert.That(same, Is.EqualTo(task));

@Dreamescaper
Copy link
Member

The source code does an Unwrap of an awaitable type regardless if it is actually awaited.

Yes, because NUnit waits for tasks in asserts, you don't have to await it explicitly, i.e. this one is perfectly valid:

var task = Task.FromResult(3);
Assert.That(task, Is.EqualTo(3));

Still, probably non-generic Task case was missed here.

@manfred-brands
Copy link
Member Author

Are you sure about NUnit implicitly waiting?

Your example fails on my machine:

Expected: 3
But was: <System.Threading.Tasks.Task`1[System.Int32]>

I fixed my original problem by changing the code in AssertHelper.UnwrapActualType to:

            if (actualType.IsAwaitable(out var awaitReturnType) && awaitReturnType.SpecialType != SpecialType.System_Void)
                actualType = awaitReturnType;

Are you happy for me to raise a PR for that?

manfred-brands added a commit to manfred-brands/nunit.analyzers that referenced this issue Jul 13, 2020
@Dreamescaper
Copy link
Member

No, you're right.

I mix it up with ActualValueDelegate overload, so this is correct in NUnit:
Assert.That(() => Task.FromResult(3), Is.EqualTo(3));

I suppose that Task handling should be reworked to account for them better.

manfred-brands added a commit to manfred-brands/nunit.analyzers that referenced this issue Jul 14, 2020
manfred-brands added a commit to manfred-brands/nunit.analyzers that referenced this issue Jul 14, 2020
mikkelbu added a commit that referenced this issue Jul 17, 2020
@mikkelbu mikkelbu added this to the Release 0.4 milestone Jul 17, 2020
manfred-brands added a commit to manfred-brands/nunit.analyzers that referenced this issue Jul 22, 2020
manfred-brands added a commit to manfred-brands/nunit.analyzers that referenced this issue Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants