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

refactor: convert bitswap tests to use async/await #391

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 30 additions & 31 deletions SPEC/BITSWAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,60 +5,59 @@

### `bitswap.wantlist`

> Returns the wantlist, optionally filtered by peer ID
> Get the list of wanted CIDs for this peer or another peer on the network.

#### `Go` **WIP**
#### Go **WIP**

#### `JavaScript` - ipfs.bitswap.wantlist([peerId], [callback])
#### JavaScript - `ipfs.bitswap.wantlist([peerId])`

`callback` must follow `function (err, list) {}` signature, where `err` is an error if the operation was not successful. `list` is an Object containing the following keys:
##### Parameters

- `Keys` An array of objects containing the following keys:
- `/` A string multihash
| Name | Type | Description |
|------|------|-------------|
| peerId | `String` | (optional) Base 58 encoded Peer ID to get the wantlist for. Default: this node's peer ID |

If no `callback` is passed, a promise is returned.
##### Returns

**Example:**
| Type | Description |
|------|-------------|
| `Promise<{`<br/>&nbsp;&nbsp;`Keys<Array<Object>>`<br/>`}>` | The list of CIDs wanted by the peer. Each object in the array has a single property "/" a string CID. |
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems weird, html inside inline code block just to add new lines and space

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 know 😢. I would like this to be clearer when viewed as markdown but I don't know how right now. Unfortunately multi-line code blocks in a table aren't possible. If you have any ideas for how to format this in markdown so that it reads clearly when rendered then please comment!

Copy link
Contributor

Choose a reason for hiding this comment

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

I've recently also started to use tables for parameters (for js-ipld). But I'll move away from that again. I find tables hard to read if you just read the unrendered Markdown.

The signature is quite complicated, I actually missed that it is an object containing a single field called Keys. I found that clearer in the old docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've recently also started to use tables for parameters (for js-ipld). But I'll move away from that again. I find tables hard to read if you just read the unrendered Markdown.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will think up a better way to layout this information that is readable rendered or not.


```JavaScript
ipfs.bitswap.wantlist((err, list) => console.log(list))
##### Example

// { Keys: [{ '/': 'QmHash' }] }
```js
const list = await ipfs.bitswap.wantlist()
Copy link
Collaborator

@achingbrain achingbrain Nov 27, 2018

Choose a reason for hiding this comment

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

Should this return an iterator instead of a list?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think in general we should either return single values or iterators of single values, never lists of single values.

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, the wantlist could get really big so an iterator makes sense.

never lists of single values

Do you think this should always be true? I think we should return async iterators where it makes sense to "stream" the output but I wouldn't have thought it would be necessary for something like ipfs.bootstrap.list, for example...but could be persuaded for the sake of consistency.

Copy link
Contributor

Choose a reason for hiding this comment

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

we talking iterable ? or really async iterator ?

console.log(list) // { Keys: [{ '/': 'QmHash' }] }
```

ipfs.bitswap.wantlist(peerId, (err, list) => console.log(list))
Wantlist for a given peer:

// { Keys: [{ '/': 'QmHash' }] }
```js
ipfs.bitswap.wantlist('QmZEYeEin6wEB7WNyiT7stYTmbYFGy7BzM7T3hRDzRxTvY')
console.log(list) // { Keys: [{ '/': 'QmHash' }] }
```

#### `bitswap.stat`

> Show diagnostic information on the bitswap agent.

##### `Go` **WIP**
##### Go **WIP**

##### `JavaScript` - ipfs.bitswap.stat([callback])
##### JavaScript - `ipfs.bitswap.stat()`

Note: `bitswap.stat` and `stats.bitswap` can be used interchangeably.

`callback` must follow `function (err, stats) {}` signature, where `err` is an error if the operation was not successful. `stats` is an Object containing the following keys:

- `provideBufLen` is an integer.
- `wantlist` (array of CIDs)
- `peers` (array of peer IDs)
- `blocksReceived` is a [Big Int][1]
- `dataReceived` is a [Big Int][1]
- `blocksSent` is a [Big Int][1]
- `dataSent` is a [Big Int][1]
- `dupBlksReceived` is a [Big Int][1]
- `dupDataReceived` is a [Big Int][1]

If no `callback` is passed, a promise is returned.
##### Returns

**Example:**
| Type | Description |
|------|-------------|
| `Promise<{`<br/>&nbsp;&nbsp;`provideBufLen<Number>,`<br/>&nbsp;&nbsp;`wantlist<Array<Object>>,`<br/>&nbsp;&nbsp;`peers<Array<String>>,`<br/>&nbsp;&nbsp;`blocksReceived<`[`Big`][1]`>,`<br/>&nbsp;&nbsp;`dataReceived<`[`Big`][1]`>,`<br/>&nbsp;&nbsp;`blocksSent<`[`Big`][1]`>,`<br/>&nbsp;&nbsp;`dataSent<`[`Big`][1]`>,`<br/>&nbsp;&nbsp;`dupBlksReceived<`[`Big`][1]`>,`<br/>&nbsp;&nbsp;`dupDataReceived<`[`Big`][1]`>,`<br/>`}>` | Diagnostic information on the bitswap agent |

```JavaScript
ipfs.bitswap.stat((err, stats) => console.log(stats))
##### Example

```js
const stats = await ipfs.bitswap.stat()
console.log(stats)
// { provideBufLen: 0,
// wantlist: [ { '/': 'QmSoLPppuBtQSGwKDZT2M73ULpjvfd3aZ6ha4oFGL1KrGM' } ],
// peers:
Expand Down
1 change: 0 additions & 1 deletion js/src/bitswap/.tern-port

This file was deleted.

50 changes: 16 additions & 34 deletions js/src/bitswap/stat.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint-env mocha */
'use strict'

const waterfall = require('async/waterfall')
const { getDescribe, getIt, expect } = require('../utils/mocha')
const { expectIsBitswap } = require('../stats/utils')

Expand All @@ -13,50 +12,33 @@ module.exports = (createCommon, options) => {
describe('.bitswap.stat', () => {
let ipfs

before(function (done) {
before(async function () {
// CI takes longer to instantiate the daemon, so we need to increase the
// timeout for the before step
this.timeout(60 * 1000)

common.setup((err, factory) => {
expect(err).to.not.exist()
factory.spawnNode((err, node) => {
expect(err).to.not.exist()
ipfs = node
done()
})
})
const factory = await common.setup()
ipfs = await factory.spawnNode()
})

after((done) => common.teardown(done))
after(() => common.teardown())

it('should get bitswap stats', (done) => {
ipfs.bitswap.stat((err, res) => {
expectIsBitswap(err, res)
done()
})
it('should get bitswap stats', async () => {
const res = await ipfs.bitswap.stat()
expectIsBitswap(res)
})

it('should get bitswap stats (promised)', () => {
return ipfs.bitswap.stat().then((res) => {
expectIsBitswap(null, res)
})
})

it('should not get bitswap stats when offline', function (done) {
it('should not get bitswap stats when offline', async function () {
this.timeout(60 * 1000)

waterfall([
(cb) => createCommon().setup(cb),
(factory, cb) => factory.spawnNode(cb),
(node, cb) => node.stop((err) => cb(err, node))
], (err, node) => {
expect(err).to.not.exist()
node.bitswap.wantlist((err) => {
expect(err).to.exist()
done()
})
})
const common = createCommon()
const factory = await common.setup()
const ipfs = await factory.spawnNode()
await ipfs.stop()

// TODO: assert on error message/code
await expect(ipfs.bitswap.stat()).to.be.rejected()
return common.teardown()
})
})
}
26 changes: 7 additions & 19 deletions js/src/bitswap/utils.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,17 @@
'use strict'

const until = require('async/until')

function waitForWantlistKey (ipfs, key, opts, cb) {
if (typeof opts === 'function') {
cb = opts
opts = {}
}

async function waitForWantlistKey (ipfs, key, opts) {
opts = opts || {}
opts.timeout = opts.timeout || 1000

let list = { Keys: [] }
let timedOut = false
const start = Date.now()

setTimeout(() => { timedOut = true }, opts.timeout)

const test = () => timedOut ? true : list.Keys.every(k => k['/'] === key)
const iteratee = (cb) => ipfs.bitswap.wantlist(opts.peerId, cb)
while (Date.now() <= start + opts.timeout) {
const list = await ipfs.bitswap.wantlist(opts.peerId)
if (list.Keys.find(k => k['/'] === key)) return
}

until(test, iteratee, (err) => {
if (err) return cb(err)
if (timedOut) return cb(new Error(`Timed out waiting for ${key} in wantlist`))
cb()
})
throw new Error(`Timed out waiting for ${key} in wantlist`)
}

module.exports.waitForWantlistKey = waitForWantlistKey
59 changes: 23 additions & 36 deletions js/src/bitswap/wantlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
/* eslint max-nested-callbacks: ["error", 6] */
'use strict'

const waterfall = require('async/waterfall')
const { spawnNodesWithId } = require('../utils/spawn')
const { getDescribe, getIt, expect } = require('../utils/mocha')
const { waitForWantlistKey } = require('./utils')
Expand All @@ -18,58 +17,46 @@ module.exports = (createCommon, options) => {
let ipfsB
const key = 'QmUBdnXXPyoDFXj3Hj39dNJ5VkN3QFRskXxcGaYFBB8CNR'

before(function (done) {
before(async function () {
// CI takes longer to instantiate the daemon, so we need to increase the
// timeout for the before step
this.timeout(60 * 1000)

common.setup((err, factory) => {
expect(err).to.not.exist()
const factory = await common.setup()
const nodes = await spawnNodesWithId(2, factory)

spawnNodesWithId(2, factory, (err, nodes) => {
expect(err).to.not.exist()
ipfsA = nodes[0]
ipfsB = nodes[1]

ipfsA = nodes[0]
ipfsB = nodes[1]
// Add key to the wantlist for ipfsB
ipfsB.block.get(key)

// Add key to the wantlist for ipfsB
ipfsB.block.get(key, () => {})

connect(ipfsA, ipfsB.peerId.addresses[0], done)
})
})
return connect(ipfsA, ipfsB.peerId.addresses[0])
})

after(function (done) {
after(async function () {
this.timeout(30 * 1000)
common.teardown(done)
return common.teardown()
})

it('should get the wantlist', (done) => {
waitForWantlistKey(ipfsB, key, done)
})
it('should get the wantlist', () => waitForWantlistKey(ipfsB, key))

it('should get the wantlist by peer ID for a diffreent node', (done) => {
ipfsB.id((err, info) => {
expect(err).to.not.exist()
waitForWantlistKey(ipfsA, key, { peerId: info.id }, done)
})
it('should get the wantlist by peer ID for a diffreent node', async () => {
const info = await ipfsB.id()
return waitForWantlistKey(ipfsA, key, { peerId: info.id })
})

it('should not get the wantlist when offline', function (done) {
it('should not get the wantlist when offline', async function () {
this.timeout(60 * 1000)

waterfall([
(cb) => createCommon().setup(cb),
(factory, cb) => factory.spawnNode(cb),
(node, cb) => node.stop((err) => cb(err, node))
], (err, node) => {
expect(err).to.not.exist()
node.bitswap.wantlist((err) => {
expect(err).to.exist()
done()
})
})
const common = createCommon()
const factory = await common.setup()
const ipfs = factory.spawnNode()
await ipfs.stop()

// TODO: assert on error message/code
await expect(ipfs.bitswap.wantlist()).to.be.rejected()
return common.teardown()
})
})
}
9 changes: 3 additions & 6 deletions js/src/stats/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ const isBigInt = (n) => {
return n.constructor.name === 'Big'
}

exports.expectIsBitswap = (err, stats) => {
expect(err).to.not.exist()
exports.expectIsBitswap = stats => {
expect(stats).to.exist()
expect(stats).to.have.a.property('provideBufLen')
expect(stats).to.have.a.property('wantlist')
Expand All @@ -30,8 +29,7 @@ exports.expectIsBitswap = (err, stats) => {
expect(isBigInt(stats.dupDataReceived)).to.eql(true)
}

exports.expectIsBandwidth = (err, stats) => {
expect(err).to.not.exist()
exports.expectIsBandwidth = stats => {
expect(stats).to.exist()
expect(stats).to.have.a.property('totalIn')
expect(stats).to.have.a.property('totalOut')
Expand All @@ -43,8 +41,7 @@ exports.expectIsBandwidth = (err, stats) => {
expect(isBigInt(stats.rateOut)).to.eql(true)
}

exports.expectIsRepo = (err, res) => {
expect(err).to.not.exist()
exports.expectIsRepo = res => {
expect(res).to.exist()
expect(res).to.have.a.property('numObjects')
expect(res).to.have.a.property('repoSize')
Expand Down