fix: code review

libp2p · Feb 19, 2019 · 1fc9f88 · 1fc9f88
1 parent 86e947d
commit 1fc9f88
Show file tree

Hide file tree

Showing 8 changed files with 107 additions and 46 deletions.
diff --git a/package.json b/package.json
@@ -10,9 +10,9 @@
   },
   "scripts": {
     "lint": "aegir lint",
-    "test": "cross-env IPFS_REUSEPORT=false aegir test -t node -f test/node.js",
-    "test:node": "cross-env IPFS_REUSEPORT=false aegir test -t node -f test/node.js",
-    "test:browser": "cross-env IPFS_REUSEPORT=false aegir test -t browser --no-cors -f test/browser.js"
+    "test": "aegir test -t node -f test/node.js",
+    "test:node": "aegir test -t node -f test/node.js",
+    "test:browser": "aegir test -t browser --no-cors -f test/browser.js"
   },
   "repository": {
     "type": "git",
@@ -27,7 +27,7 @@
   },
   "homepage": "https://github.com/libp2p/interop#readme",
   "devDependencies": {
-    "aegir": "^17.0.1",
+    "aegir": "^18.1.0",
     "chai": "^4.2.0",
     "chai-checkmark": "^1.0.1",
     "cross-env": "^5.2.0",

diff --git a/src/daemon.js b/src/daemon.js
@@ -3,14 +3,11 @@
 const assert = require('assert')
 const execa = require('execa')
 const fs = require('fs')
-const os = require('os')
 const path = require('path')
 const rimraf = require('rimraf')
 
 const Client = require('libp2p-daemon-client')
-const { getSockPath } = require('./utils')
-
-const isWindows = os.platform() === 'win32'
+const { getSockPath, isWindows } = require('./utils')
 
 // process path
 const processPath = process.cwd()
@@ -48,6 +45,7 @@ class Daemon {
 
   /**
    * Get binary path according to implementation and OS
+   * @private
    * @param {String} type daemon implementation type ("go" or "js")
    * @returns {String}
    */
@@ -89,6 +87,7 @@ class Daemon {
 
   /**
    * Starts the specifiec daemon and wait for its start.
+   * @private
    * @returns {Promise}
    */
   _startDaemon () {
@@ -117,6 +116,7 @@ class Daemon {
 
   /**
    * Cleans the unix socket.
+   * @private
    * @returns {Promise}
    */
   _cleanUnixSocket () {

diff --git a/src/utils.js b/src/utils.js
@@ -2,7 +2,9 @@
 
 const os = require('os')
 const path = require('path')
-const isWindows = Boolean(os.type().match(/windows/gi))
+const isWindows = os.platform() === 'win32'
+
+exports.isWindows = isWindows
 
 exports.getSockPath = (sockPath) => isWindows
   ? path.join('\\\\?\\pipe', sockPath)

diff --git a/test/connect/README.md b/test/connect/README.md
@@ -1,3 +1,3 @@
 # Connect
 
-In this set of tests, we intend to guarantee that nodes implemented in a specific language are able to connect with nodes implemented in a different language.
+In this set of tests, we intend to guarantee that nodes implemented in a specific language are able to connect with other nodes, regardless of their implementation language.
diff --git a/test/connect/go2go.js b/test/connect/go2go.js
@@ -19,29 +19,43 @@ describe('connect', () => {
     goDaemon1 = new Daemon('go')
     goDaemon2 = new Daemon('go', '/tmp/p2pd-go2.sock')
 
-    await goDaemon1.start()
-    await goDaemon2.start()
+    await Promise.all([
+      goDaemon1.start(),
+      goDaemon2.start()
+    ])
   })
 
   // Stop daemons
   after(async function () {
-    await goDaemon1.stop()
-    await goDaemon2.stop()
+    await Promise.all([
+      goDaemon1.stop(),
+      goDaemon2.stop()
+    ])
   })
 
   it('go peer to go peer', async function () {
     this.timeout(10 * 1000)
 
     const identify1 = await goDaemon1.client.identify()
+    const identify2 = await goDaemon2.client.identify()
 
-    const knownPeersBeforeConnect = await goDaemon2.client.listPeers()
-    expect(knownPeersBeforeConnect).to.have.lengthOf(0)
+    // verify connected peers
+    const knownPeersBeforeConnect1 = await goDaemon1.client.listPeers()
+    expect(knownPeersBeforeConnect1).to.have.lengthOf(0)
+
+    const knownPeersBeforeConnect2 = await goDaemon2.client.listPeers()
+    expect(knownPeersBeforeConnect2).to.have.lengthOf(0)
 
     // connect peers
     await goDaemon2.client.connect(identify1.peerId, identify1.addrs)
 
-    const knownPeersAfterConnect = await goDaemon2.client.listPeers()
-    expect(knownPeersAfterConnect).to.have.lengthOf(1)
-    expect(knownPeersAfterConnect[0].toB58String()).to.equal(identify1.peerId.toB58String())
+    // verify connected peers
+    const knownPeersAfterConnect1 = await goDaemon1.client.listPeers()
+    expect(knownPeersAfterConnect1).to.have.lengthOf(1)
+    expect(knownPeersAfterConnect1[0].toB58String()).to.equal(identify2.peerId.toB58String())
+
+    const knownPeersAfterConnect2 = await goDaemon2.client.listPeers()
+    expect(knownPeersAfterConnect2).to.have.lengthOf(1)
+    expect(knownPeersAfterConnect2[0].toB58String()).to.equal(identify1.peerId.toB58String())
   })
 })
diff --git a/test/connect/go2js.js b/test/connect/go2js.js
@@ -19,29 +19,43 @@ describe('connect', () => {
     goDaemon = new Daemon('go')
     jsDaemon = new Daemon('js')
 
-    await goDaemon.start()
-    await jsDaemon.start()
+    await Promise.all([
+      goDaemon.start(),
+      jsDaemon.start()
+    ])
   })
 
   // Stop daemons
   after(async function () {
-    await goDaemon.stop()
-    await jsDaemon.stop()
+    await Promise.all([
+      goDaemon.stop(),
+      jsDaemon.stop()
+    ])
   })
 
   it('go peer to js peer', async function () {
     this.timeout(10 * 1000)
 
     const identifyJs = await jsDaemon.client.identify()
+    const identifyGo = await goDaemon.client.identify()
 
-    const knownPeersBeforeConnect = await goDaemon.client.listPeers()
-    expect(knownPeersBeforeConnect).to.have.lengthOf(0)
+    // verify connected peers
+    const knownPeersBeforeConnectJs = await jsDaemon.client.listPeers()
+    expect(knownPeersBeforeConnectJs).to.have.lengthOf(0)
+
+    const knownPeersBeforeConnectGo = await goDaemon.client.listPeers()
+    expect(knownPeersBeforeConnectGo).to.have.lengthOf(0)
 
     // connect peers
     await goDaemon.client.connect(identifyJs.peerId, identifyJs.addrs)
 
-    const knownPeersAfterConnect = await goDaemon.client.listPeers()
-    expect(knownPeersAfterConnect).to.have.lengthOf(1)
-    expect(knownPeersAfterConnect[0].toB58String()).to.equal(identifyJs.peerId.toB58String())
+    // verify connected peers
+    const knownPeersAfterConnectGo = await goDaemon.client.listPeers()
+    expect(knownPeersAfterConnectGo).to.have.lengthOf(1)
+    expect(knownPeersAfterConnectGo[0].toB58String()).to.equal(identifyJs.peerId.toB58String())
+
+    const knownPeersAfterConnectJs = await jsDaemon.client.listPeers()
+    expect(knownPeersAfterConnectJs).to.have.lengthOf(1)
+    expect(knownPeersAfterConnectJs[0].toB58String()).to.equal(identifyGo.peerId.toB58String())
   })
 })
diff --git a/test/connect/js2go.js b/test/connect/js2go.js
@@ -19,29 +19,43 @@ describe('connect', () => {
     jsDaemon = new Daemon('js')
     goDaemon = new Daemon('go')
 
-    await jsDaemon.start()
-    await goDaemon.start()
+    await Promise.all([
+      jsDaemon.start(),
+      goDaemon.start()
+    ])
   })
 
   // Stop daemons
   after(async function () {
-    await jsDaemon.stop()
-    await goDaemon.stop()
+    await Promise.all([
+      jsDaemon.stop(),
+      goDaemon.stop()
+    ])
   })
 
   it('js peer to go peer', async function () {
     this.timeout(10 * 1000)
 
+    const identifyJs = await jsDaemon.client.identify()
     const identifyGo = await goDaemon.client.identify()
 
-    const knownPeersBeforeConnect = await jsDaemon.client.listPeers()
-    expect(knownPeersBeforeConnect).to.have.lengthOf(0)
+    // verify connected peers
+    const knownPeersBeforeConnectJs = await jsDaemon.client.listPeers()
+    expect(knownPeersBeforeConnectJs).to.have.lengthOf(0)
+
+    const knownPeersBeforeConnectGo = await goDaemon.client.listPeers()
+    expect(knownPeersBeforeConnectGo).to.have.lengthOf(0)
 
     // connect peers
     await jsDaemon.client.connect(identifyGo.peerId, identifyGo.addrs)
 
-    const knownPeersAfterConnect = await jsDaemon.client.listPeers()
-    expect(knownPeersAfterConnect).to.have.lengthOf(1)
-    expect(knownPeersAfterConnect[0].toB58String()).to.equal(identifyGo.peerId.toB58String())
+    // verify connected peers
+    const knownPeersAfterConnectJs = await jsDaemon.client.listPeers()
+    expect(knownPeersAfterConnectJs).to.have.lengthOf(1)
+    expect(knownPeersAfterConnectJs[0].toB58String()).to.equal(identifyGo.peerId.toB58String())
+
+    const knownPeersAfterConnectGo = await goDaemon.client.listPeers()
+    expect(knownPeersAfterConnectGo).to.have.lengthOf(1)
+    expect(knownPeersAfterConnectGo[0].toB58String()).to.equal(identifyJs.peerId.toB58String())
   })
 })
diff --git a/test/connect/js2js.js b/test/connect/js2js.js
@@ -19,29 +19,46 @@ describe('connect', () => {
     jsDaemon1 = new Daemon('js')
     jsDaemon2 = new Daemon('js', '/tmp/p2pd-js2.sock')
 
-    await jsDaemon1.start()
-    await jsDaemon2.start()
+    await Promise.all([
+      jsDaemon1.start(),
+      jsDaemon2.start()
+    ])
   })
 
   // Stop daemons
   after(async function () {
-    await jsDaemon1.stop()
-    await jsDaemon2.stop()
+    await Promise.all([
+      jsDaemon1.stop(),
+      jsDaemon2.stop()
+    ])
   })
 
   it('js peer to js peer', async function () {
     this.timeout(10 * 1000)
 
     const identify1 = await jsDaemon1.client.identify()
+    const identify2 = await jsDaemon2.client.identify()
 
-    const knownPeersBeforeConnect = await jsDaemon2.client.listPeers()
-    expect(knownPeersBeforeConnect).to.have.lengthOf(0)
+    // verify connected peers
+    const knownPeersBeforeConnect1 = await jsDaemon1.client.listPeers()
+    expect(knownPeersBeforeConnect1).to.have.lengthOf(0)
+
+    const knownPeersBeforeConnect2 = await jsDaemon2.client.listPeers()
+    expect(knownPeersBeforeConnect2).to.have.lengthOf(0)
 
     // connect peers
     await jsDaemon2.client.connect(identify1.peerId, identify1.addrs)
 
-    const knownPeersAfterConnect = await jsDaemon2.client.listPeers()
-    expect(knownPeersAfterConnect).to.have.lengthOf(1)
-    expect(knownPeersAfterConnect[0].toB58String()).to.equal(identify1.peerId.toB58String())
+    // jsDaemon1 will take some time to get the peers
+    await new Promise(resolve => setTimeout(resolve, 1000))
+
+    // verify connected peers
+    const knownPeersAfterConnect1 = await jsDaemon1.client.listPeers()
+    expect(knownPeersAfterConnect1).to.have.lengthOf(1)
+    expect(knownPeersAfterConnect1[0].toB58String()).to.equal(identify2.peerId.toB58String())
+
+    const knownPeersAfterConnect2 = await jsDaemon2.client.listPeers()
+    expect(knownPeersAfterConnect2).to.have.lengthOf(1)
+    expect(knownPeersAfterConnect2[0].toB58String()).to.equal(identify1.peerId.toB58String())
   })
 })