Skip to content

Commit

Permalink
fix: race condition between the Karma shutdown and coverage writing
Browse files Browse the repository at this point in the history
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
  • Loading branch information
devoto13 authored and Jonathan Ginsburg committed Feb 5, 2022
1 parent 4adb3ef commit 44b31eb
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
11 changes: 7 additions & 4 deletions lib/reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// ------------

var path = require('path')
const { promisify } = require('util')
var istanbulLibCoverage = require('istanbul-lib-coverage')
var istanbulLibReport = require('istanbul-lib-report')
var minimatch = require('minimatch')
Expand Down Expand Up @@ -264,10 +265,12 @@ var CoverageReporter = function (rootConfig, helper, logger, emitter) {
return
}

helper.mkdirIfNotExists(outputPath, function () {
log.debug('Writing coverage to %s', outputPath)
report.execute(context)
})
const mkdirIfNotExists = promisify(helper.mkdirIfNotExists)
await mkdirIfNotExists(outputPath)

log.debug('Writing coverage to %s', outputPath)
report.execute(context)

return results
}

Expand Down
10 changes: 4 additions & 6 deletions test/reporter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ describe('reporter', () => {
const mockHelper = {
isDefined: (v) => helper.isDefined(v),
merge: (...arg) => helper.merge(...arg),
mkdirIfNotExists: mkdirIfNotExistsStub,
mkdirIfNotExists: (dir, done) => {
mkdirIfNotExistsStub(dir, done)
setTimeout(done, 0)
},
normalizeWinPath: (path) => helper.normalizeWinPath(path)
}

Expand Down Expand Up @@ -166,7 +169,6 @@ describe('reporter', () => {
const dir = rootConfig.coverageReporter.dir
expect(mkdirIfNotExistsStub.getCall(0).args[0]).to.deep.equal(resolve('/base', dir, fakeChrome.name))
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', dir, fakeOpera.name))
mkdirIfNotExistsStub.getCall(0).args[1]()
expect(reportCreateStub).to.have.been.called
expect(mockPackageSummary.execute).to.have.been.called
const createArgs = reportCreateStub.getCall(0).args
Expand All @@ -192,7 +194,6 @@ describe('reporter', () => {
const subdir = customConfig.coverageReporter.subdir
expect(mkdirIfNotExistsStub.getCall(0).args[0]).to.deep.equal(resolve('/base', dir, subdir))
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', dir, subdir))
mkdirIfNotExistsStub.getCall(0).args[1]()
expect(reportCreateStub).to.have.been.called
expect(mockPackageSummary.execute).to.have.been.called
})
Expand All @@ -213,7 +214,6 @@ describe('reporter', () => {
const dir = customConfig.coverageReporter.dir
expect(mkdirIfNotExistsStub.getCall(0).args[0]).to.deep.equal(resolve('/base', dir, 'chrome'))
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', dir, 'opera'))
mkdirIfNotExistsStub.getCall(0).args[1]()
expect(reportCreateStub).to.have.been.called
expect(mockPackageSummary.execute).to.have.been.called
})
Expand Down Expand Up @@ -246,7 +246,6 @@ describe('reporter', () => {
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', 'reporter1', 'opera'))
expect(mkdirIfNotExistsStub.getCall(2).args[0]).to.deep.equal(resolve('/base', 'reporter2', 'CHROME'))
expect(mkdirIfNotExistsStub.getCall(3).args[0]).to.deep.equal(resolve('/base', 'reporter2', 'OPERA'))
mkdirIfNotExistsStub.getCall(0).args[1]()
expect(reportCreateStub).to.have.been.called
expect(mockPackageSummary.execute).to.have.been.called
})
Expand Down Expand Up @@ -277,7 +276,6 @@ describe('reporter', () => {
expect(mkdirIfNotExistsStub.getCall(1).args[0]).to.deep.equal(resolve('/base', 'reporter1', 'defaultsubdir'))
expect(mkdirIfNotExistsStub.getCall(2).args[0]).to.deep.equal(resolve('/base', 'defaultdir', 'CHROME'))
expect(mkdirIfNotExistsStub.getCall(3).args[0]).to.deep.equal(resolve('/base', 'defaultdir', 'OPERA'))
mkdirIfNotExistsStub.getCall(0).args[1]()
expect(reportCreateStub).to.have.been.called
expect(mockPackageSummary.execute).to.have.been.called
})
Expand Down

0 comments on commit 44b31eb

Please sign in to comment.