-
Notifications
You must be signed in to change notification settings - Fork 2.3k
allow asynchronous callbacks for jasmine tests #728
Conversation
Automated CLA checker says: please sign CLA at http://code.google.com/legal/individual-cla-v1.0.html Please make sure that the email associated with your PR is the same as the email you use to sign. |
CLA signature found, thank you! |
@@ -39,12 +39,19 @@ function wrapInControlFlow(globalFn, fnName) { | |||
|
|||
function asyncTestFn(fn) { | |||
return function(done) { | |||
//deferred object for signaling completion of asychronous function within globalFn |
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.
Nit: space between //
and the first letter of the comment
Looks good except for the possible error added to the done callback. Can you add handling for that? |
var asyncFnDone = webdriver.promise.defer(); | ||
|
||
if (fn.length == 0) { | ||
//function with globalFn not asychronous |
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.
nit: space after // here as well
Error handling isn't working - added a comment. |
// deferred object for signaling completion of asychronous function within globalFn | ||
var asyncFnDone = webdriver.promise.defer(); | ||
|
||
if (fn.length == 0) { |
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.
Isn't this asyncTestFunction always called with just a single non-array (Function) parameter which probably doesn't have a length property which then would cause a non-strict comparison of undefined and 0?
Not sure if you are trying to check if the function is defined to take a done callback as a parameter, which I think you will have to do some function source pattern matching to check.
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.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/length Ok so it seems length returns number of parameter for a function. Didn't know that.
I just ran into this trying to use |
Hmm I see that it was, just trying to resolve if a release has been pushed to NPM with this. |
No description provided.