mlison / protractor-jasmine2-screenshot-reporter

Protractor screenshot reporter for Jasmine2
https://www.npmjs.com/package/protractor-jasmine2-screenshot-reporter
MIT License
81 stars 79 forks source link

jasmineDone is no longer synchronous #117

Open jrm06c opened 7 years ago

jrm06c commented 7 years ago

Why is jasmineDone asynchronous?

The jasmineDone function was synchronous in version 0.3.5 (I just upgraded to 0.5.0). Now it uses the asynchronous function of mkdirp which makes jasmineDone non-blocking.

Some thoughts:

  1. Does this create a race condition? Since there's no callback or promise provided to jasmine, it seems this would create a race condition between jasmineDone and afterLaunch, though the file IO probably accounts for this indirectly and the race is unlikely to be close enough to cause concern.
  2. Some of those handy listeners provided by nodejs such as process.on('exit') and process.on('uncaughtException') allow for cleanup methods to dump the report contents before exiting. jasmineDone being non-blocking renders it useless within this context since node does not resume its event loop before exiting. This means anything that interrupts node's flow control will prevent jasmineDone from completing. For example, calls to process.exit will complete before jasmineDone's mkdirp callback can execute, much less a callback provided to jasmineDone (of which there is none anyway which seems prone to race conditions?).
  3. Couldn't a try/catch be used for the error handling rather than handling it in a callback?
  4. If jasmineDone needs to be asynchronous, can another synchronous function be provided for cleanup?