From d576e4418ff5c84dabd52cdd1c8715c32a7be130 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Fri, 1 Mar 2019 14:24:31 +1100 Subject: [PATCH 01/11] Added `include` and `exclude` options for instrument command This allows you to better select which files are instrumented when running the instrument command. It's a pretty minimal implementation to make it as easy as possible to integrate with other proposed changes. Test support is included with this change. --- index.js | 5 +++ lib/commands/instrument.js | 14 ++++++ test/nyc-integration.js | 87 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index dc6a461d9..ef21d7656 100755 --- a/index.js +++ b/index.js @@ -208,6 +208,11 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { var ext var transform var inFile = path.resolve(inputDir, filename) + + if (!_this.exclude.shouldInstrument(filename)) { + return + } + var code = fs.readFileSync(inFile, 'utf-8') for (ext in _this.transforms) { diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index 7c098d9a2..8206082bc 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -1,3 +1,5 @@ +const testExclude = require('test-exclude') + var NYC try { NYC = require('../../index.covered.js') @@ -51,6 +53,16 @@ exports.builder = function (yargs) { type: 'boolean', description: 'should nyc exit when an instrumentation failure occurs?' }) + .option('include', { + alias: 'n', + default: [], + describe: 'a list of specific files and directories that should be instrumented, glob patterns are supported' + }) + .option('exclude', { + alias: 'x', + default: testExclude.defaultExclude, + describe: 'a list of specific files and directories that should not be instrumented, glob patterns are supported' + }) .example('$0 instrument ./lib ./output', 'instrument all .js files in ./lib with coverage and output in ./output') } @@ -68,6 +80,8 @@ exports.handler = function (argv) { require: argv.require, compact: argv.compact, preserveComments: argv.preserveComments, + include: argv.include, + exclude: argv.exclude, exitOnError: argv.exitOnError }) diff --git a/test/nyc-integration.js b/test/nyc-integration.js index aa6b9a39c..358801af6 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') @@ -548,6 +548,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 +565,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 +582,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,7 +598,84 @@ 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() + }) + }) + + it('allows a subdirectory to be excluded', function (done) { + const args = [bin, 'instrument', '--exclude', '**/subdir/**', './', './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(false) + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.length.should.not.equal(0) + files.should.not.include('subdir') + done() + }) + }) + + it('allows a file to be excluded', function (done) { + const args = [bin, 'instrument', '--exclude', 'subdir/input-dir/bad.js', './', './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/subdir/input-dir')) + files.should.include('index.js') + files.should.not.include('bad.js') + done() + }) + }) + + it('allows specifying a single sub-directory to be included', function (done) { + const args = [bin, 'instrument', '--include', '**/subdir/input-dir/bad.js', './', './output'] + + const proc = spawn(process.execPath, args, { + cwd: fixturesCLI, + env: env + }) + + proc.on('close', function (code) { + code.should.equal(0) + let files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.length.should.equal(1) + const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) + subdirExists.should.equal(true) + files = fs.readdirSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) + files.should.include('bad.js') + files.should.not.include('index.js') + done() + }) + }) + + it('allows a file to be excluded from an included directory', function (done) { + const args = [bin, 'instrument', '--exclude', '**/bad.js', '--include', '**/subdir/input-dir/**', './', './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/subdir/input-dir')) + files.should.include('index.js') + files.should.not.include('bad.js') done() }) }) From 5174d127dc976278cde564fad725e1d6513db749 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Wed, 20 Mar 2019 15:55:04 +1100 Subject: [PATCH 02/11] `nyc instrument` copies all files to an output directory, then adds instrumented versions where needed So I've implemented this using `fs-extra`, not sure if that's the module you wish to go with here. This is now relying on `testExclude.globSync` to get the correct files for instrumentation, so no more `shouldInstrument` There's quite a few test changes in here, as `fs-extra.copySync` won't let you copy from `./` to `./outputMkdirHere`, but they're probably for the best anyway. This also means that any attempt to store the instrumentation result in a subdir of the input dir will fail. --- index.js | 6 +- package-lock.json | 61 ++++++--- package.json | 1 + .../cli/subdir/input-dir/exclude-me/index.js | 2 + test/nyc-integration.js | 129 +++++++++++------- 5 files changed, 127 insertions(+), 72 deletions(-) create mode 100644 test/fixtures/cli/subdir/input-dir/exclude-me/index.js diff --git a/index.js b/index.js index 43e20884b..f01bb6381 100755 --- a/index.js +++ b/index.js @@ -6,7 +6,7 @@ const arrify = require('arrify') const cachingTransform = require('caching-transform') const util = require('util') const findCacheDir = require('find-cache-dir') -const fs = require('fs') +const fs = require('fs-extra') const Hash = require('./lib/hash') const libCoverage = require('istanbul-lib-coverage') const libHook = require('istanbul-lib-hook') @@ -200,6 +200,10 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { try { const stats = fs.lstatSync(input) if (stats.isDirectory()) { + if (output) { + fs.copySync(input, output) + } + inputDir = input this.walkAllFiles(input, visitor) } else { diff --git a/package-lock.json b/package-lock.json index 44931e4a9..0e3a57b9c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -293,7 +293,7 @@ }, "chalk": { "version": "1.1.3", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", "dev": true, "requires": { @@ -312,7 +312,7 @@ }, "strip-ansi": { "version": "3.0.1", - "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "resolved": "http://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "dev": true, "requires": { @@ -854,7 +854,7 @@ "dependencies": { "minimist": { "version": "1.2.0", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true } @@ -1408,7 +1408,7 @@ }, "load-json-file": { "version": "2.0.0", - "resolved": "https://registry.npmjs.org/load-json-file/-/load-json-file-2.0.0.tgz", + "resolved": "http://registry.npmjs.org/load-json-file/-/load-json-file-2.0.0.tgz", "integrity": "sha1-eUfkIUmvgNaWy/eXvKq8/h/inKg=", "dev": true, "requires": { @@ -1660,7 +1660,7 @@ }, "external-editor": { "version": "2.2.0", - "resolved": "https://registry.npmjs.org/external-editor/-/external-editor-2.2.0.tgz", + "resolved": "http://registry.npmjs.org/external-editor/-/external-editor-2.2.0.tgz", "integrity": "sha512-bSn6gvGxKt+b7+6TKEv1ZycHleA7aHhRHyAqJyp5pbUFuYYNIzpZnQDk7AsYckyWdEnTeAnay0aCy2aV6iTk9A==", "dev": true, "requires": { @@ -1765,7 +1765,7 @@ }, "foreground-child": { "version": "1.5.6", - "resolved": "https://registry.npmjs.org/foreground-child/-/foreground-child-1.5.6.tgz", + "resolved": "http://registry.npmjs.org/foreground-child/-/foreground-child-1.5.6.tgz", "integrity": "sha1-T9ca0t/elnibmApcCilZN8svXOk=", "requires": { "cross-spawn": "^4", @@ -1804,6 +1804,16 @@ "integrity": "sha1-zyVVTKBQ3EmuZla0HeQiWJidy84=", "dev": true }, + "fs-extra": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-7.0.1.tgz", + "integrity": "sha512-YJDaCJZEnBmcbw13fvdAM9AwNOJwOzrE4pqMqBq5nFiEqXUqHwlK4B+3pUw6JNvfSPtX05xFHtYy/1ni01eGCw==", + "requires": { + "graceful-fs": "^4.1.2", + "jsonfile": "^4.0.0", + "universalify": "^0.1.0" + } + }, "fs.realpath": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz", @@ -1894,7 +1904,7 @@ }, "load-json-file": { "version": "1.1.0", - "resolved": "https://registry.npmjs.org/load-json-file/-/load-json-file-1.1.0.tgz", + "resolved": "http://registry.npmjs.org/load-json-file/-/load-json-file-1.1.0.tgz", "integrity": "sha1-lWkFcI1YtLq0wiYbBPWfMcmTdMA=", "dev": true, "requires": { @@ -1931,7 +1941,7 @@ }, "minimist": { "version": "1.2.0", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true }, @@ -2329,7 +2339,7 @@ }, "is-obj": { "version": "1.0.1", - "resolved": "https://registry.npmjs.org/is-obj/-/is-obj-1.0.1.tgz", + "resolved": "http://registry.npmjs.org/is-obj/-/is-obj-1.0.1.tgz", "integrity": "sha1-PkcprB9f3gJc19g6iW2rn09n2w8=", "dev": true }, @@ -2553,6 +2563,14 @@ "integrity": "sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=", "dev": true }, + "jsonfile": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-4.0.0.tgz", + "integrity": "sha1-h3Gq4HmbZAdrdmQPygWPnBDjPss=", + "requires": { + "graceful-fs": "^4.1.6" + } + }, "jsonparse": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/jsonparse/-/jsonparse-1.3.1.tgz", @@ -2777,7 +2795,7 @@ }, "minimist": { "version": "1.2.0", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true }, @@ -2862,7 +2880,7 @@ }, "minimist": { "version": "0.0.10", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.10.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-0.0.10.tgz", "integrity": "sha1-3j+YVD2/lggr5IrRoMfNqDYwHc8=" }, "minimist-options": { @@ -2895,7 +2913,7 @@ }, "mkdirp": { "version": "0.5.1", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", + "resolved": "http://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "requires": { "minimist": "0.0.8" @@ -2903,7 +2921,7 @@ "dependencies": { "minimist": { "version": "0.0.8", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=" } } @@ -4429,7 +4447,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -4791,7 +4809,7 @@ "dependencies": { "minimist": { "version": "1.2.0", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true } @@ -5067,7 +5085,7 @@ }, "through": { "version": "2.3.8", - "resolved": "https://registry.npmjs.org/through/-/through-2.3.8.tgz", + "resolved": "http://registry.npmjs.org/through/-/through-2.3.8.tgz", "integrity": "sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU=", "dev": true }, @@ -5262,7 +5280,7 @@ }, "strip-ansi": { "version": "3.0.1", - "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "resolved": "http://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "dev": true, "requires": { @@ -5277,6 +5295,11 @@ "integrity": "sha1-sxxa6CVIRKOoKBVBzisEuGWnNP8=", "dev": true }, + "universalify": { + "version": "0.1.2", + "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", + "integrity": "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==" + }, "uri-js": { "version": "4.2.2", "resolved": "https://registry.npmjs.org/uri-js/-/uri-js-4.2.2.tgz", @@ -5343,7 +5366,7 @@ }, "wrap-ansi": { "version": "2.1.0", - "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", + "resolved": "http://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", "integrity": "sha1-2Pw9KE3QV5T+hJc8rs3Rz4JP3YU=", "requires": { "string-width": "^1.0.1", @@ -5375,7 +5398,7 @@ }, "strip-ansi": { "version": "3.0.1", - "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "resolved": "http://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "requires": { "ansi-regex": "^2.0.0" diff --git a/package.json b/package.json index 14d6cafe3..6d4f2e964 100644 --- a/package.json +++ b/package.json @@ -74,6 +74,7 @@ "find-cache-dir": "^2.0.0", "find-up": "^3.0.0", "foreground-child": "^1.5.6", + "fs-extra": "^7.0.1", "istanbul-lib-coverage": "^2.0.4", "istanbul-lib-hook": "^2.0.4", "istanbul-lib-instrument": "^3.1.1", diff --git a/test/fixtures/cli/subdir/input-dir/exclude-me/index.js b/test/fixtures/cli/subdir/input-dir/exclude-me/index.js new file mode 100644 index 000000000..239a04e74 --- /dev/null +++ b/test/fixtures/cli/subdir/input-dir/exclude-me/index.js @@ -0,0 +1,2 @@ +'use strict'; +console.log('Hello, World!') diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 0af8bca6f..9d301f9b3 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -589,17 +589,17 @@ describe('the nyc cli', function () { }) it('works in directories without a package.json', function (done) { - var args = [bin, 'instrument', './input-dir', './output-dir'] + const args = [bin, 'instrument', './input-dir', './output-dir'] - var subdir = path.resolve(fixturesCLI, 'subdir') - var proc = spawn(process.execPath, args, { + const subdir = path.resolve(fixturesCLI, 'subdir') + const proc = spawn(process.execPath, args, { cwd: subdir, env: env }) proc.on('exit', function (code) { code.should.equal(0) - var target = path.resolve(subdir, 'output-dir', 'index.js') + const target = path.resolve(subdir, 'output-dir', 'index.js') fs.readFileSync(target, 'utf8') .should.match(/console.log\('Hello, World!'\)/) done() @@ -607,16 +607,10 @@ describe('the nyc cli', function () { }) it('can be configured to exit on error', function (done) { - var args = [ - bin, - 'instrument', - '--exit-on-error', - './input-dir', - './output-dir' - ] + const args = [bin, 'instrument', '--exit-on-error', './input-dir', './output-dir'] - var subdir = path.resolve(fixturesCLI, 'subdir') - var proc = spawn(process.execPath, args, { + const subdir = path.resolve(fixturesCLI, 'subdir') + const proc = spawn(process.execPath, args, { cwd: subdir, env: env }) @@ -628,16 +622,16 @@ describe('the nyc cli', function () { }) it('allows a single file to be instrumented', function (done) { - var args = [bin, 'instrument', './half-covered.js', './output'] + const args = [bin, 'instrument', './half-covered.js', './output'] - var proc = spawn(process.execPath, args, { + const proc = spawn(process.execPath, args, { cwd: fixturesCLI, env: env }) proc.on('close', function (code) { code.should.equal(0) - var files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.equal(1) files.should.include('half-covered.js') done() @@ -645,40 +639,62 @@ describe('the nyc cli', function () { }) it('allows a directory of files to be instrumented', function (done) { - var args = [bin, 'instrument', './', './output'] + const args = [bin, 'instrument', './nyc-config-js', './output'] - var proc = spawn(process.execPath, args, { + const proc = spawn(process.execPath, args, { cwd: fixturesCLI, env: env }) proc.on('close', function (code) { code.should.equal(0) - var files = fs.readdirSync(path.resolve(fixturesCLI, './output')) - files.should.include('env.js') - files.should.include('es6.js') + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.should.include('index.js') + files.should.include('ignore.js') + files.should.include('package.json') + files.should.include('node_modules') done() }) }) it('allows a sub-directory of files to be instrumented', function (done) { - var args = [bin, 'instrument', './subdir/input-dir', './output'] + const args = [bin, 'instrument', './subdir/input-dir', './output'] - var proc = spawn(process.execPath, args, { + const proc = spawn(process.execPath, args, { cwd: fixturesCLI, env: env }) proc.on('close', function (code) { code.should.equal(0) - var files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.should.include('index.js') done() }) }) + it('prevents writing output to a sub-dir of the input dir', function (done) { + const args = [bin, 'instrument', './', './output'] + + 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.match(/Cannot copy .* to a subdirectory of itself/) + done() + }) + }) + it('allows a subdirectory to be excluded', function (done) { - const args = [bin, 'instrument', '--exclude', '**/subdir/**', './', './output'] + const args = [bin, 'instrument', '--exclude', '**/exclude-me/**', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -687,17 +703,18 @@ describe('the nyc cli', function () { proc.on('close', function (code) { code.should.equal(0) - const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) - subdirExists.should.equal(false) const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.not.equal(0) - files.should.not.include('subdir') + files.should.include('exclude-me') + const target = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') + fs.readFileSync(target, 'utf8') + .should.not.match(/_cov/) done() }) }) it('allows a file to be excluded', function (done) { - const args = [bin, 'instrument', '--exclude', 'subdir/input-dir/bad.js', './', './output'] + const args = [bin, 'instrument', '--exclude', 'exclude-me/index.js', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -706,17 +723,18 @@ describe('the nyc cli', function () { 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/subdir/input-dir')) - files.should.include('index.js') - files.should.not.include('bad.js') + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.length.should.not.equal(0) + files.should.include('exclude-me') + const target = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') + fs.readFileSync(target, 'utf8') + .should.not.match(/_cov/) done() }) }) it('allows specifying a single sub-directory to be included', function (done) { - const args = [bin, 'instrument', '--include', '**/subdir/input-dir/bad.js', './', './output'] + const args = [bin, 'instrument', '--include', '**/exclude-me/**', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -725,19 +743,21 @@ describe('the nyc cli', function () { proc.on('close', function (code) { code.should.equal(0) - let files = fs.readdirSync(path.resolve(fixturesCLI, './output')) - files.length.should.equal(1) - const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) - subdirExists.should.equal(true) - files = fs.readdirSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) - files.should.include('bad.js') - files.should.not.include('index.js') + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.length.should.not.equal(0) + files.should.include('exclude-me') + const instrumented = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') + fs.readFileSync(instrumented, 'utf8') + .should.match(/_cov/) + const uninstrumented = path.resolve(fixturesCLI, 'output', 'index.js') + fs.readFileSync(uninstrumented, 'utf8') + .should.not.match(/_cov/) done() }) }) it('allows a file to be excluded from an included directory', function (done) { - const args = [bin, 'instrument', '--exclude', '**/bad.js', '--include', '**/subdir/input-dir/**', './', './output'] + const args = [bin, 'instrument', '--exclude', '**/index.js', '--include', '**/exclude-me/**', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -746,11 +766,15 @@ describe('the nyc cli', function () { 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/subdir/input-dir')) - files.should.include('index.js') - files.should.not.include('bad.js') + const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.length.should.not.equal(0) + files.should.include('exclude-me') + const instrumented = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') + fs.readFileSync(instrumented, 'utf8') + .should.not.match(/_cov/) + const uninstrumented = path.resolve(fixturesCLI, 'output', 'index.js') + fs.readFileSync(uninstrumented, 'utf8') + .should.not.match(/_cov/) done() }) }) @@ -808,7 +832,7 @@ describe('the nyc cli', function () { }) it('cleans the output directory if `--delete` is specified', function (done) { - const args = [bin, 'instrument', '--delete', 'true', './', './output'] + const args = [bin, 'instrument', '--delete', 'true', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -817,16 +841,17 @@ describe('the nyc cli', function () { proc.on('close', function (code) { code.should.equal(0) - const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) + const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output')) subdirExists.should.equal(true) const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.should.not.include('removed-by-clean') + files.should.include('exclude-me') done() }) }) it('does not clean the output directory by default', function (done) { - const args = [bin, 'instrument', './', './output'] + const args = [bin, 'instrument', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -835,7 +860,7 @@ describe('the nyc cli', function () { proc.on('close', function (code) { code.should.equal(0) - const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output/subdir/input-dir')) + const subdirExists = fs.existsSync(path.resolve(fixturesCLI, './output')) subdirExists.should.equal(true) const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.should.include('removed-by-clean') From bd25a383fe4283bfd1a4c20a3e7063af2f2fa66b Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Wed, 20 Mar 2019 16:05:13 +1100 Subject: [PATCH 03/11] Use `var cov_` rather than `_cov` in tests --- test/nyc-integration.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 9d301f9b3..6f649b0ac 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -708,7 +708,7 @@ describe('the nyc cli', function () { files.should.include('exclude-me') const target = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') fs.readFileSync(target, 'utf8') - .should.not.match(/_cov/) + .should.not.match(/var cov_/) done() }) }) @@ -728,7 +728,7 @@ describe('the nyc cli', function () { files.should.include('exclude-me') const target = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') fs.readFileSync(target, 'utf8') - .should.not.match(/_cov/) + .should.not.match(/var cov_/) done() }) }) @@ -748,10 +748,10 @@ describe('the nyc cli', function () { files.should.include('exclude-me') const instrumented = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') fs.readFileSync(instrumented, 'utf8') - .should.match(/_cov/) + .should.match(/var cov_/) const uninstrumented = path.resolve(fixturesCLI, 'output', 'index.js') fs.readFileSync(uninstrumented, 'utf8') - .should.not.match(/_cov/) + .should.not.match(/var cov_/) done() }) }) @@ -771,10 +771,10 @@ describe('the nyc cli', function () { files.should.include('exclude-me') const instrumented = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') fs.readFileSync(instrumented, 'utf8') - .should.not.match(/_cov/) + .should.not.match(/var cov_/) const uninstrumented = path.resolve(fixturesCLI, 'output', 'index.js') fs.readFileSync(uninstrumented, 'utf8') - .should.not.match(/_cov/) + .should.not.match(/var cov_/) done() }) }) From 02bc210877d64b0d91bd517ff9e99e5fcc5725ca Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Fri, 22 Mar 2019 14:13:54 +1100 Subject: [PATCH 04/11] nyc instrument now copies all files, with permissions, from `input` to `output` dir It ignores files in the `.git` folder and copies empty directories Unfortunately I had to keep `fs-extra`, although this should be easy to drop when the project moves to node 8+ when `fs.copyFileSync` is made available. We need to use `fs.chmodSync` to set the mode as any `write` mode flags get filtered when trying to set them through `fs.writeFileSync`. What's missing? * No check to see where the working directory is relative to the project root directory * Tests confirming includes/excludes can be set in a config file or stanza and are observed by the instrument command * Tests confirming file permissions match between files in the input and output directories * Tests confirming we can't instrument outside of the project root directory --- index.js | 27 +++++++++++++++++++++++---- package.json | 2 +- test/nyc-integration.js | 20 -------------------- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/index.js b/index.js index f01bb6381..c14fdf141 100755 --- a/index.js +++ b/index.js @@ -6,7 +6,9 @@ const arrify = require('arrify') const cachingTransform = require('caching-transform') const util = require('util') const findCacheDir = require('find-cache-dir') -const fs = require('fs-extra') +const { copySync } = require('fs-extra') +const fs = require('fs') +const glob = require('glob') const Hash = require('./lib/hash') const libCoverage = require('istanbul-lib-coverage') const libHook = require('istanbul-lib-hook') @@ -187,9 +189,11 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { const outCode = transform(inCode, { filename: inFile }) if (output) { + const mode = fs.statSync(inFile).mode const outFile = path.resolve(output, filename) mkdirp.sync(path.dirname(outFile)) - fs.writeFileSync(outFile, outCode, 'utf-8') + fs.writeFileSync(outFile, outCode) + fs.chmodSync(outFile, mode) } else { console.log(outCode) } @@ -200,12 +204,27 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { try { const stats = fs.lstatSync(input) if (stats.isDirectory()) { - if (output) { - fs.copySync(input, output) + const globOptions = { dot: true, mark: true, ignore: ['**/.git', '**/.git/**/*'] } + const outputPaths = (output) ? glob.sync(`${path.resolve(input)}/**/*`, globOptions) : [] + + const partition = (universal, subsetFilter) => { + return universal.reduce(([a, aDash], member) => { + return subsetFilter(member) ? [[...a, member], aDash] : [a, [...aDash, member]] + }, [[], []]) } + const [dirs, files] = partition(outputPaths, file => file.endsWith('/')) + inputDir = input this.walkAllFiles(input, visitor) + + if (files.length || dirs.length) { + dirs.map(file => path.resolve(output, path.relative(input, file))) + .forEach(dir => mkdirp.sync(dir)) + + files.map(file => path.relative(input, file)) + .forEach(file => { copySync(path.join(input, file), path.join(output, file), { overwrite: false }) }) + } } else { visitor(input) } diff --git a/package.json b/package.json index 6d4f2e964..3204a6ab1 100644 --- a/package.json +++ b/package.json @@ -75,6 +75,7 @@ "find-up": "^3.0.0", "foreground-child": "^1.5.6", "fs-extra": "^7.0.1", + "glob": "^7.1.3", "istanbul-lib-coverage": "^2.0.4", "istanbul-lib-hook": "^2.0.4", "istanbul-lib-instrument": "^3.1.1", @@ -96,7 +97,6 @@ "any-path": "^1.3.0", "chai": "^4.2.0", "coveralls": "^3.0.3", - "glob": "^7.1.3", "is-windows": "^1.0.2", "lodash": "^4.17.11", "newline-regex": "^0.2.1", diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 6f649b0ac..153c97167 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -673,26 +673,6 @@ describe('the nyc cli', function () { }) }) - it('prevents writing output to a sub-dir of the input dir', function (done) { - const args = [bin, 'instrument', './', './output'] - - 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.match(/Cannot copy .* to a subdirectory of itself/) - done() - }) - }) - it('allows a subdirectory to be excluded', function (done) { const args = [bin, 'instrument', '--exclude', '**/exclude-me/**', './subdir/input-dir', './output'] From b2eb67009d78de64ba9db975a6ff5d1e78ce1b96 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Mon, 25 Mar 2019 11:01:38 +1100 Subject: [PATCH 05/11] Update tests, prevent in place instrumentation and instrumentation outside of cwd Tests include * using a `.nycrc` file to specify excludes * checking that file mode is preserved after instrumentation, * copying all uninstrumented files across to the output directory * Ensure that we can't instrument in place, overwriting the existing source * Ensure that we can't instrument source from outside the project root directory --- index.js | 2 +- lib/commands/instrument.js | 32 +++++++++----- test/fixtures/cli/.instrument-nycrc | 5 +++ test/nyc-integration.js | 68 ++++++++++++++++++++++++++--- 4 files changed, 91 insertions(+), 16 deletions(-) create mode 100644 test/fixtures/cli/.instrument-nycrc diff --git a/index.js b/index.js index c14fdf141..096b9a07a 100755 --- a/index.js +++ b/index.js @@ -213,7 +213,7 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { }, [[], []]) } - const [dirs, files] = partition(outputPaths, file => file.endsWith('/')) + const [dirs, files] = partition(outputPaths, filename => filename.endsWith('/')) inputDir = input this.walkAllFiles(input, visitor) diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index e5ebdc8fd..3c0fe9d3b 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -73,6 +73,28 @@ exports.builder = function (yargs) { } 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 (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 + } + + 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) + } + } + // If instrument is set to false enable a noop instrumenter. argv.instrumenter = (argv.instrument) ? './lib/instrumenters/istanbul' @@ -93,16 +115,6 @@ exports.handler = function (argv) { exitOnError: argv.exitOnError }) - 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) - } - } - nyc.instrumentAllFiles(argv.input, argv.output, err => { if (err) { console.error(err.message) diff --git a/test/fixtures/cli/.instrument-nycrc b/test/fixtures/cli/.instrument-nycrc new file mode 100644 index 000000000..4063df5c0 --- /dev/null +++ b/test/fixtures/cli/.instrument-nycrc @@ -0,0 +1,5 @@ +{ + "exclude": [ + "**/exclude-me/**" + ] +} diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 153c97167..f579400c2 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -624,6 +624,11 @@ describe('the nyc cli', function () { it('allows a single file to be instrumented', function (done) { const args = [bin, 'instrument', './half-covered.js', './output'] + const inputPath = path.resolve(fixturesCLI, './half-covered.js') + const inputMode = fs.statSync(inputPath).mode & 0o7777 + const newMode = 0o775 + fs.chmodSync(inputPath, newMode) + const proc = spawn(process.execPath, args, { cwd: fixturesCLI, env: env @@ -634,6 +639,13 @@ describe('the nyc cli', function () { const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.equal(1) files.should.include('half-covered.js') + + const outputPath = path.resolve(fixturesCLI, 'output', 'half-covered.js') + const outputMode = fs.statSync(outputPath).mode & 0o7777 + outputMode.should.equal(newMode) + + fs.chmodSync(inputPath, inputMode) + done() }) }) @@ -673,8 +685,8 @@ describe('the nyc cli', function () { }) }) - it('allows a subdirectory to be excluded', function (done) { - const args = [bin, 'instrument', '--exclude', '**/exclude-me/**', './subdir/input-dir', './output'] + it('allows a subdirectory to be excluded via nycrc file', function (done) { + const args = [bin, 'instrument', '--nycrc-path', './.instrument-nycrc', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -686,9 +698,15 @@ describe('the nyc cli', function () { const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.not.equal(0) files.should.include('exclude-me') - const target = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') - fs.readFileSync(target, 'utf8') + files.should.include('node_modules') + files.should.include('index.js') + files.should.include('bad.js') + const excludeTarget = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') + fs.readFileSync(excludeTarget, 'utf8') .should.not.match(/var cov_/) + const includeTarget = path.resolve(fixturesCLI, 'output', 'index.js') + fs.readFileSync(includeTarget, 'utf8') + .should.match(/var cov_/) done() }) }) @@ -759,6 +777,46 @@ describe('the nyc cli', function () { }) }) + it('aborts if trying to write files in place', 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: cannot instrument files in place') + done() + }) + }) + + it('aborts if trying to instrument files from outside the project root 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: cannot instrument files outside of project root directory') + done() + }) + }) + describe('es-modules', function () { afterEach(function () { rimraf.sync(path.resolve(fixturesCLI, './output')) @@ -849,7 +907,7 @@ describe('the nyc cli', function () { }) it('aborts if trying to clean process.cwd()', function (done) { - const args = [bin, 'instrument', '--delete', './', './'] + const args = [bin, 'instrument', '--delete', './src', './'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, From 995aa1a034f257fc883a0967d24e92bff09e3188 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Mon, 25 Mar 2019 11:10:49 +1100 Subject: [PATCH 06/11] Include missing test files A file `node_modules` file involved in the tests was being git ignored --- test/fixtures/cli/subdir/.gitignore | 1 + test/fixtures/cli/subdir/input-dir/node_modules/index.js | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 test/fixtures/cli/subdir/input-dir/node_modules/index.js diff --git a/test/fixtures/cli/subdir/.gitignore b/test/fixtures/cli/subdir/.gitignore index 21ba3bc07..a5cb0329a 100644 --- a/test/fixtures/cli/subdir/.gitignore +++ b/test/fixtures/cli/subdir/.gitignore @@ -1 +1,2 @@ output-dir +!node_modules diff --git a/test/fixtures/cli/subdir/input-dir/node_modules/index.js b/test/fixtures/cli/subdir/input-dir/node_modules/index.js new file mode 100644 index 000000000..239a04e74 --- /dev/null +++ b/test/fixtures/cli/subdir/input-dir/node_modules/index.js @@ -0,0 +1,2 @@ +'use strict'; +console.log('Hello, World!') From 5a289419bbab55751cd97ea8746d8dbe8da4d7d8 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Mon, 25 Mar 2019 12:22:49 +1100 Subject: [PATCH 07/11] Ignore file permissions test if running on Windows --- test/nyc-integration.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/nyc-integration.js b/test/nyc-integration.js index f579400c2..a0ce4e58a 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -14,6 +14,7 @@ const rimraf = require('rimraf') const makeDir = require('make-dir') const spawn = require('child_process').spawn const si = require('strip-indent') +const os = require('os') require('chai').should() require('tap').mochaGlobals() @@ -627,7 +628,9 @@ describe('the nyc cli', function () { const inputPath = path.resolve(fixturesCLI, './half-covered.js') const inputMode = fs.statSync(inputPath).mode & 0o7777 const newMode = 0o775 - fs.chmodSync(inputPath, newMode) + if (os.platform !== 'win32') { + fs.chmodSync(inputPath, newMode) + } const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -640,11 +643,13 @@ describe('the nyc cli', function () { files.length.should.equal(1) files.should.include('half-covered.js') - const outputPath = path.resolve(fixturesCLI, 'output', 'half-covered.js') - const outputMode = fs.statSync(outputPath).mode & 0o7777 - outputMode.should.equal(newMode) + if (os.platform !== 'win32') { + const outputPath = path.resolve(fixturesCLI, 'output', 'half-covered.js') + const outputMode = fs.statSync(outputPath).mode & 0o7777 + outputMode.should.equal(newMode) - fs.chmodSync(inputPath, inputMode) + fs.chmodSync(inputPath, inputMode) + } done() }) From d22c585b4ed384b53e971854ee6a586eefc3fca5 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Mon, 25 Mar 2019 12:33:06 +1100 Subject: [PATCH 08/11] Use process.platform instead of os.platform --- test/nyc-integration.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/nyc-integration.js b/test/nyc-integration.js index a0ce4e58a..2266e19b1 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -14,7 +14,6 @@ const rimraf = require('rimraf') const makeDir = require('make-dir') const spawn = require('child_process').spawn const si = require('strip-indent') -const os = require('os') require('chai').should() require('tap').mochaGlobals() @@ -628,7 +627,7 @@ describe('the nyc cli', function () { const inputPath = path.resolve(fixturesCLI, './half-covered.js') const inputMode = fs.statSync(inputPath).mode & 0o7777 const newMode = 0o775 - if (os.platform !== 'win32') { + if (process.platform !== 'win32') { fs.chmodSync(inputPath, newMode) } @@ -643,7 +642,7 @@ describe('the nyc cli', function () { files.length.should.equal(1) files.should.include('half-covered.js') - if (os.platform !== 'win32') { + if (process.platform !== 'win32') { const outputPath = path.resolve(fixturesCLI, 'output', 'half-covered.js') const outputMode = fs.statSync(outputPath).mode & 0o7777 outputMode.should.equal(newMode) From 1fbc20411ae3bf923c406ab3173656bc04fea2b5 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Tue, 2 Apr 2019 11:19:57 +1100 Subject: [PATCH 09/11] Clean up copy remaining files for nyc instrument command In the last attempt at this I was jumping through hoops trying to maintain empty folders and structure etc. Turns out none of that is necessary. So this change runs the instrumentation process over the included files, then copies across anything remaining excluding freshly created output directory. --- index.js | 21 ++++----------------- test/nyc-integration.js | 2 +- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/index.js b/index.js index 62191e178..25685e2f3 100755 --- a/index.js +++ b/index.js @@ -195,26 +195,13 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { try { const stats = fs.lstatSync(input) if (stats.isDirectory()) { - const globOptions = { dot: true, mark: true, ignore: ['**/.git', '**/.git/**/*'] } - const outputPaths = (output) ? glob.sync(`${path.resolve(input)}/**/*`, globOptions) : [] - - const partition = (universal, subsetFilter) => { - return universal.reduce(([a, aDash], member) => { - return subsetFilter(member) ? [[...a, member], aDash] : [a, [...aDash, member]] - }, [[], []]) - } - - const [dirs, files] = partition(outputPaths, filename => filename.endsWith('/')) - inputDir = input this.walkAllFiles(input, visitor) - if (files.length || dirs.length) { - dirs.map(file => path.resolve(output, path.relative(input, file))) - .forEach(dir => mkdirp.sync(dir)) - - files.map(file => path.relative(input, file)) - .forEach(file => { copySync(path.join(input, file), path.join(output, file), { overwrite: false }) }) + if (output) { + const globOptions = { dot: true, mark: true, ignore: ['**/.git', '**/.git/**/*', `${output}/**/*`] } + glob.sync(`${path.resolve(input)}/*`, globOptions) + .forEach(src => copySync(src, path.join(output, path.relative(input, src)), { overwrite: false })) } } else { visitor(input) diff --git a/test/nyc-integration.js b/test/nyc-integration.js index c324175cb..fc0931028 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -709,7 +709,7 @@ describe('the nyc cli', function () { }) }) - it('allows a subdirectory to be excluded via nycrc file', function (done) { + it('allows a subdirectory to be excluded via .nycrc file', function (done) { const args = [bin, 'instrument', '--nycrc-path', './.instrument-nycrc', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { From e1a1b9493ed9f4e1e9a6e6172b9569138a8d3183 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Tue, 2 Apr 2019 14:34:08 +1100 Subject: [PATCH 10/11] Make sure we can instrument the root directory And a test to prove it! --- index.js | 2 +- test/nyc-integration.js | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 25685e2f3..ff763fcad 100755 --- a/index.js +++ b/index.js @@ -199,7 +199,7 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { this.walkAllFiles(input, visitor) if (output) { - const globOptions = { dot: true, mark: true, ignore: ['**/.git', '**/.git/**/*', `${output}/**/*`] } + const globOptions = { dot: true, ignore: ['**/.git', `**/${path.basename(output)}`] } glob.sync(`${path.resolve(input)}/*`, globOptions) .forEach(src => copySync(src, path.join(output, path.relative(input, src)), { overwrite: false })) } diff --git a/test/nyc-integration.js b/test/nyc-integration.js index fc0931028..6cbb688ba 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -693,6 +693,23 @@ describe('the nyc cli', function () { }) }) + it('can instrument the root directory', 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 files = fs.readdirSync(path.resolve(fixturesCLI, './output')) + files.should.include('args.js') + files.should.include('subdir') + done() + }) + }) + it('allows a sub-directory of files to be instrumented', function (done) { const args = [bin, 'instrument', './subdir/input-dir', './output'] From 89033659f8c7f7d86468cafc591c7b693f4f0b4c Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Thu, 4 Apr 2019 11:00:49 +1100 Subject: [PATCH 11/11] Split PR such that this only handles `nyc instrument --include --exclude` Previously this was adding `nyc instrument` full copy of `src` to `dst` as well as `include`/`exclude` The full copy work was proving troublesome so it's been moved to it's own branch/PR This commit removes the full copy, removes now unused dependencies, and updates tests accordingly --- index.js | 8 ---- package-lock.json | 23 ----------- package.json | 3 +- .../subdir/input-dir/include-me/exclude-me.js | 2 + .../subdir/input-dir/include-me/include-me.js | 2 + test/nyc-integration.js | 40 ++++++++----------- 6 files changed, 21 insertions(+), 57 deletions(-) create mode 100644 test/fixtures/cli/subdir/input-dir/include-me/exclude-me.js create mode 100644 test/fixtures/cli/subdir/input-dir/include-me/include-me.js diff --git a/index.js b/index.js index e75e10e7c..00e6cc81f 100755 --- a/index.js +++ b/index.js @@ -6,9 +6,7 @@ const arrify = require('arrify') const cachingTransform = require('caching-transform') const util = require('util') const findCacheDir = require('find-cache-dir') -const { copySync } = require('fs-extra') const fs = require('fs') -const glob = require('glob') const Hash = require('./lib/hash') const libCoverage = require('istanbul-lib-coverage') const libHook = require('istanbul-lib-hook') @@ -198,12 +196,6 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { if (stats.isDirectory()) { inputDir = input this.walkAllFiles(input, visitor) - - if (output) { - const globOptions = { dot: true, ignore: ['**/.git', `**/${path.basename(output)}`] } - glob.sync(`${path.resolve(input)}/*`, globOptions) - .forEach(src => copySync(src, path.join(output, path.relative(input, src)), { overwrite: false })) - } } else { visitor(input) } diff --git a/package-lock.json b/package-lock.json index da5b07822..dd60e2f9f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1736,16 +1736,6 @@ "integrity": "sha1-zyVVTKBQ3EmuZla0HeQiWJidy84=", "dev": true }, - "fs-extra": { - "version": "7.0.1", - "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-7.0.1.tgz", - "integrity": "sha512-YJDaCJZEnBmcbw13fvdAM9AwNOJwOzrE4pqMqBq5nFiEqXUqHwlK4B+3pUw6JNvfSPtX05xFHtYy/1ni01eGCw==", - "requires": { - "graceful-fs": "^4.1.2", - "jsonfile": "^4.0.0", - "universalify": "^0.1.0" - } - }, "fs.realpath": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz", @@ -2514,14 +2504,6 @@ "integrity": "sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=", "dev": true }, - "jsonfile": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-4.0.0.tgz", - "integrity": "sha1-h3Gq4HmbZAdrdmQPygWPnBDjPss=", - "requires": { - "graceful-fs": "^4.1.6" - } - }, "jsonparse": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/jsonparse/-/jsonparse-1.3.1.tgz", @@ -5297,11 +5279,6 @@ "integrity": "sha1-sxxa6CVIRKOoKBVBzisEuGWnNP8=", "dev": true }, - "universalify": { - "version": "0.1.2", - "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", - "integrity": "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==" - }, "uri-js": { "version": "4.2.2", "resolved": "https://registry.npmjs.org/uri-js/-/uri-js-4.2.2.tgz", diff --git a/package.json b/package.json index 30d640158..e07cf8d66 100644 --- a/package.json +++ b/package.json @@ -74,8 +74,6 @@ "find-cache-dir": "^2.1.0", "find-up": "^3.0.0", "foreground-child": "^1.5.6", - "fs-extra": "^7.0.1", - "glob": "^7.1.3", "istanbul-lib-coverage": "^2.0.4", "istanbul-lib-hook": "^2.0.5", "istanbul-lib-instrument": "^3.1.2", @@ -97,6 +95,7 @@ "any-path": "^1.3.0", "chai": "^4.2.0", "coveralls": "^3.0.3", + "glob": "^7.1.3", "is-windows": "^1.0.2", "lodash": "^4.17.11", "newline-regex": "^0.2.1", diff --git a/test/fixtures/cli/subdir/input-dir/include-me/exclude-me.js b/test/fixtures/cli/subdir/input-dir/include-me/exclude-me.js new file mode 100644 index 000000000..239a04e74 --- /dev/null +++ b/test/fixtures/cli/subdir/input-dir/include-me/exclude-me.js @@ -0,0 +1,2 @@ +'use strict'; +console.log('Hello, World!') diff --git a/test/fixtures/cli/subdir/input-dir/include-me/include-me.js b/test/fixtures/cli/subdir/input-dir/include-me/include-me.js new file mode 100644 index 000000000..239a04e74 --- /dev/null +++ b/test/fixtures/cli/subdir/input-dir/include-me/include-me.js @@ -0,0 +1,2 @@ +'use strict'; +console.log('Hello, World!') diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 62a5ea088..999e5102b 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -687,8 +687,8 @@ describe('the nyc cli', function () { const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.should.include('index.js') files.should.include('ignore.js') - files.should.include('package.json') - files.should.include('node_modules') + files.should.not.include('package.json') + files.should.not.include('node_modules') done() }) }) @@ -738,13 +738,10 @@ describe('the nyc cli', function () { code.should.equal(0) const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.not.equal(0) - files.should.include('exclude-me') - files.should.include('node_modules') + files.should.not.include('exclude-me') + files.should.not.include('node_modules') files.should.include('index.js') files.should.include('bad.js') - const excludeTarget = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') - fs.readFileSync(excludeTarget, 'utf8') - .should.not.match(/var cov_/) const includeTarget = path.resolve(fixturesCLI, 'output', 'index.js') fs.readFileSync(includeTarget, 'utf8') .should.match(/var cov_/) @@ -764,16 +761,13 @@ describe('the nyc cli', function () { code.should.equal(0) const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.not.equal(0) - files.should.include('exclude-me') - const target = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') - fs.readFileSync(target, 'utf8') - .should.not.match(/var cov_/) + files.should.not.include('exclude-me') done() }) }) it('allows specifying a single sub-directory to be included', function (done) { - const args = [bin, 'instrument', '--include', '**/exclude-me/**', './subdir/input-dir', './output'] + const args = [bin, 'instrument', '--include', '**/include-me/**', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -784,19 +778,16 @@ describe('the nyc cli', function () { code.should.equal(0) const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.not.equal(0) - files.should.include('exclude-me') - const instrumented = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') + files.should.include('include-me') + const instrumented = path.resolve(fixturesCLI, 'output', 'include-me', 'include-me.js') fs.readFileSync(instrumented, 'utf8') .should.match(/var cov_/) - const uninstrumented = path.resolve(fixturesCLI, 'output', 'index.js') - fs.readFileSync(uninstrumented, 'utf8') - .should.not.match(/var cov_/) done() }) }) it('allows a file to be excluded from an included directory', function (done) { - const args = [bin, 'instrument', '--exclude', '**/index.js', '--include', '**/exclude-me/**', './subdir/input-dir', './output'] + const args = [bin, 'instrument', '--exclude', '**/exclude-me.js', '--include', '**/include-me/**', './subdir/input-dir', './output'] const proc = spawn(process.execPath, args, { cwd: fixturesCLI, @@ -807,13 +798,14 @@ describe('the nyc cli', function () { code.should.equal(0) const files = fs.readdirSync(path.resolve(fixturesCLI, './output')) files.length.should.not.equal(0) - files.should.include('exclude-me') - const instrumented = path.resolve(fixturesCLI, 'output', 'exclude-me', 'index.js') + files.should.include('include-me') + const includeMeFiles = fs.readdirSync(path.resolve(fixturesCLI, 'output', 'include-me')) + includeMeFiles.length.should.not.equal(0) + includeMeFiles.should.include('include-me.js') + includeMeFiles.should.not.include('exclude-me.js') + const instrumented = path.resolve(fixturesCLI, 'output', 'include-me', 'include-me.js') fs.readFileSync(instrumented, 'utf8') - .should.not.match(/var cov_/) - const uninstrumented = path.resolve(fixturesCLI, 'output', 'index.js') - fs.readFileSync(uninstrumented, 'utf8') - .should.not.match(/var cov_/) + .should.match(/var cov_/) done() }) })