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

Calling the same method with the same object does not differentiate the delta at call time #531

Closed
chrishaland opened this issue Nov 24, 2017 · 3 comments

Comments

@chrishaland
Copy link

chrishaland commented Nov 24, 2017

Considering the following unit test, where the subject calls the same method with the same object twice, I expect that you can verify that the method is called twice, and being able to distinguish the two calls.

The premise is that you can't go into and make changes to the house, if the doors are locked. If the doors are locked you first have to open them before making changes.

It seems that verifying method calls just keep the object reference and don't make a copy of the object at the time of the method call. I expect the first two assertions to be verified, differentiating the changes, while it actually verifies the last assertion.

[TestFixture]
    public class Test
    {
        private Mock<IUpdater> _updateMock;
        private RemoveTvCommand _subject;

        [SetUp]
        public void Before()
        {
            _updateMock = new Mock<IUpdater>();
            _subject = new RemoveTvCommand(_updateMock.Object);
        }

        [Test]
        public void Verify_should_differentiate_the_object_on_update_call()
        {
            var tv1 = new Tv {Id = 1};
            var tv2 = new Tv {Id = 2};

            var house = new House
            {
                Id = 1,
                DoorStatus = DoorStatus.Locked,
                Tvs = new List<Tv> {tv1, tv2}
            };

            _subject.Handle(house);

            _updateMock.Verify(m => m.Update(It.IsAny<House>()), Times.Exactly(2));

            // Expected
            _updateMock.Verify(m => m.Update(It.Is<House>(h => h.Id == house.Id && h.DoorStatus == DoorStatus.Open && h.Tvs.Contains(tv1) && h.Tvs.Contains(tv2))), Times.Once);
            _updateMock.Verify(m => m.Update(It.Is<House>(h => h.Id == house.Id && h.DoorStatus == DoorStatus.Locked && !h.Tvs.Any())), Times.Once);

            // Actual
            _updateMock.Verify(m => m.Update(It.Is<House>(h => h.Id == house.Id && h.DoorStatus == DoorStatus.Locked && !h.Tvs.Any())), Times.Exactly(2));
        }
    }

    public class RemoveTvCommand
    {
        private readonly IUpdater _updater;

        public RemoveTvCommand(IUpdater updater)
        {
            _updater = updater;
        }

        public void Handle(House house)
        {
            var doorStatus = house.DoorStatus;
            var isClosed = house.DoorStatus == DoorStatus.Locked;

            if (isClosed)
            {
                house.DoorStatus = DoorStatus.Open;
                _updater.Update(house);
            }

            if (house.Tvs == null || !house.Tvs.Any()) return;

            foreach (var tv in house.Tvs.ToList())
            {
                house.Tvs.Remove(tv);
            }

            if (isClosed)
            {
                house.DoorStatus = doorStatus;
            }

            _updater.Update(house);
        }
    }

    public interface IUpdater
    {
        void Update(House house);
    }

    public class House
    {
        public int Id { get; set; }
        public DoorStatus DoorStatus { get; set; }
        public List<Tv> Tvs { get; set; }
    }

    public class Tv
    {
        public int Id { get; set; }
    }

    public enum DoorStatus
    {
        Locked,
        Open
    }
@stakx
Copy link
Contributor

stakx commented Nov 24, 2017

Moq does not capture whole object graphs for each proxy invocation (and that is not going to change because the memory burden would likely outweigh the rare advantages—imagine someone mocking an Entity Framework context and Moq trying to capture the context's complete state!). At call time, the actual argument values are captured, nothing else. Therefore, since House is a reference type, at the time of the calls, only the object reference to some House is captured, not the specific state that House is in at that time. Your IUpdater keeps changing the same House object's state, Verify only sees the final state.

A few options:

  1. You could make House a value type, but that change probably wouldn't make much sense for your object model.

  2. Moq has Capture matchers and CaptureMatch<T> which you can use to capture argument values at the time of invocation—or, in your case, you'd want to capture a House's current state instead of just the House object reference—into a collection, which you can then run Assert's against. It takes a little bit of work to set things up and it's not super-neat, but it's feasible.

    See also Set Times expectation on Setup #373, where @IanYates83 submitted a PR to solve pretty much the same problem.

  3. Another way of approaching your scenario might be to use a strict mock and conditional setups. Something like this:

    var updateMock = new Mock<IUpdater>(MockBehavior.Strict);
    
    // don't allow house updates when `house` is locked:
    updateMock.When(() => house.DoorStatus == DoorStatus.Locked)
              .Setup(u => u.Update(house))
              .Throws(new InvalidOperationException());
    
    // allow house update when `house` is open and for any house with some specific TVs inside:
    updateMock.When(() => house.DoorStatus == DoorStatus.Open)
              .Setup(u => u.Update(It.Is<House>(h => h == house && h.Tvs.Contains(...) && h.Tvs.Contains(...))));
    ...
    // check that the house was updated twice:
    // (if the house was updated while locked, you won't get here due to the thrown exception):
    updateMock.Verify(u => u.Update(house), Times.Exactly(2));

@stakx stakx closed this as completed Nov 24, 2017
@chrishaland
Copy link
Author

Understandable, especially considering larger, complex models, like a EF context. Thanks for detailed respone!

@stakx
Copy link
Contributor

stakx commented Nov 24, 2017

You're welcome! :)

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