Mock setup even if dependency is called in a private method

[radeilic]

Hello,

I love the feature that generates setup for mocks that is generated when call to a dependency is detected. However, if tested method calls private method, and mocked dependency is called in that private method, mock setup will not be created for that call. This happens a lot so i think it would be a great addition!

[mattwhitfield]

Great idea ❤️

How far would you go with it? If that private method called another method that then called the dependency - would you want it to detect that?

Obviously it's a tree, and the deeper we inspect that tree the slower it becomes...

[radeilic]

Yes, i would like to detect that as well because it is very common to go 2 methods deep.

Sometimes it can be a graph, as there are recursive methods, and two methods can call each other so we need to make sure the method is not checked more than once to avoid an infinite loop.

Do you really think we would get a significant performance downgrade because of it? I would suggest we do performance testing before merging to see where that point is. Even if it is 30 methods checked, for example, we can set a limit of 30 methods checked, and still cover most real-life cases. And if you happen to have a method with more sub-private methods, that is a good sign you need to refactor or split your class!

In an ideal world, it would be best to go all the way down (and wide). Because every private method that is directly or indirectly called by the public method is essentially part of that public method and should be covered by unit tests.

[mattwhitfield]

The problem is that when we look at a method, we're effectively looking at a textual representation. So we know that something looks like a method call, but we have to query the semantic model. The other issue is that it's not just method calls...

I might have

void PublicMethod()
{
    SomePrivateMethod();
}

But I may also have

void PublicMethod()
{
    LoggingContext.Run(() => SomePrivateMethod());
}

or

void PublicMethod()
{
    Action a = () => SomePrivateMethod();
    a();
}

So, effectively, we would say that both of those code samples called SomePrivateMethod - even though in the abstract syntax tree the reference to SomePrivateMethod is actually contained within a parameter to another method call or a variable. In the case of a variable it's significantly harder to know whether the method is, in fact, called. Maybe it's passed to some long running operation as a callback.

And each time you come across something that looks like a method call - that's another semantic model query.

I will definitely give it a go - it's just a bunch more complex than it seems at first glance.

[radeilic]

True, that is complex. Although if you make lets say v1, that doesn't cover complex cases you mentioned, it would still be huge improvement!

[mattwhitfield]

I noticed you forked the repo - if you wanted to have a go at this yourself, you'd want to start with https://github.com/mattwhitfield/Unitverse/blob/master/src/Unitverse.Core/Helpers/InvocationExtractor.cs

Basically that's currently called for the method being tested - and you'd want to modify it so that VisitInvocationExpression finds any invocations that can be resolved in the current class and then collects from those - probably by calling node.Accept() on the resolved method definition, while protecting against double visits. 👍

[mattwhitfield]

Let me know if you're intending to have a go at implementing it - if so I'm happy to wait. If not then I'll probably get to it in a few days 👍

[radeilic]

I have a working version on my fork, just have to finish testing and check some things. Will open PR in the next couple of days.

[mattwhitfield]

That is awesome - really appreciate it - thank you!

[mattwhitfield]

This is integrated in 0.177 which is now live on the marketplace 😁

Thanks again for the contribution! ❤

I'll leave this open just to wait to make sure it's working for you in the scenario that you intended 👍

[mattwhitfield]

Closing this now - thanks again for the contribution!