Production code to be good for unit testing

Question

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.

• Lstor: 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.

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().

• SBel: I don't understand what you mean. Keywords.getKeywords() is the method under test and I wrote its implementation.
• Ross Patterson: 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.