Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Add files.get command and tests #296

Closed
wants to merge 5 commits into from

Conversation

hackergrrl
Copy link
Contributor

@hackergrrl hackergrrl commented Jun 15, 2016

Builds on @nginnever's work, bringing the rest of the implementation to completion.

Requires ipfs-inactive/interface-js-ipfs-core#28 to be merged first to get tests passing.

@daviddias
Copy link
Contributor

@noffle I got a:

 1) .files .get with a multihash:
     Uncaught AssertionError: expected 'QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB' to deeply equal <Buffer 12 20 12 0f 6a f6 01 d4 6e 10 b2 d2 e1 1e d7 1c 55 d2 5f 30 42 c2 25 01 e4 1d 12 46 e7 a1 e9 d3 d8 ec>

Seems that on the interface-ipfs-core tests expect two different result types, check:

@hackergrrl
Copy link
Contributor Author

@@ -0,0 +1,125 @@
/* eslint-env mocha */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these tests cover something that interface-ipfs-core tests don't cover? If yes, why not have all of them at interface-ipfs-core for get?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also getting am error from this file, which makes all the browser tests to fail:

ERROR in ./test/api/get.spec.js
Module build failed: TypeError: Cannot read property 'apply' of undefined

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it, these are the tests for the options supported by go-ipfs

@daviddias
Copy link
Contributor

@daviddias daviddias mentioned this pull request Aug 9, 2016
1 task
@hackergrrl
Copy link
Contributor Author

Closed in favour of #337.

@hackergrrl hackergrrl closed this Aug 9, 2016
@Kubuxu Kubuxu removed the milestone 2 label Aug 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants