-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Fix series arguments having 0 lenght #75
Conversation
641fe31
to
9464919
Compare
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.
Some nitpicking in the wording on assertion and test titles.
lib/helpers/normalizeArgs.js
Outdated
@@ -16,7 +16,10 @@ function normalizeArgs(registry, args) { | |||
return fn; | |||
} | |||
|
|||
return map(flatten(args), getFunction); | |||
var flattenArgs = flatten(args); | |||
assert(flattenArgs.length, 'Arguments must be a non-empty array or string'); |
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.
I think this message should be more user friendly. How does "One or more tasks must be combined using series or parallel" sound?
test/series.js
Outdated
@@ -34,6 +34,42 @@ describe('series', function() { | |||
done(); | |||
}); | |||
|
|||
it('should throw on empty object on registered tasks array', function(done) { | |||
function fail() { | |||
taker.series(['test1', 'test2', 'test3', {}]); |
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.
You can add another test for taker.series('test1', 'test2', 'test3', {});
(that's without the array).
test/series.js
Outdated
done(); | ||
}); | ||
|
||
it('should throw on non registered tasks', function(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.
"should throw when no tasks specified"
test/series.js
Outdated
done(); | ||
}); | ||
|
||
it('should throw on empty object of registered task', function(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.
"should throw on non-valid task"
@AlexHenkel this needs to mirror the |
What does this PR do?
TODO