interface-js-ipfs-core icon indicating copy to clipboard operation
interface-js-ipfs-core copied to clipboard

test(files): add test to ensure meaningful error is thrown in `cat()`

Open 0x-r4bbit opened this issue 6 years ago • 4 comments

To verify changes discussed in https://github.com/ipfs/js-ipfs-api/issues/799#issuecomment-402086401, this commit adds a test that checks files.cat() throws a meaningful error when a key or ipfs path with unsupported encoding is given.

License: MIT Signed-off-by: Pascal Precht [email protected]

0x-r4bbit avatar Jul 18 '18 09:07 0x-r4bbit

Hey @PascalPrecht did you run this against js-ipfs as well as js-ipfs-api to check it passes for go-ipfs as well as js-ipfs?

alanshaw avatar Jul 23 '18 12:07 alanshaw

Ha good point @alanshaw !

I've only tested this against js-ipfs-api (where it passes). Running this against js-ipfs gives me:

  1) interface-ipfs-core tests
       .files.cat
         should error on key/path with invalid encoding:
     AssertionError: expected 'Error: Non-base58 character' to include 'Error: selected encoding not supported'
      at ipfs.files.cat.catch (/Users/pascalprecht/projects/ipfs/interface-ipfs-core/js/src/files/cat.js:139:37)

So js-ipfs seems to behave differently to go-ipfs here as well. At least they throw different errors (while both seem legit). What should do you think should be done? Shall I change the test to just expecting some error?

I think though, it'd be better to specify what error we're expecting...

0x-r4bbit avatar Jul 23 '18 19:07 0x-r4bbit

@VictorBjelkholm Hi and thank you for getting back to me here!

Sure, I can add that.

Do you also have some thoughts on

What should do you think should be done? Shall I change the test to just expecting some error?

By any chance?

0x-r4bbit avatar Sep 03 '18 07:09 0x-r4bbit

@PascalPrecht can you rebase master onto this branch?

daviddias avatar Dec 09 '18 15:12 daviddias