Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow nyc instrument to instrument code in place #1149

Merged
merged 3 commits into from
Aug 1, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
coreyfarrell marked this conversation as resolved.
Show resolved Hide resolved

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
54 changes: 32 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,40 +97,45 @@ 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.`)
}
}

console.log(`Running nyc from ${process.cwd()}`)
coreyfarrell marked this conversation as resolved.
Show resolved Hide resolved

// 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
coreyfarrell marked this conversation as resolved.
Show resolved Hide resolved
}

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)
}
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
66 changes: 56 additions & 10 deletions test/nyc-integration-old.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
coreyfarrell marked this conversation as resolved.
Show resolved Hide resolved
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']
coreyfarrell marked this conversation as resolved.
Show resolved Hide resolved

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 = ''
Expand All @@ -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()
})
})
Expand All @@ -349,7 +395,7 @@ describe('the nyc cli', function () {

const proc = spawn(process.execPath, args, {
cwd: fixturesCLI,
env: env
env
})

let stderr = ''
Expand All @@ -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()
})
})
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -411,7 +457,7 @@ describe('the nyc cli', function () {

const proc = spawn(process.execPath, args, {
cwd: fixturesCLI,
env: env
env
})

let stderr = ''
Expand All @@ -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()
})
})
Expand All @@ -431,7 +477,7 @@ describe('the nyc cli', function () {

const proc = spawn(process.execPath, args, {
cwd: fixturesCLI,
env: env
env
})

let stderr = ''
Expand All @@ -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()
})
})
Expand Down