Skip to content

Commit

Permalink
feat: Allow nyc instrument to instrument code in place (#1149)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AndrewFinlay authored and coreyfarrell committed Aug 1, 2019
1 parent c358ce1 commit 7783284
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 30 deletions.
6 changes: 4 additions & 2 deletions docs/instrument.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ 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.

**Note:** `--complete-copy` will dereference any symlinks during the copy process, this may stop scripts running properly from the output directory.

## 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
Expand Down
52 changes: 30 additions & 22 deletions lib/commands/instrument.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -92,26 +97,30 @@ 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) {
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()}' or containing directory.`)
process.exit(1)
return errorExit(`attempt to delete '${process.cwd()}' or containing directory.`)
}
}

Expand All @@ -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)
}
8 changes: 7 additions & 1 deletion tap-snapshots/test-nyc-integration.js-TAP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/cli/instrument-inplace/file1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function test() {
return 'file1'
}

module.exports = test
5 changes: 5 additions & 0 deletions test/fixtures/cli/instrument-inplace/file2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function test() {
return 'file2'
}

module.exports = test
58 changes: 53 additions & 5 deletions test/nyc-integration-old.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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,
Expand All @@ -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()
})
})
Expand All @@ -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()
})
})
Expand Down Expand Up @@ -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()
})
})
Expand All @@ -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()
})
})
Expand Down

0 comments on commit 7783284

Please sign in to comment.