search

bensu / doo

doo is a library and lein plugin to run cljs.test on different js environments.
Eclipse Public License 1.0
324 stars 63 forks source link

update the logic for selecting exit function #141

Closed loomis closed 6 years ago

loomis commented 6 years ago

With the clojurescript release 1.9.854, a shim has been added for the js/process object. David Nolan has indicated that this will always present regardless of the target platform (Node.js, browser, etc.). Consequently, the existence of the js/process object cannot be used reliably to determine if the Node.js environment is being used.

This change affects the doo.runner namespace, specifically the functions node? and exit!. Currently, with the 1.9.854 the testing process fails, after running the unit tests with the exception:

WARNING: doo's init function was not set

object[TypeError TypeError: undefined is not an object (evaluating 'process_exit.call')]

TypeError: undefined is not an object (evaluating 'process_exit.call’)

This is caused by doo not finding the correct exit function, when targeting the browser.

This PR removes the node? function, as this doesn't reliably determine running on Node.js, and updates the logic for selecting the exit function to default to *exit-fn* if one does not exist in the js/process object. This will continue to work correctly on Node.js, which provides a full js/process object, and default to the *exit-fn* when targeting the browser.

loomis commented 6 years ago

All the CI tests work for me on OS X, but I see that they have failed with AppVeyor on Windows. It appears to be an installation problem with slimerjs. Unfortunately, I don't have a Windows machine to debug the problem, but if I can help validate this PR in some other way, please let me know.

loomis commented 6 years ago

For anyone wanting to workaround this problem and use the latest clojurescript, you can set the compiler option :process-shim false when running your tests.

MatthewDarling commented 6 years ago

I've hit this error on ClojureScript 1.9.473, older than what this PR mentions. Using :process-shim false didn't resolve the error on this particular version.

miikka commented 6 years ago

For whatever it's worth, I think the Slimer issue on AppVeyor can likely be solved by upgrading Slimer.

loomis commented 6 years ago

@miikka If you have an (easy?) way to do the slimerjs upgrade, let me know. I tried but quickly got stuck with the karma launcher pinning the slimerjs version and didn't see an easy fix for that.

miikka commented 6 years ago

@loomis, did you see #137? If that doesn't help, then I don't have an easy fix.

loomis commented 6 years ago

@miikka No, didn't see that. I've merged those changes in and the AppVeyor tests are passing now.