Is it a good practice to mock Automapper in unit t

2019-06-22 08:01发布

问题:

There is this codebase where we use automapper and have 2 layers, Domain and Service. Each has its object for data representation, DomainItem and ServiceItem. The service gets data from domain, the uses constructor injected automapper instance to map

class Service 
{
  public ServiceItem Get(int id)
  {
    var domainItem = this.domain.Get(id);
    return this.mapper.Map<DomainItem, ServiceItem>(domainItem);
  }
}

Assume best practices, so mapper has no side-effects and no external dependencies. You'd write a static function to convert one object to another within seconds, just mapping fields.

With this in mind, is it a good practice to mock the mapper in unit tests like this?

[TestClass]
class UnitTests
{
  [TestMethod]
  public void Test()
  {
    var expected = new ServiceItem();

    var mockDomain = new Mock<IDomain>();
    // ... setup
    var mockMapper = new Mock<IMapper>();
    mockMapper.Setup(x => x.Map<DomainItem, ServiceItem>(It.IsAny<DomainItem>()))
      .Returns(expected);


    var service = new Service(mockDomain.Object, mockMapper.Object);
    var result = service.Get(0);

    Assert.AreEqual(expected, result);
  }
}

To me, it seems that such unit test does not really bring any value, because it is effectively testing only the mocks, So i'd either not write it at all OR I'd use the actual mapper, not the mocked one. Am I right or do I overlook something?

回答1:

I think the issue here is that the test is badly written for what it is actually trying to achieve which is testing Service.Get().

The way I would write this test is as follows:

[TestMethod]
public void Test()
{
  var expected = new ServiceItem();

  var mockDomain = new Mock<IDomain>();
  var expectedDomainReturn = new DomainItem(0); //Illustrative purposes only
  mockDomain.Setup(x => x.DomainCall(0)).Returns(expectedDomainReturn); //Illustrative purposes only

  var mockMapper = new Mock<IMapper>();
  mockMapper.Setup(x => x.Map<DomainItem, ServiceItem>(It.IsAny<DomainItem>()))
      .Returns(expected);    


  var service = new Service(mockDomain.Object, mockMapper.Object);
  var result = service.Get(0);

  mockDomain.Verify(x => x.DomainCall(0), Times.Once);
  mockMapper.Verify(x => x.Map<DomainItem, ServiceItem>(expectedDomainReturn), Times.Once);
}

This test instead of not really checking the functionality of the service.Get(), checks that the parameters passed are correct for the individual dependency calls based on the responses. You are thus not testing AutoMapper itself and should not need to.

Checking result is basically useless but will get the code coverage up.