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

Provide better error reporting in Verify() #84

Closed
ashmind opened this issue Jan 16, 2014 · 5 comments
Closed

Provide better error reporting in Verify() #84

ashmind opened this issue Jan 16, 2014 · 5 comments

Comments

@ashmind
Copy link
Contributor

ashmind commented Jan 16, 2014

Example of reporting I get in 4.1.1311.615:

Moq.MockException : 
Expected invocation on the mock at least once, but was never performed:
r => r.Save(It.Is<Entity>(e => e.Description == .expectedDescription))
No setups configured.

Performed invocations:
IRepository`2.Save(My.Entity)

Reporting that would be more useful:

Moq.MockException : 
Expected invocation on the mock at least once, but was never performed:
r => r.Save(It.Is<Entity>(e => e.Description == "ABC"))
No setups configured.

Performed invocations:
IRepository`2.Save(My.Entity { Description = "XYZ" })

As for the second point, I have done similar thing once -- can be done by finding all distinct property/method paths in expression tree passed to It.Is and requesting their values for actual argument.

@icalvo
Copy link

icalvo commented Nov 5, 2015

I think that for objects you have a nice workaround most of the times, which is to override ToString(). But if you do not have control over the class, this would be nice to have. You have to take into account the possible length of the resulting string, and also circular references if you recursively print property values. Not trivial at all!

Also interesting for enhancing reporting is when you have array parameters, which is specially useful for params methods. Also in-memory collections such as Dictionary, List and Collection. I'd not recommend to implement it for IEnumerable or other interfaces because you could have a huge (or even infinite) sequence there. The same caveats about the length of the resulting string and circular references apply.

@stakx
Copy link
Contributor

stakx commented Jun 12, 2017

I think this idea has a lot of potential. But I don't think it would be good to decide on one fixed way of reporting additional object data, because of the problems this could cause (see @icalvo's post above).

I would suggest that this new functionality be exposed in a slightly more general way. For example, a new public contract could be introduced, say:

namespace Moq.Diagnostics
{
    public interface IObjectFormatter
    {
        string Format(object obj);
    }
}

Exactly one instance of this new interface type could then be registered on a Mock or MockRepository. This would then be used in messages to (you guessed it) create a string representing a specific object. Such a string could consist only of the type name, but it could also include the property names and values of an object, or the first n items of a sequence, etc.

By default, Moq would be using a DefaultObjectFormatter implementation that produces the exact same output as Moq does today.

If the questions raised in the post above can be solved such that the resulting formatting would work well for most objects, Moq could ship with one (or several) additional concrete implementations in the new Moq.Diagnostics namespace that people could use to quickly switch over to more advanced error reporting.

@stakx
Copy link
Contributor

stakx commented Nov 12, 2017

Closing this issue due to inactivity. It also belatedly occurred to me that it wouldn't be straightforward to figure out when to use an object's .ToString() and when to perform extended formatting. Implementing this feature could easily lead to simple types such as decimal or DateTime be formatted as objects, which might be unexpected.

@stakx stakx closed this as completed Nov 12, 2017
@ashmind
Copy link
Contributor Author

ashmind commented Nov 21, 2017

Implementing this feature could easily lead to simple types such as decimal or DateTime be formatted as objects, which might be unexpected.

Well, an easy workaround would be to ignore all IFormattable for this feature, which is not precise, but might be enough in practice.

But even if we don't, that would just provide some extra information for those:

Expected invocation on the mock at least once, but was never performed:
r => r.SetDate(It.Is<DateTime>(d => d.Ticks == 12345))
No setups configured.

Performed invocations:
Something.SetDate(11/21/2017 8:00:11 PM { Ticks = 636468912254430306 })

@kamranayub
Copy link

kamranayub commented Jul 19, 2022

This would be super valuable... with BDD-style testing, I often have to use Verify and I am only asserting a few values match an expression on a complex object. Right now, it's not easy (read: impossible?) to figure out what values in the expression didn't match without debugging.

Example:

_context.MailNotifier.Verify(
                x => x.SendNotification(It.Is<NotificationViewModel>(n =>
                    n.Username == currentUser.Username &&
                    n.IsOptional)), Times.Once);

Either expression could be false and I won't know which. This is a simple example, when I do things like "StartsWith(str)" it becomes even more annoying to figure out why it failed.

I mention BDD-style specifically Setup calls are separated from Verify calls, sometimes between classes, so I can't use Callback easily to capture args (also that feels smelly). I'm using SpecFlow as my BDD framework, for example.

what I want

What I want is what I can do in Jest projects (pseudo code equivalent of above):

expect(_context.mailNotifier.sendNotification).toHaveBeenCalledWith(expect.objectContaining({
  username: currentUser.username,
  isOptional: true
});

When this expectation fails, Jest gives me a full diff of the properties I was comparing.

Perhaps for Moq, another similar API could allow the same thing?

_context.MailNotifier.Verify(
  x => x.SendNotification(It.IsObjectContaining<NotificationViewModel>()
    .Where(n => n.Username, Is.EqualTo(currentUser.Username)
    .Where(n => n.IsOptional, Is.True)
  )
), Times.Once);

This has the added benefit of reusing existing Is.(...) API 👍

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

4 participants