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

Any reason there is no Mock.Of<>() overload with MockBehavior? #154

Closed
JohanLarsson opened this issue Feb 11, 2015 · 13 comments
Closed

Any reason there is no Mock.Of<>() overload with MockBehavior? #154

JohanLarsson opened this issue Feb 11, 2015 · 13 comments

Comments

@JohanLarsson
Copy link
Contributor

public static T Of<T>(Expression<Func<T, bool>> predicate, MockBehavior behavior) where T : class

I think ^ could be useful but don't know if there is a reason it is not there. Hence this question.

@JohanLarsson
Copy link
Contributor Author

Related question: Does mock.VerifyAll(); do anything if not strict? If not it should throw if called when not strict.

@kzu
Copy link
Member

kzu commented Feb 12, 2015

VerifyAll is unaffected by MockBehavior. It verifies that all setups have
been invoked.

It could be that in a loose mock you do want to verify certain setups. And
it could be that in a strict mock some setups were not invoked.

As to the previous question: the Linq provider ends up calling
SetupProperty on all the members used in the "query", so it couldn't be a
strict mock. BUT, I think that should be doable if the mock is initially
created as loose and later changed to strict internally, after the
expressions are evaluated...

Do you want to take a shot at a PR to fix that? That would be very helpful!

Thanks in advance

/kzu

Daniel Cazzulino | Team Lead | Xamarin for Visual Studio

On Thu, Feb 12, 2015 at 3:01 PM, Johan Larsson notifications@github.com
wrote:

Related question: Does mock.VerifyAll(); do anything if not strict? If not
it should throw if called when not strict.

Reply to this email directly or view it on GitHub
#154 (comment).

@JohanLarsson
Copy link
Contributor Author

Hopefully I can give it a try this weekend. Will update here if I give up or can't find time for it.
Thanks for a beautiful lib btw!

@kzu
Copy link
Member

kzu commented Feb 13, 2015

Keep us posted!

Thanks again for volunteering to look into this :)

/kzu from mobile

On Feb 12, 2015 7:18 PM, "Johan Larsson" notifications@github.com wrote:

Hopefully I can give it a try this weekend. Will update here if I give up
or can't find time for it.
Thanks for a beautiful lib btw!

Reply to this email directly or view it on GitHub
#154 (comment).

@JohanLarsson
Copy link
Contributor Author

https://gitter.im/oxyplot/oxyplot
^ Gitter is pretty nice.

@stakx stakx added the question label Jun 21, 2017
@stakx
Copy link
Contributor

stakx commented Jun 21, 2017

@JohanLarsson: Just a friendly ping. It's been a while since this thread was active. Did you ever get around to look into this? Do you still think this should be followed up at all?

@JohanLarsson
Copy link
Contributor Author

I wrote a PR with it but there was a whitespace mess so it was never merged. Then I forgot about it many times. Here is the diff, agreed it is messy.

@stakx
Copy link
Contributor

stakx commented Jun 21, 2017

OK, thanks for letting me know. This will probably be difficult to merge due to the diff's relative age. Nevertheless, I should eventually get around to taking a closer look at it. Once I do, I'll report back here.

@JohanLarsson
Copy link
Contributor Author

I should clean it up.

Adding public static T Of<T>(MockBehavior behavior) where T : class is trivial and does not need many tests.
I'll PR it tomorrow.

The overload public static T Of<T>(MockBehavior behavior, Expression<Func<T, bool>> predicate) where T : class is a bit trickier but the code & tests are already written.

@JohanLarsson
Copy link
Contributor Author

Maybe there will not be a PR, I cloned it and tried building but it looked like a couple of hours of dev tool fighting. Is there a readme for how to build it?

image

image

image

Tried both Moq.sln & Moq.NetCore.sln, did just click build in VS.

@stakx
Copy link
Contributor

stakx commented Jun 21, 2017

This looks as if the NuGet package restore didn't kick in. AttributesToAvoidReplicating comes from the Castle.Core NuGet package, IFluentInterface from the IFluentInterface NuGet package.

You should be able to open either of the two solutions you mentioned. I am working on Moq in VS 2017, works fine.

@JohanLarsson
Copy link
Contributor Author

Got it to build now, looks like having more than one package source confused nuget.exe.
Paket.exe is nice btw.

@stakx
Copy link
Contributor

stakx commented Nov 24, 2017

I closed the PR accompanying this issue a while ago, but overlooked the issue and left it open.

Any reason there is no Mock.Of<>() overload with MockBehavior?

After looking into Moq's performance issues some more I've actually found one reason against this proposed additional overload: compared to new Mock<T>(), Mock.Of<T>() is super slow (slower by more than 1 order of magnitude) because it performs recursive property setups and some more work.

Given that the only time when you'd use the new Mock.Of<T>(MockBehavior) is to get a strict mock that isn't to be invoked (as you showed in the PR), all the additional work of setting up all properties is actually superfluous.

Thus adding this new overload at a time when the performance of Mock.Of<T>() hasn't yet been much improved would make it very easy for people to write inefficient (slow) tests.

The only real advantage here would be that Mock.Of<T>(MockBehavior.Strict) is a little neater than new Mock<T>(MockBehavior.Strict).Object, but that alone isn't worth the performance penalty IMO.

I suggest people who want that overload write their own utility method that returns a strict mocked object; adding this to Moq's main library at this time would be counterproductive.

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

No branches or pull requests

3 participants