Skip to content

Commit 44b31eb

Browse files
devoto13Jonathan Ginsburg
authored andcommitted
fix: race condition between the Karma shutdown and coverage writing
The creation of parent directories is asynchronous process, which was not properly awaited, which resulted in the coverage reports not being written to the disk sometimes as Karma process has exited before the reporter completed the writing of the coverage report. Also update the test case to use async implementation of the corresponding stub to prevent regressions in the future. Remove manual calls for `done` parameter in tests as it is handled by the stub now. Fixes #434
1 parent 4adb3ef commit 44b31eb

File tree

2 files changed

+11
-10
lines changed

2 files changed

+11
-10
lines changed

lib/reporter.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
// ------------
1212

1313
var path = require('path')
14+
const { promisify } = require('util')
1415
var istanbulLibCoverage = require('istanbul-lib-coverage')
1516
var istanbulLibReport = require('istanbul-lib-report')
1617
var minimatch = require('minimatch')
@@ -264,10 +265,12 @@ var CoverageReporter = function (rootConfig, helper, logger, emitter) {
264265
return
265266
}
266267

267-
helper.mkdirIfNotExists(outputPath, function () {
268-
log.debug('Writing coverage to %s', outputPath)
269-
report.execute(context)
270-
})
268+
const mkdirIfNotExists = promisify(helper.mkdirIfNotExists)
269+
await mkdirIfNotExists(outputPath)
270+
271+
log.debug('Writing coverage to %s', outputPath)
272+
report.execute(context)
273+
271274
return results
272275
}
273276

test/reporter.spec.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ describe('reporter', () => {
2727
const mockHelper = {
2828
isDefined: (v) => helper.isDefined(v),
2929
merge: (...arg) => helper.merge(...arg),
30-
mkdirIfNotExists: mkdirIfNotExistsStub,
30+
mkdirIfNotExists: (dir, done) => {
31+
mkdirIfNotExistsStub(dir, done)
32+
setTimeout(done, 0)
33+
},
3134
normalizeWinPath: (path) => helper.normalizeWinPath(path)
3235
}
3336

@@ -166,7 +169,6 @@ describe('reporter', () => {
166169
const dir = rootConfig.coverageReporter.dir
167170
expect(mkdirIfNotExistsStub.getCall(0).args[0]).to.deep.equal(resolve('/base', dir, fakeChrome.name))
168171
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', dir, fakeOpera.name))
169-
mkdirIfNotExistsStub.getCall(0).args[1]()
170172
expect(reportCreateStub).to.have.been.called
171173
expect(mockPackageSummary.execute).to.have.been.called
172174
const createArgs = reportCreateStub.getCall(0).args
@@ -192,7 +194,6 @@ describe('reporter', () => {
192194
const subdir = customConfig.coverageReporter.subdir
193195
expect(mkdirIfNotExistsStub.getCall(0).args[0]).to.deep.equal(resolve('/base', dir, subdir))
194196
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', dir, subdir))
195-
mkdirIfNotExistsStub.getCall(0).args[1]()
196197
expect(reportCreateStub).to.have.been.called
197198
expect(mockPackageSummary.execute).to.have.been.called
198199
})
@@ -213,7 +214,6 @@ describe('reporter', () => {
213214
const dir = customConfig.coverageReporter.dir
214215
expect(mkdirIfNotExistsStub.getCall(0).args[0]).to.deep.equal(resolve('/base', dir, 'chrome'))
215216
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', dir, 'opera'))
216-
mkdirIfNotExistsStub.getCall(0).args[1]()
217217
expect(reportCreateStub).to.have.been.called
218218
expect(mockPackageSummary.execute).to.have.been.called
219219
})
@@ -246,7 +246,6 @@ describe('reporter', () => {
246246
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', 'reporter1', 'opera'))
247247
expect(mkdirIfNotExistsStub.getCall(2).args[0]).to.deep.equal(resolve('/base', 'reporter2', 'CHROME'))
248248
expect(mkdirIfNotExistsStub.getCall(3).args[0]).to.deep.equal(resolve('/base', 'reporter2', 'OPERA'))
249-
mkdirIfNotExistsStub.getCall(0).args[1]()
250249
expect(reportCreateStub).to.have.been.called
251250
expect(mockPackageSummary.execute).to.have.been.called
252251
})
@@ -277,7 +276,6 @@ describe('reporter', () => {
277276
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', 'reporter1', 'defaultsubdir'))
278277
expect(mkdirIfNotExistsStub.getCall(2).args[0]).to.deep.equal(resolve('/base', 'defaultdir', 'CHROME'))
279278
expect(mkdirIfNotExistsStub.getCall(3).args[0]).to.deep.equal(resolve('/base', 'defaultdir', 'OPERA'))
280-
mkdirIfNotExistsStub.getCall(0).args[1]()
281279
expect(reportCreateStub).to.have.been.called
282280
expect(mockPackageSummary.execute).to.have.been.called
283281
})

0 commit comments

Comments
 (0)