This is the eighth installment in a series of building a Test-Driven Grocery List application using Jasmine and RequireJS. To learn more about the intent and general concept of the series please visit The Making of a Test-Driven Grocery List Application in JavaScript: Part I
—

Introduction

In the past several articles we have sort of have been living in the spec runner. We fiddled about making it turn red and green, and – until the last article – we never really ran the Grocery List application we were building to give it a proper User Testing. If you did play around with the actual application we have been busy writing tests for, you may have found a bug. An unexpected result from supplying no grocery item name before saving it to the list:

As you can see from the screenshot, there is an empty item in the third entry of the list. While I do think the act of walking into a store and picking up nothing is an accomplishment at times, this Empty Item bug is not an intended feature. In this article, I want to squash that bug… but – in keep with the theme of this series – we are going to do it through tests! Calm down. It is hard to read when you jump up and down like that.

Bug Ticket

Before we begin fixing the bug, let’s get it down in writing what we think the problem is and how we think the intended behavior should be. To start, we don’t want empty items added to the Grocery List. We are currently able to add them – and rather easily – so let’s list out the steps that allows us to do so:

Empty Item Added to List
—
Steps to Reproduce

1. Launch the Grocery List application
2. Click ‘add item’ button
3. In focused input field, type the name of a grocery item, ie. ‘apples’ (sans quotes)
4. Click the Tab button to save to list
5. Click ‘add item’ button
6. Leave the focused input field blank
7. Click the Tab button

Result
Empty item is added to the list

Expected Result
An item without any name value provided is not added to the list

Additional Notes
It is not a requirement to first have a valid item added to the list in order to reproduce the issue. Steps 1-4 are intended to show expected behavior with a non-empty item.
—

If you are out there reading this and file bug tickets, please, for the love of software, log a ticket with a structure similar to this. Most modern bug tracking systems have fields like this, but you’d be surprised (or maybe not) how tickets come in sometimes. My least favorite is explained bug in the title. I can only imagine that they are the same people who write the whole email in the Subject field.

Alright, so now we know what we are dealing with. We have clear steps to reproduce the issue we think is a bug and have explained the difference between the actual result and the intended/expected result. Let’s tackle it.

Tests

As the developer of the application, we probably know exactly where this is happening. Our initial response is to go write to the source and resolve this issue. But we’re not going to do that. Don’t give me that look. I know, I know. The less we have to be in bug-fixing-mode, the happier developer we be, but I think if we have a test that supports the validity of this claim then, when we do make changes to the application code and it still passes, we know we have not committed a regression. I think it is a confidence builder in relying on my code to work properly and takes away some stress when refactoring comes into play.

Before we begin, let’s think about where we will add the tests. We could append another spec suite to the Add Item specs – after all, this bug occurs upon the completion of the Add Item feature. But in thinking about how to reproduce the issue, it seems like it may be another feature, or at least could be discovered in a feature we haven’t addressed – Edit Item. Essentially, when we get to the 3rd step – In focused input field, type the name of a grocery item, ie. ‘apples’ (sans quotes) – we have already added the item to the list. It has entered an edit mode, from which we need to ensure the validity of the value provided before saving it to the list.

Now, a true TDD‘er probably would stop me right now and tell me I am thinking too much to get this resolved. And, in part, they would be correct in doing so. But it is not my intent to start dreaming up new features during bug fixing. I just want to properly think about where the tests make the most sense. I don’t think they belong as an addendum to the Add Item specs and should live in there own spec suite in a separate file:

/test/jasmine/spec/feature/saveitem.spec.js

define(['jquery', 'script/controller/list-controller', 'script/controller/list-item-controller'],

        function($, listController, itemControllerFactory) {



  describe('Save item to grocery list', function() {



    it('should not save an empty item to the list') {

      expect(true).toEqual(false);

    }



  }

}

Given the steps provided in the bug ticket, we could easily translate that into the setup for the spec suite:

/test/jasmine/spec/feature/saveitem.spec.js

define(['jquery', 'script/controller/list-controller', 'script/controller/list-item-controller'],

        function($, listController, itemControllerFactory) {



  describe('Save item to grocery list', function() {



    var item,

        itemName = 'apples',

        invalidName = '',

        itemRenderer,

        async = new AsyncSpec(this);



    beforeEach( function() {

      item = listController.createNewItem();

      itemRenderer = listController.getRendererFromItem(item);

    });



    it('should not save an empty item to the list', function() {

      expect(true).toEqual(false);

    });



    afterEach( function() {

      item = undefined;

      itemRenderer = undefined;

    });

  });



});

In the beforeEach() setup, we have progressed the application – or at least a list item – to essentially the 3rd step in the Steps to Reproduce: we have added a new item to the list, and internally upon addition of an item in the list-controller, it has entered edit-mode for that item. In the variable declarations, we have also defined what we know of as being valid and invalid item entry names: ‘apples’ and ”, respectively.

We should be ready to go in our specs now to define the rest of the steps and expected results:

/test/jasmine/spec/feature/saveitem.spec.js

it('should not save an empty item to the list', function() {

  var listLength = listController.getItemList().itemLength();



  item.name = invalidName;

  // save item... ?

  expect(listController.getItemList().itemLength()).toEqual(listLength);

});

Oh, boy. How do we actually save an item? There is no API on the list-controller that saves an item. There used to be, but we refactored that out when we handed over item management to the list-item-controller. Furthermore, an item is added and kept on the list since its request for creation – the actual reason for the bug, I suppose. I am fine with such functionality internally to the list-controller as it stands now and don’t intend to change the item creation. I think an additional event should be dispatched from a list-item-controller signifying a request to commit the item to the list after being edited. Let’s finish up this spec under such assumptions, then work our way through the failing test:

/test/jasmine/spec/feature/saveitem.spec.js

async.it('should not save an empty item to the list', function(done) {

  var listLength = listController.getItemList().itemLength();



  $(itemRenderer).on('commit', function(event) {

    expect(listController.getItemList().itemLength()).toEqual(listLength-1);

    done();

  });



  item.name = invalidName;

  itemRenderer.save();

});

We marked the spec as asynchronous to support events from the list-item-controller and placed the expectation within the ‘commit’ event handler.

Run that, and we’ll be waiting for about 5 seconds until we are told that save() is not a method on the list-item-controller instance:

I have temporarily commented out the other tests in order to remove any noise and focus solely on resolving this bug. This is for local testing and would not commit the spec runner in such a state when committing back to the repo and running latest on a CI server.

To resolve that, let’s open up list-item-controller and add that method:

/script/controller/list-item-controller.js

listItemController = {

  $editableView: undefined,

  $uneditableView: undefined,

  init: function() {

    ...

    return this;

  },

  save: function() {

  },

  dispose: function() {

    ...

  }

};

Run that again, and now we are down to just the timeout failure:

To resolve that, we’ll first create a new event factory method and invoke it from save() to dispatch the commit event, of which we have assigned a handler for in the test:

/script/controller/list-item-controller.js

function createSaveEvent(controller) {

  var event = $.Event('commit');

  event.controller = controller;

  return event;

}

and…

save: function() {

  $(this).trigger(createSaveEvent(this));

},

No more timeout!

But still failing. Reason is that the item had not been removed from the collection. In previous tests we have verified its addition to the collection from the createNewItem() method on list-controller. Now that we are saving the list-item-controller with a modified model that has an invalid name, we are expecting such an action to remove the item from the collection – the crux of our Empty Item bug.

Let’s modify the list-controller in order to handle the commit event from a list-item-controller:

/script/controller/list-controller.js

$collection.on('collection-change', function(event) {

  var model, itemController, $itemView;

  switch( event.kind ) {

    case EventKindEnum.ADD:

      $itemView = $('<li>');

      model = event.items.shift();

      itemController = itemControllerFactory.create($itemView, model);



      $itemView.appendTo(listController.$view);

      rendererList.addItem(itemController);

      itemController.state = itemControllerFactory.state.EDITABLE;

      $(itemController).on('remove', function(event) {

        listController.removeItem(model);

      });

      $(itemController).on('commit', function(event) {

        if(!isValidValue(model.name)) {

          listController.removeItem(model);

        }

      });

      break;

    case EventKindEnum.REMOVE:

      model = event.items.shift();

      itemController = listController.getRendererFromItem(model);



      if(itemController) {

        $itemView = itemController.parentView;

        $itemView.remove();

        itemController.dispose();

        $(itemController).off('remove');

        $(itemController).off('commit');

        rendererList.removeItem(itemController);

      }

      break;

    case EventKindEnum.RESET:

      break;

  }

});

We’ve added another event listener to the list-item-controller when it is added to the collection to handle the commit event. Within the handler we check for its validity – which we have predetermined as not being an empty string – and if it does not pass, then we remove it.
Note: In shipped code I would hold a single reference to a wrapped non-jQuery object and not wrap it multiple times as is shown here when adding and removing event handlers. I left it in this example to not add extra noise to the task at hand.

Run that, and we are back to green!

User Test

Let’s run the application and see if the issue is resolved:

I think that picture says it all…

The bug is still there because save() is not being called on the list-item-controller. In our test, we explicitly called it after a change to the model, but in the application itself it is not being invoked upon change to the model.

But before we start adding calls to save() in our code, let’s think about the steps to reproduce the bug… or at least the fourth step – Click the Tab button to save to list. The tab keystroke, internally to the list-item-controller, causes a blur to the input field. If we look at the code from list-item-controller, that blur event performs a change to state:

/script/controller/list-item-controller.js

$('input', this.$editableView).on('blur', (function(controller) {

  return function(event) {

    controller.model.name = $(this).val();

    controller.state = stateEnum.UNEDITABLE;

  };

}(this)));

This snippet is taken from the event handler assignment in the init() function. When in edit mode of the list-item-controller, once focus is lost from the input field – which happens upon tab – the model is updated and state changed to non-edit mode. My first inkling is to put it here. It is true that, in doing so, it will pass and get rid of the bug… but the real commit of changes to the list-item-controller and its underlying model I feel lie in its change from edit mode to non-edit mode. I would argue that save() should be called from that occurrence. But before we add that to the code, let’s write up an expectation of that behavior.

More Tests

‘WHAT?!? The bug was in our sights. We know how to get rid of it, and you want to write more tests?!?’
That is the voice in my head most of the time when I decide to go back to tests. It has lessened, and the swearing really has decreased. And when I finally do get around to passing tests, it goes away and is replaced with: ‘Nice job! You deserve a raise.. or at least an egg sandwich!’

Anyway, let’s get to Egg Sandwich Status and add another test to ensure that upon change to non-edit mode, if the model properties are not valid, that the item is not saved:

/test/jasmine/spec/feature/saveitem.spec.js

async.it('should not save an empty item upon change to non-edit mode', function(done) {

   var listLength = listController.getItemList().itemLength();



  $(itemRenderer).on('commit', function(event) {

    expect(listController.getItemList().itemLength()).toEqual(listLength - 1);

    done();

  });



  item.name = invalidName;

  itemRenderer.state = itemControllerFactory.state.UNEDITABLE;

});

We have already verified the expectation that a list-item-controller is entered into an EDITABLE state from our additem.spec.js tests, so we don’t need to test for that here – just know that setting the list-item-controller instance to an UNEDITABLE state will trigger it to go into non-edit mode.

Run that, and we get the timeout failure from that test:

Good! Before you slap me… save that hand for modifying the list-item-controller to invoke the save() method from its state-change handler:

/script/controller/list-item-controller.js

function handleStateChange(controller, event) {

  // remove state-based item.

  if( typeof event.oldState !== 'undefined') {

    if(event.oldState === stateEnum.UNEDITABLE) {

      controller.$uneditableView.detach();

    }

    else if(event.oldState === stateEnum.EDITABLE) {

      controller.$editableView.detach();

    }

  }

  // append state-based item.

  if(event.newState === stateEnum.UNEDITABLE) {

    controller.parentView.append(controller.$uneditableView);

    controller.save();

  }

  else if(event.newState === stateEnum.EDITABLE) {

    var inputTimeout = setTimeout( function()  {

      clearTimeout(inputTimeout);

      $('input', controller.$editableView).focus();

    }, 100);

    controller.parentView.append(controller.$editableView);

  }

}

One little addition. Now run the tests:

Hooray! Now use that hand you were gonna slap me with and high-five yourself. Now look at yourself… you look silly. Run the application you crazy solo-high-fiver:

That picture doesn’t say much, but – if all has gone well – it conveys that we are no longer able to save an item to the list when nothing has been provided in the input field and we can close the bug.

Can’t Stop. Won’t Stop

I would normally stop there, but I do like to sew things up nicely and verify all expectations. Such as:

1. Item controller view is not retained in list view if not valid:
   /test/jasmine/spec/feature/saveitem.spec.js

   async.it('should not add an empty item to list view upon change to non-edit mode', function(done) {

   var listViewLength = $listView.children().length;
   

  $(itemRenderer).on('commit', function(event) {

    expect($listView.children().length).toEqual(listViewLength - 1);

    done();

  });



  item.name = invalidName;

  itemRenderer.state = itemControllerFactory.state.UNEDITABLE;

});

1. When going from edit to non-edit mode, valid items are retained:
   /test/jasmine/spec/feature/saveitem.spec.js

   async.it('should save a valid item upon change to non-edit mode', function(done) {

   var listLength = listController.getItemList().itemLength();
   

  $(itemRenderer).on('commit', function(event) {

    expect(listController.getItemList().itemLength()).toEqual(listLength);

    done();

  });



  item.name = itemName;

  itemRenderer.state = itemControllerFactory.state.UNEDITABLE;

});



async.it('should add a valid item to list view upon change to non-edit mode', function(done) {

   var listViewLength = $listView.children.length;



  $(itemRenderer).on('commit', function(event) {

    expect($listView.children().length).toEqual(listViewLength);

    done();

  });



  item.name = itemName;

  itemRenderer.state = itemControllerFactory.state.UNEDITABLE;

});

Those additional specs will pass with flying colors and without having to modify any more application code:

Tagged 0.1.12: https://github.com/bustardcelly/grocery-ls/tree/0.1.12

Conclusion

In this article we tested our way to closing a bug.

Just as we had done in the previous article, we adhered more closely to the principles of TDD and trudge along making things turn red before green – even when we found the reason and knew the resolution for the Empty Item bug. In doing so, we sort of verified and documented in test a Save Item feature. Now we know where to add or modify tests if the usability in committing an item to the list is changed in our requirements.

Todd can’t leave well enough alone

The application is pretty much ready to use as is to boot! There is just one more thing that is nagging me – persistence. No pun intended. We can create a list of all the items we need at the grocery store just fine, but if we close the browser window… oh-noes. It’s lost. I gotta fix that… next.

Cheers for sticking around!

—-

Link Dump

Reference

Test-Driven JavaScript Development by Christian Johansen
Introducing BDD by Dan North
TDD as if you Meant it by Keith Braithwaite
RequireJS
AMD
Jasmine
Sinon
Jasmine.Async

Post Series

grocery-ls github repo
Part I – Introduction
Part II – Feature: Add Item
Part III – Feature: Mark-Off Item
Part IV – Feature: List-Item-Controller
Part V – Feature: List-Controller Refactoring
Part VI – Back to Passing
Part VII – Remove Item
Part VIII – Bug Fixing
Part IX – Persistence
Part X – It Lives!

Posted in AMD, JavaScript, RequireJS, grocery-ls, jasmine, unit-testing.