Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: mfs preload test #1551

Merged
merged 1 commit into from
Sep 12, 2018
Merged

fix: mfs preload test #1551

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 15 additions & 12 deletions test/core/mfs-preload.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)

const waitFor = require('../utils/wait-for')
const mfsPreload = require('../../src/core/mfs-preload')

const createMockFilesStat = (cids = []) => {
Expand All @@ -15,11 +16,12 @@ const createMockFilesStat = (cids = []) => {
}

const createMockPreload = () => {
return function preload (cid, cb) {
preload.cids = preload.cids || []
const preload = (cid, cb) => {
preload.cids.push(cid)
cb()
}
preload.cids = []
return preload
}

describe('MFS preload', () => {
Expand All @@ -41,16 +43,17 @@ describe('MFS preload', () => {
preloader.start((err) => {
expect(err).to.not.exist()

setTimeout(() => {
preloader.stop((err) => {
expect(err).to.not.exist()
expect(
// Slice off any extra CIDs it processed
mockPreload.cids.slice(0, expectedPreloadCids.length)
).to.deep.equal(expectedPreloadCids)
done()
})
}, statCids.length * (interval * 2))
const test = (cb) => {
// Slice off any extra CIDs it processed
const cids = mockPreload.cids.slice(0, expectedPreloadCids.length)
if (cids.length !== expectedPreloadCids.length) return cb(null, false)
cb(null, cids.every((cid, i) => cid === expectedPreloadCids[i]))
}

waitFor(test, { name: 'CIDs to be preloaded' }, (err) => {
expect(err).to.not.exist()
Copy link
Member

Choose a reason for hiding this comment

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

rather than expect(err).to.not.exist() we should be using if (err) return done(err) so we properly finish off the test when it errors

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean so that we always call preloader.stop?

I'm not sure I understand why using done(err) is necessary in this case and not elsewhere in the tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

ping @victorbjelkholm

preloader.stop(done)
})
})
})
})