Mocking IElasticClient in unit tests

Question

I'm trying to unit test how my class responds to a NEST 1.7 IElasticClient's return values, using NUnit 2.6.4 and Moq 4.2. I feel mocking the Get function requires very "heavy" It.IsAny calls, and am mainly looking for feedback on how to mock IElasticClient in a way that my tests remain readable.

My code is mostly 100% actual code, with the exception of:

• The Repository and Person classes are simplified (I'm mainly looking for feedback on the Test method);
• I'm expecting a generic Exception but in reality I've got a custom exception class there;

Here's the code:

public class Person
{
    public string Id { get; set; }
    public string FullName { get; set; }
}

public class PeopleRepository
{
    private IElasticClient client;

    public PeopleRepository(IElasticClient Client)
    {
        this.client = Client;
    }

    public Person Get(string id)
    {
        var getResponse = client.Get<Person>(p => p.Id(id));

        if (getResponse.Source == null)
        {
            throw new Exception("Person was not found for id: " + id);
        }

        return getResponse.Source;
    }
}

[TestFixture]
public class PeopleRepositoryTests
{
    [Test]
    public void RetrieveProduct_WhenDocNotFoundInElastic_ThrowsException()
    {
        var clientMock = new Mock<IElasticClient>();
        var getRetvalMock = new Mock<IGetResponse<Person>>();

        getRetvalMock
            .Setup(r => r.Source)
            .Returns((Person)null);

        clientMock
            .Setup(c => c.Get<Person>(It.IsAny<Func<GetDescriptor<Person>, GetDescriptor<Person>>>()))
            .Returns(getRetvalMock.Object);

        var repo = new PeopleRepository(clientMock.Object);

        Assert.Throws<Exception>(() => repo.Get("invalid-id"));
    }
}

Any way I can improve the way I'm mocking IElasticClient?

Answer 1

It's fine. Yes, in an ideal world, we'd verify that the id we passed into the PeopleRepository.Get() method is the same one that's getting passed to the client.Get() method, but that's also getting down into testing the internal implementation of the method. What you really care about here is that when the Person isn't found, a specific exception is thrown, so in this particular case it's fine.

Remember, we want to tests for specific results, not the internal implementation.

I'm going to quote Jon Skeet's comment to a related SO question.

I would test that the usage of those functions do the right thing, rather than testing that the exact same function is used. – Jon Skeet Oct 8 '12 at 9:33