30 Days of TDD – Day 21 – A Tale of Two Defects
If you’ve never done Test Driven Development or aren’t even sure what this "crazy TDD stuff” is all about than this is the series for you. Over the next 30 days this series of posts take you from “I can spell TDD” to being able to consider yourself a “functional” TDD developer. Of course TDD is a very deep topic and truly mastering it will take quite a bit of time, but the rewards are well worth it. Along the way I’ll be showing you how
tools like JustCode and JustMock can help you in your practice of TDD.
Previous Posts in this Series: 30 Days of TDD – Day 20 – Refactoring Revisited Pt. 3
Test Driven Development changes the way you approach development of new features in your application. But it doesn’t stop there. Good TDD practice extends to the way you deal with all features and code you add to your application. In this post you’ll see how the practices of TDD extend to defects and demonstrates that your tests are part of your code and why it’s important to pay attention to their quality as well.
Dealing with Defects
As I stated in a previous post, defects are really just requirements that have yet to be discovered. When I say this I am not necessarily referring to defects that are the result of poor workmanship, lackadaisical testing or just plain old fashion incompetence. That aside, if most defects are simply newly discovered requirements then we should be able to deal with these new defects just like any other requirement. When a defect is first reported I want to first look at my existing tests to verify that I haven’t missed something. If that’s not the case then this defect truly is a new requirement and the before I start noodling around in the application code I want to create a test that exposes the lack or inaccurate functionality that is causing the defect. Once this new test passes (and my previously existing tests still pass!) I’ll know my defect has been address. An additional benefit of having a test is that I should be able to eliminate the dreaded “zombie” defect; a defect that has been “fixed” on several occasions but always seems to find a way to come back.
Our First Defect
For the sake of this example, we’re going to assume that our application has been the subject of nightly builds and has the benefit of a Quality Assurance (QA) department testing those builds daily. After a while a number of defects are reported, triaged and prioritized. The first defect that we must fix is:
“As a user of the application I was able to place multiple items in my shopping cart, but only one was delivered via the order fulfillment service”
Yes, as we’re building an online shopping application it’s reasonable to expect that customer might want to buy more than one item, but in the case of this example we’ll work under the assumption that this requirement was missed (don’t laugh, I’ve seen worse, one resulting in a $2,000,000 mistake!) or the QA folks are working from a different feature schedule than we are. In any case, the Project Manager (PM) has decided that this requirement must be addressed now, so we’ll do so.
I’ve verified that this defect is not the result of an incorrect test, so that means the first thing I want to do is write a test:
1: [Test] 2: public void WhenUserPlacesACorrectOrderWithMoreThenOneItemThenAnOrderNumberShouldBeReturned()
3: { 4: //Arrange
5: var shoppingCart = new ShoppingCart();
6: var itemOneId = Guid.NewGuid(); 7: var itemTwoId = Guid.NewGuid(); 8: int itemOneQuantity = 1;
9: int itemTwoQuantity = 4;
10: shoppingCart.Items.Add(new ShoppingCartItem { ItemId = itemOneId, Quantity = itemOneQuantity });
11: shoppingCart.Items.Add(new ShoppingCartItem { ItemId = itemTwoId, Quantity = itemTwoQuantity });
12: var customerId = Guid.NewGuid(); 13: var expectedOrderId = Guid.NewGuid(); 14: var orderFulfillmentSessionId = Guid.NewGuid(); 15: var customer = new Customer { Id = customerId };
16: 17: Mock.Arrange(() => _orderDataService.Save(Arg.IsAny<Order>())) 18: .Returns(expectedOrderId) 19: .OccursOnce(); 20: Mock.Arrange(() => _customerService.GetCustomer(customerId)).Returns(customer).OccursOnce(); 21: Mock.Arrange(() => _orderFulfillmentService.OpenSession(Arg.IsAny<string>(), Arg.IsAny<string>()))
22: .Returns(orderFulfillmentSessionId); 23: Mock.Arrange(() => _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, itemOneId, itemOneQuantity)) 24: .Returns(true)
25: .OccursOnce(); 26: Mock.Arrange(() => _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, itemTwoId, itemTwoQuantity)) 27: .Returns(true)
28: .OccursOnce(); 29: Mock.Arrange(() => 30: _orderFulfillmentService. 31: PlaceOrder(orderFulfillmentSessionId, Arg.IsAny<IDictionary<Guid, int>>(), Arg.IsAny<string>()))
32: .Returns(true);
33: //Act
34: var result = _orderService.PlaceOrder(customerId, shoppingCart); 35: 36: //Assert
37: Assert.AreEqual(expectedOrderId, result); 38: Mock.Assert(_orderDataService); 39: Mock.Assert(_orderFulfillmentService); 40: } By now the code in this test should be very familiar and easy to understand. This test is very similar to the WhenUserPlacesOrdeWithItemThatIsInInventoryOrderFulfillmentWorkflowShouldComplete test; I’ve added a second item to the shopping cart. I’ve also arranged a mock on the order fulfillment service mock for the call to IsInInventory for the second item. To make sure that IsInInventory is called for both items I’ve added the OccursOnce condition to both calls and will assert the mock was called correctly at the end of the test.
Running this test exposes our defect, as you can see by the failing test in figure one:
Figure 1 – Our defect is exposed
The order fulfillment mock is never called for the second item in our shopping cart. After some careful analysis, the offending method is the CheckInventoryLevels method:
1: private Dictionary<Guid, int> CheckInventoryLevels(ShoppingCart shoppingCart, Guid orderFulfillmentSessionId)
2: { 3: var firstItemId = shoppingCart.Items[0].ItemId; 4: var firstItemQuantity = shoppingCart.Items[0].Quantity; 5: 6: //Check Inventory Level
7: var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, firstItemId, firstItemQuantity); 8: 9: var orderForFulfillmentService = new Dictionary<Guid, int>();
10: orderForFulfillmentService.Add(firstItemId, firstItemQuantity); 11: return orderForFulfillmentService;
12: }
It seems that our code is only getting the first item out of the shopping cart and ordering that from the order fulfillment service. That explains why orders with multiple items are not being processed correctly. I’ll take a stab at fixing this code:
1: private Dictionary<Guid, int> CheckInventoryLevels(ShoppingCart shoppingCart, Guid orderFulfillmentSessionId)
2: { 3: var orderForFulfillmentService = new Dictionary<Guid, int>();
4: 5: foreach (var shoppingCartItem in shoppingCart.Items)
6: { 7: var itemId = shoppingCartItem.ItemId; 8: var itemQuantity = shoppingCartItem.Quantity; 9: 10: var itemIsInInventory = _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, itemId, itemQuantity); 11: 12: orderForFulfillmentService.Add(itemId, itemQuantity); 13: } 14: return orderForFulfillmentService;
15: }
This was the easiest solution I could come up with that may make our new test pass, and it does (figure two):
Figure 2 – The new test passes
The next step is to make sure that while we were fixing this defect we didn’t break anything else. We do that by running all the unit tests (figure three):
Figure 3 – All the tests pass
Defect Number Twof
There is no rule that says only QA testers will find defects. Everyone who works with the application from BA’s to developers to end users can find problems with an application. You should constantly be on the lookout for possible issues with your application. For example; the team developing our TDD Store application participates in a code review. As unit tests are part of our applications code they are reviewed as well. The team finds a problem with the WhenUserPlacesACorrectOrderThenAnOrderNumberShouldBeReturned test. Here’s what this code looks like now:
1: [Test] 2: public void WhenUserPlacesACorrectOrderThenAnOrderNumberShouldBeReturned()
3: { 4: //Arrange
5: var shoppingCart = new ShoppingCart();
6: shoppingCart.Items.Add(new ShoppingCartItem { ItemId = Guid.NewGuid(), Quantity = 1 });
7: var customerId = Guid.NewGuid(); 8: var expectedOrderId = Guid.NewGuid(); 9: var orderFulfillmentSessionId = Guid.NewGuid(); 10: var itemId = Guid.NewGuid(); 11: var customer = new Customer { Id = customerId };
12: 13: Mock.Arrange(() => _orderDataService.Save(Arg.IsAny<Order>())) 14: .Returns(expectedOrderId) 15: .OccursOnce(); 16: Mock.Arrange(() => _customerService.GetCustomer(customerId)).Returns(customer).OccursOnce(); 17: Mock.Arrange(() => _orderFulfillmentService.OpenSession(Arg.IsAny<string>(), Arg.IsAny<string>()))
18: .Returns(orderFulfillmentSessionId); 19: Mock.Arrange(() => _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, itemId, 1)) 20: .Returns(true);
21: Mock.Arrange(() => 22: _orderFulfillmentService. 23: PlaceOrder(orderFulfillmentSessionId, Arg.IsAny<IDictionary<Guid, int>>(), Arg.IsAny<string>()))
24: .Returns(true);
25: //Act
26: var result = _orderService.PlaceOrder(customerId, shoppingCart); 27: 28: //Assert
29: Assert.AreEqual(expectedOrderId, result); 30: Mock.Assert(_orderDataService); 31: }
Reviewing this test we found a couple issues. The first is around the mock arrangement that starts on line 19. We set this mock up to expect an item id defined in the itemId variable which we declare and populate on line ten. The idea behind this being that the IsInInventory method will be called with the item id in the itemId variable. IsInInventory takes this value from a ShoppingCartItem. We are creating a ShoppingCartItem on line six, but we are not populating the item id with the value expected by the mock.
We’re also not asserting that the _orderFulfillmentService mock was used properly, but it wouldn’t matter right now; there’s no condition on the mock arranged on line 19 that specifies that this method had to be called. This is because these mocks are “loose” by default. Specifically they are “recursive loose” but we’ll cover the differences between those two (as well as Strict and Call Original) in another post. Since our mocks are loose it means that if our code calls a method that we have not arranged, which is happening here since we don’t have an arrangement that takes the actual item id that is being passed to IsInInventory, JustMock returns the default value of the return type, in our case “false.”
Because of our requirements, there may not be a specific problem with this test. But many developers practicing TDD consider it a bad practice to have mocks arranged in ways that do not reflect how the actual business rules and workflow are used. This is a defensive programming practice that helps make sure that as your application, your code and your tests evolve these test continue to properly reflect how the business rules and workflow should be used.
Luckily this is an easy fix. The first thing I want to do is make this test fail. This is because I always want to be working from a failing test to a passing test:
1: [Test] 2: public void WhenUserPlacesACorrectOrderThenAnOrderNumberShouldBeReturned()
3: { 4: //Arrange
5: var shoppingCart = new ShoppingCart();
6: shoppingCart.Items.Add(new ShoppingCartItem { ItemId = Guid.NewGuid(), Quantity = 1 });
7: var customerId = Guid.NewGuid(); 8: var expectedOrderId = Guid.NewGuid(); 9: var orderFulfillmentSessionId = Guid.NewGuid(); 10: var itemId = Guid.NewGuid(); 11: var customer = new Customer { Id = customerId };
12: 13: Mock.Arrange(() => _orderDataService.Save(Arg.IsAny<Order>())) 14: .Returns(expectedOrderId) 15: .OccursOnce(); 16: Mock.Arrange(() => _customerService.GetCustomer(customerId)).Returns(customer).OccursOnce(); 17: Mock.Arrange(() => _orderFulfillmentService.OpenSession(Arg.IsAny<string>(), Arg.IsAny<string>()))
18: .Returns(orderFulfillmentSessionId); 19: Mock.Arrange(() => _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, itemId, 1)) 20: .Returns(true)
21: .OccursOnce(); 22: Mock.Arrange(() => 23: _orderFulfillmentService. 24: PlaceOrder(orderFulfillmentSessionId, Arg.IsAny<IDictionary<Guid, int>>(), Arg.IsAny<string>()))
25: .Returns(true);
26: Mock.Arrange(() => _orderFulfillmentService.CloseSession(orderFulfillmentSessionId)); 27: //Act
28: var result = _orderService.PlaceOrder(customerId, shoppingCart); 29: 30: //Assert
31: Assert.AreEqual(expectedOrderId, result); 32: Mock.Assert(_orderDataService); 33: Mock.Assert(_orderFulfillmentService); 34: } I’ve added a couple things here. First I’ve added the OccursOnce condition to the arrangement for the mock of IsInInventory on line 19. I also added a missing call to the CloseSession method of the order fulfillment service. Finally I assert the order fulfillment mock on line 33. Running the test results in failure (figure four):
Figure 4 – Our test fails
This is what we expected; the mock of the IsInIventory is not called because we are not passing it the expected item id. We need to change that.
1: [Test] 2: public void WhenUserPlacesACorrectOrderThenAnOrderNumberShouldBeReturned()
3: { 4: //Arrange
5: var shoppingCart = new ShoppingCart();
6: var itemId = Guid.NewGuid(); 7: shoppingCart.Items.Add(new ShoppingCartItem { ItemId = itemId, Quantity = 1 });
8: var customerId = Guid.NewGuid(); 9: var expectedOrderId = Guid.NewGuid(); 10: var orderFulfillmentSessionId = Guid.NewGuid(); 11: var customer = new Customer { Id = customerId };
12: 13: Mock.Arrange(() => _orderDataService.Save(Arg.IsAny<Order>())) 14: .Returns(expectedOrderId) 15: .OccursOnce(); 16: Mock.Arrange(() => _customerService.GetCustomer(customerId)).Returns(customer).OccursOnce(); 17: Mock.Arrange(() => _orderFulfillmentService.OpenSession(Arg.IsAny<string>(), Arg.IsAny<string>()))
18: .Returns(orderFulfillmentSessionId).OccursOnce(); 19: Mock.Arrange(() => _orderFulfillmentService.IsInInventory(orderFulfillmentSessionId, itemId, 1)) 20: .Returns(true).OccursOnce();
21: Mock.Arrange(() => 22: _orderFulfillmentService. 23: PlaceOrder(orderFulfillmentSessionId, Arg.IsAny<IDictionary<Guid, int>>(), Arg.IsAny<string>()))
24: .Returns(true);
25: Mock.Arrange(() => _orderFulfillmentService.CloseSession(orderFulfillmentSessionId)).OccursOnce(); 26: //Act
27: var result = _orderService.PlaceOrder(customerId, shoppingCart); 28: 29: //Assert
30: Assert.AreEqual(expectedOrderId, result); 31: Mock.Assert(_orderDataService); 32: Mock.Assert(_orderFulfillmentService); 33: }
First I moved the declaration and population of the itemId variable from line ten to line six. Then I updated the code that creates the ShoppingCartItem on line seven to use the itemId variable as the ItemId for its item id. This change should cause the IsInInventory method to be called with the expected item id. When I run the test I can see that it now passes (figure five)
Figure 5 – Our test passes again
Summary
Defects happen, there is no way to completely eliminate them. But when you practice TDD your approach to defects are slightly different. Remember that defects are just new requirements and should be treated like them. Before you change your code, write a test. This will not only ensure that your defect is fixed, but will also help make sure that it stays fixed.
Continue the TDD journey:
James Bender
James Bender is a Developer and has been involved in software development and architecture for almost 20 years. He has built everything from small, single-user applications to Enterprise-scale, multi-user systems. His specialties are .NET development and architecture, TDD, Web Development, cloud computing, and agile development methodologies. James is a Microsoft MVP and the author of two books; "Professional Test Driven Development with C#" which was released in May of 2011 and "Windows 8 Apps with HTML5 and JavaScript" which will be available soon. James has a blog at JamesCBender.com and his Twitter ID is @JamesBender.
Related Posts
Comments
All articles
Topics
Latest Stories
in Your Inbox
Subscribe to be the first to get our expert-written articles and tutorials for developers!
All fields are required