make callback optional in retry, refactor a bitretry-optional-callback

3 files changed, 147 insertions, 119 deletions

diff --git a/lib/retry.js b/lib/retry.js
index 506a424..aecd07f 100644
--- a/lib/retry.js
+++ b/lib/retry.js
@@ -1,12 +1,12 @@
 'use strict';
 
 import series from './series';
+import noop from 'lodash/noop';
 
 export default function retry(times, task, callback) {
     var DEFAULT_TIMES = 5;
     var DEFAULT_INTERVAL = 0;
 
-    var attempts = [];
 
     var opts = {
         times: DEFAULT_TIMES,
@@ -14,64 +14,62 @@ export default function retry(times, task, callback) {
     };
 
     function parseTimes(acc, t) {
-        if (typeof t === 'number') {
-            acc.times = parseInt(t, 10) || DEFAULT_TIMES;
-        } else if (typeof t === 'object') {
-            acc.times = parseInt(t.times, 10) || DEFAULT_TIMES;
-            acc.interval = parseInt(t.interval, 10) || DEFAULT_INTERVAL;
+        if (typeof t === 'object') {
+            acc.times = +t.times || DEFAULT_TIMES;
+            acc.interval = +t.interval || DEFAULT_INTERVAL;
+        } else if (typeof t === 'number' || typeof t === 'string') {
+            acc.times = +t || DEFAULT_TIMES;
         } else {
-            throw new Error('Unsupported argument type for \'times\': ' + typeof t);
+            throw new Error("Invalid arguments for async.retry");
         }
     }
 
-    var length = arguments.length;
-    if (length < 1 || length > 3) {
-        throw new Error('Invalid arguments - must be either (task), (task, callback), (times, task) or (times, task, callback)');
-    } else if (length <= 2 && typeof times === 'function') {
-        callback = task;
+
+    if (arguments.length < 3 && typeof times === 'function') {
+        callback = task || noop;
         task = times;
-    }
-    if (typeof times !== 'function') {
+    } else {
         parseTimes(opts, times);
+        callback = callback || noop;
     }
-    opts.callback = callback;
-    opts.task = task;
 
-    function wrappedTask(wrappedCallback, wrappedResults) {
-        function retryAttempt(task, finalAttempt) {
-            return function(seriesCallback) {
-                task(function(err, result) {
-                    seriesCallback(!err || finalAttempt, {
-                        err: err,
-                        result: result
-                    });
-                }, wrappedResults);
-            };
-        }
 
-        function retryInterval(interval) {
-            return function(seriesCallback) {
-                setTimeout(function() {
-                    seriesCallback(null);
-                }, interval);
-            };
-        }
+    if (typeof task !== 'function') {
+        throw new Error("Invalid arguments for async.retry");
+    }
 
-        while (opts.times) {
 
-            var finalAttempt = !(opts.times -= 1);
-            attempts.push(retryAttempt(opts.task, finalAttempt));
-            if (!finalAttempt && opts.interval > 0) {
-                attempts.push(retryInterval(opts.interval));
-            }
+    var attempts = [];
+    while (opts.times) {
+        var isFinalAttempt = !(opts.times -= 1);
+        attempts.push(retryAttempt(isFinalAttempt));
+        if (!isFinalAttempt && opts.interval > 0) {
+            attempts.push(retryInterval(opts.interval));
         }
+    }
+
+    series(attempts, function(done, data) {
+        data = data[data.length - 1];
+        callback(data.err, data.result);
+    });
 
-        series(attempts, function(done, data) {
-            data = data[data.length - 1];
-            (wrappedCallback || opts.callback)(data.err, data.result);
-        });
+
+    function retryAttempt(isFinalAttempt) {
+        return function(seriesCallback) {
+            task(function(err, result) {
+                seriesCallback(!err || isFinalAttempt, {
+                    err: err,
+                    result: result
+                });
+            });
+        };
     }
 
-    // If a callback is passed, run this as a controll flow
-    return opts.callback ? wrappedTask() : wrappedTask;
+    function retryInterval(interval) {
+        return function(seriesCallback) {
+            setTimeout(function() {
+                seriesCallback(null);
+            }, interval);
+        };
+    }
 }
diff --git a/mocha_test/retry.js b/mocha_test/retry.js
new file mode 100644
index 0000000..d0df1fe
--- /dev/null
+++ b/mocha_test/retry.js
@@ -0,0 +1,101 @@
+var async = require('../lib');
+var expect = require('chai').expect;
+var assert = require('assert');
+
+describe("retry", function () {
+
+    // Issue 306 on github: https://github.com/caolan/async/issues/306
+    it('retry when attempt succeeds',function(done) {
+        var failed = 3;
+        var callCount = 0;
+        var expectedResult = 'success';
+        function fn(callback) {
+            callCount++;
+            failed--;
+            if (!failed) callback(null, expectedResult);
+            else callback(true); // respond with error
+        }
+        async.retry(fn, function(err, result){
+            assert(err === null, err + " passed instead of 'null'");
+            assert.equal(callCount, 3, 'did not retry the correct number of times');
+            assert.equal(result, expectedResult, 'did not return the expected result');
+            done();
+        });
+    });
+
+    it('retry when all attempts succeeds',function(done) {
+        var times = 3;
+        var callCount = 0;
+        var error = 'ERROR';
+        var erroredResult = 'RESULT';
+        function fn(callback) {
+            callCount++;
+            callback(error + callCount, erroredResult + callCount); // respond with indexed values
+        }
+        async.retry(times, fn, function(err, result){
+            assert.equal(callCount, 3, "did not retry the correct number of times");
+            assert.equal(err, error + times, "Incorrect error was returned");
+            assert.equal(result, erroredResult + times, "Incorrect result was returned");
+            done();
+        });
+    });
+
+    it('retry fails with invalid arguments',function(done) {
+        expect(function() {
+            async.retry("");
+        }).to.throw();
+        expect(function() {
+            async.retry();
+        }).to.throw();
+        expect(function() {
+            async.retry(function() {}, 2, function() {});
+        }).to.throw();
+        done();
+    });
+
+    it('retry with interval when all attempts succeeds',function(done) {
+        var times = 3;
+        var interval = 500;
+        var callCount = 0;
+        var error = 'ERROR';
+        var erroredResult = 'RESULT';
+        function fn(callback) {
+            callCount++;
+            callback(error + callCount, erroredResult + callCount); // respond with indexed values
+        }
+        var start = new Date().getTime();
+        async.retry({ times: times, interval: interval}, fn, function(err, result){
+            var now = new Date().getTime();
+            var duration = now - start;
+            assert(duration >= (interval * (times -1)),  'did not include interval');
+            assert.equal(callCount, 3, "did not retry the correct number of times");
+            assert.equal(err, error + times, "Incorrect error was returned");
+            assert.equal(result, erroredResult + times, "Incorrect result was returned");
+            done();
+        });
+    });
+
+    it("should not require a callback", function (done) {
+        var called = false;
+        async.retry(3, function(cb) {
+            called = true;
+            cb();
+        });
+        setTimeout(function () {
+            assert(called);
+            done();
+        }, 10);
+    });
+
+    it("should not require a callback and use the default times", function (done) {
+        var calls = 0;
+        async.retry(function(cb) {
+            calls++;
+            cb("fail");
+        });
+        setTimeout(function () {
+            expect(calls).to.equal(5);
+            done();
+        }, 50);
+    });
+});
diff --git a/test/test-async.js b/test/test-async.js
index b4aef6c..2e04178 100755
--- a/test/test-async.js
+++ b/test/test-async.js
@@ -278,77 +278,6 @@ exports['seq without callback'] = function (test) {
     add2mul3.call(testcontext, 3);
 };
 
-// Issue 306 on github: https://github.com/caolan/async/issues/306
-exports['retry when attempt succeeds'] = function(test) {
-    var failed = 3;
-    var callCount = 0;
-    var expectedResult = 'success';
-    function fn(callback) {
-        callCount++;
-        failed--;
-        if (!failed) callback(null, expectedResult);
-        else callback(true); // respond with error
-    }
-    async.retry(fn, function(err, result){
-        test.ok(err === null, err + " passed instead of 'null'");
-        test.equal(callCount, 3, 'did not retry the correct number of times');
-        test.equal(result, expectedResult, 'did not return the expected result');
-        test.done();
-    });
-};
-
-exports['retry when all attempts succeeds'] = function(test) {
-    var times = 3;
-    var callCount = 0;
-    var error = 'ERROR';
-    var erroredResult = 'RESULT';
-    function fn(callback) {
-        callCount++;
-        callback(error + callCount, erroredResult + callCount); // respond with indexed values
-    }
-    async.retry(times, fn, function(err, result){
-        test.equal(callCount, 3, "did not retry the correct number of times");
-        test.equal(err, error + times, "Incorrect error was returned");
-        test.equal(result, erroredResult + times, "Incorrect result was returned");
-        test.done();
-    });
-};
-
-exports['retry fails with invalid arguments'] = function(test) {
-    test.throws(function() {
-        async.retry("");
-    });
-    test.throws(function() {
-        async.retry();
-    });
-    test.throws(function() {
-        async.retry(function() {}, 2, function() {});
-    });
-    test.done();
-};
-
-exports['retry with interval when all attempts succeeds'] = function(test) {
-    var times = 3;
-    var interval = 500;
-    var callCount = 0;
-    var error = 'ERROR';
-    var erroredResult = 'RESULT';
-    function fn(callback) {
-        callCount++;
-        callback(error + callCount, erroredResult + callCount); // respond with indexed values
-    }
-    var start = new Date().getTime();
-    async.retry({ times: times, interval: interval}, fn, function(err, result){
-        var now = new Date().getTime();
-        var duration = now - start;
-        test.ok(duration > (interval * (times -1)),  'did not include interval');
-        test.equal(callCount, 3, "did not retry the correct number of times");
-        test.equal(err, error + times, "Incorrect error was returned");
-        test.equal(result, erroredResult + times, "Incorrect result was returned");
-        test.done();
-    });
-};
-
 // need to fix retry, this isn't working
 /*
 exports['retry as an embedded task'] = function(test) {
@@ -371,7 +300,7 @@ exports['retry as an embedded task'] = function(test) {
         test.equal(fooResults, retryResults, "Incorrect results were passed to retry function");
         test.done();
     });
-};*/
+};
 
 exports['retry as an embedded task with interval'] = function(test) {
     var start = new Date().getTime();
@@ -390,7 +319,7 @@ exports['retry as an embedded task with interval'] = function(test) {
         test.ok(duration >= expectedMinimumDuration, "The duration should have been greater than " + expectedMinimumDuration + ", but was " + duration);
         test.done();
     });
-};
+};*/
 
 exports['waterfall'] = {