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: Fix excludeAfterRemap functionality. #1010

Merged
merged 3 commits into from
Mar 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 21 additions & 38 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@ const glob = require('glob')
const Hash = require('./lib/hash')
const libCoverage = require('istanbul-lib-coverage')
const libHook = require('istanbul-lib-hook')
const libReport = require('istanbul-lib-report')
const mkdirp = require('make-dir')
const Module = require('module')
const onExit = require('signal-exit')
const path = require('path')
const reports = require('istanbul-reports')
const resolveFrom = require('resolve-from')
const rimraf = require('rimraf')
const SourceMaps = require('./lib/source-maps')
const testExclude = require('test-exclude')
const uuid = require('uuid/v4')
const api = require('istanbul-api')

const debugLog = util.debuglog('nyc')

Expand Down Expand Up @@ -83,12 +84,6 @@ function NYC (config) {
this.rootId = this.processInfo.root || this.generateUniqueID()

this.hashCache = {}

this.config.reporting = config.reporting || {}
this.config.reporting['dir'] = this.reportDirectory()
this.config.reporting['report-config'] = this._reportConfig()
this.config.reporting['summarizer'] = this._reportSummarizer()
this.config.reporting['watermarks'] = this._reportWatermarks()
}

NYC.prototype._createTransform = function (ext) {
Expand Down Expand Up @@ -420,32 +415,41 @@ function coverageFinder () {
}

NYC.prototype.getCoverageMapFromAllCoverageFiles = function (baseDirectory) {
var _this = this
var map = libCoverage.createCoverageMap({})

this.eachReport(undefined, (report) => {
map.merge(report)
}, baseDirectory)

map.data = this.sourceMaps.remapCoverage(map.data)

// depending on whether source-code is pre-instrumented
// or instrumented using a JIT plugin like @babel/require
// you may opt to exclude files after applying
// source-map remapping logic.
if (this.config.excludeAfterRemap) {
map.filter(function (filename) {
return _this.exclude.shouldInstrument(filename)
})
map.filter(filename => this.exclude.shouldInstrument(filename))
}
map.data = this.sourceMaps.remapCoverage(map.data)

return map
}

NYC.prototype.report = function () {
const config = api.config.loadObject(this.config)
const reporter = api.createReporter(config)
const map = this.getCoverageMapFromAllCoverageFiles()
var tree
var map = this.getCoverageMapFromAllCoverageFiles()
var context = libReport.createContext({
dir: this.reportDirectory(),
watermarks: this.config.watermarks
})

reporter.addAll(this.reporter)
reporter.write(map)
tree = libReport.summarizers.pkg(map)

this.reporter.forEach((_reporter) => {
tree.visit(reports.create(_reporter, {
skipEmpty: this.config.skipEmpty,
skipFull: this.config.skipFull
}), context)
})

if (this._showProcessTree) {
this.showProcessTree()
Expand Down Expand Up @@ -553,25 +557,4 @@ NYC.prototype.processInfoDirectory = function () {
return path.resolve(this.tempDirectory(), 'processinfo')
}

NYC.prototype._reportConfig = function () {
const config = {}

this.reporter.forEach(_reporter => {
config[_reporter] = {
skipEmpty: this.config.skipEmpty,
skipFull: this.config.skipFull
}
})

return config
}

NYC.prototype._reportSummarizer = function () {
return 'pkg'
}

NYC.prototype._reportWatermarks = function () {
return this.config.watermarks
}

module.exports = NYC
19 changes: 19 additions & 0 deletions lib/commands/check-coverage.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const testExclude = require('test-exclude')
const NYC = require('../../index.js')

exports.command = 'check-coverage'
Expand All @@ -6,6 +7,24 @@ exports.describe = 'check whether coverage is within thresholds provided'

exports.builder = function (yargs) {
yargs
.option('exclude', {
alias: 'x',
default: testExclude.defaultExclude,
describe: 'a list of specific files and directories that should be excluded from coverage, glob patterns are supported, node_modules is always excluded',
global: false
})
.option('exclude-after-remap', {
default: true,
type: 'boolean',
description: 'should exclude logic be performed after the source-map remaps filenames?',
global: false
})
.option('include', {
alias: 'n',
default: [],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coreyfarrell sorry for the post-review - shouldn't this be ['**']?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. An empty list is functionally equal to ['**'] but [] actually bypasses the check so it's more efficient.

https://github.com/istanbuljs/istanbuljs/blob/master/packages/test-exclude/index.js#L50-L54 is where the default is processed, [] is replaced with false which short-circuits the include check.

describe: 'a list of specific files that should be covered, glob patterns are supported',
global: false
})
.option('branches', {
default: 0,
description: 'what % of branches must be covered?'
Expand Down
19 changes: 19 additions & 0 deletions lib/commands/report.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const testExclude = require('test-exclude')
const NYC = require('../../index.js')

exports.command = 'report'
Expand All @@ -23,6 +24,24 @@ exports.builder = function (yargs) {
.option('temp-directory', {
hidden: true
})
.option('exclude', {
alias: 'x',
default: testExclude.defaultExclude,
describe: 'a list of specific files and directories that should be excluded from coverage, glob patterns are supported, node_modules is always excluded',
global: false
})
.option('exclude-after-remap', {
default: true,
type: 'boolean',
description: 'should exclude logic be performed after the source-map remaps filenames?',
global: false
})
.option('include', {
alias: 'n',
default: [],
describe: 'a list of specific files that should be covered, glob patterns are supported',
global: false
})
.option('show-process-tree', {
describe: 'display the tree of spawned processes',
default: false,
Expand Down
42 changes: 6 additions & 36 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
"find-up": "^3.0.0",
"foreground-child": "^1.5.6",
"glob": "^7.1.3",
"istanbul-api": "^2.1.0",
"istanbul-lib-coverage": "^2.0.3",
"istanbul-lib-hook": "^2.0.3",
"istanbul-lib-instrument": "^3.1.0",
Expand Down
33 changes: 33 additions & 0 deletions test/nyc-integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,39 @@ describe('the nyc cli', function () {
done()
})
})

it('uses source-maps to exclude original sources from reports', (done) => {
const args = [
bin,
'--all',
'--cache', 'false',
'--exclude', 'original/s1.js',
process.execPath, './instrumented/s1.min.js'
]

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

var stdout = ''
proc.stdout.on('data', function (chunk) {
stdout += chunk
})

proc.on('close', function (code) {
code.should.equal(0)
stdoutShouldEqual(stdout, `
----------|----------|----------|----------|----------|-------------------|
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files | 0 | 100 | 0 | 0 | |
s2.js | 0 | 100 | 0 | 0 | 1,2,4,6 |
----------|----------|----------|----------|----------|-------------------|`
)
done()
})
})
})

describe('.map file', () => {
Expand Down