Skip to content

Commit 33a069f

Browse files
devoto13devoto13
authoredMar 18, 2020
refactor: use native Promise instead of Bluebird (#3436)
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 #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
·
v6.4.4v5.0.0
1 parent 131d154 commit 33a069f

18 files changed

+69
-159
lines changed
 

‎lib/file-list.js

+8-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
'use strict'
22

3-
const Promise = require('bluebird')
3+
const { promisify } = require('util')
44
const mm = require('minimatch')
55
const Glob = require('glob').Glob
6-
const fs = Promise.promisifyAll(require('graceful-fs'))
6+
const fs = require('graceful-fs')
7+
fs.statAsync = promisify(fs.stat)
78
const pathLib = require('path')
89
const _ = require('lodash')
910

@@ -60,8 +61,8 @@ class FileList {
6061
const matchedFiles = new Set()
6162

6263
let lastCompletedRefresh = this._refreshing
63-
lastCompletedRefresh = Promise
64-
.map(this._patterns, async ({ pattern, type, nocache, isBinary }) => {
64+
lastCompletedRefresh = Promise.all(
65+
this._patterns.map(async ({ pattern, type, nocache, isBinary }) => {
6566
if (helper.isUrlAbsolute(pattern)) {
6667
this.buckets.set(pattern, [new Url(pattern, type)])
6768
return
@@ -86,7 +87,7 @@ class FileList {
8687
if (nocache) {
8788
log.debug(`Not preprocessing "${pattern}" due to nocache`)
8889
} else {
89-
await Promise.map(files, (file) => this._preprocess(file))
90+
await Promise.all(files.map((file) => this._preprocess(file)))
9091
}
9192

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

203205
if (!file) {
204206
log.debug(`Changed file "${path}" ignored. Does not match any file in the list.`)
205-
return Promise.resolve(this.files)
207+
return this.files
206208
}
207209

208210
const [stat] = await Promise.all([fs.statAsync(path), this._refreshing])

‎lib/helper.js

-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const fs = require('graceful-fs')
44
const path = require('path')
55
const _ = require('lodash')
66
const useragent = require('useragent')
7-
const Promise = require('bluebird')
87
const mm = require('minimatch')
98

109
exports.browserFullNameToShort = (fullName) => {

‎lib/launcher.js

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict'
22

3-
const Promise = require('bluebird')
43
const Jobs = require('qjobs')
54

65
const log = require('./logger').create('launcher')

‎lib/launchers/base.js

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
const KarmaEventEmitter = require('../events').EventEmitter
22
const EventEmitter = require('events').EventEmitter
3-
const Promise = require('bluebird')
43

54
const log = require('../logger').create('launcher')
65
const helper = require('../helper')

‎lib/server.js

-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
const SocketIO = require('socket.io')
44
const di = require('di')
55
const util = require('util')
6-
const Promise = require('bluebird')
76
const spawn = require('child_process').spawn
87
const tmp = require('tmp')
98
const fs = require('fs')

‎lib/utils/bundle-utils.js

-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use strict'
22
const PathUtils = require('./path-utils')
33
const fs = require('fs')
4-
const Promise = require('bluebird')
54

65
const BundleUtils = {
76
bundleResource (inPath, outPath) {

‎lib/utils/net-utils.js

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict'
22

3-
const Promise = require('bluebird')
43
const net = require('net')
54

65
const NetUtils = {

‎package-lock.json

+10-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

-1
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,6 @@
390390
"Karl Lindmark <karl.lindmark@ninetwozero.com>"
391391
],
392392
"dependencies": {
393-
"bluebird": "^3.3.0",
394393
"body-parser": "^1.16.1",
395394
"braces": "^3.0.2",
396395
"chokidar": "^3.0.0",

‎test/.eslintrc

+1-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
},
55
"globals": {
66
"expect": true,
7-
"sinon": true,
8-
"scheduleNextTick": true
7+
"sinon": true
98
},
109
"rules": {
1110
"no-unused-expressions": "off"

‎test/unit/file-list.spec.js

+6-18
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict'
22

3-
const Promise = require('bluebird')
43
const EventEmitter = require('events').EventEmitter
54
const mocks = require('mocks')
65
const proxyquire = require('proxyquire')
@@ -448,16 +447,14 @@ describe('FileList', () => {
448447
helper: helper,
449448
glob: glob,
450449
'graceful-fs': mockFs,
451-
path: pathLib.posix,
452-
bluebird: Promise
450+
path: pathLib.posix
453451
})
454452

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

458456
afterEach(() => {
459457
clock.restore()
460-
Promise.setScheduler((fn) => process.nextTick(fn))
461458
})
462459

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

515512
return list.refresh().then(() => {
516513
preprocess.resetHistory()
517-
sinon.spy(mockFs, 'stat')
514+
sinon.spy(mockFs, 'statAsync')
518515

519516
return Promise.all([
520517
list.addFile('/some/d.js'),
521518
list.addFile('/some/d.js')
522519
]).then(() => {
523520
expect(preprocess).to.have.been.calledOnce
524-
expect(mockFs.stat).to.have.been.calledOnce
521+
expect(mockFs.statAsync).to.have.been.calledOnce
525522
})
526523
})
527524
})
@@ -555,7 +552,6 @@ describe('FileList', () => {
555552
beforeEach(() => {
556553
patternList = PATTERN_LIST
557554
mg = MG
558-
Promise.setScheduler((fn) => fn())
559555

560556
emitter = new EventEmitter()
561557

@@ -576,8 +572,7 @@ describe('FileList', () => {
576572
helper: helper,
577573
glob: glob,
578574
'graceful-fs': mockFs,
579-
path: pathLib.posix,
580-
bluebird: Promise
575+
path: pathLib.posix
581576
})
582577

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

587582
afterEach(() => {
588583
clock.restore()
589-
Promise.setScheduler((fn) => process.nextTick(fn))
590584
})
591585

592586
it('updates mtime and fires "file_list_modified"', () => {
@@ -680,7 +674,6 @@ describe('FileList', () => {
680674
beforeEach(() => {
681675
patternList = PATTERN_LIST
682676
mg = MG
683-
Promise.setScheduler((fn) => fn())
684677

685678
emitter = new EventEmitter()
686679

@@ -701,8 +694,7 @@ describe('FileList', () => {
701694
helper: helper,
702695
glob: glob,
703696
'graceful-fs': mockFs,
704-
path: pathLib.posix,
705-
bluebird: Promise
697+
path: pathLib.posix
706698
})
707699

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

712704
afterEach(() => {
713705
clock.restore()
714-
Promise.setScheduler((fn) => process.nextTick(fn))
715706
})
716707

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

767757
emitter = new EventEmitter()
768758

@@ -786,14 +776,12 @@ describe('FileList', () => {
786776
helper: helper,
787777
glob: glob,
788778
'graceful-fs': mockFs,
789-
path: pathLib.posix,
790-
bluebird: Promise
779+
path: pathLib.posix
791780
})
792781
})
793782

794783
afterEach(() => {
795784
clock.restore()
796-
Promise.setScheduler((fn) => process.nextTick(fn))
797785
})
798786

799787
it('debounces calls to emitModified', () => {

‎test/unit/launcher.spec.js

+6-16
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict'
22

3-
const Promise = require('bluebird')
43
const di = require('di')
54

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

68-
before(() => {
69-
Promise.setScheduler((fn) => fn())
70-
})
71-
72-
after(() => {
73-
Promise.setScheduler((fn) => process.nextTick(fn))
74-
})
75-
7667
beforeEach(() => {
7768
lastGeneratedId = 0
7869
FakeBrowser._instances = []
@@ -252,7 +243,7 @@ describe('launcher', () => {
252243
expect(browser.forceKill).to.have.been.called
253244
})
254245

255-
it('should call callback when all processes killed', () => {
246+
it('should call callback when all processes killed', (done) => {
256247
const exitSpy = sinon.spy()
257248

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

267-
scheduleNextTick(() => {
258+
setImmediate(() => {
268259
expect(exitSpy).not.to.have.been.called
269-
})
270260

271-
scheduleNextTick(() => {
272261
// finish the second browser
273262
browser = FakeBrowser._instances.pop()
274263
browser.forceKill.resolve()
275-
})
276264

277-
scheduleNextTick(() => {
278-
expect(exitSpy).to.have.been.called
265+
setImmediate(() => {
266+
expect(exitSpy).to.have.been.called
267+
done()
268+
})
279269
})
280270
})
281271

‎test/unit/launchers/base.spec.js

+27-47
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ describe('launchers/base.js', () => {
8181
})
8282
})
8383

84-
it('should not restart when being force killed', (done) => {
84+
it('should not restart when being force killed', async () => {
8585
const spyOnStart = sinon.spy()
8686
const spyOnKill = sinon.spy()
8787
launcher.on('start', spyOnStart)
@@ -98,22 +98,18 @@ describe('launchers/base.js', () => {
9898
launcher._done()
9999
spyOnKill.callArg(0)
100100

101-
onceKilled.done(() => {
102-
expect(spyOnStart).to.not.have.been.called
103-
done()
104-
})
101+
await onceKilled
102+
expect(spyOnStart).to.not.have.been.called
105103
})
106104
})
107105

108106
describe('kill', () => {
109-
it('should manage state', (done) => {
107+
it('should manage state', async () => {
110108
const onceKilled = launcher.kill()
111109
expect(launcher.state).to.equal(launcher.STATE_BEING_KILLED)
112110

113-
onceKilled.done(() => {
114-
expect(launcher.state).to.equal(launcher.STATE_FINISHED)
115-
done()
116-
})
111+
await onceKilled
112+
expect(launcher.state).to.equal(launcher.STATE_FINISHED)
117113
})
118114

119115
it('should fire "kill" and wait for all listeners to finish', (done) => {
@@ -152,66 +148,51 @@ describe('launchers/base.js', () => {
152148
spyOnKill.callArg(0)
153149
})
154150

155-
it('should not fire "kill" if already being killed, but wait for all listeners', (done) => {
151+
it('should not fire "kill" second time if already being killed', async () => {
156152
const spyOnKill = sinon.spy()
157153
launcher.on('kill', spyOnKill)
158154

159-
const expectOnKillListenerIsAlreadyFinishedAndHasBeenOnlyCalledOnce = () => {
160-
expect(spyOnKill).to.have.been.called
161-
expect(spyOnKill.callCount).to.equal(1)
162-
expect(spyOnKill.finished).to.equal(true)
163-
expect(launcher.state).to.equal(launcher.STATE_FINISHED)
164-
}
165-
166155
launcher.start('http://localhost:9876/')
167-
const firstKilling = launcher.kill().then(() => {
168-
expectOnKillListenerIsAlreadyFinishedAndHasBeenOnlyCalledOnce()
169-
})
170-
171-
const secondKilling = launcher.kill().then(() => {
172-
expectOnKillListenerIsAlreadyFinishedAndHasBeenOnlyCalledOnce()
173-
})
156+
launcher.kill()
157+
const killing = launcher.kill()
174158

175159
expect(launcher.state).to.equal(launcher.STATE_BEING_KILLED)
176160

177-
_.defer(() => {
178-
spyOnKill.finished = true
179-
spyOnKill.callArg(0)
180-
})
161+
spyOnKill.callArg(0)
181162

182-
// finish the test once everything is done
183-
firstKilling.done(() => secondKilling.done(() => done()))
163+
await killing
164+
165+
expect(spyOnKill).to.have.been.called
166+
expect(spyOnKill.callCount).to.equal(1)
167+
expect(launcher.state).to.equal(launcher.STATE_FINISHED)
184168
})
185169

186-
it('should not kill already crashed browser', (done) => {
170+
it('should not kill already crashed browser', async () => {
187171
const spyOnKill = sinon.spy((killDone) => killDone())
188172
launcher.on('kill', spyOnKill)
189173

190174
launcher._done('crash')
191-
launcher.kill().done(() => {
192-
expect(spyOnKill).to.not.have.been.called
193-
done()
194-
})
175+
await launcher.kill()
176+
expect(spyOnKill).to.not.have.been.called
195177
})
196178
})
197179

198180
describe('forceKill', () => {
199-
it('should cancel restart', (done) => {
181+
it('should cancel restart', async () => {
200182
const spyOnStart = sinon.spy()
201183
launcher.on('start', spyOnStart)
202184

203185
launcher.start('http://localhost:9876/')
204186
spyOnStart.resetHistory()
205187
launcher.restart()
206188

207-
launcher.forceKill().done(() => {
208-
expect(launcher.state).to.equal(launcher.STATE_FINISHED)
209-
expect(spyOnStart).to.not.have.been.called
210-
done()
211-
})
189+
await launcher.forceKill()
190+
191+
expect(launcher.state).to.equal(launcher.STATE_FINISHED)
192+
expect(spyOnStart).to.not.have.been.called
212193
})
213194

214-
it('should not fire "browser_process_failure" even if browser crashes', (done) => {
195+
it('should not fire "browser_process_failure" even if browser crashes', async () => {
215196
const spyOnBrowserProcessFailure = sinon.spy()
216197
emitter.on('browser_process_failure', spyOnBrowserProcessFailure)
217198

@@ -223,10 +204,9 @@ describe('launchers/base.js', () => {
223204
})
224205

225206
launcher.start('http://localhost:9876/')
226-
launcher.forceKill().done(() => {
227-
expect(spyOnBrowserProcessFailure).to.not.have.been.called
228-
done()
229-
})
207+
await launcher.forceKill()
208+
209+
expect(spyOnBrowserProcessFailure).to.not.have.been.called
230210
})
231211
})
232212

‎test/unit/launchers/capture_timeout.spec.js

+6-8
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe('launchers/capture_timeout.js', () => {
3838
expect(launcher.kill).not.to.have.been.called
3939
})
4040

41-
it('should clear timeout between restarts', (done) => {
41+
it('should clear timeout between restarts', async () => {
4242
CaptureTimeoutLauncher.call(launcher, timer, 10)
4343

4444
// simulate process finished
@@ -49,12 +49,10 @@ describe('launchers/capture_timeout.js', () => {
4949

5050
launcher.start()
5151
timer.wind(8)
52-
launcher.kill().done(() => {
53-
launcher.kill.resetHistory()
54-
launcher.start()
55-
timer.wind(8)
56-
expect(launcher.kill).not.to.have.been.called
57-
done()
58-
})
52+
await launcher.kill()
53+
launcher.kill.resetHistory()
54+
launcher.start()
55+
timer.wind(8)
56+
expect(launcher.kill).not.to.have.been.called
5957
})
6058
})

‎test/unit/launchers/process.spec.js

+4-5
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ describe('launchers/process.js', () => {
133133
})
134134

135135
// the most common scenario, when everything works fine
136-
it('start -> capture -> kill', (done) => {
136+
it('start -> capture -> kill', async () => {
137137
// start the browser
138138
launcher.start('http://localhost/')
139139
expect(mockSpawn).to.have.been.calledWith(BROWSER_PATH, ['http://localhost/?id=fake-id'])
@@ -150,10 +150,9 @@ describe('launchers/process.js', () => {
150150
mockSpawn._processes[0].emit('exit', 0)
151151
mockTempDir.remove.callArg(1)
152152

153-
killingLauncher.done(() => {
154-
expect(launcher.state).to.equal(launcher.STATE_FINISHED)
155-
done()
156-
})
153+
await killingLauncher
154+
155+
expect(launcher.state).to.equal(launcher.STATE_FINISHED)
157156
})
158157

159158
// when the browser fails to get captured in default timeout, it should restart

‎test/unit/middleware/runner.spec.js

-9
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
const path = require('path')
22
const EventEmitter = require('events').EventEmitter
33
const mocks = require('mocks')
4-
const Promise = require('bluebird')
54
const _ = require('lodash')
65

76
const Browser = require('../../../lib/browser')
@@ -38,14 +37,6 @@ describe('middleware.runner', () => {
3837
)
3938
}
4039

41-
before(() => {
42-
Promise.setScheduler((fn) => fn())
43-
})
44-
45-
after(() => {
46-
Promise.setScheduler((fn) => process.nextTick(fn))
47-
})
48-
4940
beforeEach(() => {
5041
mockReporter = {
5142
adapters: [],

‎test/unit/mocha-globals.js

-37
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ const sinon = require('sinon')
22
const chai = require('chai')
33
const logger = require('../../lib/logger')
44

5-
require('bluebird').longStackTraces()
6-
75
// publish globals that all specs can use
86
global.expect = chai.expect
97
global.should = chai.should()
@@ -46,38 +44,3 @@ chai.use((chai, utils) => {
4644
`expected response body to not be set, it was '${response._body}'`)
4745
})
4846
})
49-
50-
// TODO(vojta): move it somewhere ;-)
51-
const nextTickQueue = []
52-
const nextTickCallback = () => {
53-
if (!nextTickQueue.length) throw new Error('Nothing scheduled!')
54-
nextTickQueue.shift()()
55-
56-
if (nextTickQueue.length) process.nextTick(nextTickCallback)
57-
}
58-
global.scheduleNextTick = (action) => {
59-
nextTickQueue.push(action)
60-
61-
if (nextTickQueue.length === 1) process.nextTick(nextTickCallback)
62-
}
63-
const nextQueue = []
64-
const nextCallback = () => {
65-
// if not nextQueue.length then throw new Error 'Nothing scheduled!'
66-
nextQueue.shift()()
67-
}
68-
69-
global.scheduleNextTick = (action) => {
70-
nextTickQueue.push(action)
71-
72-
if (nextTickQueue.length === 1) process.nextTick(nextTickCallback)
73-
}
74-
global.scheduleNext = (action) => {
75-
nextQueue.push(action)
76-
}
77-
78-
global.next = nextCallback
79-
80-
beforeEach(() => {
81-
nextTickQueue.length = 0
82-
nextQueue.length = 0
83-
})

‎test/unit/utils/net-utils.spec.js

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ describe('NetUtils.bindAvailablePort', () => {
2222
const port = boundServer.address().port
2323
expect(port).to.be.equal(9877)
2424
expect(boundServer).not.to.be.null
25+
boundServer.close()
2526
server.close(done)
2627
})
2728
})

0 commit comments

Comments
 (0)
Please sign in to comment.