From cdffff83b78e35de404c868ffec6f614cf2cb248 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Wed, 27 Feb 2019 11:20:07 +1100 Subject: [PATCH 1/5] Enable '--cwd' option for nyc instrument command - Notes: Add instrument command support for the 'cwd' option Refactor nyc-integration instrument tests so tests with output folder specified aren't in the no output folder suite Refactor instrument.js and instrumentAllFiles command to use modern javascript Remove call to deprecated 'process.exit()' in instrument command Use file extension as an index to find the correct transform, rather than trying to match a transform name to the rightmost characters of a filename * There is another one of these in the code base but it's out of the scope of this PR BTW, I'm not going to widen the scope of this PR, this is it! - Assumptions: If you're instrumenting a single file cwd/a/b/d.js, and specifying an output directory cwd/e, the result will be cwd/e/d.js That we want to use file extensions for find transforms, the existing implementation would use the '.js' instrumenter for both '.js' and '.mjs' files, these changes assume that was wrong. That the filename in the covered file should be of the form './path/to/file.js' --- index.js | 50 ++++++++++++++++---------------------- lib/commands/instrument.js | 16 ++++++------ test/nyc-integration.js | 8 ++---- 3 files changed, 30 insertions(+), 44 deletions(-) diff --git a/index.js b/index.js index 31fec4bbd..cdea7ef71 100755 --- a/index.js +++ b/index.js @@ -191,48 +191,40 @@ NYC.prototype.addAllFiles = function () { } NYC.prototype.instrumentAllFiles = function (input, output, cb) { - var _this = this - var inputDir = '.' + path.sep - var visitor = function (filename) { - var ext - var transform - var inFile = path.resolve(inputDir, filename) - var code = fs.readFileSync(inFile, 'utf-8') - - for (ext in _this.transforms) { - if (filename.toLowerCase().substr(-ext.length) === ext) { - transform = _this.transforms[ext] - break - } - } - - if (transform) { - code = transform(code, { filename: inFile, relFile: inFile }) - } - - if (!output) { - console.log(code) + const visitor = (filename, singleFile = false) => { + const inputPath = path.resolve(this.cwd, input, filename) + const inCode = fs.readFileSync(inputPath, 'utf-8') + + const extname = path.extname(filename).toLowerCase() + const transform = this.transforms[extname] || false + const outCode = (transform) + ? transform(inCode, { filename: `./${path.relative(this.cwd, inputPath)}` }) + : inCode + + if (output) { + const subpath = singleFile || path.relative(input, path.resolve(input, filename)) + const outputPath = path.resolve(this.cwd, output, subpath) + mkdirp.sync(path.dirname(outputPath)) + fs.writeFileSync(outputPath, outCode, 'utf-8') } else { - var outFile = path.resolve(output, filename) - mkdirp.sync(path.dirname(outFile)) - fs.writeFileSync(outFile, code, 'utf-8') + console.log(outCode) } } this._loadAdditionalModules() try { - var stats = fs.lstatSync(input) + const inputPath = path.resolve(this.cwd, input) + const stats = fs.lstatSync(inputPath) if (stats.isDirectory()) { - inputDir = input - this.walkAllFiles(input, visitor) + this.walkAllFiles(inputPath, visitor) } else { - visitor(input) + visitor(inputPath, path.basename(inputPath)) } } catch (err) { return cb(err) } - cb() + return cb() } NYC.prototype.walkAllFiles = function (dir, visitor) { diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index 9e255a757..13b7e9760 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -62,12 +62,12 @@ exports.builder = function (yargs) { } exports.handler = function (argv) { - // if instrument is set to false, - // enable a noop instrumenter. - if (!argv.instrument) argv.instrumenter = './lib/instrumenters/noop' - else argv.instrumenter = './lib/instrumenters/istanbul' + // If instrument is set to false enable a noop instrumenter. + argv.instrumenter = (!argv.instrument) + ? './lib/instrumenters/noop' + : './lib/instrumenters/istanbul' - var nyc = new NYC({ + const nyc = new NYC({ instrumenter: argv.instrumenter, sourceMap: argv.sourceMap, produceSourceMap: argv.produceSourceMap, @@ -89,12 +89,10 @@ exports.handler = function (argv) { } } - nyc.instrumentAllFiles(argv.input, argv.output, function (err) { + nyc.instrumentAllFiles(argv.input, argv.output, err => { if (err) { console.error(err.message) - process.exit(1) - } else { - process.exit(0) + process.exitCode = 1 } }) } diff --git a/test/nyc-integration.js b/test/nyc-integration.js index a34bbdbc8..89a83f38d 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -581,7 +581,9 @@ describe('the nyc cli', function () { done() }) }) + }) + describe('output folder specified', function () { it('works in directories without a package.json', function (done) { var args = [bin, 'instrument', './input-dir', './output-dir'] @@ -620,12 +622,6 @@ describe('the nyc cli', function () { done() }) }) - }) - - 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'] From aaece80e4bb281c26622b6eb6045ca24b91cbc60 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Tue, 12 Mar 2019 15:51:33 +1100 Subject: [PATCH 2/5] Remove any left over `instrument --cwd` changes --- index.js | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/index.js b/index.js index cdea7ef71..3abd00a82 100755 --- a/index.js +++ b/index.js @@ -192,20 +192,18 @@ NYC.prototype.addAllFiles = function () { NYC.prototype.instrumentAllFiles = function (input, output, cb) { const visitor = (filename, singleFile = false) => { - const inputPath = path.resolve(this.cwd, input, filename) - const inCode = fs.readFileSync(inputPath, 'utf-8') + const inFile = path.resolve(input, filename) + const inCode = fs.readFileSync(inFile, 'utf-8') const extname = path.extname(filename).toLowerCase() - const transform = this.transforms[extname] || false - const outCode = (transform) - ? transform(inCode, { filename: `./${path.relative(this.cwd, inputPath)}` }) - : inCode + const transform = this.transforms[extname] || (code => code) + const outCode = transform(inCode, { filename: inFile }) if (output) { - const subpath = singleFile || path.relative(input, path.resolve(input, filename)) - const outputPath = path.resolve(this.cwd, output, subpath) - mkdirp.sync(path.dirname(outputPath)) - fs.writeFileSync(outputPath, outCode, 'utf-8') + const subPath = singleFile || path.relative(input, path.resolve(input, filename)) + const outFile = path.resolve(output, subPath) + mkdirp.sync(path.dirname(outFile)) + fs.writeFileSync(outFile, outCode, 'utf-8') } else { console.log(outCode) } @@ -214,12 +212,12 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { this._loadAdditionalModules() try { - const inputPath = path.resolve(this.cwd, input) - const stats = fs.lstatSync(inputPath) + const stats = fs.lstatSync(input) if (stats.isDirectory()) { - this.walkAllFiles(inputPath, visitor) + this.walkAllFiles(input, visitor) } else { - visitor(inputPath, path.basename(inputPath)) + const inputPath = path.resolve('.', input) + visitor(inputPath, path.basename(input)) } } catch (err) { return cb(err) From 0f4194bc95e56022260a144c02258627d5e4edab Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Tue, 12 Mar 2019 16:29:25 +1100 Subject: [PATCH 3/5] Add support for `--cwd` option to nyc instrument command This is intended to sit on top of the nyc es2015+ commit. It uses the cwd option and its defaults in nyc to make paths more consistent. --- index.js | 12 ++++++------ lib/commands/instrument.js | 6 ++++++ test/nyc-integration.js | 40 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 3abd00a82..e77248bb6 100755 --- a/index.js +++ b/index.js @@ -192,7 +192,7 @@ NYC.prototype.addAllFiles = function () { NYC.prototype.instrumentAllFiles = function (input, output, cb) { const visitor = (filename, singleFile = false) => { - const inFile = path.resolve(input, filename) + const inFile = path.resolve(this.cwd, input, filename) const inCode = fs.readFileSync(inFile, 'utf-8') const extname = path.extname(filename).toLowerCase() @@ -201,7 +201,7 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { if (output) { const subPath = singleFile || path.relative(input, path.resolve(input, filename)) - const outFile = path.resolve(output, subPath) + const outFile = path.resolve(this.cwd, output, subPath) mkdirp.sync(path.dirname(outFile)) fs.writeFileSync(outFile, outCode, 'utf-8') } else { @@ -212,12 +212,12 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { this._loadAdditionalModules() try { - const stats = fs.lstatSync(input) + const inputPath = path.resolve(this.cwd, input) + const stats = fs.lstatSync(inputPath) if (stats.isDirectory()) { - this.walkAllFiles(input, visitor) + this.walkAllFiles(inputPath, visitor) } else { - const inputPath = path.resolve('.', input) - visitor(inputPath, path.basename(input)) + visitor(inputPath, path.basename(inputPath)) } } catch (err) { return cb(err) diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index 13b7e9760..4b531d2c4 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -7,6 +7,7 @@ exports.command = 'instrument [output]' exports.describe = 'instruments a file or a directory tree and writes the instrumented code to the desired output location' exports.builder = function (yargs) { + const cwd = process.cwd() return yargs .option('require', { alias: 'i', @@ -28,6 +29,10 @@ exports.builder = function (yargs) { type: 'boolean', description: "should nyc's instrumenter produce source maps?" }) + .option('cwd', { + describe: 'working directory used when resolving paths', + default: cwd + }) .option('compact', { default: true, type: 'boolean', @@ -73,6 +78,7 @@ exports.handler = function (argv) { produceSourceMap: argv.produceSourceMap, extension: argv.extension, require: argv.require, + cwd: argv.cwd, compact: argv.compact, preserveComments: argv.preserveComments, esModules: argv.esModules, diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 89a83f38d..b965dcf48 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -584,6 +584,10 @@ describe('the nyc cli', function () { }) describe('output folder specified', function () { + afterEach(function () { + rimraf.sync(path.resolve(fixturesCLI, 'output')) + }) + it('works in directories without a package.json', function (done) { var args = [bin, 'instrument', './input-dir', './output-dir'] @@ -602,6 +606,42 @@ describe('the nyc cli', function () { }) }) + it('works with a different working directory', function (done) { + const subdir = path.resolve(fixturesCLI, 'subdir') + const args = [bin, 'instrument', '--cwd', subdir, './input-dir', './output-dir'] + + const proc = spawn(process.execPath, args, { + cwd: fixturesCLI, + env: env + }) + + proc.on('exit', code => { + code.should.equal(0) + const target = path.resolve(subdir, 'output-dir', 'index.js') + fs.readFileSync(target, 'utf8') + .should.match(/console.log\('Hello, World!'\)/) + done() + }) + }) + + it('works with a deep folder structure working directory', function (done) { + const subdir = path.resolve(fixturesCLI, 'subdir') + const args = [bin, 'instrument', '--cwd', subdir, '.', './output-dir'] + + const proc = spawn(process.execPath, args, { + cwd: fixturesCLI, + env: env + }) + + proc.on('exit', code => { + code.should.equal(0) + const target = path.resolve(subdir, 'output-dir', 'input-dir', 'index.js') + fs.readFileSync(target, 'utf8') + .should.match(/console.log\('Hello, World!'\)/) + done() + }) + }) + it('can be configured to exit on error', function (done) { var args = [ bin, From 23f875f782e0772ae6577142ea412ce7596913a5 Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Wed, 13 Mar 2019 11:40:25 +1100 Subject: [PATCH 4/5] Update PR to work with latest --- index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 10c1f4527..7b1577d98 100755 --- a/index.js +++ b/index.js @@ -212,10 +212,11 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { this._loadAdditionalModules() try { - const stats = fs.lstatSync(input) + const inputPath = path.resolve(this.cwd, input) + const stats = fs.lstatSync(inputPath) if (stats.isDirectory()) { - inputDir = input - this.walkAllFiles(input, visitor) + inputDir = inputPath + this.walkAllFiles(inputPath, visitor) } else { visitor(input) } From c454b56d70d88b3221bdd218607c60f02328f69a Mon Sep 17 00:00:00 2001 From: Andrew Finlay Date: Tue, 19 Mar 2019 12:16:01 +1100 Subject: [PATCH 5/5] Address review issues All changed, so all this does now is propagate `cwd` so that it can influence `testExclude` and `require` path settings. --- index.js | 11 +++++------ lib/commands/instrument.js | 5 ----- test/nyc-integration.js | 36 ------------------------------------ 3 files changed, 5 insertions(+), 47 deletions(-) diff --git a/index.js b/index.js index 15f1582e2..43e20884b 100755 --- a/index.js +++ b/index.js @@ -179,7 +179,7 @@ NYC.prototype.addAllFiles = function () { NYC.prototype.instrumentAllFiles = function (input, output, cb) { let inputDir = '.' + path.sep const visitor = filename => { - const inFile = path.resolve(this.cwd, inputDir, filename) + const inFile = path.resolve(inputDir, filename) const inCode = fs.readFileSync(inFile, 'utf-8') const extname = path.extname(filename).toLowerCase() @@ -187,7 +187,7 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { const outCode = transform(inCode, { filename: inFile }) if (output) { - const outFile = path.resolve(this.cwd, output, filename) + const outFile = path.resolve(output, filename) mkdirp.sync(path.dirname(outFile)) fs.writeFileSync(outFile, outCode, 'utf-8') } else { @@ -198,11 +198,10 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { this._loadAdditionalModules() try { - const inputPath = path.resolve(this.cwd, input) - const stats = fs.lstatSync(inputPath) + const stats = fs.lstatSync(input) if (stats.isDirectory()) { - inputDir = inputPath - this.walkAllFiles(inputPath, visitor) + inputDir = input + this.walkAllFiles(input, visitor) } else { visitor(input) } diff --git a/lib/commands/instrument.js b/lib/commands/instrument.js index 325713cf5..1820c5837 100644 --- a/lib/commands/instrument.js +++ b/lib/commands/instrument.js @@ -7,7 +7,6 @@ exports.command = 'instrument [output]' exports.describe = 'instruments a file or a directory tree and writes the instrumented code to the desired output location' exports.builder = function (yargs) { - const cwd = process.cwd() return yargs .option('require', { alias: 'i', @@ -29,10 +28,6 @@ exports.builder = function (yargs) { type: 'boolean', description: "should nyc's instrumenter produce source maps?" }) - .option('cwd', { - describe: 'working directory used when resolving paths', - default: cwd - }) .option('compact', { default: true, type: 'boolean', diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 1fa30d6ba..8124f671d 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -606,42 +606,6 @@ describe('the nyc cli', function () { }) }) - it('works with a different working directory', function (done) { - const subdir = path.resolve(fixturesCLI, 'subdir') - const args = [bin, 'instrument', '--cwd', subdir, './input-dir', './output-dir'] - - const proc = spawn(process.execPath, args, { - cwd: fixturesCLI, - env: env - }) - - proc.on('exit', code => { - code.should.equal(0) - const target = path.resolve(subdir, 'output-dir', 'index.js') - fs.readFileSync(target, 'utf8') - .should.match(/console.log\('Hello, World!'\)/) - done() - }) - }) - - it('works with a deep folder structure working directory', function (done) { - const subdir = path.resolve(fixturesCLI, 'subdir') - const args = [bin, 'instrument', '--cwd', subdir, '.', './output-dir'] - - const proc = spawn(process.execPath, args, { - cwd: fixturesCLI, - env: env - }) - - proc.on('exit', code => { - code.should.equal(0) - const target = path.resolve(subdir, 'output-dir', 'input-dir', 'index.js') - fs.readFileSync(target, 'utf8') - .should.match(/console.log\('Hello, World!'\)/) - done() - }) - }) - it('can be configured to exit on error', function (done) { var args = [ bin,