views:

131

answers:

4

I'm only new to Unit Testing and ASP.NET MVC. I've been trying to get my head into both using Steve Sanderson's "Pro ASP.NET MVC Framework". In the book there is this piece of code:

public class AdminController : Controller
{
 ...

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Edit(Product product, HttpPostedFileBase image)
    {
      ...
       productsRepository.SaveProduct(product);

       TempData["message"] = product.Name + " has been saved.";
       return RedirectToAction("Index");
    }
}

That he tests like so:

[Test]
public void Edit_Action_Saves_Product_To_Repository_And_Redirects_To_Index()
{
    // Arrange
    AdminController controller = new AdminController(mockRepos.Object);

    Product newProduct = new Product();

    // Act
    var result = (RedirectToRouteResult)controller.Edit(newProduct, null);

    // Assert: Saved product to repository and redirected
    mockRepos.Verify(x => x.SaveProduct(newProduct));
    Assert.AreEqual("Index", result.RouteValues["action"]);
}

THE TEST PASSES.

So I intentionally corrupt the code by adding "productsRepository.DeleteProduct(product);" after the "SaveProduct(product);" as in:

            ...
       productsRepository.SaveProduct(product);
       productsRepository.DeleteProduct(product);
            ...

THE TEST PASSES.(i.e Condones a calamitous [hypnosis + intellisense]-induced typo :) )

Could this test be written better? Or is there something I should know? Thanks a lot.

A: 

The idea is to "write the simplest code that works". This helps you avoid doing stupid things like deleting files from disk on an increment counter operation. Obviously, not deleting the files is simpler.

Martinho Fernandes
+2  A: 

Your intentional "corrupting" of the code does not break the test as you have done it after the call that you Verify() to SaveProduct(). I have always found Moq's Verify() very reliable.

Some psuedo code for a more robust test might be to have your repository implement an interface and have a simple in memory testable version

// Arrange

var repo = SetupTestableRepository() 
var product = CreateProduct(productId)

// Act 
repo.SaveProduct(product)

// Assert
Assert.IsNotNull(repo.Fetch(productId))

Kindness,

Dan

Daniel Elliott
+5  A: 

I think you are maybe misinterpreting the .Verify() methods purpose.

It verifies that the given method was called with the expected value.

On page 187 of the book Steve says 'Notice how it uses Moqs .Verify() method to ensure that the AdminController really did call the DeleteProduct() with the correct parameter.'

So in your case the test passes as it is merely Verifying the call and not the functionality.

As TDD is being followed during the course of the book the addition of

productsRepository.DeleteProduct(product);

should first be added to the test

// Assert: Saved product to repository, then deleted and redirected
mockRepos.Verify(x => x.SaveProduct(newProduct))
mockRepos.Verify(x => x.DeleteProduct(newProduct));
Assert.AreEqual("Index", result.RouteValues["action"]);

and then added to the code

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Edit(Product product, HttpPostedFileBase image)
{

     ...
productsRepository.SaveProduct(product);
productsRepository.DeleteProduct(product);
    ...
Nicholas Murray
+3  A: 

As others have said, the test passes because your assertion:

mockRepos.Verify(x => x.SaveProduct(newProduct));

was fulfilled. Your code did call the SaveProduct method.

The Mock.Verify() method can't verify that some other method wasn't called, which is what you are expecting it to do.

If you are worried about something weird happening (like a Delete() being called after a Save()) and want to prevent it with a test, you'd have to add a Verify() for that condition, too. Something like:

mockRepos.Verify(x => x.DeleteProduct(newProduct), Times.Never());
Eric King
Yep. Some mocking frameworks support strict mocks that will verify that no other method was called on the mock, but these tend to lead to brittle tests.
TrueWill