Refactoring for good
In my previous post I’ve shown a passing, but strikingly ugly written, test code
it("should load new content when an internal link is clicked", function() {
// mock the check
spyOn($, 'isUrlExternal').and.returnValue(false);
// make the ajax request, will use the 'load' method internally
$frag.ajaxify().find('a[href="/some/url"]').trigger('click');
// mock the ajax request response
jasmine.Ajax.requests.mostRecent().response({
"status": 200,
"content/type": "text/html",
"responseText": "<p>some new lorem</p>"
});
// BAD testing code, BAD, temporary solution
// check the new content after some time has passed
setTimeout(function(){
expect($frag.find("#content").text()).toBe('some new lorem');
}, 5000);
});
where the bad part is clearly pointed out. I’ve modified the test code first to use a better approach
it("should load new content when an internal link is clicked", function() {
var content = '<div id="content"><p>some new lorem</p></div>';
// mock the check for the external link
spyOn($, 'isUrlExternal').and.returnValue(false);
// create a spy fucntion to be injected in the ajax callback method
var spyFunc = jasmine.createSpy('afterLoad');
// trigger the method that will make the ajax request
$frag.ajaxify({afterLoad: spyFunc}).find('a[href="/some/url"]').trigger('click');
// mock the ajax request, it will be a 'load' one with the '#content' selector
jasmine.Ajax.requests.mostRecent().response({
'status': 200,
'content/type': 'text/html',
'responseText': content
});
// verify the call to the success callback function has been made
expect(spyFunc).toHaveBeenCalledWith(jasmine.any(Object), jasmine.any(Object), content);
});
and that implies the possibility to inject of a callback function to be fired when the ajax request succeeds; I’m not fond of modifying the code under test to allow testing, aside for normal object-oriented best practices, but allowing plugin users to specify a custom success callback function only makes sense, the plugin code I will modify to
bindAnchors: function() {
var self = this,
$contentArea = self.$elem.find(self.loadToSelector),
content;
self.$elem.find('a').on('click', function(e) {
var url = e.target.href;
if ($.isUrlExternal(url)) {
return;
}
// do not follow the link
e.preventDefault();
// will be an url like 'http://testing.dev/abc #content'
url = e.target.href + ' ' + self.toLoadSelector;
// before load callback
if (typeof self.options.beforeLoad === 'function') {
content = $contentArea.html();
self.options.beforeLoad(self.$elem, $contentArea, content);
}
// load the new content
$contentArea.parent().load(url, function(){
if (typeof self.options.afterLoad === 'function') {
content = self.$elem.find(self.loadToSelector).html();
self.options.afterLoad(self.$elem, $contentArea, content);
}
});
});
}
and, to stick with the idea that tests are the first form of code documentation, I will change the description of the test to
should allow a plugin user to specify the beforeLoad callback function
Setting spies on prototypes
In JavaScript prototypes are objects and that means that a jasmine method like jasmine.spyOn will work on prototypes as expected.
When the plugin user did not specify a beforeLoad callback function then the plugin will fade out the old content and will fade in, when available, the new one. To test this default behavior I will use
it("should use fadeOut when no beforeLoad method is specified", function() {
// set a spy on the jQuery prototype for the 'fadeOut' method
var fadeOutSpy = spyOn($.prototype, 'fadeOut');
// usual mock of the internal check
spyOn($, 'isUrlExternal').and.returnValue(false);
// trigger the ajax call
$frag.ajaxify().find('a[href="/some/url"]').trigger('click');
// before the ajax request is sent the fadeOut of the old content should
// kick in
expect(fadeOutSpy).toHaveBeenCalled();
});
and the same goes for the default fade in of the new content
it("should use fadeIn when no afterLoad success method is specified", function() {
// set a spy on the jQuery prototype for the 'fadeIn' method
var fadeInSpy = spyOn($.prototype, 'fadeIn');
// usual mock of the internal check
spyOn($, 'isUrlExternal').and.returnValue(false);
// trigger the ajax request
$frag.ajaxify().find('a[href="/some/url"]').trigger('click');
// mock the ajax response
jasmine.Ajax.requests.mostRecent().response({
'status': 200,
'content/type': 'text/html',
'responseText': 'whatever'
});
// the method will be called in the success callback
expect(fadeInSpy).toHaveBeenCalled();
});
Too tight a coupling
The possibility to test the calls on the prototype makes for great power and great responsibility: after finding out it was possible I’ve written a test like
it("should call load with the parameters specified by the plugin user", function() {
var content = '<div class="main-wrapper"><p>some new lorem</p></div>';
// usual mock of the internal link check
spyOn($, 'isUrlExternal').and.returnValue(false);
// I know the plugin uses '$.load' and that will use
// '$.find' when filtering the response
var findSpy = spyOn($.prototype, 'find').and.callThrough();
// activate the plugin with custom parameters and trigger an
// anchor click
$frag.ajaxify({
loadFromSelector: '.main-wrapper',
loadToSelector: '#load-target'
// loadToSelector: '#content-here'
}).find('a[href="/some/url"]').trigger('click');
// the method is used elsewhere, only register new calls
findSpy.calls.reset();
// return the mock ajax response
jasmine.Ajax.requests.mostRecent().response({
'status': 200,
'content/type': 'text/html',
'responseText': content
});
// test the find method has been called with this parameters
expect(findSpy.calls.argsFor(0)).toContain(' .main-wrapper');
expect(findSpy.calls.argsFor(1)).toContain('#load-target');
});
while the thing will work the amount of coupling involved in the test is amazing:
- I know the plugin uses the
loadmethod and presume it will always do - I know the plugin will call the
findmethod in theloadmethod first - and will then call the
findmethod again to attach the retrieved HTML
The sum of it is that I’m not testing that at all to avoid such an amount of coupling and will test outputs in respect to the inputs instead.
The “tell, don’t ask” principle is a fully functional and guiding one in my tests as well.