-
Notifications
You must be signed in to change notification settings - Fork 15
Conversation
//cc @victorbjelkholm |
👍 anything special needs to be done or just running |
just running |
@dryajov great, seems everything worked out-of-the-box! One error on windows though:
Make sure the tests uses port Edit: and |
yeah, this is how we did it before the rewritten |
@daviddias will do asap, afk atm. |
f0c5097
to
93c1809
Compare
@diasdavid do we expect these tests to run on windows %100? |
@dryajov the answer is yes. However, we do know that there are some blockers, can you list what's stopping Travis from running them successfully? |
I'll run this on my windows virtual box see what the issues are - as for travis, I believe it hasn't really started running yet, the current run is queued. |
After some further investigation, I was able to trace both types of failures on windows. The first failure
The failure above is currently due to the fact that graceful daemon shutdown with signals on windows is not possible, due to its lack of full signal support. Node.js will emulate some signals, however as stated in the docs https://nodejs.org/api/process.html#process_signal_events (under Note at the end of that section):
Hence, gracefully shutting down a daemon on windows will not work. In the context of the failing test, this means that, when the Note that,
There is currently no workaround to that limitation - perhaps an alternative way of shutting down daemons needs to be implemented. I'll open an issue with a proposal, but so far I've thought of either an HTTP endpoint ( The second failure:
Is due to some issues with
The above output, indicates that it's failing to create the directories - not entirely sure why tho, looking at the code, its pretty straight forward - just I think we should skip this failing tests on windows for now but open individual issues so we can fix them ASAP. |
issue to implement graceful shutdown - ipfs/js-ipfs#1192 |
@dryajov can we track both of those issues in individual issues? Thank you so much for investigating the cause. I'm going to go ahead and merge this PR since it only is adding Jenkins CI |
No description provided.