Repository: istanbuljs/nyc
Pull request #1149 (2019-07-21 23:30:52)

feat: Allow `nyc instrument` to instrument code in place
This PR addresses #1135

This change adds the `--in-place` switch to nyc instrument
If `--in-place` is specified, the output directory will be set to equal the input directory for the instrument command.
This has the effect of replacing any file in the input directory that should be instrumented, with its instrumented counterpart.
The command will throw an error if the --delete option is specified.
The only way to instrument in place is with the --in-place switch, setting the input and output directories to be the same will not work
If `--in-place` is set the instrument command ignores any output directory specified with the command
If `--in-place` is set the instrument command will disable the `--complete-copy` switch as it is unnecessary.

I've made a few small code improvements to the instrument command spec.
I've also added tests to the old integration tests folder, I figured I could put tests here for the time being before they get moved to the new tap format (by me or someone else).

AndrewFinlay: Just a quick note that the I'm aware that this breaks the test snapshot and you probably don't want additions to the old integration test file.  Since I'm a little low on time at the moment I figured that it'd be good to get this out there and it'd be quicker to make tests for the old integration set that I'm familiar with.  Anyway let me know what needs to be done from here, thanks
coreyfarrell: Additional note the snapshots getting broken by this test is actually referenced in #1096.  I just haven't had time to tweak those tests which use `--all` to use an isolated directory.
coveralls: 
[![Coverage Status](https://coveralls.io/builds/24729930/badge)](https://coveralls.io/builds/24729930)

Coverage increased (+0.4%) to 97.854% when pulling **bfd6e62e01dbc819926cbe5a9184ae895749def4 on AndrewFinlay:instrument-overwrite** into **e94e2e75d27c10afac4646ff6822bcefe7e8aa7b on istanbuljs:master**.


Patch:
diff --git a/docs/instrument.md b/docs/instrument.md
index af19b6d..bce1a3b 100644
--- a/docs/instrument.md
+++ b/docs/instrument.md
@@ -20,6 +20,8 @@ For example, `nyc instrument . ./output` will produce instrumented versions of a
 
 The `--delete` option will remove the existing output directory before instrumenting.
 
+The `--in-place` option will allow you to run the instrument command.
+
 The `--complete-copy` option will copy all remaining files from the `input` directory to the `output` directory.
 When using `--complete-copy` nyc will not copy the contents of a `.git` folder to the output directory.
 
@@ -27,8 +29,8 @@ When using `--complete-copy` nyc will not copy the contents of a `.git` folder t
 
 ## Streaming instrumentation
 
-`nyc instrument` will stream instrumented source directly to `stdout`, that can then be piped to another process.
-You can use this behaviour to create a server that can instrument files on request.
+`nyc instrument <input>` will stream instrumented source directly to `stdout` and that output can then be piped to another process.
+You can use this behaviour to create a server that dynamically instruments files on request.
 The following example shows streaming instrumentation middleware capable of instrumenting files on request.
 
 ```javascript
diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js
index 553a1af..541878b 100644
--- a/lib/commands/instrument.js
+++ b/lib/commands/instrument.js
@@ -31,32 +31,37 @@ exports.builder = function (yargs) {
     .option('source-map', {
       default: true,
       type: 'boolean',
-      description: 'should nyc detect and handle source maps?'
+      describe: 'should nyc detect and handle source maps?'
     })
     .option('produce-source-map', {
       default: false,
       type: 'boolean',
-      description: "should nyc's instrumenter produce source maps?"
+      describe: "should nyc's instrumenter produce source maps?"
     })
     .option('compact', {
       default: true,
       type: 'boolean',
-      description: 'should the output be compacted?'
+      describe: 'should the output be compacted?'
     })
     .option('preserve-comments', {
       default: true,
       type: 'boolean',
-      description: 'should comments be preserved in the output?'
+      describe: 'should comments be preserved in the output?'
     })
     .option('instrument', {
       default: true,
       type: 'boolean',
-      description: 'should nyc handle instrumentation?'
+      describe: 'should nyc handle instrumentation?'
+    })
+    .option('in-place', {
+      default: false,
+      type: 'boolean',
+      describe: 'should nyc run the instrumentation in place?'
     })
     .option('exit-on-error', {
       default: false,
       type: 'boolean',
-      description: 'should nyc exit when an instrumentation failure occurs?'
+      describe: 'should nyc exit when an instrumentation failure occurs?'
     })
     .option('include', {
       alias: 'n',
@@ -92,17 +97,22 @@ exports.builder = function (yargs) {
     .example('$0 instrument ./lib ./output', 'instrument all .js files in ./lib with coverage and output in ./output')
 }
 
+const errorExit = msg => {
+  console.error(`nyc instrument failed:  ${msg}`)
+  process.exitCode = 1
+}
+
 exports.handler = function (argv) {
-  if (argv.output && (path.resolve(argv.cwd, argv.input) === path.resolve(argv.cwd, argv.output))) {
-    console.error(`nyc instrument failed: cannot instrument files in place, <input> must differ from <output>`)
-    process.exitCode = 1
-    return
+  if (argv.output && !argv.inPlace && (path.resolve(argv.cwd, argv.input) === path.resolve(argv.cwd, argv.output))) {
+    return errorExit(`cannot instrument files in place, <input> must differ from <output>.  Set '--in-place' to force`)
   }
 
   if (path.relative(argv.cwd, path.resolve(argv.cwd, argv.input)).startsWith('..')) {
-    console.error(`nyc instrument failed: cannot instrument files outside of project root directory`)
-    process.exitCode = 1
-    return
+    return errorExit('cannot instrument files outside project root directory')
+  }
+
+  if (argv.delete && argv.inPlace) {
+    return errorExit(`cannot use '--delete' when instrumenting files in place`)
   }
 
   if (argv.delete && argv.output && argv.output.length !== 0) {
@@ -110,8 +120,7 @@ exports.handler = function (argv) {
     if (relPath !== '' && !relPath.startsWith('..')) {
       rimraf.sync(argv.output)
     } else {
-      console.error(`nyc instrument failed: attempt to delete '${process.cwd()}' or containing directory.`)
-      process.exit(1)
+      return errorExit(`attempt to delete '${process.cwd()}' or containing directory.`)
     }
   }
 
@@ -120,12 +129,11 @@ exports.handler = function (argv) {
     ? './lib/instrumenters/istanbul'
     : './lib/instrumenters/noop'
 
-  const nyc = new NYC(argv)
+  if (argv.inPlace) {
+    argv.output = argv.input
+    argv.completeCopy = false
+  }
 
-  nyc.instrumentAllFiles(argv.input, argv.output, err => {
-    if (err) {
-      console.error(err.message)
-      process.exitCode = 1
-    }
-  })
+  const nyc = new NYC(argv)
+  nyc.instrumentAllFiles(argv.input, argv.output, err => err ? errorExit(err.message) : null)
 }


Test patch:
diff --git a/tap-snapshots/test-nyc-integration.js-TAP.test.js b/tap-snapshots/test-nyc-integration.js-TAP.test.js
index 4635391..5c7adce 100644
--- a/tap-snapshots/test-nyc-integration.js-TAP.test.js
+++ b/tap-snapshots/test-nyc-integration.js-TAP.test.js
@@ -103,6 +103,9 @@ All files                        |       0 |        0 |       0 |       0 |
   test.js                        |       0 |        0 |       0 |       0 |                         
  cli/fakebin                     |       0 |      100 |     100 |       0 |                         
   npm-template.js                |       0 |      100 |     100 |       0 | 2,3,4,7,9               
+ cli/instrument-inplace          |       0 |      100 |       0 |       0 |                         
+  file1.js                       |       0 |      100 |       0 |       0 | 2,5                     
+  file2.js                       |       0 |      100 |       0 |       0 | 2,5                     
  cli/nyc-config-js               |       0 |        0 |     100 |       0 |                         
   ignore.js                      |       0 |      100 |     100 |       0 | 1                       
   index.js                       |       0 |        0 |     100 |       0 | 1,3,5,7,8,9,10          
@@ -126,7 +129,7 @@ exports[`test/nyc-integration.js TAP --ignore-class-method skips methods that ma
 ---------------------------------|---------|----------|---------|---------|-------------------------
 File                             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s       
 ---------------------------------|---------|----------|---------|---------|-------------------------
-All files                        |    1.49 |        0 |    5.56 |    1.96 |                         
+All files                        |    1.45 |        0 |       5 |    1.89 |                         
  cli                             |    2.08 |        0 |    5.56 |    3.13 |                         
   args.js                        |       0 |      100 |     100 |       0 | 1                       
   by-arg2.js                     |       0 |        0 |     100 |       0 | 1,2,3,4,5,7             
@@ -144,6 +147,9 @@ All files                        |    1.49 |        0 |    5.56 |    1.96 |
   skip-full.js                   |       0 |      100 |     100 |       0 | 1,2                     
  cli/fakebin                     |       0 |      100 |     100 |       0 |                         
   npm-template.js                |       0 |      100 |     100 |       0 | 2,3,4,7,9               
+ cli/instrument-inplace          |       0 |      100 |       0 |       0 |                         
+  file1.js                       |       0 |      100 |       0 |       0 | 2,5                     
+  file2.js                       |       0 |      100 |       0 |       0 | 2,5                     
  cli/nyc-config-js               |       0 |        0 |     100 |       0 |                         
   ignore.js                      |       0 |      100 |     100 |       0 | 1                       
   index.js                       |       0 |        0 |     100 |       0 | 1,3,5,7,8,9,10          
diff --git a/test/fixtures/cli/instrument-inplace/file1.js b/test/fixtures/cli/instrument-inplace/file1.js
new file mode 100644
index 0000000..d8c7ef4
--- /dev/null
+++ b/test/fixtures/cli/instrument-inplace/file1.js
@@ -0,0 +1,5 @@
+function test() {
+  return 'file1'
+}
+
+module.exports = test
diff --git a/test/fixtures/cli/instrument-inplace/file2.js b/test/fixtures/cli/instrument-inplace/file2.js
new file mode 100644
index 0000000..5ca85a3
--- /dev/null
+++ b/test/fixtures/cli/instrument-inplace/file2.js
@@ -0,0 +1,5 @@
+function test() {
+	return 'file2'
+}
+
+module.exports = test
diff --git a/test/nyc-integration-old.js b/test/nyc-integration-old.js
index 6190dc0..4004187 100644
--- a/test/nyc-integration-old.js
+++ b/test/nyc-integration-old.js
@@ -7,6 +7,7 @@ const fixturesCLI = path.resolve(__dirname, './fixtures/cli')
 const fakebin = path.resolve(fixturesCLI, 'fakebin')
 const fs = require('fs')
 const isWindows = require('is-windows')()
+const cpFile = require('cp-file')
 const rimraf = require('rimraf')
 const makeDir = require('make-dir')
 const { spawn } = require('child_process')
@@ -325,7 +326,7 @@ describe('the nyc cli', function () {
       })
 
       it('aborts if trying to write files in place', function (done) {
-        const args = [bin, 'instrument', '--delete', './', './']
+        const args = [bin, 'instrument', './', './']
 
         const proc = spawn(process.execPath, args, {
           cwd: fixturesCLI,
@@ -339,7 +340,54 @@ describe('the nyc cli', function () {
 
         proc.on('close', function (code) {
           code.should.equal(1)
-          stderr.should.include('nyc instrument failed: cannot instrument files in place')
+          stderr.should.include('cannot instrument files in place')
+          done()
+        })
+      })
+
+      it('can write files in place with --in-place switch', function (done) {
+        const args = [bin, 'instrument', '--in-place', '--include', '*/file1.js', './test-instrument-inplace']
+
+        const sourceDir = path.resolve(fixturesCLI, 'instrument-inplace')
+        const destDir = path.resolve(fixturesCLI, 'test-instrument-inplace')
+        makeDir.sync(destDir)
+        cpFile.sync(path.join(sourceDir, 'file1.js'), path.join(destDir, 'file1.js'))
+        cpFile.sync(path.join(sourceDir, 'file2.js'), path.join(destDir, 'file2.js'))
+
+        const proc = spawn(process.execPath, args, {
+          cwd: fixturesCLI,
+          env: env
+        })
+
+        proc.on('close', function (code) {
+          code.should.equal(0)
+          const file1 = path.resolve(destDir, 'file1.js')
+          fs.readFileSync(file1, 'utf8')
+            .should.match(/var cov_/)
+          const file2 = path.resolve(destDir, 'file2.js')
+          fs.readFileSync(file2, 'utf8')
+            .should.not.match(/var cov_/)
+          rimraf.sync(destDir)
+          done()
+        })
+      })
+
+      it('aborts if trying to delete while writing files in place', function (done) {
+        const args = [bin, 'instrument', '--in-place', '--delete', '--include', 'file1.js', './instrument-inplace']
+
+        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(`cannot use '--delete' when instrumenting files in place`)
           done()
         })
       })
@@ -359,7 +407,7 @@ describe('the nyc cli', function () {
 
         proc.on('close', function (code) {
           code.should.equal(1)
-          stderr.should.include('nyc instrument failed: cannot instrument files outside of project root directory')
+          stderr.should.include('cannot instrument files outside project root directory')
           done()
         })
       })
@@ -421,7 +469,7 @@ describe('the nyc cli', function () {
 
           proc.on('close', function (code) {
             code.should.equal(1)
-            stderr.should.include('nyc instrument failed: attempt to delete')
+            stderr.should.include('attempt to delete')
             done()
           })
         })
@@ -441,7 +489,7 @@ describe('the nyc cli', function () {
 
           proc.on('close', function (code) {
             code.should.equal(1)
-            stderr.should.include('nyc instrument failed: attempt to delete')
+            stderr.should.include('attempt to delete')
             done()
           })
         })