Add diagnostics for mock method resolutions

[Arithmomaniac]

I have a MockRepository with MockBehavior.Loose set, and it would be nice to be able to globally or per-mock be able to log information about how each method is resolved (e.g. which expression was used to resolve the call, or whether if was auto-resolved with MockBehavior.Loose).

This would be of big help in fixing unexpected behavior due to default implementations, or right before migrating an existing repository to MockBehavior.Strict.

I was imaging some sort of OnResolution(string resolutionInformation) event.

[stakx]

Some thoughts:

• I think it would be better to log events using the functionality in System.Diagnostics.* than triggering an event, simply because it shouldn't be possible for event consumers to affect that part of Moq operation by e.g. throwing an exception. Logging shouldn't mean that Moq needs to protect itself using catch blocks.

• Should be implemented in a very light-weight way so as not to degrade performance if the feature isn't used.

• Needs a more precise spec. There's lots of things that could be logged: information per call (with or without arguments), information per setup against which a call was matched (even when the match wasn't successful), or only the final result of the call-setup matching. That would probably make the most sense. There would likely be 7 distinct cases:

void methods:

• matches a setup
• does not match any setup, causes failure with MockBehavior.Strict (log message doesn't tell you more than the exception thrown)
• does not match any setup, is ignored

non-void methods

• matches a setup which has a return value configured, lets setup compute the return value
• matches a setup which has no return value configured, returns default(T)
• does not match any setup, causes failure with MockBehavior.Strict (same caveat as above)
• does not match any setup, lets DefaultValue setting determine return value.

So 5 out of these 7 would actually be potentially useful.

Just my 2 cents. :)

[Arithmomaniac]

Agreed that you don't need an event.
It would also be nice to log "Setup considered, but was passed over because the condition for parameter {someParameter} was not met".

[stakx]

@Arithmomaniac - sorry for only getting back to you now, I've been a little busy elsewhere.

I think something might be done at the setup / method level, but I suspect that the feature you're requesting will be too costly regarding runtime performance once you start collecting diagnostic information at the argument / matcher level. Keeping diagnostic info at the method level might be preferable for a start.

My priorities currently lie elsewhere (mainly getting rid of remaining bugs and improving performance) so I won't be working on this, but you're of course free to try to come up with an efficient implementation for this feature and send a PR for review. (I'd also suggest that emitting diagnostic info should be an opt-in feature enabled via the new Switches.)

If you would like to work on this feature, please quickly let us know here. Otherwise, I suggest we close this issue.

[Arithmomaniac]

Understood. Instead of closing it, why not just tag it with a backlog tag?

[stakx]

Instead of closing it, why not just tag it with a backlog tag?

The backlog tag is already there ("up for grabs"). 😉 We can leave your issue open a while longer (let's say, another month or so) to see if it attracts any more interest. However, if there's no more activity—no upvotes, no comments, noone willing to implement, no nothing—I'll close it.

Let me explain why.

A backlog is a great idea if there's a pool of people actively working on it. While Moq has seen a few user contributions here and there during the past couple of months, I've been pretty much the only one working through the backlog.

For that reason, let's be realistic: If you, who opened this issue, aren't willing to work on it, then it's unlikely that anyone else will. (Like I said before, my own main focus is on bugfixes and performance.)

If issues are left open indefinitely, we'll eventually end up with a ton of seemingly abandoned issues, and it's easy for people to perceive this as a sign of a stale or even dead project. This has actually happened. When I started contributing to Moq back in May, there were nearly 160 unresolved issues (mostly reported defects), many of which were older than a year. Several people voiced their impression that Moq appeared to be an abandoned project.

For the sake of Moq, I think we need to avoid returning to that state. And it's for this reason I'd rather be brutally honest and say, "right now, noone's interested enough in this feature to actually work on it" than to keep people waiting and wondering if anything's gonna happen at all forever. Finally, issues can be closed, but this can be seen as merely temporary. They can easily be reopened if there's renewed interest in something.

If you see a better way of handling a backlog of a project with relatively few user contributions, please let me know your thoughts!

[kzu]

Totally agree with the policy btw. Same thing other OSS projects are doing to keep things tidy and manageable: https://twitter.com/bradwilson/status/929046059640225793

[stakx]

@kzu - Glad to hear you approve, and that others are handling things in a similar fashion. It really doesn't feel nice to close issues that are actually good and valid ideas, and at times I've asked myself whether if I've drifted off too far into dictatorship mode. 😨 Cheers!

[Arithmomaniac]

One minor suggestion, though: maybe make a tag for issues that were closed due to inactivity, as opposed to invalid, completed, etc. "up-for-grabs" may do that already.

[stakx]

@Arithmomaniac - That's a good idea! 👍 How about a tag called "unresolved"? "up-for-grabs" would possibly work, too. In either case, we'll have to go through all closed issues and re-tag them properly.

[stakx]

@Arithmomaniac - I went through all closed issues and added the "unresolved" tag, where appropriate. Feel free to pick up any of those issues. 😉