Skip to content

Commit

Permalink
fix(helper): make mkdirIfNotExists helper resilient to concurrent calls
Browse files Browse the repository at this point in the history
The main motivation for this change is karma-runner/karma-coverage#434 (comment) where concurrent calls to the helper fail because of the race between the check and directory creation. This is a temporary solution and should be replaced with the native [`mkdir`](https://nodejs.org/api/fs.html#fsmkdirpath-options-callback) with the `recursive` option once minimum supported Node is bumped to 12.
  • Loading branch information
devoto13 authored and Jonathan Ginsburg committed Feb 5, 2022
1 parent 653c762 commit d9dade2
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 41 deletions.
16 changes: 2 additions & 14 deletions lib/helper.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
'use strict'

const fs = require('graceful-fs')
const path = require('path')
const _ = require('lodash')
const mkdirp = require('mkdirp')
const useragent = require('ua-parser-js')
const mm = require('minimatch')

Expand Down Expand Up @@ -141,18 +140,7 @@ const replaceWinPath = (path) => {
exports.normalizeWinPath = process.platform === 'win32' ? replaceWinPath : _.identity

exports.mkdirIfNotExists = (directory, done) => {
// TODO(vojta): handle if it's a file
/* eslint-disable handle-callback-err */
fs.stat(directory, (err, stat) => {
if (stat && stat.isDirectory()) {
done()
} else {
exports.mkdirIfNotExists(path.dirname(directory), () => {
fs.mkdir(directory, done)
})
}
})
/* eslint-enable handle-callback-err */
mkdirp(directory, done)
}

exports.defer = () => {
Expand Down
5 changes: 2 additions & 3 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@
"log4js": "^6.4.1",
"mime": "^2.5.2",
"minimatch": "^3.0.4",
"mkdirp": "^0.5.5",
"qjobs": "^1.2.0",
"range-parser": "^1.2.1",
"rimraf": "^3.0.2",
Expand Down Expand Up @@ -479,7 +480,6 @@
"karma-mocha": "^1.0.1",
"karma-mocha-reporter": "^2.0.0",
"karma-script-launcher": "^1.0.0",
"mkdirp": "^0.5.0",
"mocha": "^4.1.0",
"mocks": "^0.0.15",
"proxyquire": "^2.1.3",
Expand Down
33 changes: 10 additions & 23 deletions test/unit/helper.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,36 +229,23 @@ describe('helper', () => {
})

describe('mkdirIfNotExists', () => {
const fsMock = require('mocks').fs
const loadFile = require('mocks').loadFile

const fs = fsMock.create({
home: { 'some.js': fsMock.file() }
})
const spy = sinon.spy()

// load file under test
const m = loadFile(path.join(__dirname, '/../../lib/helper.js'), { 'graceful-fs': fs, lodash: require('lodash') })

const mkdirIfNotExists = m.exports.mkdirIfNotExists

it('should not do anything, if dir already exists', (done) => {
mkdirIfNotExists('/home', done)
})

it('should create directory if it does not exist', (done) => {
mkdirIfNotExists('/home/new', () => {
const stat = fs.statSync('/home/new')
expect(stat).to.exist
expect(stat.isDirectory()).to.equal(true)
const m = loadFile(path.join(__dirname, '/../../lib/helper.js'), {
mkdirp: (path, done) => {
spy(path)
done()
})
}
})

it('should create even parent directories if it does not exist', (done) => {
mkdirIfNotExists('/home/new/parent/child', () => {
const stat = fs.statSync('/home/new/parent/child')
expect(stat).to.exist
expect(stat.isDirectory()).to.equal(true)
const mkdirIfNotExists = m.exports.mkdirIfNotExists

it('should call mkdirp', (done) => {
mkdirIfNotExists('/path/to/dir', () => {
expect(spy).to.have.been.calledOnceWith('/path/to/dir')
done()
})
})
Expand Down

0 comments on commit d9dade2

Please sign in to comment.