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

Moq verify uses objects modified in Returns, rather than what was actually passed in #276

Closed
mrzappit opened this issue Aug 1, 2016 · 1 comment

Comments

@mrzappit
Copy link

mrzappit commented Aug 1, 2016

FYI, I have asked this question on Stack Overflow (http://stackoverflow.com/q/38062321/691790), but the only thing I have seen so far are workarounds to this issue.

Background

I have a class that uses NHibernate to persist objects to a database. When you call MergeEntity for an object that does not have an ID set, NHibernate populates that object with an ID when it is returned. In order to make sure I always use the same object as NHibernate is using, I pass that updated object back from my "Save" function.

Problem

I am trying to mock that same behavior using Moq, which is usually very intuitive and easy to use; however, I am having some trouble verifying that the calls to Save() are being made with the correct arguments. I would like to verify that the ID of the object being passed in is zero, then that it is set properly by the Save function. Unfortunately, when I modify the ID in the Moq.Returns() function, the Moq.Verify function uses the modified value rather than the value of the ID that was passed in.

To illustrate, here is a very basic class (I override the ToString() function so my test output will show me what ID was used when the mocked Save() is called):

public class Class1
{
    private readonly IPersistence _persistence;

    /// <summary>Initializes a new instance of the <see cref="T:System.Object" /> class.</summary>
    public Class1(IPersistence persistence)
    {
        _persistence = persistence;
    }

    public int Id { get; set; }

    public void Save()
    {
       _persistence.Save(this);
    }

    public override string ToString()
    {
        return Id.ToString();
    }
}

Here is the Interface (very straight forward):

public interface IPersistence
{
    Class1 Save(Class1 one);
}

And here is the test that I think should pass:

[TestFixture]
public class Class1Tests
{
    [Test]
    public void Save_NewObjects_IdsUpdated()
    {
        var mock = new Mock<IPersistence>();
        mock.Setup(x => x.Save(It.IsAny<Class1>()))
            .Returns((Class1 c) =>
            {
                // If it is a new object, then update the ID
                if (c.Id == 0) c.Id = 1;

                return c;
            });

        // Verify that the IDs are updated for new objects when saved
        var one = new Class1(mock.Object);

        Assert.AreEqual(0, one.Id);

        one.Save();

        mock.Verify(x => x.Save(It.Is<Class1>(o => o.Id == 0)));
    }
}

Unfortunately, it fails saying that it was never called with arguments that match that criteria. The only invocation to Save that was made on the mock is with an object that had an ID of 1. I have verified that the objects have an ID of 0 when they enter the Returns function. Is there no way for me to distinguish what was passed into the mock from what those objects are updated to be if I update the values in my Returns() function?

@stakx
Copy link
Contributor

stakx commented Jun 12, 2017

The only invocation to Save that was made on the mock is with an object that had an ID of 1.

No. The only invocation to Save that was made on the mock is with an object that at the time of verification had an ID of 1.

Moq correctly records the made call, but since Class1 is a reference type, it records that object's identity, not its value. Moq won't magically create a complete snapshot of the passed-in Class1 at the time of the call.

If you changed Class1 to be a struct, you'd get the behavior you expect. But it's probably easier to register a Callback on the setup where you can set a flag, which you can then check using an Assert.

If you don't want to have a callback, you could also look into rewriting your test a different way. Your current test appears to consist of these steps:

  • Check whether the initial state of a Class1 one implies that its ID is 0.
  • Then call that object's Save method.
  • Then (you wanted to) check whether the interface's Save method was called for an object with an ID of 0.

Wouldn't it be more or less the same test if you rewrote the last three lines as:

    Assert.Equal(0, one.Id);
    one.Save();
    mock.Verify(x => x.Save(one));

That is, instead of checking whether the Save method was called with any object with ID == 0, you check whether Save was called with a specific object for which you've previously asserted that it had an ID of 0. In that test, you don't use the same mock.Object to save several different objects, so that rewrite would appear to be a "good enough" solution.

@stakx stakx closed this as completed Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants