From 70dd99319728fc69bbb4310f5624cc3e495db77e Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Thu, 28 Feb 2019 09:28:48 +1100 Subject: [PATCH 1/4] Implement '--clean' option for nyc instrument command With this change you can now tell nyc to clean the output directory when instrumenting, to do this an output directory must be specified. I've currently set the default value for this to false so it can be a non-breaking change, although the default behaviour should be changed to true in the next major release. I've included tests for this behaviour, and introduced lint support for `afterEach` and required the `makeDir` module --- lib/commands/instrument.js | 11 ++++++++ test/nyc-integration.js | 52 +++++++++++++++++++++++++++++++++++--- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index 7c098d9a2..99835575f 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -1,3 +1,5 @@ +const rimraf = require('rimraf') + var NYC try { NYC = require('../../index.covered.js') @@ -51,6 +53,11 @@ exports.builder = function (yargs) { type: 'boolean', description: 'should nyc exit when an instrumentation failure occurs?' }) + .option('clean', { + describe: 'should the output folder be cleaned before instrumenting files?', + default: false, + type: 'boolean' + }) .example('$0 instrument ./lib ./output', 'instrument all .js files in ./lib with coverage and output in ./output') } @@ -71,6 +78,10 @@ exports.handler = function (argv) { exitOnError: argv.exitOnError }) + if (argv.clean && argv.output && argv.output.length !== 0) { + rimraf.sync(argv.output) + } + nyc.instrumentAllFiles(argv.input, argv.output, function (err) { if (err) { console.error(err.message) diff --git a/test/nyc-integration.js b/test/nyc-integration.js index aa6b9a39c..8bb821306 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -1,4 +1,4 @@ -/* global describe, it, beforeEach */ +/* global describe, it, beforeEach, afterEach */ const _ = require('lodash') const path = require('path') @@ -11,6 +11,7 @@ const fs = require('fs') const glob = require('glob') const isWindows = require('is-windows')() const rimraf = require('rimraf') +const makeDir = require('make-dir') const spawn = require('child_process').spawn const si = require('strip-indent') @@ -548,6 +549,10 @@ describe('the nyc cli', function () { }) describe('output folder specified', function () { + afterEach(function () { + rimraf.sync(path.resolve(fixturesCLI, 'output')) + }) + it('allows a single file to be instrumented', function (done) { var args = [bin, 'instrument', './half-covered.js', './output'] @@ -561,7 +566,6 @@ describe('the nyc cli', function () { var files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.equal(1) files.should.include('half-covered.js') - rimraf.sync(path.resolve(fixturesCLI, 'output')) done() }) }) @@ -579,7 +583,6 @@ describe('the nyc cli', function () { var files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.should.include('env.js') files.should.include('es6.js') - rimraf.sync(path.resolve(fixturesCLI, 'output')) done() }) }) @@ -596,10 +599,51 @@ describe('the nyc cli', function () { code.should.equal(0) var files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.should.include('index.js') - rimraf.sync(path.resolve(fixturesCLI, 'output')) done() }) }) + + describe('clean', function () { + beforeEach(function () { + makeDir.sync(path.resolve(fixturesCLI, 'output', 'removed-by-clean')) + }) + + it('cleans the output directory if `--clean` is specified', function (done) { + const args = [bin, 'instrument', '--clean', 'true', './', './output'] + + const proc = spawn(process.execPath, args, { + cwd: fixturesCLI, + env: env + }) + + proc.on('close', function (code) { + code.should.equal(0) + const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) + subdirExists.should.equal(true) + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.should.not.include('removed-by-clean') + done() + }) + }) + + it('does not clean the output directory by default', function (done) { + const args = [bin, 'instrument', './', './output'] + + const proc = spawn(process.execPath, args, { + cwd: fixturesCLI, + env: env + }) + + proc.on('close', function (code) { + code.should.equal(0) + const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) + subdirExists.should.equal(true) + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.should.include('removed-by-clean') + done() + }) + }) + }) }) }) From 6b26a60778286d30c65646ff0dcfe171cf81e422 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Thu, 7 Mar 2019 16:17:02 +1100 Subject: [PATCH 2/4] Changed command name to delete, added protection against accidentally deleting cwd The protection against deleting cwd is a little heavy handed, but I figure if you don't want to delete your cwd and probably don't want your output files going there either. Added test support for these changes. --- lib/commands/instrument.js | 22 +++++++++++----------- test/nyc-integration.js | 26 +++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index 99835575f..d3fc594fb 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -1,11 +1,6 @@ +const path = require('path') const rimraf = require('rimraf') - -var NYC -try { - NYC = require('../../index.covered.js') -} catch (e) { - NYC = require('../../index.js') -} +const NYC = require('../../index.js') exports.command = 'instrument [output]' @@ -53,8 +48,8 @@ exports.builder = function (yargs) { type: 'boolean', description: 'should nyc exit when an instrumentation failure occurs?' }) - .option('clean', { - describe: 'should the output folder be cleaned before instrumenting files?', + .option('delete', { + describe: 'should the output folder be deleted before instrumenting files?', default: false, type: 'boolean' }) @@ -78,8 +73,13 @@ exports.handler = function (argv) { exitOnError: argv.exitOnError }) - if (argv.clean && argv.output && argv.output.length !== 0) { - rimraf.sync(argv.output) + if (argv.delete && argv.output && argv.output.length !== 0) { + if (path.relative(process.cwd(), path.resolve(argv.output)) !== '') { + rimraf.sync(argv.output) + } else { + console.error(`nyc instrument failed: attempt to delete '${process.cwd()}'`) + process.exit(1) + } } nyc.instrumentAllFiles(argv.input, argv.output, function (err) { diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 8bb821306..4f39c8134 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -603,13 +603,13 @@ describe('the nyc cli', function () { }) }) - describe('clean', function () { + describe('delete', function () { beforeEach(function () { makeDir.sync(path.resolve(fixturesCLI, 'output', 'removed-by-clean')) }) - it('cleans the output directory if `--clean` is specified', function (done) { - const args = [bin, 'instrument', '--clean', 'true', './', './output'] + it('cleans the output directory if `--delete` is specified', function (done) { + const args = [bin, 'instrument', '--delete', 'true', './', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -643,6 +643,26 @@ describe('the nyc cli', function () { done() }) }) + + it('aborts if trying to clean process.cwd()', function (done) { + const args = [bin, 'instrument', '--delete', './', './'] + + 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('nyc instrument failed: attempt to delete') + done() + }) + }) }) }) }) From 66bdf0bae1b25e86b16108db5a33d7b1f549a4e0 Mon Sep 17 00:00:00 2001 From: AndrewFinlay Date: Fri, 8 Mar 2019 13:10:15 +1100 Subject: [PATCH 3/4] Attempt to resolve conflicts Never edited a file through Github's web interface before --- lib/commands/instrument.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index d3fc594fb..279faf3a4 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -1,6 +1,6 @@ +const NYC = require('../../index.js') const path = require('path') const rimraf = require('rimraf') -const NYC = require('../../index.js') exports.command = 'instrument [output]' From 005f7f8430411e5c678db0835c92cf67f038360b Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Tue, 12 Mar 2019 09:46:21 +1100 Subject: [PATCH 4/4] Ensure 'rimraf' can only remove subdirectories of the cwd --- lib/commands/instrument.js | 3 ++- test/nyc-integration.js | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index 9fdf59e58..9e255a757 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -80,7 +80,8 @@ exports.handler = function (argv) { }) if (argv.delete && argv.output && argv.output.length !== 0) { - if (path.relative(process.cwd(), path.resolve(argv.output)) !== '') { + 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()}'`) diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 4d3d11a58..a34bbdbc8 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -784,6 +784,26 @@ describe('the nyc cli', function () { done() }) }) + + it('aborts if trying to clean outside working directory', function (done) { + const args = [bin, 'instrument', '--delete', './', '../'] + + 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('nyc instrument failed: attempt to delete') + done() + }) + }) }) }) })