Slay the Dragon! #165
Slay the Dragon! #165
Conversation
| const dir = `${os.tmpdir()}/tmp-${Date.now() + '-' + Math.random().toString(36)}` | ||
|
|
||
| async.waterfall([ | ||
| (cb) => ipfsd.local(dir, cb), | ||
| (node, cb) => { | ||
| daemon = node | ||
| node.init((err) => cb(err, node)) |
There was a problem hiding this comment.
Seems like in Node.js 8 a copy was being made instead of a reference.. WAT?
@dignifiedquire @victorbjelkholm any clue?
There was a problem hiding this comment.
Hmm that sounds dubious, we should not change the test, if this is making copies there is likely a larger issue at play which will be a problem for users of ipfsd-ctl
There was a problem hiding this comment.
It seems that this didn't fixed it after all, I just got lucky
|
This PR solves the issues that #150 was trying to solve. |
daviddias
changed the title
|
|
daviddias
requested review from
victorb,
thisconnect,
dignifiedquire,
JGAntunes and
haadcode
|
Given the sensitivity of this module, I appreciate all the CR I can get :) tl;dr; There were some big issues on how daemons were being spawned and it mostly worked because the race condition was in our favor. Node.js 8 changed that. |
|
wubbalubbadubdub 🎉 |
| expect(api).to.have.property('apiHost') | ||
| expect(api).to.have.property('apiPort') | ||
| expect(api).to.have.property('gatewayHost') | ||
| expect(api).to.have.property('gatewayPort') |
There was a problem hiding this comment.
These checks are unnecessary as the ones below will check if the property exists and is equal to the value.
src/index.js
Outdated
| }, | ||
|
|
||
| /** | ||
| * Create a new disposable node and already start the daemon. |
There was a problem hiding this comment.
Minor typo
already started daemon
|
Thanks @victorbjelkholm :) |
It seems that running our tests in Node.js 8 always fail in AppVeyor and locally (not in Travis or Circle CI interestingly enough)