Merge pull request #1224 from caolan/whilst-test-params

Fix test function arguments for whilst/until/during

7 files changed, 68 insertions, 47 deletions

diff --git a/lib/doDuring.js b/lib/doDuring.js
index d12d668..c536f30 100644
--- a/lib/doDuring.js
+++ b/lib/doDuring.js
@@ -1,4 +1,6 @@
-import during from './during';
+import noop from 'lodash/noop';
+import rest from 'lodash/rest';
+import onlyOnce from './internal/onlyOnce';
 
 /**
  * The post-check version of [`during`]{@link module:ControlFlow.during}. To reflect the difference in
@@ -15,17 +17,28 @@ import during from './during';
  * The function is passed a `callback(err)`, which must be called once it has
  * completed with an optional `err` argument. Invoked with (callback).
  * @param {Function} test - asynchronous truth test to perform before each
- * execution of `fn`. Invoked with (callback).
+ * execution of `fn`. Invoked with (...args, callback), where `...args` are the
+ * non-error args from the previous callback of `fn`.
  * @param {Function} [callback] - A callback which is called after the test
  * function has failed and repeated execution of `fn` has stopped. `callback`
- * will be passed an error and any arguments passed to the final `fn`'s
- * callback. Invoked with (err, [results]);
+ * will be passed an error if one occured, otherwise `null`.
  */
 export default function doDuring(fn, test, callback) {
-    var calls = 0;
+    callback = onlyOnce(callback || noop);
+
+    var next = rest(function(err, args) {
+        if (err)  return callback(err);
+        args.push(check);
+        test.apply(this, args);
+    });
+
+    function check(err, truth) {
+        if (err) return callback(err);
+        if (!truth) return callback(null);
+        fn(next);
+    }
+
+    check(null, true);
 
-    during(function(next) {
-        if (calls++ < 1) return next(null, true);
-        test.apply(this, arguments);
-    }, fn, callback);
 }
+
diff --git a/lib/doWhilst.js b/lib/doWhilst.js
index a425999..ee8f12a 100644
--- a/lib/doWhilst.js
+++ b/lib/doWhilst.js
@@ -1,8 +1,11 @@
-import whilst from './whilst';
+import noop from 'lodash/noop';
+import rest from 'lodash/rest';
+
+import onlyOnce from './internal/onlyOnce';
 
 /**
  * The post-check version of [`whilst`]{@link module:ControlFlow.whilst}. To reflect the difference in
- * the order of operations, the arguments `test` and `fn` are switched.
+ * the order of operations, the arguments `test` and `iteratee` are switched.
  *
  * `doWhilst` is to `whilst` as `do while` is to `while` in plain JavaScript.
  *
@@ -12,20 +15,23 @@ import whilst from './whilst';
  * @method
  * @see [async.whilst]{@link module:ControlFlow.whilst}
  * @category Control Flow
- * @param {Function} fn - A function which is called each time `test` passes.
- * The function is passed a `callback(err)`, which must be called once it has
- * completed with an optional `err` argument. Invoked with (callback).
+ * @param {Function} iteratee - A function which is called each time `test`
+ * passes. The function is passed a `callback(err)`, which must be called once
+ * it has completed with an optional `err` argument. Invoked with (callback).
  * @param {Function} test - synchronous truth test to perform after each
- * execution of `fn`. Invoked with Invoked with the non-error callback results
- * of `fn`.
+ * execution of `iteratee`. Invoked with Invoked with the non-error callback
+ * results of `iteratee`.
  * @param {Function} [callback] - A callback which is called after the test
- * function has failed and repeated execution of `fn` has stopped. `callback`
- * will be passed an error and any arguments passed to the final `fn`'s
- * callback. Invoked with (err, [results]);
+ * function has failed and repeated execution of `iteratee` has stopped.
+ * `callback` will be passed an error and any arguments passed to the final
+ * `iteratee`'s callback. Invoked with (err, [results]);
  */
-export default function doWhilst(fn, test, callback) {
-    var calls = 0;
-    whilst(function() {
-        return ++calls <= 1 || test.apply(this, arguments);
-    }, fn, callback);
+export default function doWhilst(iteratee, test, callback) {
+    callback = onlyOnce(callback || noop);
+    var next = rest(function(err, args) {
+        if (err) return callback(err);
+        if (test.apply(this, args)) return iteratee(next);
+        callback.apply(null, [null].concat(args));
+    });
+    iteratee(next);
 }
diff --git a/lib/during.js b/lib/during.js
index 55af1b9..549ad44 100644
--- a/lib/during.js
+++ b/lib/during.js
@@ -1,5 +1,5 @@
 import noop from 'lodash/noop';
-import rest from 'lodash/rest';
+import onlyOnce from './internal/onlyOnce';
 
 /**
  * Like [`whilst`]{@link module:ControlFlow.whilst}, except the `test` is an asynchronous function that
@@ -20,8 +20,7 @@ import rest from 'lodash/rest';
  * completed with an optional `err` argument. Invoked with (callback).
  * @param {Function} [callback] - A callback which is called after the test
  * function has failed and repeated execution of `fn` has stopped. `callback`
- * will be passed an error and any arguments passed to the final `fn`'s
- * callback. Invoked with (err, [results]);
+ * will be passed an error, if one occured, otherwise `null`.
  * @example
  *
  * var count = 0;
@@ -40,22 +39,18 @@ import rest from 'lodash/rest';
  * );
  */
 export default function during(test, fn, callback) {
-    callback = callback || noop;
+    callback = onlyOnce(callback || noop);
 
-    var next = rest(function(err, args) {
-        if (err) {
-            callback(err);
-        } else {
-            args.push(check);
-            test.apply(this, args);
-        }
-    });
+    function next(err) {
+        if (err) return callback(err);
+        test(check);
+    }
 
-    var check = function(err, truth) {
+    function check(err, truth) {
         if (err) return callback(err);
         if (!truth) return callback(null);
         fn(next);
-    };
+    }
 
     test(check);
 }
diff --git a/lib/whilst.js b/lib/whilst.js
index 3311f4c..9012144 100644
--- a/lib/whilst.js
+++ b/lib/whilst.js
@@ -1,6 +1,8 @@
 import noop from 'lodash/noop';
 import rest from 'lodash/rest';
 
+import onlyOnce from './internal/onlyOnce';
+
 /**
  * Repeatedly call `fn`, while `test` returns `true`. Calls `callback` when
  * stopped, or an error occurs.
@@ -37,11 +39,11 @@ import rest from 'lodash/rest';
  * );
  */
 export default function whilst(test, iteratee, callback) {
-    callback = callback || noop;
+    callback = onlyOnce(callback || noop);
     if (!test()) return callback(null);
     var next = rest(function(err, args) {
         if (err) return callback(err);
-        if (test.apply(this, args)) return iteratee(next);
+        if (test()) return iteratee(next);
         callback.apply(null, [null].concat(args));
     });
     iteratee(next);
diff --git a/mocha_test/during.js b/mocha_test/during.js
index 851acf1..13db8b3 100644
--- a/mocha_test/during.js
+++ b/mocha_test/during.js
@@ -16,7 +16,7 @@ describe('during', function() {
             function (cb) {
                 call_order.push(['iteratee', count]);
                 count++;
-                cb();
+                cb(null, count);
             },
             function (err) {
                 assert(err === null, err + " passed instead of 'null'");
@@ -42,9 +42,10 @@ describe('during', function() {
             function (cb) {
                 call_order.push(['iteratee', count]);
                 count++;
-                cb();
+                cb(null, count);
             },
-            function (cb) {
+            function (c, cb) {
+                expect(c).to.equal(count);
                 call_order.push(['test', count]);
                 cb(null, count < 5);
             },
diff --git a/mocha_test/until.js b/mocha_test/until.js
index a738218..e98b184 100644
--- a/mocha_test/until.js
+++ b/mocha_test/until.js
@@ -7,7 +7,8 @@ describe('until', function(){
         var call_order = [];
         var count = 0;
         async.until(
-            function () {
+            function (c) {
+                expect(c).to.equal(undefined);
                 call_order.push(['test', count]);
                 return (count == 5);
             },
@@ -42,7 +43,8 @@ describe('until', function(){
                 count++;
                 cb(null, count);
             },
-            function () {
+            function (c) {
+                expect(c).to.equal(count);
                 call_order.push(['test', count]);
                 return (count == 5);
             },
diff --git a/mocha_test/whilst.js b/mocha_test/whilst.js
index a384916..e04c2b7 100644
--- a/mocha_test/whilst.js
+++ b/mocha_test/whilst.js
@@ -8,7 +8,8 @@ describe('whilst', function(){
 
         var count = 0;
         async.whilst(
-            function () {
+            function (c) {
+                expect(c).to.equal(undefined);
                 call_order.push(['test', count]);
                 return (count < 5);
             },
@@ -57,7 +58,8 @@ describe('whilst', function(){
                 count++;
                 cb(null, count);
             },
-            function () {
+            function (c) {
+                expect(c).to.equal(count);
                 call_order.push(['test', count]);
                 return (count < 5);
             },