0

I have a method in my project that receives an array of promise returning methods. When the first one is finished it moves to the next one and so forth. I am having a hard time figuring how to unit test this method.

fireAllBatches: function (batchQueue, resolve, reject) {
    if (batchQueue.length) {
        var batch = batchQueue.pop();

        // this returns a promise
        googleCalendarService.fireBatch(batch)
            .then(function (results) {                      
                // when done fires the next one
                this.fireAllBatches(batchQueue, resolve, reject);

            }.bind(this)).catch(reject);
     } else {
        console.log('resolving firing of batches.');
        resolve();
     }
}

This is the test:

it('fireAllBatches should call fireBatch as many times as the number of the batches', function () {
    spyOn(mockGoogleCalendarService, "fireBatch").and.returnValue(q.when({}));

    datalayerObject.fireAllBatches([1, 2, 3, 4, 5, 6]);

    expect(mockGoogleCalendarService.fireBatch).toHaveBeenCalled();
    expect(mockGoogleCalendarService.fireBatch.calls.count()).toBe(6);

});

Update

After investigating and reading this answer. I was able to transform the recursive method to this:

fireAllBatches: function (batchQueue, resolve, reject) {
    var methodArray = _.map(batchQueue, function (batch) {
        return function () {
            console.log('firing batch');
            return googleCalendarService.fireBatch(batch)
        }
    });

    var resolvedPromise = $q.when(true);

    methodArray.reduce(function(cur, next) {
        return cur.then(next);
    }, resolvedPromise).then(resolve).catch(reject);

}

However, I am not sure whether it will catch errors correctly.

Community
  • 1
  • 1
Guy
  • 12,488
  • 16
  • 79
  • 119
  • Guy, sorry to be harsh but before writing your tests, you need to start out with something worth testing. The second attempt looks better than the first but I don't think you want to map before reducing. Try `batchQueue.reduce(...)`. – Roamer-1888 Sep 29 '15 at 17:16
  • At first I did it all at once in the reduce. The code was very short but I had no idea what it was doing. I find that sometimes short efficient code is less readable than expressive code. And since there is no real performance hit why not do it in two stages. Won't you agree? – Guy Sep 30 '15 at 10:37
  • Guy, the reasoning behind my comment is as follows: From the question's first code block, I understand that `googleCalendarService.fireBatch(...)` returns a promise; therefore, in the second code block, `_.map()` creates an array of promises, which is further processed by `methodArray.reduce(...)`; therefore, the `next` var is a promise and `cur.then(next)` will fail because `.then()` expects a function, not a promise. S McCrohan's point below is almost correct (though, as he acknowledges, it's more a comment than an answer). – Roamer-1888 Sep 30 '15 at 11:10

2 Answers2

1

This isn't specifically an answer with regards to unit testing. However, if you're working in ES6, you could do something along these lines to avoid the recursion, and it might simplify your testing:

batchQueue.reduce( (chain,batch) => {
    return chain.then(googleCalendarService.fireBatch(batch))
}, Promise.resolve(null)).then(resolve).catch(reject);
S McCrohan
  • 6,663
  • 1
  • 30
  • 39
1

I would mock or stub out the googleCalendarService.fireBatch() function, so you can verify what it was called with, and then you can just use a spy for the resolve and reject parameters.

Here is what I would test:

  • (Optional) Consider case where batchQueue is null/undefined.
  • It should call resolve immediately if batchQueue is empty
  • It should call the googleCalendarService.fireBatch stub once with the first batch, and then call resolve if you pass in a queue with one batch.
  • It should call the googleCalendarService.fireBatch stub twice and the resolve spy once for a queue of 2 batches.
  • Test if the googleCalendarService.fireBatch function throws an error that the reject spy gets called.

You may think of additional tests as well.

GregL
  • 37,147
  • 8
  • 62
  • 67