views:

757

answers:

3

Using Moq, I have a very odd issue where the setup on a mock only seems to work if the method I am setting up is public. I don't know if this is a Moq bug or if I just have this wrong (newbie to Moq). Here is the test case:

    public class TestClass
    {
        public string Say()
        {
    return Hello();
        }
        internal virtual string Hello()
        {
            return "";
        }
    }

    [TestMethod]
    public void Say_WhenPublic_CallsHello()
    {
        Mock<TestClass> mock = new Mock<TestClass>();
        mock.Setup(x => x.Hello()).Returns("Hello World");

        string result = mock.Object.Say();
        mock.Verify(x => x.Hello(), Times.Exactly(1));
        Assert.AreEqual("Hello World", result);     
    }

Which fails with this message: Say_WhenPublic_CallsHello failed: Moq.MockException: Invocation was not performed on the mock 1 times: x => x.Hello() at Moq.Mock.ThrowVerifyException(IProxyCall expected, Expression expression, Times times)...

If I make the Hello method public like this, the test passes. What is the issue here?

        public virtual string Hello()
        {
            return "";
        }

Thanks in advance!

+7  A: 

The test fails when Hello() is internal because Moq cannot provide an override of the method in this case. This means that the internal implementation of Hello() will run, rather than mock's version, causing the Verify() to fail.

Incidentally, what you are doing here makes no sense in the context of a unit test. A unit test should not care that Say() calls an internal Hello() method. This is implementation internal to your assembly and not a concern of consuming code.

AdamRalph
+1 Additionally, unit tests should be written in a different library than the System Under Test. Had this been done, the code wouldn't even have compiled.
Mark Seemann
@Mark - true, although internals can be made visible to other assemblies using the InternalVisibleTo attribute
AdamRalph
@Mark - That is an obvious statement - meaning that of course you should write your tests in a different library. This code was meant to provide a simple example. Why would I go through the extra effort of separating these two into separate classes to make a simple point?
jle
@AdamRalph - Have you not heard of partial mocks? This is what this test case (and my entire post) is about. In partial mocks you can mock the subject under test only partially, meaning that you can call a real method on your mock object and do mock setups on other methods in the subject under test. This is extremely valuable when writing tests for legacy code that would otherwise not be testable. Consider a legacy class that has several methods, some of which internally call the database. Further consider that you might want to mock only the methods that call the db.
jle
@jle - I'm not disputing that, and I agree that partial mocks can be very useful. what I don't understand is the motiviation to isolate the test target from the internal Hello() call. perhaps your example does not convey enough information about your test target and what the internal call is doing in the concrete implementation
AdamRalph
+2  A: 

I don't know enough about how this works underneath the covers to give you a technical answer as to exactly why that is the behaviour, but I think I can help with your confusion.

In your example you are calling a method Say(), and it is returning the expected text. Your expectation should not enforce a particular implementation of Say() ie that it calls an internal method called Hello() to return that string. This is why it does not pass the verify, and also why the string returned is "", ie the actual implementation of Hello() has been called.

By making the Hello method public it appears that this has enabled Moq to intercept the call to it, and use it's implementation instead. Therefore, in this case the test appears to pass. However, in this scenario you haven't really achieved anything useful, because your test says that when you call Say() the result is "Hello World" when in acutal fact the result is "".

I have rewritten your example to show how I would expect Moq to be used (not necessarily definitive, but hopefully clear.

public interface IHelloProvider
{
    string Hello();
}

public class TestClass
{
    private readonly IHelloProvider _provider;

    public TestClass(IHelloProvider provider)
    {
        _provider = provider;
    }

    public string Say()
    {
        return _provider.Hello();
    }
}

[TestMethod]
public void WhenSayCallsHelloProviderAndReturnsResult()
{
    //Given
    Mock<IHelloProvider> mock = new Mock<IHelloProvider>();
    TestClass concrete = new TestClass(mock.Object);
    //Expect
    mock.Setup(x => x.Hello()).Returns("Hello World");
    //When
    string result = concrete.Say();
    //Then
    mock.Verify(x => x.Hello(), Times.Exactly(1));
    Assert.AreEqual("Hello World", result);
}

In my example, I have introduced an interface to an IHelloProvider. You will notice that there is no implementation of IHelloProvider. This is at the heart of what we are trying to achieve by using a mocking solution.

We are trying to test a class (TestClass), which is dependant on something external (IHelloProvider). If you are using Test Driven Development then you probably haven't written an IHelloProvider yet, but you are aware that you will need one at some point. You want to get TestClass working first though, rather than get distracted. Or perhaps IHelloProvider uses a database, or a flat file, or is difficult to configure.

Even if you have a fully working IHelloProvider, you are still only trying to test the behaviour of TestClass, so using a concrete HelloProvider is likely to make your test more prone to failure, for instance if there is a change to the behaviour of HelloProvider, you don't want to have to change the tests of every class that uses it, you just want to change the HelloProvider tests.

Getting back to the code, we now have a class TestClass, which is dependant on an interface IHelloProvider, the implementation of which is provided at construction time (this is dependency injection).

The behaviour of Say() is that it calls the method Hello() on the IHelloProvider.

If you look back at the test, we have created an actual TestClass object, since we actually want to test the code that we have written. We have created a Mock IHelloProvider, and said that we expect it to have it's Hello() method called, and when it does to return the string "Hello World".

We then call Say(), and verify the results as before.

The important thing to realise is that we are not interested in the behaviour of the IHelloProvider, and so we can mock it to make testing easier. We are interested in the behaviour of TestClass, so we have created an actual TestClass not a Mock, so that we can test it's actual behaviour.

I hope this has helped to clarify what is going on.

Modan
+1 - An important concept to remember (as you pointed out) is that you don't mock the class under test, you mock it's dependencies.
Eric King
@Modan - Thanks for spending so much time in your response. I should have however been more clear in my post as to my ultimate goal. I was exploring the concept of partial mocks using Moq and was bewildered as to why doing a Setup on a internal method of a class does not work. You see, I am in the middle of making some changes to some legacy code and my ability to change this code in its entirety is limited. What I thought I could do is mock only the methods of my subject under test that cannot be executed in a unit test (such as hitting a database). Thanks again.
jle
"What I thought I could do is mock only the methods of my subject under test that cannot be executed in a unit test (such as hitting a database"In this scenario I would advise splitting out that code into a separate class as I did in my example. It is useful (maybe even essential) to keep code that can't be easily tested separate from testable code. If you don't do this, it is likely that you will likely end up with some business logic that is difficult/impossible to test because of your code structure.
Modan
+1  A: 

Moq doesn't do partial mocking and can only mock public virtual methods or interfaces. When you create a Mock you are creating a completely new T and removing all implementation of any public virtual methods.

My suggestion would be to concentrate on testing the public surface area of your objects and not their internals. Your internals will get coverage. Just make sure you have a clear idea of what your target class is and don't mock that (in most cases).

Partial mocking is only useful when you want to test the functionality of an abstract class with mocked implementations of abstract methods. If you aren't doing this, you are likely not going to see much benefit from doing a partial mock.

If this is not an abstract class, you will want to focus more on the approach Modan suggests, which is to say that you should mock dependencies rather than your target class itself. All of this boils down to the rule don't mock what you are testing.

Anderson Imes