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

feat: add tests for add data using File DOM api #461

Merged
merged 6 commits into from
May 16, 2019
Merged

feat: add tests for add data using File DOM api #461

merged 6 commits into from
May 16, 2019

Conversation

hugomrdias
Copy link
Contributor

@hugomrdias hugomrdias commented May 2, 2019 •
edited
Loading

This was referenced May 2, 2019
Merged
Merged
@ghost ghost assigned hugomrdias May 2, 2019
@ghost ghost added the in progress label May 2, 2019
@hugomrdias hugomrdias requested review from alanshaw and olizilla May 2, 2019 11:03
@dirkmc
Copy link
Contributor

dirkmc commented May 4, 2019

The code for these three PRs LGTM 👍

I wonder if it would make sense to put the shared util code into this repo (interface-js-ipfs-core) instead of having another repo (js-ipfs-utils), so that there is less overhead in terms of releases etc when there are changes made to the utils code?

@hugomrdias
Copy link
Contributor Author

Yeah I also thought of that, and discussed a bit with Alan and because this repo is only tests a new repo seemed appropriate. Release overhead is a strong point I can revert utils into this repo if we all agree.
Please take a look at the ipfs-utils tests and give feedback, I introduced there the concept of self skipping tests, so we don't need to skip multiple times in the interface-core implementations.

alanshaw
alanshaw suggested changes May 8, 2019
View reviewed changes
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/files-regular/add.js
@@ -125,7 +138,7 @@ module.exports = (createCommon, options) => {

ipfs.add(data, (err) => {
expect(err).to.exist()
expect(err.message).to.contain('invalid input')
expect(err.message).to.contain('Input not supported')
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this pass when these tests are run against js-ipfs-http-client?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@alanshaw
Copy link
Contributor

ping @hugomrdias

@hugomrdias hugomrdias force-pushed the feat/support-file-dom-api branch from 65fa506 to 1a98fab Compare May 16, 2019 14:56
@alanshaw alanshaw merged commit 86a1f3f into master May 16, 2019
@alanshaw alanshaw deleted the feat/support-file-dom-api branch May 16, 2019 20:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alanshaw alanshaw alanshaw approved these changes

@olizilla olizilla Awaiting requested review from olizilla

Assignees

@hugomrdias hugomrdias

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hugomrdias @dirkmc @alanshaw @whyrusleeping