wrap a timer around the rimraf call in npm-ci

Fix: npm/arborist#207

PR-URL: #2573
Credit: @isaacs
Close: #2573
Reviewed-by: @nlf

lib/ci.js

@@ -3,6 +3,8 @@ const Arborist = require('@npmcli/arborist')
 const rimraf = util.promisify(require('rimraf'))
 const reifyFinish = require('./utils/reify-finish.js')
 const runScript = require('@npmcli/run-script')
+const fs = require('fs')
+const readdir = util.promisify(fs.readdir)
 
 const log = require('npmlog')
 const npm = require('./npm.js')
@@ -13,6 +15,16 @@ const completion = require('./utils/completion/none.js')
 
 const cmd = (args, cb) => ci().then(() => cb()).catch(cb)
 
+const removeNodeModules = async where => {
+  const rimrafOpts = { glob: false }
+  process.emit('time', 'npm-ci:rm')
+  const path = `${where}/node_modules`
+  // get the list of entries so we can skip the glob for performance
+  const entries = await readdir(path, null).catch(er => [])
+  await Promise.all(entries.map(f => rimraf(`${path}/${f}`, rimrafOpts)))
+  process.emit('timeEnd', 'npm-ci:rm')
+}
+
 const ci = async () => {
   if (npm.flatOptions.global) {
     const err = new Error('`npm ci` does not work for global packages')
@@ -33,7 +45,7 @@ const ci = async () => {
         'later to generate a package-lock.json file, then try again.'
       throw new Error(msg)
     }),
-    rimraf(`${where}/node_modules/*`, { glob: { dot: true, nosort: true, silent: true } }),
+    removeNodeModules(where),
   ])
   // npm ci should never modify the lockfile or package.json
   await arb.reify({ ...npm.flatOptions, save: false })

test/lib/ci.js

@@ -51,9 +51,45 @@ test('should use Arborist and run-script', (t) => {
     'prepare',
     'postprepare',
   ]
+
+  // set to true when timer starts, false when it ends
+  // when the test is done, we assert that all timers ended
+  const timers = {}
+  const onTime = msg => {
+    if (timers[msg])
+      throw new Error(`saw duplicate timer: ${msg}`)
+    timers[msg] = true
+  }
+  const onTimeEnd = msg => {
+    if (!timers[msg])
+      throw new Error(`ended timer that was not started: ${msg}`)
+    timers[msg] = false
+  }
+  process.on('time', onTime)
+  process.on('timeEnd', onTimeEnd)
+  t.teardown(() => {
+    process.removeListener('time', onTime)
+    process.removeListener('timeEnd', onTimeEnd)
+  })
+
+  const path = t.testdir({
+    node_modules: {
+      foo: {
+        'package.json': JSON.stringify({
+          name: 'foo',
+          version: '1.2.3',
+        }),
+      },
+      '.dotdir': {},
+      '.dotfile': 'a file with a dot',
+    },
+  })
+  const expectRimrafs = 3
+  let actualRimrafs = 0
+
   const ci = requireInject('../../lib/ci.js', {
     '../../lib/npm.js': {
-      prefix: 'foo',
+      prefix: path,
       flatOptions: {
         global: false,
       },
@@ -72,13 +108,11 @@ test('should use Arborist and run-script', (t) => {
         t.ok(true, 'reify is called')
       }
     },
-    util: {
-      inherits: () => {},
-      promisify: (fn) => fn,
-    },
-    rimraf: (path) => {
+    rimraf: (path, ...args) => {
+      actualRimrafs++
       t.ok(path, 'rimraf called with path')
-      return Promise.resolve(true)
+      // callback is always last arg
+      args.pop()()
     },
     '../../lib/utils/reify-output.js': function (arb) {
       t.ok(arb, 'gets arborist tree')
@@ -87,6 +121,10 @@ test('should use Arborist and run-script', (t) => {
   ci(null, er => {
     if (er)
       throw er
+    for (const [msg, result] of Object.entries(timers))
+      t.notOk(result, `properly resolved ${msg} timer`)
+    t.match(timers, { 'npm-ci:rm': false }, 'saw the rimraf timer')
+    t.equal(actualRimrafs, expectRimrafs, 'removed the right number of things')
     t.strictSame(scripts, [], 'called all scripts')
     t.end()
   })