fix: don't error to specific #235
Conversation
ghost
assigned 
|
I'm not sure if that's the proper fix. I hope @richardschneider would know. |
|
It's not the proper fix. It also failed for me. I need to revisit this on Monday. |
|
Sorry for the noise, with increasing the timeout it seems to now pass all the time. So please review. |
js/src/miscellaneous.js
Outdated
| // TODO: go-ipfs returns an error, https://github.com/ipfs/go-ipfs/issues/4078 | ||
| ipfs.stop((err) => { | ||
| if (err && err.message !== 'read ECONNRESET') { |
There was a problem hiding this comment.
Why the removal. Normally ipfs.stop should not return an error. There is a bug in go, in that it closes the connection before sending the result.
There was a problem hiding this comment.
The reason is that the error message on Browsers is different from read ECONNRESET. So this check would fail, and the whole test case would fail. Hence I allow one error (no matter which message) to happen (due to the bug in Go).
Edit: Another way doing this would be:
if (err && err.message !== 'read ECONNRESET' && isNode) {
expect(err).to.not.exist()
}This way it would be skipped if the test if run from the Browser.
js/src/miscellaneous.js
Outdated
| if (err && err.message !== 'read ECONNRESET') { | ||
| expect(err).to.not.exist() | ||
| } | ||
| // Retry | ||
| ipfs.stop((err) => { | ||
| expect(err).to.exist() |
There was a problem hiding this comment.
error should not exist.
There was a problem hiding this comment.
Gotcha, then we need to check for go and then expect an error
if (!withGo)
expect(err).to.not.exist()Here's an example of detecting go-ipfs.
When stopping an IPFS node it returns an error. The error message depends on the environment. In Browsers it might also return `Failed to fetch` or `XHR error`. Hence removing the specific error check as it would fail for Browsers. Instead check if the node is an go-ipfs one.
|
I finally understood what the test is actually doing. I added a commit which hopefully also makes it clearer for others. |
|
@vmx do you confirm that this indeed passes with js-ipfs and js-ipfs-api? If so, I'll merge and release |
|
@diasdavid I confirm that the miscellaneous.spec.js passes with js-ipfs-api on Windows and Linux. js-ipfs doesn't run these tests at all atm. |
|
@vmx does run them as well, it just never got the file renamed, still named generic.js https://github.com/ipfs/js-ipfs/blob/master/test/core/interface/generic.js |
|
@diasdavid js-ipfs generic.js passes on Windows and Linux. |
When stopping an IPFS node it returns an error. The error message
depends on the environment. In Browsers it might also return
Failed to fetchorXHR error. Hence removing the specificerror check as it would fail for Browsers.