PubSub Interop Tests and CLI+HTTP-API Implementation #1081

richardschneider · 2017-11-16T23:33:40Z

enable testing Impl the http endpoints and cli cmds for PubSub #1068
fix red lights https://github.com/ipfs/js-ipfs/blob/master/test/http-api/spec/pubsub.js
fix red lights https://github.com/ipfs/js-ipfs/blob/master/test/http-api/interface/pubsub.js
~~fix red lights https://github.com/ipfs/js-ipfs/blob/master/test/cli/pubsub.js~~
write an interop test Pubsub message fields #1077

richardschneider · 2017-11-17T01:08:31Z

The pubsub tests need to enable the pubsub experiment in the IPFS server.

daviddias

I like the direction of this PR. I've rebased master after merging the other PubSub changes. Will wait for the rest of the changes :)

daviddias · 2017-11-17T09:39:31Z

src/core/components/no-floodsub.js

+
+}
+
+module.exports = NoFloodSub


Good approach!

daviddias · 2017-11-17T09:39:41Z

src/core/components/start.js

+      self._pubsub = self._options.EXPERIMENTAL.pubsub
+        ? new FloodSub(self._libp2pNode)
+        : new NoFloodSub()
+      self._pubsub.start(done)


…sponse

richardschneider · 2017-11-17T21:37:19Z

The pubsub spec specifies that published data is a buffer. However the go api has it as a string.

How can these two interop? Consider a buffer with [0x00, 0x01]. What is its string representation?

daviddias · 2017-11-17T22:40:30Z

@richardschneider go-ipfs will just toJSON and base64 that when it comes down to the CLI, since you will be using js-ipfs and js-ipfs-api as a client to go-ipfs, you won't need to think about Strings.

richardschneider · 2017-11-17T22:59:42Z

@diasdavid Yes, at the CLI level everything works. This is because CLI starts with a string not a buffer.

At the API things fail if we start with a buffer that does not contain UTF-8 encoded data. This test fails.

 2) .pubsub callback API multiple nodes connected multiple nodes round trips a non-utf8 binary buffer correctly:

      AssertionError: expected 'efbfbd6161636179656162efbfbd0103056164efbfbd6466666666efbfbd' to deeply equal 'a36161636179656162830103056164a16466666666f4'
      + expected - actual

      -efbfbd6161636179656162efbfbd0103056164efbfbd6466666666efbfbd
      +a36161636179656162830103056164a16466666666f4

richardschneider · 2017-11-18T03:00:29Z

test\cli\pubsub is "old" code and hard to test, since in brings in all the other CLI tests with require('./index').repoPath.

I'll let someone else deal with this.

@diasdavid do you want to raise an issue?

richardschneider · 2017-11-18T04:04:20Z

The interop tests are not working because of libp2p/js-libp2p-floodsub#50

richardschneider · 2017-11-18T08:07:33Z

@diasdavid Could you review the interop test. I think its correct, but js/go' and go/js are still failing. Perhaps more eyeballs will help.

Here's the test run (PS: don't worry about pubsub "after all" hook error)

>mocha test\interop\pubsub.js


  pubsub
Swarm listening on /ip4/127.0.0.1/tcp/4110/ws/ipfs/QmTXzKEvkRdoqZTZuKuUdovbNd2RorQeg9HB583VWfePQo
Swarm listening on /ip4/127.0.0.1/tcp/4009/ipfs/QmTXzKEvkRdoqZTZuKuUdovbNd2RorQeg9HB583VWfePQo
peerInfos []
API is listening on: /ip4/127.0.0.1/tcp/5008
Gateway (readonly) is listening on: /ip4/127.0.0.1/tcp/9096
    √ make connections (337ms)
    √ publish from Go, subscribe on Go (549ms)
    √ publish from JS, subscribe on JS (537ms)
    1) publish from JS, subscribe on Go
    2) publish from Go, subscribe on JS
    3) "after all" hook


  3 passing (20s)
  3 failing

  1) pubsub publish from JS, subscribe on Go:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.
      at Timeout.<anonymous> (C:\Users\Owner\AppData\Roaming\npm\node_modules\mocha\lib\runnable.js:226:19)

  2) pubsub publish from Go, subscribe on JS:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.
      at Timeout.<anonymous> (C:\Users\Owner\AppData\Roaming\npm\node_modules\mocha\lib\runnable.js:226:19)

  3) pubsub "after all" hook:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.
      at Timeout.<anonymous> (C:\Users\Owner\AppData\Roaming\npm\node_modules\mocha\lib\runnable.js:226:19)

richardschneider · 2017-11-18T11:16:54Z

@diasdavid my apologies. It looks like I broke libp2p-floodsub. topicCID is used in internal RPC messages. I should only change to topicID for our subscribers.

daviddias · 2017-11-18T21:55:35Z

@richardschneider Does that mean that you now have a fully working interop test for pubsub just waiting for some PR to be merged? If that is so, then awesome!

I do not feel that the mistake was your fault, just a fault of lack of tests + lack of well written spec that would avoid that confusion.

Fortunately, thanks to semver this problem never got shipped to users of js-ipfs. Let's fix it asap and get this interop tests in! Thanks for building all of this :)

Are you working towards fixing the bug introduced with libp2p/js-libp2p-floodsub@b8f66cd ?

richardschneider · 2017-11-19T01:21:38Z

@diasdavid Yes, I have pubsub interop tests. They may need a bit of refactoring. Some are still failing. Which I think is attributable to bad floodsub code.

I'm working on both issues now.

Note that binary data from JS to GO fails

# Conflicts: # test/fixtures/go-ipfs-repo/version # test/interop/pubsub.js

richardschneider · 2017-11-21T03:40:27Z

@diasdavid Sending binary data from JS to GO fails; see the discussion on ipfs-inactive/interface-js-ipfs-core#171

pgte · 2017-11-21T12:24:21Z

@richardschneider it looks like the problem is in the js-ipfs API server. Apparently it fails because it tries translating the query arg containing arbitrary data into a string, and those bytes the test is sending produces an invalid string..

pgte · 2017-11-21T15:17:22Z

@richardschneider 9d83fa3 should solve the problem.
One note: on my local install, for the interior pubsub tests to pass I had to comment the lines validating existence of the topicIDs field.
Could you check this out and see if this fixes this binary data issue for you?

daviddias · 2017-11-21T15:53:00Z

@pgte did you test with the latest floodsub PR?

…given

pgte · 2017-11-21T16:32:03Z

@diasdavid yes I did.
ipfs-pubsub-room tests pass.
peerpad tests pass.
js-ipfs pubsub interop tests seam to pass minus the topicIDs attribute test.

A lot of js-ipfs tests fail, though. Trying to figure out why..

richardschneider · 2017-11-21T21:16:31Z

@pgte Thanks for binary-querysting! The npmjs listing points to an unknown github repo.

One note: on my local install, for the interior pubsub tests to pass I had to comment the lines validating existence of the topicIDs field.

you need to npm link js-ipfsd-ctl to js-ipfs-api

A lot of js-ipfs tests fail, though. Trying to figure out why..

Which files? I've been looking on interface-ipfs-core\src\pubsub and may have a fix

pgte · 2017-11-22T09:07:20Z

@richardschneider thanks for the heads up about binary-querystring! — it should be fixed now.

npm-linking the master of ipfsd-ctl into ipfs-api did not solve the test issue:

Any hints?

daviddias · 2017-11-22T09:15:46Z

Updated js-ipfs-api and js-ipfsd-ctl and I still get this error:

» npm run test:interop

> ipfs@0.26.0 test:interop /Users/imp/code/js-ipfs
> IPFS_TEST=interop aegir test -t node -f test/interop

Test Node.js
non-zero exit code 1
  while running: /Users/imp/code/js-ipfs/node_modules/subcomandante/subcom 65836 /Users/imp/code/js-ipfs/node_modules/go-ipfs-dep/go-ipfs/ipfs daemon [object Object]

  Error: Unknown Command "[object Object]"

Did you mean this?

        object

USAGE
  ipfs daemon - Run a network-connected IPFS node.

  ipfs daemon [--init] [--routing=<routing>] [--mount] [--writable] [--mount-ipfs=<mount-ipfs>] [--mount-ipns=<mount-ipns>] [--unrestricted-api] [--disable-transport-encryption] [--enable-gc] [--manage-fdlimit=false] [--offline] [--migrate] [--enable-pubsub-experiment] [--enable-mplex-experiment=false]

  'ipfs daemon' runs a persistent ipfs daemon that can serve commands
  over the network. Most applications that use IPFS will do so by
  communicating with a daemon over the HTTP API. While the daemon is
  running, calls to 'ipfs' commands will be sent over the network to
  the daemon.

Use 'ipfs daemon --help' for more information about this command.

richardschneider · 2017-11-22T09:25:36Z

@pgte Did you pickup my PR for js-ipfs-api, which is now in master

daviddias · 2017-11-22T09:26:27Z

test/utils/interop-daemon-spawner/go.js

@@ -40,8 +40,7 @@ class GoDaemon {
        this.node = node
        this.node.setConfig('Bootstrap', '[]', cb)
      },
-      (res, cb) => this.node.startDaemon(cb),
-      // (res, cb) => this.node.startDaemon(this.flags, cb),


It is this change causing the latest error

The 2nd line should not be commented out. It's needed to pass --enable-pubsub-experiment to the daemon.

pgte · 2017-11-22T09:29:11Z

@richardschneider yes, my local js-pifs-api is using latest master.

daviddias · 2017-11-22T09:37:35Z

Going to squash and merge this branch into pubsub-cont because it needs the latest stuff on master to pass all tests and rebasing a rebased branch is annoying.

* feat: PubSub Interop Tests and CLI+HTTP-API Implementation (#1081) * test: enable pubsub tests * fix: generate meaniful error when pubsub is called and not enabled * test: enable pubsub for factory daemon * test: enable pubsub tests * fix: generate meaniful error when pubsub is called and not enabled * test: enable pubsub for factory daemon * fiix(pubsub-subscribe): stop HAPI gzip from buffering our streamed response * test: fix spec/pubsub * fix: lint errors * test: tests js/go pubsub interop * test: pubsub interop tests * test: enable pubsub tests * fix: generate meaniful error when pubsub is called and not enabled * test: enable pubsub for factory daemon * fiix(pubsub-subscribe): stop HAPI gzip from buffering our streamed response * test: fix spec/pubsub * fix: lint errors * test: tests js/go pubsub interop * test: pubsub interop tests * test: more tests with different data types Note that binary data from JS to GO fails * HTTP API server: parsing query string as binary in pubsub publish * HTTP API: pubsub: publish should fail gracefully when no argument is given * chore: update deps * chore: update deps * last pass * chore: update deps * test: update interop tests * trying to fix cli pubsub tests * HTTP API server: pubsub pub buffer should have content * making linter happier * pubsub cli tests: higher timeout * making the linter even happier * tests: increasing some of the timeouts * fix: test was wrong * moar * mais um pouco * fix * meh * key size * moar timeouts * taaaaaaaaaaake ooooooonnn meeeeeeee * unomas * take it all * fix * moar * almost there * almost there part II * almost there part III * take out coverage from travis and pass it to circle * small adj in the travis config

test: enable pubsub tests

61966c0

ghost assigned richardschneider Nov 16, 2017

ghost added the status/in-progress In progress label Nov 16, 2017

richardschneider changed the title ~~pubsub is broken~~ pubsub tests are broken Nov 17, 2017

richardschneider added 5 commits November 17, 2017 15:21

fix: generate meaniful error when pubsub is called and not enabled

534d849

test: enable pubsub for factory daemon

12ec8d2

test: enable pubsub tests

1e9f260

fix: generate meaniful error when pubsub is called and not enabled

15ab7a2

test: enable pubsub for factory daemon

dc5166c

daviddias force-pushed the pubsub branch from 12ec8d2 to dc5166c Compare November 17, 2017 09:39

ghost assigned daviddias Nov 17, 2017

daviddias suggested changes Nov 17, 2017

View reviewed changes

daviddias changed the title ~~pubsub tests are broken~~ Finish http-api + cli PubSub tests and write a Interop Test Nov 17, 2017

richardschneider added 2 commits November 17, 2017 23:36

fiix(pubsub-subscribe): stop HAPI gzip from buffering our streamed re…

718645d

…sponse

Merge branch 'pubsub' of https://github.com/ipfs/js-ipfs into pubsub

ad6d9a4

richardschneider mentioned this pull request Nov 17, 2017

test(pubsub): fix red lights ipfs-inactive/interface-js-ipfs-core#171

Closed

richardschneider added 2 commits November 18, 2017 15:18

test: fix spec/pubsub

533d803

fix: lint errors

08955a2

test: tests js/go pubsub interop

c5fda15

richardschneider mentioned this pull request Nov 19, 2017

Various floodsub issues libp2p/js-libp2p-floodsub#51

Merged

test: pubsub interop tests

fb726a7

daviddias force-pushed the pubsub branch from 77f8b14 to fb726a7 Compare November 20, 2017 13:29

daviddias mentioned this pull request Nov 20, 2017

⚡️ v0.27.0 RELEASE 🚀 #1095

Closed

24 tasks

daviddias changed the title ~~Finish http-api + cli PubSub tests and write a Interop Test~~ PubSub Interop Tests and CLI+HTTP-API Implementation Nov 20, 2017

richardschneider added 2 commits November 21, 2017 16:24

test: more tests with different data types

5bc9ce2

Note that binary data from JS to GO fails

Merge branch 'pubsub' of https://github.com/ipfs/js-ipfs into pubsub

97e619f

# Conflicts: # test/fixtures/go-ipfs-repo/version # test/interop/pubsub.js

HTTP API server: parsing query string as binary in pubsub publish

9d83fa3

ghost assigned pgte Nov 21, 2017

HTTP API: pubsub: publish should fail gracefully when no argument is …

fc724e1

…given

chore: update deps

7b5dc81

chore: update deps

0c98a48

daviddias reviewed Nov 22, 2017

View reviewed changes

last pass

18179d7

daviddias changed the base branch from master to pubsub-cont November 22, 2017 09:36

Merge branch 'pubsub-cont' into pubsub

7cdaeec

daviddias merged commit 99f0f87 into pubsub-cont Nov 22, 2017

ghost removed the status/in-progress In progress label Nov 22, 2017

daviddias deleted the pubsub branch November 22, 2017 09:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PubSub Interop Tests and CLI+HTTP-API Implementation #1081

PubSub Interop Tests and CLI+HTTP-API Implementation #1081

richardschneider commented Nov 16, 2017 •

edited

Loading

richardschneider commented Nov 17, 2017

daviddias left a comment

daviddias Nov 17, 2017

daviddias Nov 17, 2017

richardschneider commented Nov 17, 2017 •

edited

Loading

daviddias commented Nov 17, 2017 •

edited

Loading

richardschneider commented Nov 17, 2017

richardschneider commented Nov 18, 2017

richardschneider commented Nov 18, 2017

richardschneider commented Nov 18, 2017

richardschneider commented Nov 18, 2017

daviddias commented Nov 18, 2017 •

edited

Loading

richardschneider commented Nov 19, 2017

richardschneider commented Nov 21, 2017

pgte commented Nov 21, 2017

pgte commented Nov 21, 2017

daviddias commented Nov 21, 2017

pgte commented Nov 21, 2017

richardschneider commented Nov 21, 2017

pgte commented Nov 22, 2017

daviddias commented Nov 22, 2017

richardschneider commented Nov 22, 2017

daviddias Nov 22, 2017

richardschneider Nov 22, 2017

pgte commented Nov 22, 2017

daviddias commented Nov 22, 2017


		}

		module.exports = NoFloodSub

PubSub Interop Tests and CLI+HTTP-API Implementation #1081

PubSub Interop Tests and CLI+HTTP-API Implementation #1081

Conversation

richardschneider commented Nov 16, 2017 • edited Loading

richardschneider commented Nov 17, 2017

daviddias left a comment

Choose a reason for hiding this comment

daviddias Nov 17, 2017

Choose a reason for hiding this comment

daviddias Nov 17, 2017

Choose a reason for hiding this comment

richardschneider commented Nov 17, 2017 • edited Loading

daviddias commented Nov 17, 2017 • edited Loading

richardschneider commented Nov 17, 2017

richardschneider commented Nov 18, 2017

richardschneider commented Nov 18, 2017

richardschneider commented Nov 18, 2017

richardschneider commented Nov 18, 2017

daviddias commented Nov 18, 2017 • edited Loading

richardschneider commented Nov 19, 2017

richardschneider commented Nov 21, 2017

pgte commented Nov 21, 2017

pgte commented Nov 21, 2017

daviddias commented Nov 21, 2017

pgte commented Nov 21, 2017

richardschneider commented Nov 21, 2017

pgte commented Nov 22, 2017

daviddias commented Nov 22, 2017

richardschneider commented Nov 22, 2017

daviddias Nov 22, 2017

Choose a reason for hiding this comment

richardschneider Nov 22, 2017

Choose a reason for hiding this comment

pgte commented Nov 22, 2017

daviddias commented Nov 22, 2017

richardschneider commented Nov 16, 2017 •

edited

Loading

richardschneider commented Nov 17, 2017 •

edited

Loading

daviddias commented Nov 17, 2017 •

edited

Loading

daviddias commented Nov 18, 2017 •

edited

Loading