Unit testing, mocking - simple case: Service - Repository

Question

Consider a following chunk of service:

public class ProductService : IProductService {

   private IProductRepository _productRepository;

   // Some initlization stuff

   public Product GetProduct(int id) {
      try {
         return _productRepository.GetProduct(id);
      } catch (Exception e) {
         // log, wrap then throw
      }
   }
}

Let's consider a simple unit test:

[Test]
public void GetProduct_return_the_same_product_as_getProduct_on_productRepository() {
   var product = EntityGenerator.Product();

   _productRepositoryMock.Setup(pr => pr.GetProduct(product.Id)).Returns(product);

   Product returnedProduct = _productService.GetProduct(product.Id);

   Assert.AreEqual(product, returnedProduct);

   _productRepositoryMock.VerifyAll();
}

At first it seems that this test is ok. But let's change our service method a little bit:

public Product GetProduct(int id) {
   try {
      var product = _productRepository.GetProduct(id);

      product.Owner = "totallyDifferentOwner";

      return product;
   } catch (Exception e) {
      // log, wrap then throw
   }
}

How to rewrite a given test that it'd pass with the first service method and fail with a second one?

How do you handle this kind of simple scenarios?

HINT 1: A given test is bad coz product and returnedProduct is actually the same reference.

HINT 2: Implementing equality members (object.equals) is not the solution.

HINT 3: As for now, I create a clone of the Product instance (expectedProduct) with AutoMapper - but I don't like this solution.

HINT 4: I'm not testing that the SUT does NOT do sth. I'm trying to test that SUT DOES return the same object as it is returned from repository.

Answer 1

Personally, I wouldn't care about this. The test should make sure that the code is doing what you intend. It's very hard to test what code is not doing, I wouldn't bother in this case.

The test actually should just look like this:

[Test]
public void GetProduct_GetsProductFromRepository() 
{
   var product = EntityGenerator.Product();

   _productRepositoryMock
     .Setup(pr => pr.GetProduct(product.Id))
     .Returns(product);

   Product returnedProduct = _productService.GetProduct(product.Id);

   Assert.AreSame(product, returnedProduct);
}

I mean, it's one line of code you are testing.

Answer 2

Why don't you mock the product as well as the productRepository?

If you mock the product using a strict mock, you will get a failure when the repository touches your product.

If this is a completely ridiculous idea, can you please explain why? Honestly, I'd like to learn.

Answer 3

One way of thinking of unit tests is as coded specifications. When you use the EntityGenerator to produce instances both for the Test and for the actual service, your test can be seen to express the requirement

• The Service uses the EntityGenerator to produce Product instances.

This is what your test verifies. It's underspecified because it doesn't mention if modifications are allowed or not. If we say

• The Service uses the EntityGenerator to produce Product instances, which cannot be modified.

Then we get a hint as to the test changes needed to capture the error:

var product = EntityGenerator.Product();
// [ Change ] 
var originalOwner = product.Owner;  
// assuming owner is an immutable value object, like String
// [...] - record other properties as well.

Product returnedProduct = _productService.GetProduct(product.Id);

Assert.AreEqual(product, returnedProduct);

// [ Change ] verify the product is equivalent to the original spec
Assert.AreEqual(originalOwner, returnedProduct.Owner);
// [...] - test other properties as well

(The change is that we retrieve the owner from the freshly created Product and check the owner from the Product returned from the service.)

This embodies the fact that the Owner and other product properties must equal the the original value from the generator. This may seem like I'm stating the obvious, since the code is pretty trivial, but it runs quite deep if you think in terms of requirement specifications.

I often "test my tests" by stipulating "if I change this line of code, tweak a critical constant or two, or inject a few code burps (e.g. changing != to ==), which test will capture the error?" Doing it for real finds if there is a test that captures the problem. Sometimes not, in which case it's time to look at the requirements implicit in the tests, and see how we can tighten them up. In projects with no real requirements capture/analysis this can be a useful tool to toughen up tests so they fail when unexpected changes occur.

Of course, you have to be pragmatic. You can't reasonably expect to handle all changes - some will simply be absurd and the program will crash. But logical changes like the Owner change are good candidates for test strengthening.

By dragging talk of requirements into a simple coding fix, some may think I've gone off the deep end, but thorough requirements help produce thorough tests, and if you have no requirements, then you need to work doubly hard to make sure your tests are thorough, since you're implicitly doing requirements capture as you write the tests.

EDIT: I'm answering this from within the contraints set in the question. Given a free choice, I would suggest not using the EntityGenerator to create Product test instances, and instead create them "by hand" and use an equality comparison. Or more direct, compare the fields of the returned Product to specific (hard-coded) values in the test, again, without using the EntityGenerator in the test.

Answer 4

Q1: Don't make changes to code then write a test. Write a test first for the expected behavior. Then you can do whatever you want to the SUT.

Q2: You don't make the changes in your Product Gateway to change the owner of the product. You make the change in your model.

But if you insist, then listen to your tests. They are telling you that you have the possibility for products to be pulled from the gateway that have the incorrect owners. Oops, Looks like a business rule. Should be tested for in the model.

Also your using a mock. Why are you testing an implementation detail? The gateway only cares that the _productRepository.GetProduct(id) returns a product. Not what the product is.

If you test in this manner you will be creating fragile tests. What if product changes further. Now you have failing tests all over the place.

Your consumers of product (MODEL) are the only ones that care about the implementation of Product.

So your gateway test should look like this:

[Test]
public void GetProduct_return_the_same_product_as_getProduct_on_productRepository() {
   var product = EntityGenerator.Product();

   _productRepositoryMock.Setup(pr => pr.GetProduct(product.Id)).Returns(product);

   _productService.GetProduct(product.Id);

   _productRepositoryMock.VerifyAll();
}

Don't put business logic where it doesn't belong! And it's corollary is don't test for business logic where there should be none.

Answer 5

If you really want to guarantee that the service method doesn't change the attributes of your products, you have two options:

• Define the expected product attributes in your test and assert that the resulting product matches these values. (This appears to be what you're doing now by cloning the object.)

• Mock the product and specify expectations to verify that the service method does not change its attributes.

This is how I'd do the latter with NMock:

// If you're not a purist, go ahead and verify all the attributes in a single
// test - Get_Product_Does_Not_Modify_The_Product_Returned_By_The_Repository
[Test]
public Get_Product_Does_Not_Modify_Owner() {

    Product mockProduct = mockery.NewMock<Product>(MockStyle.Transparent);

    Stub.On(_productRepositoryMock)
        .Method("GetProduct")
        .Will(Return.Value(mockProduct);

    Expect.Never
          .On(mockProduct)
          .SetProperty("Owner");

    _productService.GetProduct(0);

    mockery.VerifyAllExpectationsHaveBeenMet();
}

Answer 6

My previous answer stands, though it assumes the members of the Product class that you care about are public and virtual. This is not likely if the class is a POCO / DTO.

What you're looking for might be rephrased as a way to do comparison of the values (not instance) of the object.

One way to compare to see if they match when serialized. I did this recently for some code... Was replacing a long parameter list with a parameterized object. The code is crufty, I don't want to refactor it though as its going away soon anyhow. So I just do this serialization comparison as a quick way to see if they have the same value.

I wrote some utility functions... Assert2.IsSameValue(expected,actual) which functions like NUnit's Assert.AreEqual(), except it serializes via JSON before comparing. Likewise, It2.IsSameSerialized() can be used to describe parameters passed to mocked calls in a manner similar to Moq.It.Is().

public class Assert2
{
    public static void IsSameValue(object expectedValue, object actualValue) {

        JavaScriptSerializer serializer = new JavaScriptSerializer();

        var expectedJSON = serializer.Serialize(expectedValue);
        var actualJSON = serializer.Serialize(actualValue);

        Assert.AreEqual(expectedJSON, actualJSON);
    }
}

public static class It2
{
    public static T IsSameSerialized<T>(T expectedRecord) {

        JavaScriptSerializer serializer = new JavaScriptSerializer();

        string expectedJSON = serializer.Serialize(expectedRecord);

        return Match<T>.Create(delegate(T actual) {

            string actualJSON = serializer.Serialize(actual);

            return expectedJSON == actualJSON;
        });
    }
}