-
Notifications
You must be signed in to change notification settings - Fork 298
Conversation
48a3aeb
to
e96c859
Compare
Ok, I thought I had fixed the adding dirs on the browser, but then realised that there is a bug that only happens sometimes (like 1 out of 20), where we don't receive all the hashes for the files added. I thought this could be an js-ipfs-api problem, but further testing (and some fixes) narrowed down the problem to how go-ipfs is responding, even when requests are 100% the same, the response may differ. The failing failing test is: https://github.com/ipfs/interface-ipfs-core/blob/feat/ipfs.files.get/src/files.js#L113-L143 (again: just fails in the browser and some of the times) (left -> success; center -> fails) Full request + response of a Success How to reproduce> git clone git@github.com:ipfs/interface-ipfs-core.git -b feat/ipfs.files.get
> cd interface-ipfs-core
> npm i && npm run build && npm link
> cd ..
> git clone git@github.com:ipfs/js-ipfs-api.git -b feat/ipfs.files.get
> cd js-ipfs-api
> npm i && npm link interface-ipfs-core
> npm run test:browser # repeat this one till it fails |
While this is definitely a bug, it's not a blocker for getting files.get in. How about we file a separate issue for this and get js-ipfs-api and interface-ipfs-core in? |
@diasdavid very strange... i'll look into it. |
@diasdavid those steps fail for me:
|
@whyrusleeping, did you see any errors pop up when running the npm It sounds like you may have a global installation conflicting with the local karma pluggins as commented on by our very own @dignifiedquire here. You could try uninstalling all of the global karma deps and then rerun the local installs. Or maybe dig knows a better solution. I was able to reproduce the bug on the first try. |
thank you @nginnever, yeah @whyrusleeping it is probably that. You can uninstall the global deps with |
@noffle I was thinking about something along those lines, @dignifiedquire would you like to give your opinion? |
I agree that this has nothing to do with this PR in particular as the test fails on a different feature. |
e96c859
to
023b553
Compare
…ade to aegir 6 and fix linting
- Move path.join call out of readStream to not mess with the AST parser - Fix big file test
023b553
to
248cd13
Compare
Ok, going to move ahead and merge. For reference, the tests that fail are:
Because it is a cascading failure, since add doesn't complete well, Just going to release after #339 is fixed |
Builds on top of #296
Missing:
The second test times out because it is looking for a different hash