From a201c6e46e5adad8de3dc21a3e5ed998dfa4d9e9 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 6 Mar 2016 17:10:29 -0800 Subject: [PATCH 1/3] convert test to mocha --- mocha_test/waterfall.js | 147 +++++++++++++++++++++++++++++++++++++++ test/test-async.js | 150 +--------------------------------------- 2 files changed, 148 insertions(+), 149 deletions(-) create mode 100644 mocha_test/waterfall.js diff --git a/mocha_test/waterfall.js b/mocha_test/waterfall.js new file mode 100644 index 000000000..f024875f1 --- /dev/null +++ b/mocha_test/waterfall.js @@ -0,0 +1,147 @@ +var async = require('../lib'); +var expect = require('chai').expect; + +describe("waterfall", function () { + + it('basics', function(done){ + var call_order = []; + async.waterfall([ + function(callback){ + call_order.push('fn1'); + setTimeout(function(){callback(null, 'one', 'two');}, 0); + }, + function(arg1, arg2, callback){ + call_order.push('fn2'); + expect(arg1).to.equal('one'); + expect(arg2).to.equal('two'); + setTimeout(function(){callback(null, arg1, arg2, 'three');}, 25); + }, + function(arg1, arg2, arg3, callback){ + call_order.push('fn3'); + expect(arg1).to.equal('one'); + expect(arg2).to.equal('two'); + expect(arg3).to.equal('three'); + callback(null, 'four'); + }, + function(arg4, callback){ + call_order.push('fn4'); + expect(call_order).to.eql(['fn1','fn2','fn3','fn4']); + callback(null, 'test'); + } + ], function(err){ + expect(err === null, err + " passed instead of 'null'"); + done(); + }); + }); + + it('empty array', function(done){ + async.waterfall([], function(err){ + if (err) throw err; + done(); + }); + }); + + it('non-array', function(done){ + async.waterfall({}, function(err){ + expect(err.message).to.equal('First argument to waterfall must be an array of functions'); + done(); + }); + }); + + it('no callback', function(done){ + async.waterfall([ + function(callback){callback();}, + function(callback){callback(); done();} + ]); + }); + + it('async', function(done){ + var call_order = []; + async.waterfall([ + function(callback){ + call_order.push(1); + callback(); + call_order.push(2); + }, + function(callback){ + call_order.push(3); + callback(); + }, + function(){ + expect(call_order).to.eql([1,2,3]); + done(); + } + ]); + }); + + it('error', function(done){ + async.waterfall([ + function(callback){ + callback('error'); + }, + function(callback){ + test.ok(false, 'next function should not be called'); + callback(); + } + ], function(err){ + expect(err).to.equal('error'); + done(); + }); + }); + + it('multiple callback calls', function(done){ + var call_order = []; + var arr = [ + function(callback){ + call_order.push(1); + // call the callback twice. this should call function 2 twice + callback(null, 'one', 'two'); + callback(null, 'one', 'two'); + }, + function(arg1, arg2, callback){ + call_order.push(2); + callback(null, arg1, arg2, 'three'); + }, + function(arg1, arg2, arg3, callback){ + call_order.push(3); + callback(null, 'four'); + }, + function(/*arg4*/){ + call_order.push(4); + arr[3] = function(){ + call_order.push(4); + expect(call_order).to.eql([1,2,2,3,3,4,4]); + done(); + }; + } + ]; + async.waterfall(arr); + }); + + it('call in another context', function(done) { + if (process.browser) { + // node only test + done(); + return; + } + + var vm = require('vm'); + var sandbox = { + async: async, + done: done + }; + + var fn = "(" + (function () { + async.waterfall([function (callback) { + callback(); + }], function (err) { + if (err) { + return done(err); + } + done(); + }); + }).toString() + "())"; + + vm.runInNewContext(fn, sandbox); + }); +}); diff --git a/test/test-async.js b/test/test-async.js index bd103bfc2..b86c3b2fd 100755 --- a/test/test-async.js +++ b/test/test-async.js @@ -1,6 +1,6 @@ /** * NOTE: We are in the process of migrating these tests to Mocha. If you are - * adding a new test, consider creating a new spec file in mocha_tests/ + * adding a new test, please create a new spec file in mocha_tests/ */ require('babel-core/register'); @@ -392,154 +392,6 @@ exports['retry as an embedded task with interval'] = function(test) { }); }; -exports['waterfall'] = { - - 'basic': function(test){ - test.expect(7); - var call_order = []; - async.waterfall([ - function(callback){ - call_order.push('fn1'); - setTimeout(function(){callback(null, 'one', 'two');}, 0); - }, - function(arg1, arg2, callback){ - call_order.push('fn2'); - test.equals(arg1, 'one'); - test.equals(arg2, 'two'); - setTimeout(function(){callback(null, arg1, arg2, 'three');}, 25); - }, - function(arg1, arg2, arg3, callback){ - call_order.push('fn3'); - test.equals(arg1, 'one'); - test.equals(arg2, 'two'); - test.equals(arg3, 'three'); - callback(null, 'four'); - }, - function(arg4, callback){ - call_order.push('fn4'); - test.same(call_order, ['fn1','fn2','fn3','fn4']); - callback(null, 'test'); - } - ], function(err){ - test.ok(err === null, err + " passed instead of 'null'"); - test.done(); - }); -}, - - 'empty array': function(test){ - async.waterfall([], function(err){ - if (err) throw err; - test.done(); - }); -}, - - 'non-array': function(test){ - async.waterfall({}, function(err){ - test.equals(err.message, 'First argument to waterfall must be an array of functions'); - test.done(); - }); -}, - - 'no callback': function(test){ - async.waterfall([ - function(callback){callback();}, - function(callback){callback(); test.done();} - ]); -}, - - 'async': function(test){ - var call_order = []; - async.waterfall([ - function(callback){ - call_order.push(1); - callback(); - call_order.push(2); - }, - function(callback){ - call_order.push(3); - callback(); - }, - function(){ - test.same(call_order, [1,2,3]); - test.done(); - } - ]); -}, - - 'error': function(test){ - test.expect(1); - async.waterfall([ - function(callback){ - callback('error'); - }, - function(callback){ - test.ok(false, 'next function should not be called'); - callback(); - } - ], function(err){ - test.equals(err, 'error'); - }); - setTimeout(test.done, 50); -}, - - 'multiple callback calls': function(test){ - var call_order = []; - var arr = [ - function(callback){ - call_order.push(1); - // call the callback twice. this should call function 2 twice - callback(null, 'one', 'two'); - callback(null, 'one', 'two'); - }, - function(arg1, arg2, callback){ - call_order.push(2); - callback(null, arg1, arg2, 'three'); - }, - function(arg1, arg2, arg3, callback){ - call_order.push(3); - callback(null, 'four'); - }, - function(/*arg4*/){ - call_order.push(4); - arr[3] = function(){ - call_order.push(4); - test.same(call_order, [1,2,2,3,3,4,4]); - test.done(); - }; - } - ]; - async.waterfall(arr); -}, - - 'call in another context': function(test) { - if (isBrowser()) { - // node only test - test.done(); - return; - } - - var vm = require('vm'); - var sandbox = { - async: async, - test: test - }; - - var fn = "(" + (function () { - async.waterfall([function (callback) { - callback(); - }], function (err) { - if (err) { - return test.done(err); - } - test.done(); - }); - }).toString() + "())"; - - vm.runInNewContext(fn, sandbox); -} - -}; - exports['parallel'] = function(test){ var call_order = []; async.parallel([ From d2fe2b323820f4fbce268ade71435d58088118bb Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Sun, 6 Mar 2016 17:33:08 -0800 Subject: [PATCH 2/3] defend against multiple callbacks --- lib/waterfall.js | 34 +++++++++++++++++++--------------- mocha_test/waterfall.js | 37 +++++++++++++++++++------------------ 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/lib/waterfall.js b/lib/waterfall.js index 8867c87e4..e25f62969 100644 --- a/lib/waterfall.js +++ b/lib/waterfall.js @@ -5,28 +5,32 @@ import noop from 'lodash/noop'; import once from 'lodash/once'; import rest from 'lodash/rest'; -import ensureAsync from './ensureAsync'; -import iterator from './iterator'; +import onlyOnce from './internal/onlyOnce'; export default function(tasks, cb) { cb = once(cb || noop); if (!isArray(tasks)) return cb(new Error('First argument to waterfall must be an array of functions')); if (!tasks.length) return cb(); + var taskIndex = 0; - function wrapIterator(iterator) { - return rest(function(err, args) { + function nextTask(args) { + if (taskIndex === tasks.length) { + return cb.apply(null, [null].concat(args)); + } + var task = tasks[taskIndex]; + taskIndex++; + + var taskCallback = onlyOnce(rest(function(err, args) { if (err) { - cb.apply(null, [err].concat(args)); - } else { - var next = iterator.next(); - if (next) { - args.push(wrapIterator(next)); - } else { - args.push(cb); - } - ensureAsync(iterator).apply(null, args); + return cb.apply(null, [err].concat(args)); } - }); + nextTask(args); + })); + + args.push(taskCallback); + + task.apply(null, args); } - wrapIterator(iterator(tasks))(); + + nextTask([]); } diff --git a/mocha_test/waterfall.js b/mocha_test/waterfall.js index f024875f1..b1d1561f9 100644 --- a/mocha_test/waterfall.js +++ b/mocha_test/waterfall.js @@ -68,7 +68,7 @@ describe("waterfall", function () { callback(); }, function(){ - expect(call_order).to.eql([1,2,3]); + expect(call_order).to.eql([1,3]); done(); } ]); @@ -89,33 +89,20 @@ describe("waterfall", function () { }); }); - it('multiple callback calls', function(done){ - var call_order = []; + it('multiple callback calls', function(){ var arr = [ function(callback){ - call_order.push(1); // call the callback twice. this should call function 2 twice callback(null, 'one', 'two'); callback(null, 'one', 'two'); }, function(arg1, arg2, callback){ - call_order.push(2); callback(null, arg1, arg2, 'three'); - }, - function(arg1, arg2, arg3, callback){ - call_order.push(3); - callback(null, 'four'); - }, - function(/*arg4*/){ - call_order.push(4); - arr[3] = function(){ - call_order.push(4); - expect(call_order).to.eql([1,2,2,3,3,4,4]); - done(); - }; } ]; - async.waterfall(arr); + expect(function () { + async.waterfall(arr, function () {}); + }).to.throw(/already called/); }); it('call in another context', function(done) { @@ -144,4 +131,18 @@ describe("waterfall", function () { vm.runInNewContext(fn, sandbox); }); + + it('should not use unnecessary deferrals', function (done) { + var sameStack = true; + + async.waterfall([ + function (cb) { cb(null, 1); }, + function (arg, cb) { cb(); } + ], function() { + expect(sameStack).to.equal(true); + done(); + }); + + sameStack = false; + }); }); From 32993350d03ec9a566ae91e5c279510282e15d17 Mon Sep 17 00:00:00 2001 From: Alexander Early Date: Mon, 7 Mar 2016 23:24:15 -0800 Subject: [PATCH 3/3] clean up code, use ES6 --- lib/waterfall.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/waterfall.js b/lib/waterfall.js index e25f62969..a31b3b444 100644 --- a/lib/waterfall.js +++ b/lib/waterfall.js @@ -15,21 +15,20 @@ export default function(tasks, cb) { function nextTask(args) { if (taskIndex === tasks.length) { - return cb.apply(null, [null].concat(args)); + return cb(null, ...args); } - var task = tasks[taskIndex]; - taskIndex++; var taskCallback = onlyOnce(rest(function(err, args) { if (err) { - return cb.apply(null, [err].concat(args)); + return cb(err, ...args); } nextTask(args); })); args.push(taskCallback); - task.apply(null, args); + var task = tasks[taskIndex++]; + task(...args); } nextTask([]);