fix: improve constructor param handling tests

- ensure explicitly passed parameters are used; dont pass in the
default values and assume the are being set correctly

TODO: needs #935
to expose the protocol and api-path properties

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>

Author: olizilla

src/index.js

@@ -10,26 +10,28 @@ function ipfsClient (hostOrMultiaddr, port, opts) {
   // convert all three params to objects that we can merge.
   let hostAndPort = {}
 
-  if (hostOrMultiaddr.host) {
-    hostAndPort = hostOrMultiaddr
+  if (!hostOrMultiaddr) {
+    // autoconfigure host and port in browser
+    if (typeof self !== 'undefined') {
+      const split = self.location.host.split(':')
+      hostAndPort.host = split[0]
+      hostAndPort.port = split[1]
+    }
 
   } else if (multiaddr.isMultiaddr(hostOrMultiaddr)) {
     hostAndPort = toHostAndPort(hostOrMultiaddr)
 
+  } else if (typeof hostOrMultiaddr === 'object') {
+    hostAndPort = hostOrMultiaddr
+
   } else if (typeof hostOrMultiaddr === 'string') {
     if (hostOrMultiaddr[0] === '/') {
       // throws if multiaddr is malformed or can't be converted to a nodeAddress
       hostAndPort = toHostAndPort(multiaddr(hostOrMultiaddr))
     } else {
-      // hostAndPort is domain or ip address as a string
-      hostAndPort = hostOrMultiaddr
+      // hostOrMultiaddr is domain or ip address as a string
+      hostAndPort.host = hostOrMultiaddr
     }
-
-  // autoconfigure host and port in browser
-  } else if (!hostOrMultiaddr && typeof self !== 'undefined') {
-    const split = self.location.host.split(':')
-    hostAndPort.host = split[0]
-    hostAndPort.port = split[1]
   }
 
   if (port && typeof port !== 'object') {

test/constructor.spec.js

@@ -1,6 +1,7 @@
 /* eslint-env mocha */
 'use strict'
 
+const multiaddr = require('multiaddr')
 const chai = require('chai')
 const dirtyChai = require('dirty-chai')
 const expect = chai.expect
@@ -9,18 +10,73 @@ chai.use(dirtyChai)
 const f = require('./utils/factory')
 const ipfsClient = require('../src/index.js')
 
-function clientWorks (client, done) {
-  client.id((err, id) => {
-    expect(err).to.not.exist()
+describe('ipfs-http-client constructor tests', () => {
+  describe('parameter permuations', () => {
+    it('none', () => {
+      const host = 'localhost'
+      const port = '5001'
+      const protocol = 'http'
+      const ipfs = ipfsClient()
+      expectConfig(ipfs, { host, port, protocol })
+    })
 
-    expect(id).to.have.a.property('id')
-    expect(id).to.have.a.property('publicKey')
-    done()
+    it('opts', () => {
+      const host = 'wizard.world'
+      const port = '999'
+      const protocol = 'https'
+      const ipfs = ipfsClient({ host, port, protocol })
+      expectConfig(ipfs, { host, port })
+    })
+
+    it('mutliaddr dns4 string, opts', () => {
+      const host = 'foo.com'
+      const port = '1001'
+      const protocol = 'https'
+      const addr = `/dns4/${host}/tcp/${port}`
+      const ipfs = ipfsClient(addr, { protocol })
+      expectConfig(ipfs, { host, port, protocol })
+    })
+
+    it('mutliaddr ipv4 string', () => {
+      const host = '101.101.101.101'
+      const port = '1001'
+      const addr = `/ip4/${host}/tcp/${port}`
+      const ipfs = ipfsClient(addr)
+      expectConfig(ipfs, { host, port })
+    })
+
+    it('mutliaddr instance', () => {
+      const host = 'ace.place'
+      const port = '1001'
+      const addr = multiaddr(`/dns4/${host}/tcp/${port}`)
+      const ipfs = ipfsClient(addr)
+      expectConfig(ipfs, { host, port })
+    })
+
+    it('host and port strings', () => {
+      const host = '1.1.1.1'
+      const port = '9999'
+      const ipfs = ipfsClient(host, port)
+      expectConfig(ipfs, { host, port })
+    })
+
+    // TOOD: need to add `api-path` to `getEndpointConfig()` so we can test that setting a non-default value works
+    it.skip('host, port and api path', () => {
+      const host = '10.100.100.255'
+      const port = '9999'
+      const apiPath = '/future/api/v1'
+      const ipfs = ipfsClient(host, port, {'api-path': apiPath})
+      expectConfig(ipfs, { host, port, apiPath })
+    })
+
+    it('throws on invalid mutliaddr', () => {
+      expect(() => ipfsClient('/dns4')).to.throw('invalid address')
+      expect(() => ipfsClient('/hello')).to.throw('no protocol with name')
+      expect(() => ipfsClient('/dns4/ipfs.io')).to.throw('multiaddr must have a valid format')
+    })
   })
-}
 
-describe('ipfs-http-client constructor tests', () => {
-  describe('parameter permuations', () => {
+  describe('integration', () => {
     let apiAddr
     let ipfsd
 
@@ -40,45 +96,31 @@ describe('ipfs-http-client constructor tests', () => {
       ipfsd.stop(done)
     })
 
-    it('opts', (done) => {
-      const splitted = apiAddr.split('/')
-      clientWorks(ipfsClient({
-        host: splitted[2],
-        port: splitted[4],
-        protocol: 'http'
-      }), done)
-    })
-
-    it('mutliaddr, opts', (done) => {
-      clientWorks(ipfsClient(apiAddr, { protocol: 'http' }), done)
-    })
-
-    it('host, port', (done) => {
-      const splitted = apiAddr.split('/')
-
-      clientWorks(ipfsClient(splitted[2], splitted[4]), done)
-    })
-
-    it('specify host, port and api path', (done) => {
-      const splitted = apiAddr.split('/')
-
-      clientWorks(ipfsClient({
-        host: splitted[2],
-        port: splitted[4],
-        'api-path': '/api/v0/'
-      }), done)
+    it('can connect to an ipfs http api', (done) => {
+      clientWorks(ipfsClient(apiAddr), done)
     })
+  })
+})
 
-    it('host, port, opts', (done) => {
-      const splitted = apiAddr.split('/')
-
-      clientWorks(ipfsClient(splitted[2], splitted[4], { protocol: 'http' }), done)
-    })
+function clientWorks (client, done) {
+  client.id((err, id) => {
+    expect(err).to.not.exist()
 
-    it('error in invalid mutliaddr', () => {
-      expect(() => ipfsClient('/dns4')).to.throw('invalid address')
-      expect(() => ipfsClient('/hello')).to.throw('no protocol with name')
-      expect(() => ipfsClient('/dns4/ipfs.io')).to.throw('multiaddr must have a valid format')
-    })
+    expect(id).to.have.a.property('id')
+    expect(id).to.have.a.property('publicKey')
+    done()
   })
-})
+}
+
+function expectConfig (ipfs, {host, port, protocol, apiPath}) {
+  const conf = ipfs.util.getEndpointConfig()
+  expect(conf.host).to.equal(host)
+  expect(conf.port).to.equal(port)
+  // TODO: needs https://github.com/ipfs/js-ipfs-http-client/pull/935
+  if (protocol) {
+    expect(conf.protocol).to.equal(protocol)
+  }
+  if (apiPath) {
+    expect(conf['api-path']).to.equal(apiPath)
+  }
+}