current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I would like to hear other people's opinion on how I unit tested a functionality in a Backbone Model. As you see below, the code under test, upon being called to toggle, toggles its completed state and saves the new state:

// js/models/todo.js

  var app = app || {};

  // Todo Model
  // ----------
  // Our basic **Todo** model has `title` and `completed` attributes.

  app.Todo = Backbone.Model.extend({

    // Default attributes ensure that each todo created has `title` and `completed` keys.
    defaults: {
      title: '',
      completed: false
    },

    // Toggle the `completed` state of this todo item.
    toggle: function() {
      this.save({
        completed: !this.get('completed')
      });
    }

  });

To unit test this, as you can see below, I stub Backbone.sync method and make sure that the Model is saved with the correct state:

describe("todo", function() {
    it("should save the toggled completed state", function() {
        var stub = sinon.stub(Backbone, "sync", function(method, model) {
            expect(model.get("completed")).to.equal(true);
        });

        var todo = new app.Todo({completed: false});

        todo.toggle();

        expect(stub.calledOnce).to.equal(true);

        Backbone.sync.restore();
    });
});
share|improve this question

1 Answer 1

up vote 1 down vote

I'm not really familiar with mocha. If I understand correctly, the true and false values here closely releated, in the sense that they should be opposites:

var stub = sinon.stub(Backbone, "sync", function(method, model) {
    expect(model.get("completed")).to.equal(true);
});

var todo = new app.Todo({completed: false});

In that case, to avoid mistakes, it would be better to include that logic in the code, for example:

var initial_state = false;
var stub = sinon.stub(Backbone, "sync", function(method, model) {
    expect(model.get("completed")).to.equal(initial_state);
});

var todo = new app.Todo({completed: !initial_state});

And maybe add another test for the inverse, for initial_state = true.

Instead of this:

var todo = new app.Todo({completed: false});
todo.toggle();

Since you only use the todo instance once, I would combine the two lines to shorten the code:

new app.Todo({completed: false}).toggle();

Instead of this .to.equal(true):

expect(stub.calledOnce).to.equal(true);

I think it would be more natural (and shorter) with .to.be.ok():

expect(stub.calledOnce).to.be.ok();
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.