I haven’t blogged for ages, mostly through laziness. The last four working days I’ve been working on a particular problem which I thought was worthy of a write up.
I’ve been retro-fitting unit tests to some of my old code, for a number of reasons – mainly the fact that you run the risk of not being taken seriously as a developer if you don’t unit test the heck out of everything. I have my own thoughts about that – I’ve no doubt they have their place in code which is intended to be deployed one day into a production environment. I spend much of my time in the realm of prototypes where in my mind they inhibit the development process: most of the time I literally don’t know how I’m going to solve a particular technical challenge until I start typing code. Some would say “stop, reflect, plan, write tests, then finally code”. However I feel this would slow down the creative process massively. As a developer you are often mentally juggling a large universe of ideas (see: http://imgur.com/gallery/3uyRWGJ), and launching in feet first enables you to answer the question of the best approach by trying out lots of possibilities, whilst retaining an open mind about implementation. You can then go back over what you’ve done and pick the ideal path through the messy proof of concept to create a vey pure production strength environment with all tests etc. in place, satisfied you’ve already answered all your most challenging questions.
That aside, my code relies heavily on javascript promises (which I’ve used to straighten out the syncroncity and flow of execution), and whilst testing frameworks such as Mocha have a little support for promises, I still really struggled to get things working.
The function I was testing looks like this:
createCollection: function (collection){
var d = Q.defer();
var that = this;
var forecastParamTypes;
collection = JSON.parse(collection);
collectionDAO.collectionDAO.createCollectionRecord(collection.window, collection.brand, collection.businessProcess.id)
.then(
function (result){
collection.id = result.result.insertId;
return that.getParameterTypes();
})
.then(function (fpts) {
forecastParamTypes = fpts.result;
return that.createCPV(forecastParamTypes, "Brand", collection.brand, collection.id);
})
.then(function (success) {
return that.createCPV(forecastParamTypes, "Year Start", collection.startYear, collection.id);
})
.then(function (success) {
return that.createCPV(forecastParamTypes, "Year End", collection.endYear, collection.id);
})
.then(function (success) {
return that.createCPV(forecastParamTypes, "Collection", collection.window, collection.id);
})
.then(function (success){
// loop through the templates
// do that by creating an array of promises
var templatePromises = _.map(collection.templates, function (template) {
return that.iteratetemplateGroups(collection, forecastParamTypes, template);
})
Q.all(templatePromises)
.then(function (status) {
d.resolve(status);
})
.done();
})
.done();
return d.promise;
},
I realise there are other criticisms one can level at this, for example, I understand deferreds are no longer flavour of the month (See Rookie Mistake #4 in this post: https://pouchdb.com/2015/05/18/we-have-a-problem-with-promises.html). and the repeated calls to create data elements in the code breaks all kinds of good coding practice. Like I said it’s a POC and I wouldn’t write it like this were it intended for production. (Basically, break through the code review WTFs/m barrier (http://www.osnews.com/story/19266/WTFs_m) to get to the meat of my post.)
This function is essentially a control structure. I have received some input from the client, and using that information I want to do A then B then C then D finally E to create the complex set of data structures that comprise a collection.
Problem encountered #1: How do I mock the dependencies? This function creates a whole bunch of stuff in the database, which I don’t want to happen when the test is run.
This was quite easy actually: Rewire enabled me to substitute collectionDAO for my own object like so:
var rewire = require('rewire');
var collBL = rewire('../../../server/bl/collectionBL');
describe('collection business logic', function(){
describe('create collection', function () {
beforeEach(function () {
this.collectionDAO = {collectionDAO: {createCollectionRecord: function(){}};
collBL.__set__('collectionDAO', this.collectionDAO);
}
}
}
I also tried another library called proxyquire which seemed to do exactly the same thing.
Problem #2: createCollectionRecord needs to return a promise
This was starting to look ugly. Ideally, instead of my own function I want to use something like a Sinon spy to work out if createCollectionRecord has been called. However this function clearly returns a promise. Sinon doesn’t seem to natively support promises, but there are several libraries which seem to extend Sinon to allow it to. First of all I tried sinon-as-promised (https://www.npmjs.com/package/sinon-as-promised) which seemed pretty simple. It handled the promises as I’d expect, even chaining them together.
Problem # 2.5: However this library didn’t like the call done() at the end (done() prevents the swallowing of errors in the chain).
I tried sinon-promise (https://www.npmjs.com/package/sinon-promise) next, which gave me no such complaints about the done keyword, presumably because it is targetted at Q’s implementation of promises which I have used here, and for this reason I have stuck with this library. There is also sinon-stub-promise(https://github.com/substantial/sinon-stub-promise) which I didn’t get to test. Why so many? NPM is proliferating with lots of seemingly duplicate content, the subtle differences are a little lost on me.
The test which implements the sinon-promise looks like this:
var sinon = require('sinon')
var sinonPromise = require('sinon-promise');
sinonPromise(sinon);
...
this.collectionDAO = {collectionDAO: {
createCollectionRecord:sinon.promise().resolves({result:{insertID:1}})
}};
This works brilliantly, autoresolving my promise, and handling the chained “then” callback correctly.
Problem #3: Mocking the “this” context object.
This really messed with me. It took me ages to firstly realise this was what I needed to do. I was seeing Mocha timeouts which I couldn’t explain. The timeout was 2 seconds, I could have merely extended the timeout, but I think it was trying to do a round trip to the database, which is obviously not desired. Eventually I remembered that console logs are actually emitted when the test is run, so I was able to debug the code and find out where it was timing out
var that = this;
collectionDAO.collectionDAO.createCollectionRecord(collection.window, collection.brand, collection.businessProcess.id)
.then(
function (result){
console.log("1");
collection.id = result.result.insertId;
return that.getParameterTypes();
})
.then(function (fpts) {
console.log("2");
forecastParamTypes = fpts.result;
return that.createCPV(forecastParamTypes, "Brand", collection.brand, collection.id);
})
I was seeing the “1” being echoed to the screen, but not “2”.
I then put a console log into the local function getParameterTypes, which echoed when I thought I’d swapped it out using rewire! So basically “this” was still using the original definition of this call in spite of my rewiring.
It took me another age to work out how to deal with this, but finally I stumbled upon this StackOverflow post: http://stackoverflow.com/questions/23196928/test-the-context-of-a-method-call-using-sinonjs
This post used _.bind() to replace the context object with your own one. I love Underscore, this is not the first time it has got me out of a “bind”!
The code which implements it looks like this:
// Make a stub context object containing a Sinon Spy so we can fake the call to this.<function>
var stubContext = {
getParameterTypes:sinon.promise().resolves({result:"getParameterTypes"}),
createCPV:sinon.promise().resolves({result:"createCPV"})
};
// Use Underscore to rewire the context object
collBL.createCollection = _.bind(collBL.createCollection, stubContext, collection);
So by replacing the context object with my own, and injecting sinon promises I’ve finally been able to get to write what is the point of the test, i.e. the assertions:
sinon.assert.callCount(stubContext.createCPV, 4);
The original code probably took me 30 minutes to write. The unit test, so far has taken me 4 days. I think I need to practice more.