From 2264600955d0a7d1b28b96f40f41bd64553e201d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9licien=20FRANCOIS?= Date: Wed, 10 Feb 2016 11:28:55 +0100 Subject: [PATCH 1/4] Added tests for falsy callback See #372 --- test/executor.test.js | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/test/executor.test.js b/test/executor.test.js index 6bd1d23..eda33c8 100755 --- a/test/executor.test.js +++ b/test/executor.test.js @@ -38,6 +38,25 @@ function testThrowInCallback (d, done) { }); } +//Test that if the callback is falsy, the next DB operations will still be executed +function testFalsyCallback (d, done) { + d.insert({ a: 1 }, null); + process.nextTick(function () { + d.update({ a: 1 }, { a: 2 }, {}, null); + process.nextTick(function () { + d.update({ a: 2 }, { a: 1 }, null); + process.nextTick(function () { + d.remove({ a: 2 }, {}, null); + process.nextTick(function () { + d.remove({ a: 2 }, null); + process.nextTick(function () { + d.find({}, done); + }); + }); + }); + }); + }); +} // Test that operations are executed in the right order // We prevent Mocha from catching the exception we throw on purpose by remembering all current handlers, remove them and register them back after test ends @@ -130,7 +149,11 @@ describe('Executor', function () { it('A throw in a callback doesnt prevent execution of next operations', function(done) { testThrowInCallback(d, done); }); - + + it('A falsy callback doesnt prevent execution of next operations', function(done) { + testFalsyCallback(d, done); + }); + it('Operations are executed in the right order', function(done) { testRightOrder(d, done); }); @@ -159,7 +182,11 @@ describe('Executor', function () { it('A throw in a callback doesnt prevent execution of next operations', function(done) { testThrowInCallback(d, done); }); - + + it('A falsy callback doesnt prevent execution of next operations', function(done) { + testFalsyCallback(d, done); + }); + it('Operations are executed in the right order', function(done) { testRightOrder(d, done); }); From cada7ff1794a30ccc2be3bb67f35fbdb2b39081c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9licien=20FRANCOIS?= Date: Wed, 10 Feb 2016 11:33:01 +0100 Subject: [PATCH 2/4] When last argument is falsy, Override by default callback instead of appending it --- lib/executor.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/executor.js b/lib/executor.js index a555bd3..70ec590 100755 --- a/lib/executor.js +++ b/lib/executor.js @@ -31,6 +31,9 @@ function Executor () { }; newArguments[newArguments.length - 1] = callback; + } else if (!lastArg && task.arguments.length) { + callback = function () { cb(); }; + newArguments[newArguments.length - 1] = callback; } else { callback = function () { cb(); }; newArguments.push(callback); From be83e8d4eec5c980d09eb21e1265d355d83eb9ea Mon Sep 17 00:00:00 2001 From: Louis Chatriot Date: Thu, 11 Feb 2016 15:06:03 +0100 Subject: [PATCH 3/4] Better formatting --- lib/executor.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/executor.js b/lib/executor.js index 70ec590..add9d1a 100755 --- a/lib/executor.js +++ b/lib/executor.js @@ -11,17 +11,16 @@ function Executor () { // This queue will execute all commands, one-by-one in order this.queue = async.queue(function (task, cb) { - var callback - , lastArg = task.arguments[task.arguments.length - 1] - , i, newArguments = [] - ; + var newArguments = []; // task.arguments is an array-like object on which adding a new field doesn't work, so we transform it into a real array - for (i = 0; i < task.arguments.length; i += 1) { newArguments.push(task.arguments[i]); } + for (var i = 0; i < task.arguments.length; i += 1) { newArguments.push(task.arguments[i]); } + var lastArg = task.arguments[task.arguments.length - 1]; // Always tell the queue task is complete. Execute callback if any was given. if (typeof lastArg === 'function') { - callback = function () { + // Callback was supplied + newArguments[newArguments.length - 1] = function () { if (typeof setImmediate === 'function') { setImmediate(cb); } else { @@ -29,14 +28,12 @@ function Executor () { } lastArg.apply(null, arguments); }; - - newArguments[newArguments.length - 1] = callback; } else if (!lastArg && task.arguments.length) { - callback = function () { cb(); }; - newArguments[newArguments.length - 1] = callback; + // false/undefined/null supplied as callbback + newArguments[newArguments.length - 1] = function () { cb(); }; } else { - callback = function () { cb(); }; - newArguments.push(callback); + // Nothing supplied as callback + newArguments.push(function () { cb(); }); } @@ -51,7 +48,8 @@ function Executor () { * @param {Object} task * task.this - Object to use as this * task.fn - Function to execute - * task.arguments - Array of arguments + * task.arguments - Array of arguments, IMPORTANT: only the last argument may be a function (the callback) + * and the last argument cannot be false/undefined/null * @param {Boolean} forceQueuing Optional (defaults to false) force executor to queue task even if it is not ready */ Executor.prototype.push = function (task, forceQueuing) { From 41827a51e15c6bd508641ca39cf8b648e23e21fd Mon Sep 17 00:00:00 2001 From: Louis Chatriot Date: Sun, 14 Feb 2016 07:47:20 +0100 Subject: [PATCH 4/4] Last necessary test --- lib/executor.js | 2 +- test/executor.test.js | 33 +++++++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/lib/executor.js b/lib/executor.js index add9d1a..979ca7d 100755 --- a/lib/executor.js +++ b/lib/executor.js @@ -28,7 +28,7 @@ function Executor () { } lastArg.apply(null, arguments); }; - } else if (!lastArg && task.arguments.length) { + } else if (!lastArg && task.arguments.length !== 0) { // false/undefined/null supplied as callbback newArguments[newArguments.length - 1] = function () { cb(); }; } else { diff --git a/test/executor.test.js b/test/executor.test.js index eda33c8..73c2060 100755 --- a/test/executor.test.js +++ b/test/executor.test.js @@ -38,7 +38,7 @@ function testThrowInCallback (d, done) { }); } -//Test that if the callback is falsy, the next DB operations will still be executed +// Test that if the callback is falsy, the next DB operations will still be executed function testFalsyCallback (d, done) { d.insert({ a: 1 }, null); process.nextTick(function () { @@ -62,7 +62,7 @@ function testFalsyCallback (d, done) { // We prevent Mocha from catching the exception we throw on purpose by remembering all current handlers, remove them and register them back after test ends function testRightOrder (d, done) { var currentUncaughtExceptionHandlers = process.listeners('uncaughtException'); - + process.removeAllListeners('uncaughtException'); process.on('uncaughtException', function (err) { @@ -99,8 +99,6 @@ function testRightOrder (d, done) { }); } - - // Note: The following test does not have any assertion because it // is meant to address the deprecation warning: // (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral. @@ -114,9 +112,20 @@ var testEventLoopStarvation = function(d, done){ }); } done(); - }; +}; + +// Test that operations are executed in the right order even with no callback +function testExecutorWorksWithoutCallback (d, done) { + d.insert({ a: 1 }); + d.insert({ a: 2 }, false); + d.find({}, function (err, docs) { + docs.length.should.equal(2); + done(); + }); +} -describe('Executor', function () { + +describe.only('Executor', function () { describe('With persistent database', function () { var d; @@ -161,7 +170,11 @@ describe('Executor', function () { it('Does not starve event loop and raise warning when more than 1000 callbacks are in queue', function(done){ testEventLoopStarvation(d, done); }); - + + it('Works in the right order even with no supplied callback', function(done){ + testExecutorWorksWithoutCallback(d, done); + }); + }); // ==== End of 'With persistent database' ==== @@ -190,7 +203,11 @@ describe('Executor', function () { it('Operations are executed in the right order', function(done) { testRightOrder(d, done); }); - + + it('Works in the right order even with no supplied callback', function(done){ + testExecutorWorksWithoutCallback(d, done); + }); + }); // ==== End of 'With non persistent database' ==== });