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

Not treating Func<T> and Action properly for type compatibility. #290

Closed
manfred-brands opened this issue Sep 2, 2020 · 4 comments · Fixed by #291
Closed

Not treating Func<T> and Action properly for type compatibility. #290

manfred-brands opened this issue Sep 2, 2020 · 4 comments · Fixed by #291
Assignees
Labels
Milestone

Comments

@manfred-brands
Copy link
Member

manfred-brands commented Sep 2, 2020

The UnwrapActualType results in the return type for every delegate.
However NUnit doesn't always evaluate its actual parameter.

Func<bool> function = () => false;
Assert.That(function, Is.Not.Null);
Assert.That(() => function, Is.Not.Null);
Assert.That(() => function(), Is.Not.Null);

Only the last one actually calls the function, but we do also evaluate the first.

The same method is called for the other rules testing for incompatible types.

@mikkelbu
Copy link
Member

mikkelbu commented Sep 3, 2020

EDIT: Ignore the comments below. If I change bool function() => true; to Func<bool> function = () => true; then it all makes sense.

I'm a bit tired, so I'm probably overlooking something :), but I just tried this out and if I paste

bool function() => true;
Assert.That(function, Is.Not.Null);
Assert.That(() => function, Is.Not.Null);
Assert.That(() => function(), Is.Not.Null);

into three different tests (one for each Assert), then the second does not compile - complains with

  • CS1662 - Cannot convert lambda expression to intended delegate type because some of the return types in the block are not implicitly convertible to the delegate return type
  • CS0428 - Cannot convert method group 'function' to non-delegate type 'bool'. Did you intend to invoke the method?
  • CS0201 - Only assignment, call, increment, decrement, await, and new object expressions can be used as a statement

For the first and the third they pass. If I change the constraint to Is.Null, then both fail with

Expected: null
But was:  True

(also If I change the function to have a side-effect like bool function() { Console.WriteLine("TEST"); return true; } then I can see that both the first and the third calls the method).

@mikkelbu
Copy link
Member

mikkelbu commented Sep 3, 2020

I can "avoid" calling function by explicitly calling the That<TActual>(TActual actual, IResolveConstraint expression) overload

Assert.That<Func<bool>>(function, Is.Null);

@manfred-brands
Copy link
Member Author

@mikkelbu I should not have changed the 'code' in the ticket from the code that causes the issue as shown in the PR.

bool functionCalled = false;

Func<bool> function = () =>
{
    functionCalled = true;
    return false;
};

Assert.That(function, Is.Not.Null); // Incorrectly shows NUnit2023
Assert.That(functionCalled, Is.False);

Assert.That(() => function, Is.Not.Null);
Assert.That(functionCalled, Is.False);

Assert.That(() => function(), Is.Not.Null); // Correctly flags NUnit2023
Assert.That(functionCalled, Is.True);

Similar for Action:

bool actionCalled = false;

Action action = () => actionCalled = true;

Assert.That(action, Is.Not.Null); // Incorrectly shows NUnit2023 
Assert.That(actionCalled, Is.False);

Assert.That(() => action, Is.Not.Null);
Assert.That(actionCalled, Is.False);

Assert.That(() => action(), Is.Not.Null); // Incorrectly flags NUnit2023!
Assert.That(actionCalled, Is.True); // Fails as the test above does not call the action

Note the difference in behaviour by NUnit itself.

Assert.That(() => function(), Is.Not.Null);  // Actually calls function
Assert.That(() => action(), Is.Not.Null);  // Does not call action

The last call invokes the Assert overload Asserts that the code represented by a delegate throws an exception.
If used with a Throws constraint, it will call the action.

Assert.That(() => action(), Throws.Nothing);

@mikkelbu
Copy link
Member

mikkelbu commented Sep 4, 2020

No problem. I looked at the PR after writing the first comments. I'll take a closer look at the code now.

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

Successfully merging a pull request may close this issue.

2 participants