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

Does SetupAllProperties actually create any setups, or does it not? #681

Closed
stakx opened this issue Sep 4, 2018 · 3 comments · Fixed by #684
Closed

Does SetupAllProperties actually create any setups, or does it not? #681

stakx opened this issue Sep 4, 2018 · 3 comments · Fixed by #684

Comments

@stakx
Copy link
Contributor

stakx commented Sep 4, 2018

Using Moq 4.9.0, given this:

public interface T
{
    T P { get; }
}

var mock = new Mock<T>();
mock.SetupAllProperties();

mock.Verify(m => m.P, Times.Once) will produce this error message:

Expected invocation on the mock once, but was 0 times: m => m.P
No setups configured.
No invocations performed.

mock.VerifyAll() will produce a contradictory error message:

The following setups on mock 'Mock<T:00000001>' were not matched:
T m => m.P

One error message says that there are no setups, while the other says that there is a setup. Which one is (or should be) correct?

@stakx
Copy link
Contributor Author

stakx commented Sep 4, 2018

In my opinion, mock.Verify(…) has it right: SetupAllProperties should not create any setups (meaning that VerifyAll should succeed).

Despite its name (which includes the word Setup), the intent behind SetupAllProperties is to conveniently "auto-implement" all properties such that they remember the values they're being set to.

VerifyAll is currently quite useless when a prior call to SetupAllProperties was made, because it then requires that each and every property accessor be invoked. The more properties a mocked type has, the more unlikely it becomes that someone is going to do just that. Therefore, it would probably be more useful in practice for VerifyAll to ignore properties "set up" by SetupAllProperties.

I'd love to get some input. What do others think about this?

@kzu
Copy link
Member

kzu commented Sep 5, 2018

Absolutely, you got it right @stakx: SetupAllProperties is just a way to get useful stubs.

@stakx
Copy link
Contributor Author

stakx commented Sep 5, 2018

Great, thanks. I'll then probably change the error messages & VerifyAll implementation so this works more seamlessly.

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.

2 participants