3
\$\begingroup\$

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?

\$\endgroup\$
  • \$\begingroup\$ Just a technical observation, those are stubs, not mocks. \$\endgroup\$ – 404 Jan 18 '16 at 20:59
  • \$\begingroup\$ Since you create objects with Moq using the Mock<T>() function I tend to call all those objects a "mock", regardless of whether it's technically a fake, stub, double, mock, or dummy. Duly noted though :-) \$\endgroup\$ – Jeroen Jan 18 '16 at 22:04
2
\$\begingroup\$

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

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.