views:

49

answers:

3

Here is the code:

public interface IAccessPoint
{
    int BackHaulMaximum { get; set; }

    bool BackHaulMaximumReached();
    void EmailNetworkProvider();
}

public class AccessPoint : IAccessPoint
{

    public int BackHaulMaximum { get; set; }

    public bool BackHaulMaximumReached()
    {
        if (BackHaulMaximum > 80)
        {
            EmailNetworkProvider();
            return true;
        }
        return false;
        }

    public void EmailNetworkProvider()
    {

    }
}

//Test
[Test]
public void NetworkProviderShouldBeEmailedWhenBackHaulMaximumIsReached()
{
        var apMock = MockRepository.GenerateMock<IAccessPoint>();

        apMock.Stub(x => x.BackHaulMaximum).Return(81);

        Assert.AreEqual(true, apMock.BackHaulMaximumReached());

        apMock.AssertWasCalled(x => x.EmailNetworkProvider());
 }
+4  A: 

You shouldn't be mocking the class that you are testing. You should only be mocking the classes that class under test depends on. Something like this:

public interface IAccessPoint
{
    int BackHaulMaximum { get; set; }

    bool BackHaulMaximumReached();
    void EmailNetworkProvider();
}

public class AccessPoint : IAccessPoint
{
    private IMailProvider Mailer { get; set; }

    public AccessPoint( IMailProvider provider )
    {
        this.Mailer = provider ?? new DefaultMailProvider();
    }

    public int BackHaulMaximum { get; set; }

    public bool BackHaulMaximumReached()
    {
        if (BackHaulMaximum > 80)
        {
            EmailNetworkProvider();
            return true;
        }
        return false;
        }

    public void EmailNetworkProvider()
    {
        this.Mailer.SendMail(...);
    }
}

[Test]
public void NetworkProviderShouldBeEmailedWhenBackHaulMaximumIsReached()  
{  
    var mailerMock = MockRepository.GenerateMock<IMailProvider>();  

    mailerMock .Expect( m => m.SendMail( ... specify argument matches ... ) ); 

    var accessPoint = new AccessPoint( mailerMock ); 

    accessPoint.BackHaulMaximum = 81;

    Assert.IsTrue( accessPoint.BackHaulMaximumReached() );

    mailerMock.VerifyAllExpectations();
}
tvanfosson
I am a little confused, can you clarify.
Xaisoft
I disagree. If functionality of a single method of a class is being tested, and that functionality has side effects (like sending out an e-mail), then by all means mock it.
KeithS
What if my Email method is in my AccessPoint class and this still does not tell me why BackHaulMaximumReached() returns false instead of true.
Xaisoft
@KeithS - if he were mocking the class and not the interface that might be possible, but it's not going to work in this case. In any event, that's not how I would choose to use a mock. If you can't tell from state that the method has performed properly and need to verify that it "did" something else, then I would say that action is a prime candidate for it's own class because it's more than likely non-related directly to the function of the class under test.
tvanfosson
@Xaisoft - it returns false because you didn't supply an expectation for it and it's simply returning the default. Note that you're mocking the interface and not the class and it doesn't know that your class implementation is what it should be using because you didn't tell it.
tvanfosson
@Xaisoft - not knowing a whole lot about your design I can't really tell you whether an access point really ought to be responsible for sending email -- I suspect not. Generally, you'd want an email client class for that purpose (as I've shown in my example). You can then mock it and verify that your access point is making use of it properly given the circumstances.
tvanfosson
I thought by doing apMock.Stub(x => x.BackHaulMaximum).Return(81);, I am assigning BackHaulMaximum the value of 81 on my mock object and my mock object is going to call BackHaulMaximumReached which should return true. This is what is confusing?
Xaisoft
@Xaisoft - you're mocking the interface, not the class. BackHaulMaximumReached() doesn't have an implementation on the interface. It simply doesn't know what code to run if you don't supply an expectation and thus returns the default for a bool, which is false. If you had created a stub of the class instead of the interface, it might work because then it would at least have a default implementation from the class to fall back on. I don't ever mock the class under test so I can't tell you exactly how it would work - I can only say that I think it's a bad idea, likely leading to bad design.
tvanfosson
tvanfosson is correct. I'd recommend hand rolling your own test doubles before you use a mocking framework. It's easy to tie yourself in knots if you go straight to using a framework without first understanding the purpose of mocks/stubs/spies/fakes etc. I've seen the exact same question asked on SO about 5+ times. http://defragdev.com/blog/?p=377
Mark Simpson
Mark, Thanks for the link. My OO design is not very good. Tvanfosson, What you are saying makes more sense? The only thing confusing is when you say you don't ever mock the class under test. Wouldn't you want to mock the class you are testing? Am I missing the point of what you are trying to say?
Xaisoft
@Xaisoft - no, you want to mock the classes that the class under test depends on, specifically to isolate it's behavior from its dependencies. A mock is a substitute and you wouldn't want to be testing a substitute, you'd want to test the real thing.
tvanfosson
A: 

I agree with tvanfosson's answer. 99% (maybe 100%) of the time, you won't want a mock for your SUT.

However the reason it is failing is because your call:

Assert.AreEqual(true, apMock.BackHaulMaximumReached()); 

is not going to result in the actual method BackHaulMaximumReached() being called! It's going to call the mock's method.

Edit

If it wasn't clear, that means that BackHaulMaximumReached() will (I think) return the default value for its return type, which is "false" in the case of bools. So you need to stub out that method to return true to get the Assert.AreEqual to to pass (which is not a good test as I said).

Your next assertion will then fail, as EmailNetworkProvider() will never get called (again, because you are calling the mock's methods, not the actual SUT's methods).

Phil Sandler
What does SUT mean?
Xaisoft
"Subject Under Test". In other words, the class (or method, etc.) you are trying to test.
Phil Sandler
A: 

I would change the code as follows:

    //Test
    [Test]
    public void NetworkProviderShouldBeEmailedWhenBackHaulMaximumIsReached()
    {

        var mockRepo = new MockRepository();
        var apMock = mockRepo.PartialMock<AccessPoint>();

        using (mockRepo.Record())
        {
            Expect.Call(apMock.EmailNetworkProvider).Repeat.Once();
        }
        using (mockRepo.Playback())
        {
            apMock.BackHaulMaximum = 81;
            Assert.AreEqual(true, apMock.BackHaulMaximumReached());
        }
        mockRepo.VerifyAll();
    }

What you're expecting is that given the input to the class, when you call a method, another method that induces some unwanted side effects is also called. You want to mock the side-effect-inducing behavior, but you want to exercise the rest of the actual logic.

The solution is a PartialMock. It allows the mocking behavior to be applied to only the members of the mocked class that you specify. You can use it in several syntaxes, but the safest, most reliable method is to record and playback expectations rather than call the Expects() method on the mock itself.

We are using it here to expect a call on our method that we want to mock; expecting the call will cause the actual method not to be called. Then, the class is exercised, and the logic of the actual class is used (hence the assertion in our test succeeds), but when the call to our expected method is reached, what is actually called is the mocked method that updates some internal counters. Then, VerifyAll() asserts that all expectations happened according to the settings prescribed.

KeithS
keith, Isn't the Record, Play syntax an old way of doing it in Rhino Mocks.
Xaisoft
Old but still totally serviceable; I find that this syntax is the most reliable way to get a PartialMock to behave the way you want it to.
KeithS