Repository: istanbuljs/nyc
Pull request #1005 (2019-02-27 23:01:23)

feat: add `delete` option to instrument command
This change adds a `--clean` switch to the nyc instrument command, it must be set true and an output directory must be specified.

I've set the default value for this to false so it can be a non-breaking change, but this should change in the next major release.  I've included tests for this function, introduced lint support for `afterEach` in the tests, and required the `makeDir` module.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/21892890/badge)](https://coveralls.io/builds/21892890)

Coverage remained the same at 96.639% when pulling **70dd99319728fc69bbb4310f5624cc3e495db77e on AndrewFinlay:instrument-clean-option** into **b64d921ee15a57779e54f7ea14102c79ebf7498b on istanbuljs:master**.

coreyfarrell: One concern I have with this PR is that the `--clean` flag has meaning in the global CLI context, it controls deletion of `.nyc_output/`.  I'm not sure how I feel about having a sub command use the same flag name as the main command but for a different purpose.

CC @JaKXz @bcoe
AndrewFinlay: After a chat with @coreyfarrell I've changed the name and added some level of defence against `rimraf` foot guns.

Patch:
diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js
index fc49089..9e255a7 100644
--- a/lib/commands/instrument.js
+++ b/lib/commands/instrument.js
@@ -1,4 +1,6 @@
 const NYC = require('../../index.js')
+const path = require('path')
+const rimraf = require('rimraf')
 
 exports.command = 'instrument <input> [output]'
 
@@ -51,6 +53,11 @@ exports.builder = function (yargs) {
       type: 'boolean',
       description: 'tell the instrumenter to treat files as ES Modules'
     })
+    .option('delete', {
+      describe: 'should the output folder be deleted before instrumenting files?',
+      default: false,
+      type: 'boolean'
+    })
     .example('$0 instrument ./lib ./output', 'instrument all .js files in ./lib with coverage and output in ./output')
 }
 
@@ -72,6 +79,16 @@ exports.handler = function (argv) {
     exitOnError: argv.exitOnError
   })
 
+  if (argv.delete && argv.output && argv.output.length !== 0) {
+    const relPath = path.relative(process.cwd(), path.resolve(argv.output))
+    if (relPath !== '' && !relPath.startsWith('..')) {
+      rimraf.sync(argv.output)
+    } else {
+      console.error(`nyc instrument failed: attempt to delete '${process.cwd()}'`)
+      process.exit(1)
+    }
+  }
+
   nyc.instrumentAllFiles(argv.input, argv.output, function (err) {
     if (err) {
       console.error(err.message)


Test patch:
diff --git a/test/nyc-integration.js b/test/nyc-integration.js
index 0eba6aa..a34bbdb 100644
--- a/test/nyc-integration.js
+++ b/test/nyc-integration.js
@@ -11,6 +11,7 @@ const fs = require('fs')
 const glob = require('glob')
 const isWindows = require('is-windows')()
 const rimraf = require('rimraf')
+const makeDir = require('make-dir')
 const spawn = require('child_process').spawn
 const si = require('strip-indent')
 
@@ -622,6 +623,10 @@ describe('the nyc cli', function () {
     })
 
     describe('output folder specified', function () {
+      afterEach(function () {
+        rimraf.sync(path.resolve(fixturesCLI, 'output'))
+      })
+
       it('allows a single file to be instrumented', function (done) {
         var args = [bin, 'instrument', './half-covered.js', './output']
 
@@ -635,7 +640,6 @@ describe('the nyc cli', function () {
           var files = fs.readdirSync(path.resolve(fixturesCLI, './output'))
           files.length.should.equal(1)
           files.should.include('half-covered.js')
-          rimraf.sync(path.resolve(fixturesCLI, 'output'))
           done()
         })
       })
@@ -653,7 +657,6 @@ describe('the nyc cli', function () {
           var files = fs.readdirSync(path.resolve(fixturesCLI, './output'))
           files.should.include('env.js')
           files.should.include('es6.js')
-          rimraf.sync(path.resolve(fixturesCLI, 'output'))
           done()
         })
       })
@@ -670,7 +673,6 @@ describe('the nyc cli', function () {
           code.should.equal(0)
           var files = fs.readdirSync(path.resolve(fixturesCLI, './output'))
           files.should.include('index.js')
-          rimraf.sync(path.resolve(fixturesCLI, 'output'))
           done()
         })
       })
@@ -721,6 +723,88 @@ describe('the nyc cli', function () {
           })
         })
       })
+
+      describe('delete', function () {
+        beforeEach(function () {
+          makeDir.sync(path.resolve(fixturesCLI, 'output', 'removed-by-clean'))
+        })
+
+        it('cleans the output directory if `--delete` is specified', function (done) {
+          const args = [bin, 'instrument', '--delete', 'true', './', './output']
+
+          const proc = spawn(process.execPath, args, {
+            cwd: fixturesCLI,
+            env: env
+          })
+
+          proc.on('close', function (code) {
+            code.should.equal(0)
+            const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir'))
+            subdirExists.should.equal(true)
+            const files = fs.readdirSync(path.resolve(fixturesCLI, './output'))
+            files.should.not.include('removed-by-clean')
+            done()
+          })
+        })
+
+        it('does not clean the output directory by default', function (done) {
+          const args = [bin, 'instrument', './', './output']
+
+          const proc = spawn(process.execPath, args, {
+            cwd: fixturesCLI,
+            env: env
+          })
+
+          proc.on('close', function (code) {
+            code.should.equal(0)
+            const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir'))
+            subdirExists.should.equal(true)
+            const files = fs.readdirSync(path.resolve(fixturesCLI, './output'))
+            files.should.include('removed-by-clean')
+            done()
+          })
+        })
+
+        it('aborts if trying to clean process.cwd()', function (done) {
+          const args = [bin, 'instrument', '--delete', './', './']
+
+          const proc = spawn(process.execPath, args, {
+            cwd: fixturesCLI,
+            env: env
+          })
+
+          let stderr = ''
+          proc.stderr.on('data', function (chunk) {
+            stderr += chunk
+          })
+
+          proc.on('close', function (code) {
+            code.should.equal(1)
+            stderr.should.include('nyc instrument failed: attempt to delete')
+            done()
+          })
+        })
+
+        it('aborts if trying to clean outside working directory', function (done) {
+          const args = [bin, 'instrument', '--delete', './', '../']
+
+          const proc = spawn(process.execPath, args, {
+            cwd: fixturesCLI,
+            env: env
+          })
+
+          let stderr = ''
+          proc.stderr.on('data', function (chunk) {
+            stderr += chunk
+          })
+
+          proc.on('close', function (code) {
+            code.should.equal(1)
+            stderr.should.include('nyc instrument failed: attempt to delete')
+            done()
+          })
+        })
+      })
     })
   })