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.

I would like to advise if the following production code is good to be unit tested. The segment of the production code gets data from a backend service and displays it on the page. It is a class:

...
// Extract this code into a separate method because it is called in more than 1 place
Keywords.prototype.addKeyword = function(keyword) {
    $("ul").append("<li>" + keyword + "</li>");
}

// This is the method that I want to test
Keywords.prototype.getKeywords = function(forGroup) {
    // AppDao contains a bunch of methods that get data
    var keywords = AppDao.getKeywords(forGroup),
        that = this;

    $.each(keywords, function(i, keyword) {
        that.addKeyword(keyword);
    };
}
...

I'm planning to test it in the following way:

  • Stub Keywords.prototype.addKeyword() and make sure that it will be called with 3 arguments: "KW A", "KW B", and "KW C"
  • Stub AppDao.getKeywords() and make sure that it will be called with "Test Group" and will return 3 keywords: "KW A", "KW B", and "KW C"
  • Invoke Keywords.prototype.getKeywords() and verify the stubs

I didn't start the actual implementation of the test yet because I would like to advise with you guys if you think that something can be better done. Thanks.

share|improve this question
    
A general tip: Write the unit test first, then write the code that makes the unit test pass. That's the spirit of proper TDD, and that ensures that the code is easily testable. –  Lstor Jun 17 '13 at 23:45

1 Answer 1

You're off to a bad start. You don't have a definition of what your getKeywords() method is supposed to do, which means you can't even begin to test it. On top of that, it seems to be named backwards - getKeywords() calls addKeyword().

share|improve this answer
    
I don't understand what you mean. Keywords.getKeywords() is the method under test and I wrote its implementation. –  SBel Feb 18 '13 at 14:30
1  
Yes, and its description is "the method I want to test". Not "returns the list of keywords from the database", or something else formal. Plus, you've got a "getWhatever" method that doesn't return anything to the caller, so it's badly named to boot. –  Ross Patterson Feb 18 '13 at 20:50

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.