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

test(pubsub): fix red lights #171

Closed
wants to merge 3 commits into from
Closed

test(pubsub): fix red lights #171

wants to merge 3 commits into from

Conversation

richardschneider
Copy link
Contributor

@ghost ghost added the in progress label Nov 17, 2017
@@ -398,7 +398,8 @@ module.exports = (common) => {
})
})

it('round trips a non-utf8 binary buffer correctly', (done) => {
// TODO: see https://github.com/ipfs/js-ipfs/pull/1081#issuecomment-345372294
it.skip('round trips a non-utf8 binary buffer correctly', (done) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

These tests work both against js-ipfs core and js-ipfs-api. If they are failing against the http-api then it is something wrong in the http-api.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. It's the sending of a non-UTF8 buffer across HTTP. How is [0x00, 0x01] suppose to be encoded as string in the URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's almost time to raise this as issue.

@daviddias
Copy link
Contributor

@richardschneider is this still necessary for ipfs/js-ipfs#1081 (comment) ?

@richardschneider
Copy link
Contributor Author

@diasdavid Well these interface tests are used all over the place and we know it won't work over HTTP. It's your call.

@daviddias
Copy link
Contributor

@richardschneider They have to work, they work over HTTP when using js-ipfs-api against go-ipfs which is how https://github.com/ipfs/js-ipfs-api gets tested

@richardschneider
Copy link
Contributor Author

@diasdavid Hey, great, Can you show me a test run that sends binary data from JS to GO and then receives the data (via js-subscribe).

@daviddias
Copy link
Contributor

daviddias commented Nov 18, 2017

@richardschneider did you try running npm test on the js-ipfs-api repo? It does that because it uses this batch of tests. I can show this you in a video call too and we can pair program/debug the rest :)

@richardschneider
Copy link
Contributor Author

>mocha test\interop\pubsub --timeout 5000


  pubsub
Swarm listening on /ip4/127.0.0.1/tcp/4110/ws/ipfs/QmV4ooYgQv8sh9CKtMASazoSsSvXVCsRVAhgkh4Wg9sDDW
Swarm listening on /ip4/127.0.0.1/tcp/4009/ipfs/QmV4ooYgQv8sh9CKtMASazoSsSvXVCsRVAhgkh4Wg9sDDW
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 (1131ms)
    ascii data
      √ publish from Go, subscribe on Go (537ms)
      √ publish from JS, subscribe on JS (550ms)
      √ publish from JS, subscribe on Go (1031ms)
      √ publish from Go, subscribe on JS (1036ms)
    non-ascii data
      √ publish from Go, subscribe on Go (526ms)
      √ publish from JS, subscribe on JS (534ms)
      √ publish from JS, subscribe on Go (1033ms)
      √ publish from Go, subscribe on JS (1031ms)
    binary data
      √ publish from Go, subscribe on Go (1012ms)
      √ publish from Go, subscribe on JS (1017ms)
      1) publish from JS, subscribe on Go
      √ publish from JS, subscribe on JS
    2) "after all" hook


  12 passing (20s)
  2 failing

  1) pubsub binary data publish from JS, subscribe on Go:

      Uncaught AssertionError: expected 'efbfbd6161636179656162efbfbd0103056164efbfbd6466666666efbfbd00010203040506070809' to equal 'a36161636179656162830103056164a16466666666f400010203040506070809'
      + expected - actual

      -efbfbd6161636179656162efbfbd0103056164efbfbd6466666666efbfbd00010203040506070809
      +a36161636179656162830103056164a16466666666f400010203040506070809

      at EventEmitter.checkMessage (C:\Users\Owner\Documents\GitHub\js-ipfs\test\interop\pubsub.js:310:45)

@pgte
Copy link
Contributor

pgte commented Nov 21, 2017

this binary data problem is described here ipfs/js-ipfs#1081 (comment) and should be fixed in ipfs/js-ipfs@9d83fa3

@richardschneider
Copy link
Contributor Author

Thanks to @pgte we can close this.

@ghost ghost removed the in progress label Nov 21, 2017
@richardschneider richardschneider deleted the pubsub-tests branch November 21, 2017 21:26
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.

3 participants