-
Notifications
You must be signed in to change notification settings - Fork 436
Conversation
- exec() is just a shortcut of childProcess.spawn() - run() returns a promise, and waits until PhantomJS get ready
cool! i like this! can you fill out a CLA real quick? |
* @returns {Promise} the process of PhantomJS | ||
*/ | ||
exports.run = function () { | ||
var program = exports.exec.apply(null, arguments) |
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.
what happens right now if you pass bogus arguments to the binary? does it resolve or just hang?
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.
Yeah, for example, --bogus
makes it hang (without error messages). Is it better to be guarded like this?
http://phantomjs.org/api/command-line.html
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.
oh, i was just imagining that we'd reject the promise if the process existed before the promise resolved.
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.
oops, meant to say "if the process exited before the promise resolved". if it's not easy to do, don't worry about it.
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.
Oh, I see. I'll have a look. Wait a bit. Thanks.
@nicks sure! Will do. |
@nicks the logic is wrapped by Updated: README.md, too. |
thanks! |
exec()
is just a shortcut ofchildProcess.spawn()
run()
returns a promise, and waits until PhantomJS get readyTo use
phantomjs-prebuilt
with WebDriver, we needed to detect when PhantomJS gets ready. But that was little bit messy, like this:By this PR, it'll be easy and readable:
If you like this PR, I'd like to add some README text, too. Thanks.