Make solid tests for node-ipfs-api #81

daviddias · 2015-10-21T21:40:32Z

This endeavour was started and pushed forward by @dignifiedquire and @victorbjelkholm (<3 ref: #78 #77), but we welcome anyone who wants to contribute.

Goal: Have a complete test suit for node-ipfs-api that test both in Node.js and in the Browser.
Stretch goal: Have code coverage checking

This PR will be focused on making tests for IPFS version 0.3.7 (version 0.3.8 will require an update on the request-api.js parser)

Historically, we have had problems because tests are using the global network to perform the DHT queries, this makes testing slow and somewhat unreliable. Tests should their own IPFS cluster, for testing against the global network, we must have integration tests, but that can be the next step.

API

…ion to 0.3.7

daviddias · 2015-10-24T03:52:25Z

@whyrusleeping ls and refs are weird, before they would fail half of the time and we thought it would be because of having to query a big chunk of the network, but now, I've made it with 3 nodes only (disabled Bootstrap and mDNS and connected them manually) and they still fail. Thoughts?

daviddias · 2015-10-24T04:38:37Z

test/test.js

+    if (isNode) {
+      startIndependentNode(ipfsNodes, apiClients, 'a', finish)
+      startIndependentNode(ipfsNodes, apiClients, 'b', finish)
+      startIndependentNode(ipfsNodes, apiClients, 'c', finish)


@dignifiedquire how can we achieve the spawning of several nodes in Karma, but in a helper script, since this can't be executed from the browser?

I suggest we write a script to start karma instead of using the cli, similar to this and then we can wrap the karma start and stop into start and stop of the nodes.

So, then starting the nodes by gulp scripts as well? What is the best way to pass the disposable Node addrs to the Karma test?

Should the Node.js tests also use Gulp to spawn the disposable Node.js?

Trying to figure out what is more common/clean for these type of situations. Thank you for the guidance :)

My thinking is along these lines:

Gulp starts x disposable nodes

Trigger node test run (only tests relying on nodes running)

Trigger browser test run

Gulp stops all nodes

Trigger node tests that involve manual start and stop of nodes (like https://github.com/ipfs/node-ipfs-api/blob/master/test/test.js#L290-L301)

nice! so, would the best way to pass the apiAddrs of the nodes be to pass them through a tmp conf object such as configFile: __dirname + '/karma.conf.js', ?

For dynamic values in the karma.conf you can just pass those through the api call, but to ensure the tests in node as well in the browser have the right apiAddrs I suggest writing them to a file like test/fixtures/addrs.txt and then reading this file from the tests.

daviddias · 2015-10-27T05:16:04Z

pushed last batch of tests for this branch. now it is all about getting it run in the browser :)

dignifiedquire · 2015-10-27T09:52:01Z

@diasdavid pushed fixes for pretty much most of the browser stuff and cleaned up gulp a bit. A couple of things left to do

Fix [0.3.7] .ping returns streaming JSON in the browser #86
Handle gulp not properly exiting, it looks like this is a bug in node-ipfs-clt, see my comment here: https://github.com/ipfs/node-ipfs-api/blob/solid-tests/gulpfile.js#L14-L19
Tweak timeouts so the tests pass on sauce labs every time
Add sauce labs credentials to circleci

…d-tests

daviddias · 2015-10-27T11:32:27Z

sweet! thank you @dignifiedquire :) Just ran it in Sauce Labs and got some passing tests too:) However, running in debug bug still yields the same error to me

» DEBUG=true gulp test:browser                                   ◉ ◼◼◼◼◼◼◼◼◼◼
module.js:339
    throw err;
    ^

Error: Cannot find module 'vinyl'

On:

Add sauce labs credentials to circleci
@lgierth, would you be the best person to handle this part? So that we can keep using it for any other browser module?

daviddias · 2015-10-27T12:10:05Z

DEBUG mode is working fine for me now (apparently I had a old version of gulp installed)

One thing I noticed is that karma fires all the browser tests on top of the same daemons, which might result well because adding to IPFS is idempotent, but kind of unfair, since if one of the add operations fails in one of the browsers, verifying that it is there will still pass. Thoughts?

dignifiedquire · 2015-10-27T12:19:52Z

Make the content of the testfile be equal to the ua string, problem solved :P

daviddias · 2015-10-27T15:58:08Z

ua string?

dignifiedquire · 2015-10-27T16:41:20Z

ua string?

User agent string:

navigator.userAgent
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.71 Safari/537.36"

daviddias · 2015-10-28T02:03:42Z

but with that, we can't know the hash beforehand as new browser versions will come up. The test is about adding something we can get the hash manually via ipfs add.

I'm thinking on pushing the remaining tasks as 'next', so that we can upgrade node-ipfs-api to 0.3.9. What is your opinion?

Remaining tasks:

Fix [0.3.7] .ping returns streaming JSON in the browser #86
Handle gulp not properly exiting, it looks like this is a bug in node-ipfs-clt, see my comment here: https://github.com/ipfs/node-ipfs-api/blob/solid-tests/gulpfile.js#L14-L19
Tweak timeouts so the tests pass on sauce labs every time
Add sauce labs credentials to circleci

dignifiedquire · 2015-10-28T08:57:21Z

Well you don't know the hash in advance but you could instead of checking the hash against a fixed version use cat to get the content after the add and compare the content.

I'm fine with pushing the remaining tasks to next and creating separate issues.

daviddias · 2015-10-28T20:19:14Z

🚢

Make solid tests for node-ipfs-api

dignifiedquire · 2015-10-28T20:33:26Z

sweet ⭐️⭐️

victorb · 2015-10-28T20:34:50Z

Finally! Great job @dignifiedquire @diasdavid 👍👍🏻👍🏻👍🏻👍🏻👍🏻

daviddias · 2015-10-28T20:35:36Z

Thank you both for kicking things off and helping shipping it :D

jbenet · 2015-11-02T00:44:16Z

gavinmcdermott · 2016-05-09T07:36:00Z

@diasdavid, I started looking into the codebase tonight and added a couple of tests. Wondering if you would add me to the repo so I can open a PR for review? If so, thanks much!

daviddias · 2016-05-09T07:49:10Z

@gavinmcdermott That is awesome! Could you make the PR from your fork?

dignifiedquire and others added 11 commits October 19, 2015 12:26

Start implementing browser tests using karma

e5ddc7f

Spawn ipfs node for running tests

700d6f6

Ready for travis

a184f1a

Fix path

105fb32

another try

1934220

fix

2ea7f73

fix

dc39d98

fix3

1665b27

Add saucelabs support

11b07d3

Remove debugging cmd

6ddd117

remove the skips, we need to pass all of the tests + update ipfs vers…

0c31e40

…ion to 0.3.7

daviddias mentioned this pull request Oct 21, 2015

Start implementing browser tests using karma #78

Closed

6 tasks

remove unnecessary second installation of go-ipfs

2771a5c

daviddias added the help wanted label Oct 21, 2015

This was referenced Oct 21, 2015

Add CI for Node.js and Browser tests #57

Closed

add (with recursive: true) returns improperly-formatted JSON #82

Closed

daviddias added 4 commits October 23, 2015 19:15

update ipfsd

39072b0

support for multi node spawning

65802e9

deactivate mdns and bootstrap on the 3 nodes

e65b042

add connect Nodes a to b and c

9d66bfd

reorg tests, update PR state

e45161b

daviddias reviewed Oct 24, 2015
View reviewed changes

daviddias added 2 commits October 23, 2015 21:55

list tests

4edcebd

rm unnecessary comments

3b0a786

dignifiedquire mentioned this pull request Oct 24, 2015

[discuss] Switch to superagent instead of raw http.request #83

Closed

daviddias added 4 commits October 25, 2015 12:28

fix issue #84

f97c0b3

add folder (recursively) test

6675e5b

config.show test

88d24e6

.version and .commands

1f611e9

daviddias added 6 commits October 27, 2015 04:28

dht findprovs test

5fd3c6d

dht findprovs test

e83863e

dht findprovs test

d174fd7

pin add, list and remove tests

0986ce7

remove gateway calls as that does not exist anymore

58d2d83

name.publish and name.resolve tests

540a263

Improve gulp setup and run browser tests

3f8a9a1

Merge branch 'solid-tests' of github.com:ipfs/node-ipfs-api into soli…

adc062c

…d-tests

add debug mode as a npm script

ebc6830

prepare for merge

106ee29

daviddias added a commit that referenced this pull request Oct 28, 2015

Merge pull request #81 from ipfs/solid-tests

645000f

Make solid tests for node-ipfs-api

daviddias merged commit 645000f into master Oct 28, 2015

daviddias mentioned this pull request Oct 28, 2015

Trouble with base64 encoded buffers via browserified ipfsApi #55

Closed

daviddias deleted the solid-tests branch November 2, 2015 04:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make solid tests for node-ipfs-api #81

Make solid tests for node-ipfs-api #81

daviddias commented Oct 21, 2015

daviddias commented Oct 24, 2015

daviddias Oct 24, 2015

dignifiedquire Oct 24, 2015

daviddias Oct 25, 2015

dignifiedquire Oct 25, 2015

daviddias Oct 25, 2015

dignifiedquire Oct 25, 2015

daviddias commented Oct 27, 2015

dignifiedquire commented Oct 27, 2015

daviddias commented Oct 27, 2015

daviddias commented Oct 27, 2015

dignifiedquire commented Oct 27, 2015

daviddias commented Oct 27, 2015

dignifiedquire commented Oct 27, 2015

daviddias commented Oct 28, 2015

dignifiedquire commented Oct 28, 2015

daviddias commented Oct 28, 2015

dignifiedquire commented Oct 28, 2015

victorb commented Oct 28, 2015

daviddias commented Oct 28, 2015

jbenet commented Nov 2, 2015

gavinmcdermott commented May 9, 2016

daviddias commented May 9, 2016

Make solid tests for node-ipfs-api #81

Make solid tests for node-ipfs-api #81

Conversation

daviddias commented Oct 21, 2015

API

daviddias commented Oct 24, 2015

daviddias Oct 24, 2015

Choose a reason for hiding this comment

dignifiedquire Oct 24, 2015

Choose a reason for hiding this comment

daviddias Oct 25, 2015

Choose a reason for hiding this comment

dignifiedquire Oct 25, 2015

Choose a reason for hiding this comment

daviddias Oct 25, 2015

Choose a reason for hiding this comment

dignifiedquire Oct 25, 2015

Choose a reason for hiding this comment

daviddias commented Oct 27, 2015

dignifiedquire commented Oct 27, 2015

daviddias commented Oct 27, 2015

daviddias commented Oct 27, 2015

dignifiedquire commented Oct 27, 2015

daviddias commented Oct 27, 2015

dignifiedquire commented Oct 27, 2015

daviddias commented Oct 28, 2015

dignifiedquire commented Oct 28, 2015

daviddias commented Oct 28, 2015

dignifiedquire commented Oct 28, 2015

victorb commented Oct 28, 2015

daviddias commented Oct 28, 2015

jbenet commented Nov 2, 2015

gavinmcdermott commented May 9, 2016

daviddias commented May 9, 2016