fix(helper): make mkdirIfNotExists helper resilient to concurrent calls

devoto13 · Jonathan Ginsburg · commit d9dade2f004a · 2022-02-04T18:48:55.000-06:00
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.
diff --git a/lib/helper.js b/lib/helper.js
@@ -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')
 
@@ -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 = () => {
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -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",
@@ -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",
diff --git a/test/unit/helper.spec.js b/test/unit/helper.spec.js
@@ -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()
       })
     })
