From 35e6295580b5235a2b638175fa4cc0ea98f31501 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 23 Oct 2018 14:55:25 +0100 Subject: [PATCH] fix: code review for tests --- package.json | 2 +- src/core/ipns/publisher.js | 2 +- test/cli/name-pubsub.js | 180 ++++++++++++++++++------------------- test/core/name-pubsub.js | 83 +++++++++++++++++ 4 files changed, 170 insertions(+), 97 deletions(-) create mode 100644 test/core/name-pubsub.js diff --git a/package.json b/package.json index 1e6a2703ff..68ff36883e 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,7 @@ "cids": "~0.5.5", "class-is": "^1.1.0", "datastore-core": "~0.4.0", - "datastore-pubsub": "~0.0.2", + "datastore-pubsub": "ipfs/js-datastore-pubsub#feat/encode-record-store-keys", "debug": "^4.1.0", "deep-extend": "~0.6.0", "err-code": "^1.1.2", diff --git a/src/core/ipns/publisher.js b/src/core/ipns/publisher.js index d33713ae75..0c77031c56 100644 --- a/src/core/ipns/publisher.js +++ b/src/core/ipns/publisher.js @@ -67,7 +67,7 @@ class IpnsPublisher { let keys try { - keys = ipns.getIdKeys(peerId.id) + keys = ipns.getIdKeys(peerId.toBytes()) } catch (err) { log.error(err) return callback(err) diff --git a/test/cli/name-pubsub.js b/test/cli/name-pubsub.js index dbd89b8698..6b252f67a1 100644 --- a/test/cli/name-pubsub.js +++ b/test/cli/name-pubsub.js @@ -83,24 +83,21 @@ describe('name-pubsub', () => { // Connect before(function () { - return ipfsA('swarm', 'connect', bMultiaddr).then((out) => { - expect(out).to.eql(`connect ${bMultiaddr} success\n`) - }) + return ipfsA('swarm', 'connect', bMultiaddr) + .then((out) => { + expect(out).to.eql(`connect ${bMultiaddr} success\n`) + }) }) after((done) => parallel(nodes.map((node) => (cb) => node.stop(cb)), done)) describe('pubsub commands', () => { - before(function (done) { - this.timeout(50 * 1000) - done() - }) - it('should get enabled state of pubsub', function () { - return ipfsA('name pubsub state').then((res) => { - expect(res).to.exist() - expect(res).to.have.string('enabled') // enabled - }) + return ipfsA('name pubsub state') + .then((res) => { + expect(res).to.exist() + expect(res).to.have.string('enabled') // enabled + }) }) it('should subscribe on name resolve', function () { @@ -110,19 +107,17 @@ describe('name-pubsub', () => { .catch((err) => { expect(err).to.exist() // Not available (subscribed) - return Promise.all([ - ipfsB('pubsub ls'), - ipfsB('name pubsub subs') - ]) - .then((res) => { - expect(res).to.exist() - - expect(res[0]).to.exist() - expect(res[0]).to.have.string('/ipns/') // have an ipns subscribtion + return ipfsB('pubsub ls') + }) + .then((res) => { + expect(res).to.exist() + expect(res).to.have.string('/record/') // have a record ipns subscribtion - expect(res[1]).to.exist() - expect(res[1]).to.have.string(`/ipns/${nodeAId.id}`) // have subscription - }) + return ipfsB('name pubsub subs') + }) + .then((res) => { + expect(res).to.exist() + expect(res).to.have.string(`/ipns/${nodeAId.id}`) // have subscription }) }) @@ -135,29 +130,27 @@ describe('name-pubsub', () => { expect(res).to.have.string('no subscription') // tried to cancel a not yet subscribed id return ipfsA(`name resolve ${nodeBId.id}`) - .catch((err) => { - expect(err).to.exist() // Not available (subscribed now) - - return ipfsA(`name pubsub cancel /ipns/${nodeBId.id}`) - .then((res) => { - expect(res).to.exist() - expect(res).to.have.string('canceled') // canceled now - - return Promise.all([ - ipfsA('pubsub ls'), - ipfsA('name pubsub subs') - ]) - .then((res) => { - expect(res).to.exist() - - expect(res[0]).to.exist() - expect(res[0]).to.not.have.string('/ipns/') // ipns subscribtion not available - - expect(res[1]).to.exist() - expect(res[1]).to.not.have.string(`/ipns/${nodeBId.id}`) // ipns subscribtion not available - }) - }) - }) + }) + .catch((err) => { + expect(err).to.exist() // Not available (subscribed now) + + return ipfsA(`name pubsub cancel /ipns/${nodeBId.id}`) + }) + .then((res) => { + expect(res).to.exist() + expect(res).to.have.string('canceled') // canceled now + + return ipfsA('pubsub ls') + }) + .then((res) => { + expect(res).to.exist() + expect(res).to.not.have.string('/ipns/') // ipns subscribtion not available + + return ipfsA('name pubsub subs') + }) + .then((res) => { + expect(res).to.exist() + expect(res).to.not.have.string(`/ipns/${nodeBId.id}`) // ipns subscribtion not available }) }) }) @@ -167,10 +160,11 @@ describe('name-pubsub', () => { before(function (done) { this.timeout(50 * 1000) - ipfsA('add src/init-files/init-docs/readme').then((out) => { - cidAdded = out.split(' ')[1] - done() - }) + ipfsA('add src/init-files/init-docs/readme') + .then((out) => { + cidAdded = out.split(' ')[1] + done() + }) }) it('should publish the received record to the subscriber', function () { @@ -182,30 +176,28 @@ describe('name-pubsub', () => { expect(res).to.satisfy(checkAll([emptyDirCid])) // Empty dir received (subscribed) return ipfsA(`name resolve ${nodeBId.id}`) - .catch((err) => { - expect(err).to.exist() // Not available (subscribed now) - - return ipfsB(`name publish ${cidAdded}`) - .then((res) => { - // published to IpfsB and published through pubsub to ipfsa - expect(res).to.exist() - expect(res).to.satisfy(checkAll([cidAdded, nodeBId.id])) - - return Promise.all([ - ipfsB(`name resolve ${nodeBId.id}`), - ipfsA(`name resolve ${nodeBId.id}`) - ]) - .then((res) => { - expect(res).to.exist() - - expect(res[0]).to.exist() - expect(res[0]).to.satisfy(checkAll([cidAdded])) - - expect(res[1]).to.exist() - expect(res[1]).to.satisfy(checkAll([cidAdded])) // value propagated to node B - }) - }) - }) + }) + .catch((err) => { + expect(err).to.exist() // Not available (subscribed now) + + return ipfsB(`name publish ${cidAdded}`) + }) + .then((res) => { + // published to IpfsB and published through pubsub to ipfsa + expect(res).to.exist() + expect(res).to.satisfy(checkAll([cidAdded, nodeBId.id])) + + return ipfsB(`name resolve ${nodeBId.id}`) + }) + .then((res) => { + expect(res).to.exist() + expect(res).to.satisfy(checkAll([cidAdded])) + + return ipfsA(`name resolve ${nodeBId.id}`) + }) + .then((res) => { + expect(res).to.exist() + expect(res).to.satisfy(checkAll([cidAdded])) // value propagated to node B }) }) }) @@ -235,30 +227,28 @@ describe('name-pubsub', () => { after((done) => parallel(nodes.map((node) => (cb) => node.stop(cb)), done)) - it('should get disabled state of pubsub', function (done) { - ipfsA('name pubsub state').then((res) => { - expect(res).to.exist() - expect(res).to.have.string('disabled') - - done() - }) + it('should get disabled state of pubsub', function () { + return ipfsA('name pubsub state') + .then((res) => { + expect(res).to.exist() + expect(res).to.have.string('disabled') + }) }) - it('should get error getting the available subscriptions', function (done) { - ipfsA('name pubsub subs').catch((err) => { - expect(err).to.exist() // error as it is disabled - expect(err.toString()).to.have.string('IPNS pubsub subsystem is not enabled') - done() - }) + it('should get error getting the available subscriptions', function () { + return ipfsA('name pubsub subs') + .catch((err) => { + expect(err).to.exist() // error as it is disabled + expect(err.toString()).to.have.string('IPNS pubsub subsystem is not enabled') + }) }) - it('should get error canceling a subscription', function (done) { - ipfsA('name pubsub cancel /ipns/QmSWxaPcGgf4TDnFEBDWz2JnbHywF14phmY9hNcAeBEK5v').catch((err) => { - expect(err).to.exist() // error as it is disabled - expect(err.toString()).to.have.string('IPNS pubsub subsystem is not enabled') - - done() - }) + it('should get error canceling a subscription', function () { + return ipfsA('name pubsub cancel /ipns/QmSWxaPcGgf4TDnFEBDWz2JnbHywF14phmY9hNcAeBEK5v') + .catch((err) => { + expect(err).to.exist() // error as it is disabled + expect(err.toString()).to.have.string('IPNS pubsub subsystem is not enabled') + }) }) }) }) diff --git a/test/core/name-pubsub.js b/test/core/name-pubsub.js new file mode 100644 index 0000000000..510fd44599 --- /dev/null +++ b/test/core/name-pubsub.js @@ -0,0 +1,83 @@ +/* eslint max-nested-callbacks: ["error", 6] */ +/* eslint-env mocha */ +'use strict' + +const hat = require('hat') +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) + +const parallel = require('async/parallel') + +const isNode = require('detect-node') +const IPFS = require('../../src') + +const DaemonFactory = require('ipfsd-ctl') +const df = DaemonFactory.create({ type: 'proc' }) + +const ipfsRef = '/ipfs/QmPFVLPmp9zv5Z5KUqLhe2EivAGccQW2r7M7jhVJGLZoZU' + +describe('name-pubsub', function () { + if (!isNode) { + return + } + + let nodes + let nodeA + let nodeB + let idA + + const createNode = (callback) => { + df.spawn({ + exec: IPFS, + args: [`--pass ${hat()}`, '--enable-namesys-pubsub'], + config: { Bootstrap: [] } + }, callback) + } + + before(function (done) { + this.timeout(40 * 1000) + + parallel([ + (cb) => createNode(cb), + (cb) => createNode(cb) + ], (err, _nodes) => { + expect(err).to.not.exist() + + nodes = _nodes + nodeA = _nodes[0].api + nodeB = _nodes[1].api + + parallel([ + (cb) => nodeA.id(cb), + (cb) => nodeB.id(cb) + ], (err, ids) => { + expect(err).to.not.exist() + + idA = ids[0] + nodeA.swarm.connect(ids[1].addresses[0], done) + }) + }) + }) + + after((done) => parallel(nodes.map((node) => (cb) => node.stop(cb)), done)) + + it('should publish and then resolve correctly', function (done) { + nodeB.name.resolve(idA.id, (err) => { + expect(err).to.exist() + + nodeA.name.publish(ipfsRef, { resolve: false }, (err, res) => { + expect(err).to.not.exist() + expect(res).to.exist() + + nodeB.name.resolve(idA.id, (err, res) => { + expect(err).to.not.exist() + expect(res).to.exist() + expect(res.path).to.equal(ipfsRef) + done() + }) + }) + }) + }) +})