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

test: add test for ipfs.pubsub.publish with a binary (non-utf8) buffer #145

Merged
merged 1 commit into from
Jun 21, 2017

Conversation

vith
Copy link
Contributor

@vith vith commented Jun 21, 2017

This currently fails in js-ipfs-api.

issue: ipfs-inactive/js-ipfs-http-client#569
fix PR: ipfs-inactive/js-ipfs-http-client#570

@daviddias
Copy link
Contributor

Thank you for catching this and coming up with a solution, @vith! Really appreciate it :)

Mind fixing the linting for clean merge?

src/pubsub.js Outdated
expect(msg.data.toString('hex')).to.be.eql(expectedHex)
expect(msg.from).to.be.eql(ipfs2.peerId.id)
check()
} catch (e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use err so that standard can catch if missing

src/pubsub.js Outdated
expect(msg.data.toString('hex')).to.be.eql(expectedHex)
expect(msg.from).to.be.eql(ipfs2.peerId.id)
check()
} catch (e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use err so that standard can catch if missing

src/pubsub.js Outdated
series([
(cb) => ipfs1.pubsub.subscribe(topic, sub1, cb),
(cb) => ipfs2.pubsub.subscribe(topic, sub2, cb),
(cb) => waitForPeers(ipfs2, topic, [ipfs1.peerId.id], cb),
Copy link
Contributor

Choose a reason for hiding this comment

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

dangling comma

@vith
Copy link
Contributor Author

vith commented Jun 21, 2017

Apologies, didn't realize my eslint editor integration wasn't working. Is that better?

@daviddias
Copy link
Contributor

perfect, thank you @vith!

@daviddias daviddias merged commit c56fac7 into ipfs-inactive:master Jun 21, 2017
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.

2 participants