add electron to travis test matrix #848
Conversation
| - if [[ -z "$ELECTRON_VERSION" && $(echo "$TRAVIS_NODE_VERSION < 4" | bc -l) == "1" ]]; then npm install npm@2 && mv node_modules npm && npm/.bin/npm --version && npm/.bin/npm install; else npm --version && npm install; fi | ||
| - if [[ -z "$ELECTRON_VERSION" ]]; then node_modules/.bin/node-gyp rebuild --directory test; else node_modules/.bin/node-gyp rebuild --target=v$ELECTRON_VERSION --dist-url=https://atom.io/download/electron --directory test; fi | ||
| script: | ||
| - if [[ -z "$ELECTRON_VERSION" ]]; then node_modules/.bin/tap --gc test/js/*-test.js; fi |
There was a problem hiding this comment.
It looks like this is just a build check, if we're not going to actually run the node.js/nan unit tests.
Personally, I'd love to see electron added to the test matrix, but it would make a lot more sense if we could actually run the tests. I don't know if there's a straight forward way to do this, or if we need to wrap them, maybe even create a separate set of tests for electron.....
There was a problem hiding this comment.
Yes, just build tests.
Honestly speaking I have no idea what's actually needed to run tests with electron but I fear it's not that easy as run nodejs which is available on the CI machines.
|
rebased, added electron 5.0.1 and updated other electron version to the current latest version. |
|
Oh wait, maybe I misread the diff the first time around. The |
|
@bnoordhuis if I understand the travis log correct it calls |
|
and looking into the variant for 5.0.1 it seems there is some more work to do in NAN as there are several deprecation warnings. But not sure if this should be done in this PR. |
|
Sorry, you're right. Travis folds the build output but unfolding didn't show anything yesterday. It's there now though. Okay, objection withdrawn. :-) |
|
Thank you. Hopefully this will help in avoiding regressions. |
Seems that #841 got stuck so tried it by myself to add electron to travis test matrix.