From 6cfbdbb526e787d40260f82303ed3ae5113d71da Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Fri, 17 May 2019 15:57:25 +1000 Subject: [PATCH 1/3] feat: Allow `nyc instrument` to instrument code in place 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. --- docs/instrument.md | 6 +- lib/commands/instrument.js | 54 ++++++++------- test/fixtures/cli/instrument-inplace/file1.js | 5 ++ test/fixtures/cli/instrument-inplace/file2.js | 5 ++ test/nyc-integration-old.js | 66 ++++++++++++++++--- 5 files changed, 102 insertions(+), 34 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..3467832df 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..c87a0829e 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,22 +120,22 @@ 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.`) } } + console.log(`Running nyc from ${process.cwd()}`) + // If instrument is set to false enable a noop instrumenter. argv.instrumenter = (argv.instrument) ? './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/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..0acb1660c 100644 --- a/test/nyc-integration-old.js +++ b/test/nyc-integration-old.js @@ -329,7 +329,53 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env: 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 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', './instrument-inplace'] + + const proc = spawn(process.execPath, args, { + cwd: fixturesCLI, + env + }) + + let stderr = '' + proc.stderr.on('data', function (chunk) { + stderr += chunk + }) + + proc.on('close', function (code) { + console.log(`stderr: ${stderr}`) + code.should.equal(0) + const file1 = path.resolve(fixturesCLI, 'instrument-inplace', 'file1.js') + fs.readFileSync(file1, 'utf8') + .should.match(/var cov_/) + const file2 = path.resolve(fixturesCLI, 'instrument-inplace', 'file2.js') + fs.readFileSync(file2, 'utf8') + .should.not.match(/var cov_/) + 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 }) let stderr = '' @@ -339,7 +385,7 @@ 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 use '--delete' when instrumenting files in place`) done() }) }) @@ -349,7 +395,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env: env + env }) let stderr = '' @@ -359,7 +405,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() }) }) @@ -374,7 +420,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env: env + env }) proc.on('close', function (code) { @@ -393,7 +439,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env: env + env }) proc.on('close', function (code) { @@ -411,7 +457,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env: env + env }) let stderr = '' @@ -421,7 +467,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() }) }) @@ -431,7 +477,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env: env + env }) let stderr = '' @@ -441,7 +487,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() }) }) From bfd6e62e01dbc819926cbe5a9184ae895749def4 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Tue, 23 Jul 2019 08:42:05 +1000 Subject: [PATCH 2/3] Update snapshot and address review concerns The instrument --in-place test now runs in a copy of the instrument in place test directory, this means that running tests locally doesn't change your local environment. --- docs/instrument.md | 2 +- .../test-nyc-integration.js-TAP.test.js | 9 ++++- test/nyc-integration-old.js | 38 ++++++++++--------- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/docs/instrument.md b/docs/instrument.md index 3467832df..bce1a3b4c 100644 --- a/docs/instrument.md +++ b/docs/instrument.md @@ -20,7 +20,7 @@ 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 `--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. diff --git a/tap-snapshots/test-nyc-integration.js-TAP.test.js b/tap-snapshots/test-nyc-integration.js-TAP.test.js index 46353911f..1dc835a78 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 @@ -753,6 +759,7 @@ Failed to instrument ./not-strict.js ` exports[`test/nyc-integration.js TAP nyc instrument fails on file with \`package\` keyword when es-modules is enabled > stdout 1`] = ` +Running nyc from . ` diff --git a/test/nyc-integration-old.js b/test/nyc-integration-old.js index 0acb1660c..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,11 +326,11 @@ 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, - env + env: env }) let stderr = '' @@ -345,27 +346,28 @@ describe('the nyc cli', function () { }) it('can write files in place with --in-place switch', function (done) { - const args = [bin, 'instrument', '--in-place', '--include', '*/file1.js', './instrument-inplace'] + 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 - }) - - let stderr = '' - proc.stderr.on('data', function (chunk) { - stderr += chunk + env: env }) proc.on('close', function (code) { - console.log(`stderr: ${stderr}`) code.should.equal(0) - const file1 = path.resolve(fixturesCLI, 'instrument-inplace', 'file1.js') + const file1 = path.resolve(destDir, 'file1.js') fs.readFileSync(file1, 'utf8') .should.match(/var cov_/) - const file2 = path.resolve(fixturesCLI, 'instrument-inplace', 'file2.js') + const file2 = path.resolve(destDir, 'file2.js') fs.readFileSync(file2, 'utf8') .should.not.match(/var cov_/) + rimraf.sync(destDir) done() }) }) @@ -375,7 +377,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env + env: env }) let stderr = '' @@ -395,7 +397,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env + env: env }) let stderr = '' @@ -420,7 +422,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env + env: env }) proc.on('close', function (code) { @@ -439,7 +441,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env + env: env }) proc.on('close', function (code) { @@ -457,7 +459,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env + env: env }) let stderr = '' @@ -477,7 +479,7 @@ describe('the nyc cli', function () { const proc = spawn(process.execPath, args, { cwd: fixturesCLI, - env + env: env }) let stderr = '' From 6b1dbc9e4ebf546b28bae52add2056f885fb684e Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Thu, 1 Aug 2019 07:33:40 +1000 Subject: [PATCH 3/3] Remove unnecessary console.log statement --- lib/commands/instrument.js | 2 -- tap-snapshots/test-nyc-integration.js-TAP.test.js | 1 - 2 files changed, 3 deletions(-) diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index c87a0829e..541878b2c 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -124,8 +124,6 @@ exports.handler = function (argv) { } } - console.log(`Running nyc from ${process.cwd()}`) - // If instrument is set to false enable a noop instrumenter. argv.instrumenter = (argv.instrument) ? './lib/instrumenters/istanbul' diff --git a/tap-snapshots/test-nyc-integration.js-TAP.test.js b/tap-snapshots/test-nyc-integration.js-TAP.test.js index 1dc835a78..5c7adcee6 100644 --- a/tap-snapshots/test-nyc-integration.js-TAP.test.js +++ b/tap-snapshots/test-nyc-integration.js-TAP.test.js @@ -759,7 +759,6 @@ Failed to instrument ./not-strict.js ` exports[`test/nyc-integration.js TAP nyc instrument fails on file with \`package\` keyword when es-modules is enabled > stdout 1`] = ` -Running nyc from . `