Skip to content

Commit

Permalink
refactor: use native Promise instead of Bluebird (karma-runner#3436)
Browse files Browse the repository at this point in the history
All supported Node versions support native promises and async/await, current code has a minimal usage of Bluebird-specific methods, so dropping a third-party library in favor of native Promises.

Tests were using `Promise.setScheduler()` from Bluebird, which allowed to customize how Promises are scheduled and made test work very different from the real code, which created tricky issues (like karma-runner#3060 (comment)). Affected tests were updated to not rely on `Promise.setScheduler()`, which improved readability in some situations. Also removed some test helpers as they are not necessary and not used anymore.

BREAKING CHANGE: Karma plugins which rely on the fact that Karma uses Bluebird promises may break as Bluebird-specific API is no longer available on Promises returned by the Karma core
  • Loading branch information
devoto13 authored Mar 18, 2020
1 parent 131d154 commit 33a069f
Show file tree
Hide file tree
Showing 18 changed files with 69 additions and 159 deletions.
14 changes: 8 additions & 6 deletions lib/file-list.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
'use strict'

const Promise = require('bluebird')
const { promisify } = require('util')
const mm = require('minimatch')
const Glob = require('glob').Glob
const fs = Promise.promisifyAll(require('graceful-fs'))
const fs = require('graceful-fs')
fs.statAsync = promisify(fs.stat)
const pathLib = require('path')
const _ = require('lodash')

Expand Down Expand Up @@ -60,8 +61,8 @@ class FileList {
const matchedFiles = new Set()

let lastCompletedRefresh = this._refreshing
lastCompletedRefresh = Promise
.map(this._patterns, async ({ pattern, type, nocache, isBinary }) => {
lastCompletedRefresh = Promise.all(
this._patterns.map(async ({ pattern, type, nocache, isBinary }) => {
if (helper.isUrlAbsolute(pattern)) {
this.buckets.set(pattern, [new Url(pattern, type)])
return
Expand All @@ -86,7 +87,7 @@ class FileList {
if (nocache) {
log.debug(`Not preprocessing "${pattern}" due to nocache`)
} else {
await Promise.map(files, (file) => this._preprocess(file))
await Promise.all(files.map((file) => this._preprocess(file)))
}

this.buckets.set(pattern, files)
Expand All @@ -97,6 +98,7 @@ class FileList {
log.warn(`All files matched by "${pattern}" were excluded or matched by prior matchers.`)
}
})
)
.then(() => {
// When we return from this function the file processing chain will be
// complete. In the case of two fast refresh() calls, the second call
Expand Down Expand Up @@ -202,7 +204,7 @@ class FileList {

if (!file) {
log.debug(`Changed file "${path}" ignored. Does not match any file in the list.`)
return Promise.resolve(this.files)
return this.files
}

const [stat] = await Promise.all([fs.statAsync(path), this._refreshing])
Expand Down
1 change: 0 additions & 1 deletion lib/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const fs = require('graceful-fs')
const path = require('path')
const _ = require('lodash')
const useragent = require('useragent')
const Promise = require('bluebird')
const mm = require('minimatch')

exports.browserFullNameToShort = (fullName) => {
Expand Down
1 change: 0 additions & 1 deletion lib/launcher.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict'

const Promise = require('bluebird')
const Jobs = require('qjobs')

const log = require('./logger').create('launcher')
Expand Down
1 change: 0 additions & 1 deletion lib/launchers/base.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const KarmaEventEmitter = require('../events').EventEmitter
const EventEmitter = require('events').EventEmitter
const Promise = require('bluebird')

const log = require('../logger').create('launcher')
const helper = require('../helper')
Expand Down
1 change: 0 additions & 1 deletion lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const SocketIO = require('socket.io')
const di = require('di')
const util = require('util')
const Promise = require('bluebird')
const spawn = require('child_process').spawn
const tmp = require('tmp')
const fs = require('fs')
Expand Down
1 change: 0 additions & 1 deletion lib/utils/bundle-utils.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict'
const PathUtils = require('./path-utils')
const fs = require('fs')
const Promise = require('bluebird')

const BundleUtils = {
bundleResource (inPath, outPath) {
Expand Down
1 change: 0 additions & 1 deletion lib/utils/net-utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict'

const Promise = require('bluebird')
const net = require('net')

const NetUtils = {
Expand Down
14 changes: 10 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,6 @@
"Karl Lindmark <[email protected]>"
],
"dependencies": {
"bluebird": "^3.3.0",
"body-parser": "^1.16.1",
"braces": "^3.0.2",
"chokidar": "^3.0.0",
Expand Down
3 changes: 1 addition & 2 deletions test/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
},
"globals": {
"expect": true,
"sinon": true,
"scheduleNextTick": true
"sinon": true
},
"rules": {
"no-unused-expressions": "off"
Expand Down
24 changes: 6 additions & 18 deletions test/unit/file-list.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict'

const Promise = require('bluebird')
const EventEmitter = require('events').EventEmitter
const mocks = require('mocks')
const proxyquire = require('proxyquire')
Expand Down Expand Up @@ -448,16 +447,14 @@ describe('FileList', () => {
helper: helper,
glob: glob,
'graceful-fs': mockFs,
path: pathLib.posix,
bluebird: Promise
path: pathLib.posix
})

list = new List(patterns('/some/*.js', '*.txt'), ['/secret/*.txt'], emitter, preprocess, 100)
})

afterEach(() => {
clock.restore()
Promise.setScheduler((fn) => process.nextTick(fn))
})

it('does not add excluded files', () => {
Expand Down Expand Up @@ -514,14 +511,14 @@ describe('FileList', () => {

return list.refresh().then(() => {
preprocess.resetHistory()
sinon.spy(mockFs, 'stat')
sinon.spy(mockFs, 'statAsync')

return Promise.all([
list.addFile('/some/d.js'),
list.addFile('/some/d.js')
]).then(() => {
expect(preprocess).to.have.been.calledOnce
expect(mockFs.stat).to.have.been.calledOnce
expect(mockFs.statAsync).to.have.been.calledOnce
})
})
})
Expand Down Expand Up @@ -555,7 +552,6 @@ describe('FileList', () => {
beforeEach(() => {
patternList = PATTERN_LIST
mg = MG
Promise.setScheduler((fn) => fn())

emitter = new EventEmitter()

Expand All @@ -576,8 +572,7 @@ describe('FileList', () => {
helper: helper,
glob: glob,
'graceful-fs': mockFs,
path: pathLib.posix,
bluebird: Promise
path: pathLib.posix
})

mockFs._touchFile('/some/a.js', '2012-04-04')
Expand All @@ -586,7 +581,6 @@ describe('FileList', () => {

afterEach(() => {
clock.restore()
Promise.setScheduler((fn) => process.nextTick(fn))
})

it('updates mtime and fires "file_list_modified"', () => {
Expand Down Expand Up @@ -680,7 +674,6 @@ describe('FileList', () => {
beforeEach(() => {
patternList = PATTERN_LIST
mg = MG
Promise.setScheduler((fn) => fn())

emitter = new EventEmitter()

Expand All @@ -701,8 +694,7 @@ describe('FileList', () => {
helper: helper,
glob: glob,
'graceful-fs': mockFs,
path: pathLib.posix,
bluebird: Promise
path: pathLib.posix
})

modified = sinon.stub()
Expand All @@ -711,7 +703,6 @@ describe('FileList', () => {

afterEach(() => {
clock.restore()
Promise.setScheduler((fn) => process.nextTick(fn))
})

it('removes the file from the list and fires "file_list_modified"', () => {
Expand Down Expand Up @@ -762,7 +753,6 @@ describe('FileList', () => {
beforeEach(() => {
patternList = PATTERN_LIST
mg = MG
Promise.setScheduler((fn) => { fn() })

emitter = new EventEmitter()

Expand All @@ -786,14 +776,12 @@ describe('FileList', () => {
helper: helper,
glob: glob,
'graceful-fs': mockFs,
path: pathLib.posix,
bluebird: Promise
path: pathLib.posix
})
})

afterEach(() => {
clock.restore()
Promise.setScheduler((fn) => process.nextTick(fn))
})

it('debounces calls to emitModified', () => {
Expand Down
22 changes: 6 additions & 16 deletions test/unit/launcher.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict'

const Promise = require('bluebird')
const di = require('di')

const events = require('../../lib/events')
Expand Down Expand Up @@ -65,14 +64,6 @@ describe('launcher', () => {
let lastGeneratedId = null
launcher.Launcher.generateId = () => ++lastGeneratedId

before(() => {
Promise.setScheduler((fn) => fn())
})

after(() => {
Promise.setScheduler((fn) => process.nextTick(fn))
})

beforeEach(() => {
lastGeneratedId = 0
FakeBrowser._instances = []
Expand Down Expand Up @@ -252,7 +243,7 @@ describe('launcher', () => {
expect(browser.forceKill).to.have.been.called
})

it('should call callback when all processes killed', () => {
it('should call callback when all processes killed', (done) => {
const exitSpy = sinon.spy()

l.launch(['Fake', 'Fake'], 1)
Expand All @@ -264,18 +255,17 @@ describe('launcher', () => {
let browser = FakeBrowser._instances.pop()
browser.forceKill.resolve()

scheduleNextTick(() => {
setImmediate(() => {
expect(exitSpy).not.to.have.been.called
})

scheduleNextTick(() => {
// finish the second browser
browser = FakeBrowser._instances.pop()
browser.forceKill.resolve()
})

scheduleNextTick(() => {
expect(exitSpy).to.have.been.called
setImmediate(() => {
expect(exitSpy).to.have.been.called
done()
})
})
})

Expand Down
Loading

0 comments on commit 33a069f

Please sign in to comment.