From 5c1eb38b91b0281ae35f5d3995467c33ab9d60ea Mon Sep 17 00:00:00 2001 From: AndrewFinlay Date: Fri, 29 Mar 2019 06:49:46 +1100 Subject: [PATCH] test(instrument): should return unmodified source if no transform found (#1036) * Test that the instrument command returns unmodified source if there is no transform found for a file extension. Currently this behaviour can only be reached when trying to instrument a single file. In the case of instrumenting a directory, files with an extension with no matching transform are filtered out before they can be instrumented. * Cleanup instrumentation code again with a focus on paths The main aim of this has been to clarify whether we're working with relative or absolute file paths, and removing unnecessary transformations. Although I've made a few other 'small' changes here and there. Key changes: * Created a new private method `NYC._transform`, common to `_maybeInstrumentSource` and `instrumentAllFiles`. * Renamed the param in `walkAllFiles` forEach handler to `relFile` to explicitly state the file representation being used. * Let the `addAllFiles` visitor function rely on `testExclude` to determine which files to instrument --- index.js | 69 ++++++++----------- .../cli/no-transform/half-covered.xjs | 9 +++ test/nyc-integration.js | 20 ++++++ 3 files changed, 57 insertions(+), 41 deletions(-) create mode 100644 test/fixtures/cli/no-transform/half-covered.xjs diff --git a/index.js b/index.js index 43e20884b..a0a7431af 100755 --- a/index.js +++ b/index.js @@ -134,9 +134,8 @@ NYC.prototype._createInstrumenter = function () { } NYC.prototype.addFile = function (filename) { - const relFile = path.relative(this.cwd, filename) - const source = this._readTranspiledSource(path.resolve(this.cwd, filename)) - this._maybeInstrumentSource(source, filename, relFile) + const source = this._readTranspiledSource(filename) + this._maybeInstrumentSource(source, filename) } NYC.prototype._readTranspiledSource = function (filePath) { @@ -154,21 +153,16 @@ NYC.prototype._readTranspiledSource = function (filePath) { } NYC.prototype.addAllFiles = function () { - var _this = this - this._loadAdditionalModules() this.fakeRequire = true - this.walkAllFiles(this.cwd, function (filename) { - filename = path.resolve(_this.cwd, filename) - if (_this.exclude.shouldInstrument(filename)) { - _this.addFile(filename) - var coverage = coverageFinder() - var lastCoverage = _this.instrumenter().lastFileCoverage() - if (lastCoverage) { - filename = lastCoverage.path - coverage[filename] = lastCoverage - } + this.walkAllFiles(this.cwd, relFile => { + const filename = path.resolve(this.cwd, relFile) + this.addFile(filename) + const coverage = coverageFinder() + const lastCoverage = this.instrumenter().lastFileCoverage() + if (lastCoverage) { + coverage[lastCoverage.path] = lastCoverage } }) this.fakeRequire = false @@ -178,16 +172,13 @@ NYC.prototype.addAllFiles = function () { NYC.prototype.instrumentAllFiles = function (input, output, cb) { let inputDir = '.' + path.sep - const visitor = filename => { - const inFile = path.resolve(inputDir, filename) + const visitor = relFile => { + const inFile = path.resolve(inputDir, relFile) const inCode = fs.readFileSync(inFile, 'utf-8') - - const extname = path.extname(filename).toLowerCase() - const transform = this.transforms[extname] || (code => code) - const outCode = transform(inCode, { filename: inFile }) + const outCode = this._transform(inCode, inFile) || inCode if (output) { - const outFile = path.resolve(output, filename) + const outFile = path.resolve(output, relFile) mkdirp.sync(path.dirname(outFile)) fs.writeFileSync(outFile, outCode, 'utf-8') } else { @@ -212,26 +203,24 @@ NYC.prototype.instrumentAllFiles = function (input, output, cb) { } NYC.prototype.walkAllFiles = function (dir, visitor) { - this.exclude.globSync(dir).forEach(filename => { - visitor(filename) + this.exclude.globSync(dir).forEach(relFile => { + visitor(relFile) }) } -NYC.prototype._maybeInstrumentSource = function (code, filename, relFile) { - var instrument = this.exclude.shouldInstrument(filename, relFile) - if (!instrument) { - return null - } +NYC.prototype._transform = function (code, filename) { + const extname = path.extname(filename).toLowerCase() + const transform = this.transforms[extname] || (() => null) - var ext, transform - for (ext in this.transforms) { - if (filename.toLowerCase().substr(-ext.length) === ext) { - transform = this.transforms[ext] - break - } + return transform(code, { filename }) +} + +NYC.prototype._maybeInstrumentSource = function (code, filename) { + if (!this.exclude.shouldInstrument(filename)) { + return null } - return transform ? transform(code, { filename: filename, relFile: relFile }) : null + return this._transform(code, filename) } NYC.prototype._transformFactory = function (cacheDir) { @@ -265,11 +254,9 @@ NYC.prototype._transformFactory = function (cacheDir) { } NYC.prototype._handleJs = function (code, options) { - var filename = options.filename - var relFile = path.relative(this.cwd, filename) // ensure the path has correct casing (see istanbuljs/nyc#269 and nodejs/node#6624) - filename = path.resolve(this.cwd, relFile) - return this._maybeInstrumentSource(code, filename, relFile) || code + const filename = path.resolve(this.cwd, options.filename) + return this._maybeInstrumentSource(code, filename) || code } NYC.prototype._addHook = function (type) { @@ -382,7 +369,7 @@ function coverageFinder () { } NYC.prototype.getCoverageMapFromAllCoverageFiles = function (baseDirectory) { - var map = libCoverage.createCoverageMap({}) + const map = libCoverage.createCoverageMap({}) this.eachReport(undefined, (report) => { map.merge(report) diff --git a/test/fixtures/cli/no-transform/half-covered.xjs b/test/fixtures/cli/no-transform/half-covered.xjs new file mode 100644 index 000000000..fd8c8f68d --- /dev/null +++ b/test/fixtures/cli/no-transform/half-covered.xjs @@ -0,0 +1,9 @@ +var a = 0 + +a++ + +if (a === 0) { + a++; + a--; + a++; +} diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 8124f671d..23745e6b7 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -581,6 +581,26 @@ describe('the nyc cli', function () { done() }) }) + + it('returns unmodified source if there is no transform', function (done) { + const args = [bin, 'instrument', './no-transform/half-covered.xjs'] + + const proc = spawn(process.execPath, args, { + cwd: fixturesCLI, + env: env + }) + + let stdout = '' + proc.stdout.on('data', function (chunk) { + stdout += chunk + }) + + proc.on('close', function (code) { + code.should.equal(0) + stdout.should.contain(`var a = 0`) + done() + }) + }) }) describe('output folder specified', function () {