fix: remove node globals #2932
fix: remove node globals #2932
Conversation
|
looks like libp2p-crypto is working |
hugomrdias
force-pushed
the
test/node-globals
branch
from
dfd985c to
38283bd
Compare
hugomrdias
force-pushed
the
test/node-globals
branch
2 times, most recently
from
e3da1e5 to
bea51ad
Compare
| }) | ||
|
|
||
| it('should add from a HTTP URL with redirection', async () => { | ||
| const text = `TEST${Math.random()}` | ||
| const url = echoUrl(text) + '?foo=bar#buzz' |
There was a problem hiding this comment.
Why did the extra stuff at the end of the url get removed?
There was a problem hiding this comment.
its not needed and the new echoserver uses searchParams
| @@ -483,7 +486,7 @@ module.exports = (common, options) => { | |||
| expect(expectedResult.err).to.not.exist() | |||
| expect(result[0].cid.toString()).to.equal(expectedResult[0].cid.toString()) | |||
| expect(result[0].size).to.equal(expectedResult[0].size) | |||
| expect(result[0].path).to.equal(text) | |||
| // expect(result[0].path).to.equal(text) | |||
There was a problem hiding this comment.
Should this be uncommented?
There was a problem hiding this comment.
same reason as below, path is download now and not the text, i will remove the comment
| const addOpts = { wrapWithDirectory: true } | ||
|
|
||
| const [result, expectedResult] = await Promise.all([ | ||
| all(ipfs.add(urlSource(url), addOpts)), | ||
| all(ipfs.add([{ path: filename, content: Buffer.from(filename) }], addOpts)) | ||
| all(ipfs.add([{ path: 'download', content: Buffer.from(filename) }], addOpts)) |
There was a problem hiding this comment.
Why has filename changed to 'download'?
There was a problem hiding this comment.
this has to do with the echoserver download endpoint, the previous server was simpler and one could do just any path and it would respond, but the new has defined endpoints for different stuff.
So urlSource uses the pathname of the url for the path (http://localhost:3000/download?data=helloworld) we get "download" in file path, i could do a catch-all endpoint just for this use-case so we can build the file path with the url pathname but i didn’t see the value because the test still is testing whats needed so i left it like that.
| const { expect } = require('interface-ipfs-core/src/utils/mocha') | ||
| const all = require('it-all') | ||
| const MockPreloadNode = require('../utils/mock-preload-node') | ||
| const MockPreloadNode = require('../utils/mock-preload-node-utils') |
There was a problem hiding this comment.
Should this not be const MockPreloadNode = require('../utils/mock-preload-node') ?
There was a problem hiding this comment.
i split the server from the helpers so we don't include the server stuff in the tests bundle
use multibase instead
aegir now has a echo server
hugomrdias
force-pushed
the
test/node-globals
branch
from
e92a542 to
5966ce0
Compare
This PR main goal is to remove node globals and is the final PR of this awesome endeavour #2924
I tried to split the commits to make it easier to digest https://github.com/ipfs/js-ipfs/pull/2932/commits
Running
aegir test -t browser --node falsestill fails in a couple of tests because im still waiting on merges inreadable-streamandsimple-peerbut its ready for review.todo: