From 9d07e6f3bef073df62a085f5af53077a7a498c40 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Mon, 2 Nov 2015 22:39:42 +0000 Subject: [PATCH] Pipeline util tests, clean & fix no issue - added comments to pipeline util, inc where it came from - added tests for pipeline util - tests uncovered a bug with promises for args, which has been fixed --- core/server/utils/pipeline.js | 20 +++- core/test/unit/utils_pipeline_spec.js | 129 ++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 core/test/unit/utils_pipeline_spec.js diff --git a/core/server/utils/pipeline.js b/core/server/utils/pipeline.js index 513a4ea540..18d99b5dc3 100644 --- a/core/server/utils/pipeline.js +++ b/core/server/utils/pipeline.js @@ -1,9 +1,17 @@ +/** + * # Pipeline Utility + * + * Based on pipeline.js from when.js: + * https://github.com/cujojs/when/blob/3.7.4/pipeline.js + */ var Promise = require('bluebird'); function pipeline(tasks /* initial arguments */) { var args = Array.prototype.slice.call(arguments, 1), runTask = function (task, args) { + // Self-optimizing function to run first task with multiple + // args using apply, but subsequent tasks via direct invocation runTask = function (task, arg) { return task(arg); }; @@ -11,11 +19,13 @@ function pipeline(tasks /* initial arguments */) { return task.apply(null, args); }; - return Promise.all(tasks).reduce(function (arg, task) { - return Promise.resolve(runTask(task, arg)).then(function (result) { - return result; - }); - }, args); + // Resolve any promises for the arguments passed in first + return Promise.all(args).then(function (args) { + // Iterate through the tasks passing args from one into the next + return Promise.reduce(tasks, function (arg, task) { + return runTask(task, arg); + }, args); + }); } module.exports = pipeline; diff --git a/core/test/unit/utils_pipeline_spec.js b/core/test/unit/utils_pipeline_spec.js new file mode 100644 index 0000000000..e5cee14d80 --- /dev/null +++ b/core/test/unit/utils_pipeline_spec.js @@ -0,0 +1,129 @@ +/*globals describe, afterEach, it*/ +/*jshint expr:true*/ +var should = require('should'), + sinon = require('sinon'), + Promise = require('bluebird'), + +// Stuff we are testing + pipeline = require('../../server/utils/pipeline'), + + sandbox = sinon.sandbox.create(); + +// To stop jshint complaining +should.equal(true, true); + +// These tests are based on the tests in https://github.com/cujojs/when/blob/3.7.4/test/pipeline-test.js +function createTask(y) { + return function (x) { + return x + y; + }; +} + +describe('Pipeline', function () { + afterEach(function () { + sandbox.restore(); + }); + + it('should execute tasks in order', function (done) { + return pipeline([createTask('b'), createTask('c'), createTask('d')], 'a').then( + function (result) { + result.should.eql('abcd'); + done(); + } + ); + }); + + it('should resolve to initial args when no tasks supplied', function (done) { + return pipeline([], 'a', 'b').then( + function (result) { + result.should.eql(['a', 'b']); + done(); + } + ); + }); + + it('should resolve to empty array when no tasks and no args supplied', function (done) { + return pipeline([]).then( + function (result) { + result.should.eql([]); + done(); + } + ); + }); + + it('should pass args to initial task', function (done) { + var expected = [1, 2, 3], + tasks = [sandbox.spy()]; + + return pipeline(tasks, 1, 2, 3).then( + function () { + tasks[0].calledOnce.should.be.true; + tasks[0].firstCall.args.should.eql(expected); + + done(); + } + ); + }); + + it('should allow initial args to be promises', function (done) { + var expected = [1, 2, 3], + tasks = [sandbox.spy()], + Resolver = Promise.resolve; + + return pipeline(tasks, new Resolver(1), new Resolver(2), new Resolver(3)).then( + function () { + tasks[0].calledOnce.should.be.true; + tasks[0].firstCall.args.should.eql(expected); + + done(); + } + ); + }); + + it('should allow tasks to be promises', function (done) { + var expected = [1, 2, 3], + tasks = [ + sandbox.stub().returns(new Promise.resolve(4)), + sandbox.stub().returns(new Promise.resolve(5)), + sandbox.stub().returns(new Promise.resolve(6)) + ]; + + return pipeline(tasks, 1, 2, 3).then( + function (result) { + result.should.eql(6); + tasks[0].calledOnce.should.be.true; + tasks[0].firstCall.args.should.eql(expected); + tasks[1].calledOnce.should.be.true; + tasks[1].firstCall.calledWith(4).should.be.true; + tasks[2].calledOnce.should.be.true; + tasks[2].firstCall.calledWith(5).should.be.true; + + done(); + } + ); + }); + + it('should allow tasks and args to be promises', function (done) { + var expected = [1, 2, 3], + tasks = [ + sandbox.stub().returns(new Promise.resolve(4)), + sandbox.stub().returns(new Promise.resolve(5)), + sandbox.stub().returns(new Promise.resolve(6)) + ], + Resolver = Promise.resolve; + + return pipeline(tasks, new Resolver(1), new Resolver(2), new Resolver(3)).then( + function (result) { + result.should.eql(6); + tasks[0].calledOnce.should.be.true; + tasks[0].firstCall.args.should.eql(expected); + tasks[1].calledOnce.should.be.true; + tasks[1].firstCall.calledWith(4).should.be.true; + tasks[2].calledOnce.should.be.true; + tasks[2].firstCall.calledWith(5).should.be.true; + + done(); + } + ); + }); +});