feat: refactored config to fix precedence of config vs. args (#388)

* feat: refactored config to fix precedence of config vs. args see #379

* fix: address standard nits

* fix: remove cruft from package.json

* fix: address @mourner's code review

Author: bcoe

bin/nyc.js

@@ -1,5 +1,5 @@
 #!/usr/bin/env node
-var arrify = require('arrify')
+var configUtil = require('../lib/config-util')
 var foreground = require('foreground-child')
 var NYC
 try {
@@ -10,68 +10,44 @@ try {
 var processArgs = require('../lib/process-args')
 
 var sw = require('spawn-wrap')
-var testExclude = require('test-exclude')
 var wrapper = require.resolve('./wrap.js')
-var Yargs = require('yargs/yargs')
 
-var yargs = decorateYargs(buildYargs())
-var argv = yargs.parse(processArgs.hideInstrumenteeArgs())
+// parse configuration and command-line arguments;
+// we keep these values in a few different forms,
+// used in the various execution contexts of nyc:
+// reporting, instrumenting subprocesses, etc.
+var yargs = configUtil.decorateYargs(configUtil.buildYargs())
+var instrumenterArgs = processArgs.hideInstrumenteeArgs()
+var argv = yargs.parse(instrumenterArgs)
+var config = configUtil.loadConfig(instrumenterArgs)
 
 if (argv._[0] === 'report') {
   // run a report.
   process.env.NYC_CWD = process.cwd()
 
-  report(argv)
+  report(config)
 } else if (argv._[0] === 'check-coverage') {
-  checkCoverage(argv)
+  checkCoverage(config)
 } else if (argv._[0] === 'instrument') {
   // look in lib/commands/instrument.js for logic.
 } else if (argv._.length) {
-  // wrap subprocesses and execute argv[1]
-  argv.require = arrify(argv.require)
-  argv.extension = arrify(argv.extension)
-  argv.exclude = arrify(argv.exclude)
-  argv.include = arrify(argv.include)
-
   // 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 (!config.instrument) config.instrumenter = './lib/instrumenters/noop'
+  else config.instrumenter = './lib/instrumenters/istanbul'
 
-  var nyc = (new NYC({
-    require: argv.require,
-    include: argv.include,
-    exclude: argv.exclude,
-    sourceMap: !!argv.sourceMap,
-    instrumenter: argv.instrumenter,
-    hookRunInContext: argv.hookRunInContext,
-    showProcessTree: argv.showProcessTree
-  }))
+  var nyc = (new NYC(config))
   nyc.reset()
 
-  if (argv.all) nyc.addAllFiles()
+  if (config.all) nyc.addAllFiles()
 
   var env = {
-    NYC_CWD: process.cwd(),
     NYC_CACHE: argv.cache ? 'enable' : 'disable',
-    NYC_SOURCE_MAP: argv.sourceMap ? 'enable' : 'disable',
-    NYC_INSTRUMENTER: argv.instrumenter,
-    NYC_HOOK_RUN_IN_CONTEXT: argv.hookRunInContext ? 'enable' : 'disable',
-    NYC_SHOW_PROCESS_TREE: argv.showProcessTree ? 'enable' : 'disable',
+    NYC_CONFIG: JSON.stringify(config),
+    NYC_CWD: process.cwd(),
     NYC_ROOT_ID: nyc.rootId,
-    BABEL_DISABLE_CACHE: 1
-  }
-  if (argv.require.length) {
-    env.NYC_REQUIRE = argv.require.join(',')
-  }
-  if (argv.extension.length) {
-    env.NYC_EXTENSION = argv.extension.join(',')
-  }
-  if (argv.exclude.length) {
-    env.NYC_EXCLUDE = argv.exclude.join(':')
-  }
-  if (argv.include.length) {
-    env.NYC_INCLUDE = argv.include.join(':')
+    BABEL_DISABLE_CACHE: 1,
+    NYC_INSTRUMENTER: config.instrumenter
   }
   sw([wrapper], env)
 
@@ -82,17 +58,17 @@ if (argv._[0] === 'report') {
   foreground(processArgs.hideInstrumenterArgs(
     // use the same argv descrption, but don't exit
     // for flags like --help.
-    buildYargs().parse(process.argv.slice(2))
+    configUtil.buildYargs().parse(process.argv.slice(2))
   ), function (done) {
     var mainChildExitCode = process.exitCode
 
-    if (argv.checkCoverage) {
-      checkCoverage(argv)
+    if (config.checkCoverage) {
+      checkCoverage(config)
       process.exitCode = process.exitCode || mainChildExitCode
-      if (!argv.silent) report(argv)
+      if (!config.silent) report(config)
       return done()
     } else {
-      if (!argv.silent) report(argv)
+      if (!config.silent) report(config)
       return done()
     }
   })
@@ -104,12 +80,7 @@ if (argv._[0] === 'report') {
 function report (argv) {
   process.env.NYC_CWD = process.cwd()
 
-  var nyc = new NYC({
-    reporter: argv.reporter,
-    reportDir: argv.reportDir,
-    tempDirectory: argv.tempDirectory,
-    showProcessTree: argv.showProcessTree
-  })
+  var nyc = new NYC(argv)
   nyc.report()
 }
 
@@ -123,158 +94,3 @@ function checkCoverage (argv, cb) {
     statements: argv.statements
   })
 }
-
-function buildYargs () {
-  return Yargs([])
-    .usage('$0 [command] [options]\n\nrun your tests with the nyc bin to instrument them with coverage')
-    .command('report', 'run coverage report for .nyc_output', function (yargs) {
-      return yargs
-        .usage('$0 report [options]')
-        .option('reporter', {
-          alias: 'r',
-          describe: 'coverage reporter(s) to use',
-          default: 'text'
-        })
-        .option('report-dir', {
-          describe: 'directory to output coverage reports in',
-          default: 'coverage'
-        })
-        .option('temp-directory', {
-          describe: 'directory from which coverage JSON files are read',
-          default: './.nyc_output'
-        })
-        .option('show-process-tree', {
-          describe: 'display the tree of spawned processes',
-          default: false,
-          type: 'boolean'
-        })
-        .example('$0 report --reporter=lcov', 'output an HTML lcov report to ./coverage')
-    })
-    .command('check-coverage', 'check whether coverage is within thresholds provided', function (yargs) {
-      return yargs
-        .usage('$0 check-coverage [options]')
-        .option('branches', {
-          default: 0,
-          description: 'what % of branches must be covered?'
-        })
-        .option('functions', {
-          default: 0,
-          description: 'what % of functions must be covered?'
-        })
-        .option('lines', {
-          default: 90,
-          description: 'what % of lines must be covered?'
-        })
-        .option('statements', {
-          default: 0,
-          description: 'what % of statements must be covered?'
-        })
-        .example('$0 check-coverage --lines 95', "check whether the JSON in nyc's output folder meets the thresholds provided")
-    })
-    .option('reporter', {
-      alias: 'r',
-      describe: 'coverage reporter(s) to use',
-      default: 'text'
-    })
-    .option('report-dir', {
-      describe: 'directory to output coverage reports in',
-      default: 'coverage'
-    })
-    .option('silent', {
-      alias: 's',
-      default: false,
-      type: 'boolean',
-      describe: "don't output a report after tests finish running"
-    })
-    .option('all', {
-      alias: 'a',
-      default: false,
-      type: 'boolean',
-      describe: 'whether or not to instrument all files of the project (not just the ones touched by your test suite)'
-    })
-    .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'
-    })
-    .option('include', {
-      alias: 'n',
-      default: [],
-      describe: 'a list of specific files that should be covered, glob patterns are supported'
-    })
-    .option('require', {
-      alias: 'i',
-      default: [],
-      describe: 'a list of additional modules that nyc should attempt to require in its subprocess, e.g., babel-register, babel-polyfill.'
-    })
-    .option('cache', {
-      alias: 'c',
-      default: false,
-      type: 'boolean',
-      describe: 'cache instrumentation results for improved performance'
-    })
-    .option('extension', {
-      alias: 'e',
-      default: [],
-      describe: 'a list of extensions that nyc should handle in addition to .js'
-    })
-    .option('check-coverage', {
-      type: 'boolean',
-      default: false,
-      describe: 'check whether coverage is within thresholds provided'
-    })
-    .option('branches', {
-      default: 0,
-      description: 'what % of branches must be covered?'
-    })
-    .option('functions', {
-      default: 0,
-      description: 'what % of functions must be covered?'
-    })
-    .option('lines', {
-      default: 90,
-      description: 'what % of lines must be covered?'
-    })
-    .option('statements', {
-      default: 0,
-      description: 'what % of statements must be covered?'
-    })
-    .option('source-map', {
-      default: true,
-      type: 'boolean',
-      description: 'should nyc detect and handle source maps?'
-    })
-    .option('instrument', {
-      default: true,
-      type: 'boolean',
-      description: 'should nyc handle instrumentation?'
-    })
-    .option('hook-run-in-context', {
-      default: true,
-      type: 'boolean',
-      description: 'should nyc wrap vm.runInThisContext?'
-    })
-    .option('show-process-tree', {
-      describe: 'display the tree of spawned processes',
-      default: false,
-      type: 'boolean'
-    })
-    .pkgConf('nyc', process.cwd())
-    .example('$0 npm test', 'instrument your tests with coverage')
-    .example('$0 --require babel-core/register npm test', 'instrument your tests with coverage and babel')
-    .example('$0 report --reporter=text-lcov', 'output lcov report after running your tests')
-    .epilog('visit https://git.io/voHar for list of available reporters')
-    .boolean('help')
-    .boolean('h')
-    .boolean('version')
-}
-
-// decorate yargs with all the actions
-// that would make it exit: help, version, command.
-function decorateYargs (yargs) {
-  return yargs
-    .help('h')
-    .alias('h', 'help')
-    .version()
-    .command(require('../lib/commands/instrument'))
-}

bin/wrap.js

@@ -9,20 +9,14 @@ try {
 var parentPid = process.env.NYC_PARENT_PID || '0'
 process.env.NYC_PARENT_PID = process.pid
 
-;(new NYC({
-  require: process.env.NYC_REQUIRE ? process.env.NYC_REQUIRE.split(',') : [],
-  extension: process.env.NYC_EXTENSION ? process.env.NYC_EXTENSION.split(',') : [],
-  exclude: process.env.NYC_EXCLUDE ? process.env.NYC_EXCLUDE.split(':') : [],
-  include: process.env.NYC_INCLUDE ? process.env.NYC_INCLUDE.split(':') : [],
-  enableCache: process.env.NYC_CACHE === 'enable',
-  sourceMap: process.env.NYC_SOURCE_MAP === 'enable',
-  instrumenter: process.env.NYC_INSTRUMENTER,
-  hookRunInContext: process.env.NYC_HOOK_RUN_IN_CONTEXT === 'enable',
-  showProcessTree: process.env.NYC_SHOW_PROCESS_TREE === 'enable',
-  _processInfo: {
-    ppid: parentPid,
-    root: process.env.NYC_ROOT_ID
-  }
-})).wrap()
+var config = {}
+if (process.env.NYC_CONFIG) config = JSON.parse(process.env.NYC_CONFIG)
+config.enableCache = process.env.NYC_CACHE === 'enable'
+config._processInfo = {
+  ppid: parentPid,
+  root: process.env.NYC_ROOT_ID
+}
+
+;(new NYC(config)).wrap()
 
 sw.runMain()

index.js

@@ -1,4 +1,5 @@
 /* global __coverage__ */
+var arrify = require('arrify')
 var fs = require('fs')
 var glob = require('glob')
 var libCoverage = require('istanbul-lib-coverage')
@@ -13,14 +14,11 @@ var path = require('path')
 var rimraf = require('rimraf')
 var onExit = require('signal-exit')
 var resolveFrom = require('resolve-from')
-var arrify = require('arrify')
 var convertSourceMap = require('convert-source-map')
 var md5hex = require('md5-hex')
 var findCacheDir = require('find-cache-dir')
 var js = require('default-require-extensions/js')
-var pkgUp = require('pkg-up')
 var testExclude = require('test-exclude')
-var yargs = require('yargs')
 
 var ProcessInfo
 try {
@@ -34,17 +32,16 @@ if (/index\.covered\.js$/.test(__filename)) {
   require('./lib/self-coverage-helper')
 }
 
-function NYC (opts) {
-  var config = this._loadConfig(opts || {})
+function NYC (config) {
+  config = config || {}
 
-  this._istanbul = config.istanbul
   this.subprocessBin = config.subprocessBin || path.resolve(__dirname, './bin/nyc.js')
   this._tempDirectory = config.tempDirectory || './.nyc_output'
   this._instrumenterLib = require(config.instrumenter || './lib/instrumenters/istanbul')
-  this._reportDir = config.reportDir
-  this._sourceMap = config.sourceMap
-  this._showProcessTree = config.showProcessTree
-  this.cwd = config.cwd
+  this._reportDir = config.reportDir || 'coverage'
+  this._sourceMap = typeof config.sourceMap === 'boolean' ? config.sourceMap : true
+  this._showProcessTree = config.showProcessTree || false
+  this.cwd = config.cwd || process.cwd()
 
   this.reporter = arrify(config.reporter || 'text')
 
@@ -80,26 +77,10 @@ function NYC (opts) {
   this.loadedMaps = null
   this.fakeRequire = null
 
-  this.processInfo = new ProcessInfo(opts && opts._processInfo)
+  this.processInfo = new ProcessInfo(config && config._processInfo)
   this.rootId = this.processInfo.root || this.generateUniqueID()
 }
 
-NYC.prototype._loadConfig = function (opts) {
-  var cwd = opts.cwd || process.env.NYC_CWD || process.cwd()
-  var pkgPath = pkgUp.sync(cwd)
-
-  if (pkgPath) {
-    cwd = path.dirname(pkgPath)
-  }
-
-  opts.cwd = cwd
-
-  return yargs([])
-    .pkgConf('nyc', cwd)
-    .default(opts)
-    .argv
-}
-
 NYC.prototype._createTransform = function (ext) {
   var _this = this
   return cachingTransform({