From 77832845b85134d21eca3a23c812c4f21f36713f Mon Sep 17 00:00:00 2001 From: AndrewFinlay Date: Thu, 1 Aug 2019 10:35:17 +1000 Subject: [PATCH] feat: Allow `nyc instrument` to instrument code in place (#1149) 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. --- docs/instrument.md | 6 +- lib/commands/instrument.js | 52 ++++++++++------- .../test-nyc-integration.js-TAP.test.js | 8 ++- test/fixtures/cli/instrument-inplace/file1.js | 5 ++ test/fixtures/cli/instrument-inplace/file2.js | 5 ++ test/nyc-integration-old.js | 58 +++++++++++++++++-- 6 files changed, 104 insertions(+), 30 deletions(-) create mode 100644 test/fixtures/cli/instrument-inplace/file1.js create mode 100644 test/fixtures/cli/instrument-inplace/file2.js diff --git a/docs/instrument.md b/docs/instrument.md index af19b6daa..bce1a3b4c 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 ` 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 553a1af7b..541878b2c 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, must differ from `) - 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, must differ from . 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) } diff --git a/tap-snapshots/test-nyc-integration.js-TAP.test.js b/tap-snapshots/test-nyc-integration.js-TAP.test.js index 46353911f..5c7adcee6 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 000000000..d8c7ef465 --- /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 000000000..5ca85a3f0 --- /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 6190dc005..40041876f 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() }) })