How do I unit test a method that is expecting an object to be updated via a reference?

Question

I'm having trouble unit testing a method that changes some properties of a reference type that is passed to it.

As an example, let's say I have a class called Policy.

Policy policy = new Policy();
policy.Status = Active;

I then pass this policy to the policy manager in order to inactivate the policy.

policyManager.InactivatePolicy(policy);

The inactivate policy method does the following:

public void InactivatePolicy(Policy policy)
{
    policy.Status = Inactive;
    UpdatePolicy(policy); //saves the updated policy details via nhibernate
}

What I'm having trouble with is unit testing this DoSomething method. (ignore the fact that what it does in this example is useless)

public void DoSomething(Policy policy)
{
    Policy policy = new Policy();
    policy.Status = Active;

    policyManager.InactivatePolicy(policy);
}

Because I mock out policy Manager, the status does not get set to inactive and as a result when I assert that after DoSomething is called the status of the policy is inactive I get a test failure since it is still Active.

[Test]
public void TheStatusShouldBeInactiveWhenWeDoSomething()
{
    Mock<IPolicyManager> policyManagerMock = new Mock<PolicyManager>();
    MyClass mc = new MyClass(policyManagerMock.Object);

    Policy policy = new Policy();
    policy.Status = Active;

    mc.DoSomething(policy);

    Assert.That(policy.Status, Is.EqualTo(Inactive)); //this fails     
}

So I'm in a situation where the code works in reality but just not when isolated in my unit tests.

The only way I've been able to work around this issue is to have the policy manager's InactivatePolicy method return the modified policy so that I can mock up the expected return value.

public Policy InactivatePolicy(Policy policy)
{
    policy.Status = Inactive;
    UpdatePolicy(policy); //saves the updated policy details via nhibernate
    return policy;
}

[Test]
public void TheStatusShouldBeInactiveWhenWeDoSomething()
{
    Mock<IPolicyManager> policyManagerMock = new Mock<PolicyManager>();
    MyClass mc = new MyClass(policyManagerMock.Object);

    Policy expectedInactivePolicy = new Policy();
    expectedInactivePolicy.Status = Inactive;

    Policy policy = new Policy();
    policy.Status = Active;

    policyManagerMock
        .Setup(p => p.InactivatePolicy(policy))
        .Returns(expectedInactivePolicy);   

    mc.DoSomething(policy);

    Assert.That(policy.Status, Is.EqualTo(Inactive)); //this now succeeds

}

Typically when I'm struggling to unit test something it's an indication I'm doing things wrong.

Does anyone know if this there is a better way to do this? Is one essentially forced to return values which were originally intended to have been updated via the reference value that was passed into the method?

Is my problem possibly that the policy manager shouldn't have an InactivatePolicy method but it should instead be on the Policy object itself and the database update called later?

Answer 1

Rambling

I don't quite follow.

If you're not testing the fact that it gets set to true via NHibernate (i.e. you are going to assume that works), then what are you even testing? Why bother testing, at all, that the value gets set, given that in production code you'll just assume it is so? Even if you were to trivially mock up a system that sets it to true, I don't see the point, because it's not the same as the production code.

At best, I would consider having a 'mocking' data store; not nHibernate, that just does nothing. In this case it would be implemented in the UpdatePolicy class, with some 'MockRepo' instead of 'nHibernateRepo'.

In this way, if you set up the repo appropriately, you could see it being set.

Though I would wonder the point of it, because maybe your nHibernate code has a bug, and effectively all you're checking is the setting of a boolean.

Summary

Why not just have a test dev db that you can run this test against?

Answer 2

You shouldn't be mocking out the PolicyManager, you should be mocking out the UpdatePolicy method as you still want to test the functionality of the DoSomething method.

Also, you're probably testing too high up the tree.

You should test the InactivatePolicy() method in isolation and test only that functionality is working and then you should test the DoSomething() method, again in Isolation.

You have 2 separate units of code here and you should have unit tests that test each unit specifically.

Answer 3

I think the test is wrong because actually you are testing the mock object and its affect on Active state of your policy object instead of testing your original object and even if the test passes in real world scenario PolicyManager may behave differently and causes DoSomething to fail. Maybe you'd better test PolicyManager and its UpdateInactive method in a unit test and have a integrity test to test DoSomething along with the real PolicyManager.