files.addStream started failing

[daviddias]

Although no changes have been made to files, unixfs or unixfs-engine code, there is one test that started failing intermittently:

  1) interface-ipfs-core tests .files callback API .add stream:
     Error: Uncaught AssertionError: expected [] to have a length of 1 but got 0 (webpack:///~/chai/lib/chai/assertion.js?fee1:141:0 <- test/core/interface/interface.spec.js:119792)

I can replicate it sometimes locally and sometimes in CI.

@pgte mind taking a look?

[pgte]

Isolated this issue to the readable stream that is created on the interface-ipfs-core.file test not passing the isstream.isReadable() test. This is because a 'readable-stream' does not pass the isStream() test.

This test is being used in js-ipfs here.

This only happens in the browser.

Any clues on how to solve this? @diasdavid @dignifiedquire

[pgte]

One thing that seams to solve this is in the tests is to use stream.Readable instead of readable-stream in the interface-ipfs-core file tests. I'm not sure about the full story of support for this in webpack and browserify...

[daviddias]

@pgte I've removed readable-stream as a dep of interface-ipfs-core and the problem persists:

 1) interface-ipfs-core tests .files callback API .add stream:
     Error: Uncaught AssertionError: expected [] to have a length of 1 but got 0 (webpack:///~/chai/lib/chai/assertion.js?fee1:141:0 <- test/core/interface/interface.spec.js:114446)

See my patch: https://github.com/ipfs/interface-ipfs-core

[daviddias]

Interestingly, js-ipfs-api is failing on the same call:

 1) .files callback API .add .createAddStream stream of valid files and dirs:
     Error: Timeout of 80000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

https://circleci.com/gh/ipfs/js-ipfs-api/1242

[pgte]

@diasdavid I think it derives from the same issue.
You're correct, I'm still experiencing that issue, depending on how I npm link things around in my local dev environment.

I've managed to build down this one problem to adding an assertion after this line on https://github.com/ipfs/interface-ipfs-core/blob/master/src/files.js#L60

expect(rs instanceof stream.Stream).to.be.true()

This one test passes when I run test:node but fails when I run test:browser.

Again, I'm not too familiar of the inner workings of webpack and how it's being used here. Any hints?

[pgte]

...which is understandable if you look at https://github.com/substack/stream-browserify/blob/master/index.js#L27-L32 . What I'm failing to understand is if and how isstream ever worked on this..

[daviddias]

fixed with #937 (comment). Thanks for all the help with debugging this, @pgte :)

tl;dr; we should never use isstream, use is-stream instead :)