-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add filter option to retry() and retryable(). PR for #1256. #1261
Changes from all commits
cbc5b4f
0337ee0
0af976c
4367751
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -156,5 +156,105 @@ describe("retry", function () { | |
async.retry(5, fn, function(err, result) { | ||
expect(result).to.be.eql({a: 1}); | ||
}); | ||
}) | ||
}); | ||
|
||
it('retry when all attempts fail and error continue test returns true',function(done) { | ||
var times = 3; | ||
var callCount = 0; | ||
var error = 'ERROR'; | ||
var special = 'SPECIAL_ERROR'; | ||
var erroredResult = 'RESULT'; | ||
function fn(callback) { | ||
callCount++; | ||
callback(error + callCount, erroredResult + callCount); | ||
} | ||
function errorTest(err) { | ||
return err && err !== special; | ||
} | ||
var options = { | ||
times: times, | ||
errorFilter: errorTest | ||
}; | ||
async.retry(options, 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 when some attempts fail and error test returns false at some invokation',function(done) { | ||
var callCount = 0; | ||
var error = 'ERROR'; | ||
var special = 'SPECIAL_ERROR'; | ||
var erroredResult = 'RESULT'; | ||
function fn(callback) { | ||
callCount++; | ||
var err = callCount === 2 ? special : error + callCount; | ||
callback(err, erroredResult + callCount); | ||
} | ||
function errorTest(err) { | ||
return err && err === error + callCount; // just a different pattern | ||
} | ||
var options = { | ||
errorFilter: errorTest | ||
}; | ||
async.retry(options, fn, function(err, result){ | ||
assert.equal(callCount, 2, "did not retry the correct number of times"); | ||
assert.equal(err, special, "Incorrect error was returned"); | ||
assert.equal(result, erroredResult + 2, "Incorrect result was returned"); | ||
done(); | ||
}); | ||
}); | ||
|
||
it('retry with interval when some attempts fail and error test returns false at some invokation',function(done) { | ||
var interval = 50; | ||
var callCount = 0; | ||
var error = 'ERROR'; | ||
var erroredResult = 'RESULT'; | ||
var special = 'SPECIAL_ERROR'; | ||
var specialCount = 3; | ||
function fn(callback) { | ||
callCount++; | ||
var err = callCount === specialCount ? special : error + callCount; | ||
callback(err, erroredResult + callCount); | ||
} | ||
function errorTest(err) { | ||
return err && err !== special; | ||
} | ||
var start = new Date().getTime(); | ||
async.retry({ interval: interval, errorFilter: errorTest }, fn, function(err, result){ | ||
var now = new Date().getTime(); | ||
var duration = now - start; | ||
assert(duration >= (interval * (specialCount - 1)), 'did not include interval'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a bit fragile of a test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hi, can you elaborate? Do you mean the specific assertion? I based it on existing test. Just checks that the appropriate time has passed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its probably fine, we have just seen issues lately with some tests erring out (especially in CI) due to slight timing issues |
||
assert.equal(callCount, specialCount, "did not retry the correct number of times"); | ||
assert.equal(err, special, "Incorrect error was returned"); | ||
assert.equal(result, erroredResult + specialCount, "Incorrect result was returned"); | ||
done(); | ||
}); | ||
}); | ||
|
||
it('retry when first attempt succeeds and error test should not be called',function(done) { | ||
var callCount = 0; | ||
var error = 'ERROR'; | ||
var erroredResult = 'RESULT'; | ||
var continueTestCalled = false; | ||
function fn(callback) { | ||
callCount++; | ||
callback(null, erroredResult + callCount); | ||
} | ||
function errorTest(err) { | ||
continueTestCalled = true; | ||
return err && err === error; | ||
} | ||
var options = { | ||
errorFilter: errorTest | ||
}; | ||
async.retry(options, fn, _.rest(function(args) { | ||
assert.equal(callCount, 1, "did not retry the correct number of times"); | ||
expect(args).to.be.eql([null, erroredResult + callCount]); | ||
assert.equal(continueTestCalled, false, "error test function was called"); | ||
done(); | ||
})); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you try to merge these two else conditions