From df4de4d490f8cd32204fba66a810ed0444c26d0d Mon Sep 17 00:00:00 2001 From: Corey Farrell Date: Sun, 6 Oct 2019 21:29:37 -0400 Subject: [PATCH] feat: Add `--use-spawn-wrap=true` option (#1169) Completely bypass spawn-wrap unless overridden with this new option. --- bin/nyc.js | 33 ++++++--- bin/wrap.js | 31 +-------- index.js | 10 +++ lib/config-util.js | 6 ++ lib/wrap.js | 29 ++++++++ package-lock.json | 5 ++ package.json | 1 + .../test-nyc-integration.js-TAP.test.js | 68 +++++++++---------- test/add-all-files.js | 8 ++- test/fixtures/cli/selfspawn-fibonacci.js | 2 + test/nyc-integration.js | 8 +++ 11 files changed, 128 insertions(+), 73 deletions(-) create mode 100644 lib/wrap.js diff --git a/bin/nyc.js b/bin/nyc.js index 6dd78bf46..da9f45631 100755 --- a/bin/nyc.js +++ b/bin/nyc.js @@ -3,11 +3,9 @@ const configUtil = require('../lib/config-util') const foreground = require('foreground-child') +const resolveFrom = require('resolve-from') const NYC = require('../index.js') -const sw = require('spawn-wrap') -const wrapper = require.resolve('./wrap.js') - // parse configuration and command-line arguments; // we keep these values in a few different forms, // used in the various execution contexts of nyc: @@ -44,10 +42,7 @@ async function main () { await nyc.addAllFiles() } - var env = { - // Support running nyc as a user without HOME (e.g. linux 'nobody'), - // https://github.com/istanbuljs/nyc/issues/951 - SPAWN_WRAP_SHIM_ROOT: process.env.SPAWN_WRAP_SHIM_ROOT || process.env.XDG_CACHE_HOME || require('os').homedir(), + const env = { NYC_CONFIG: JSON.stringify(argv), NYC_CWD: process.cwd() } @@ -58,7 +53,29 @@ async function main () { env.BABEL_DISABLE_CACHE = process.env.BABEL_DISABLE_CACHE = '1' } - sw([wrapper], env) + if (!argv.useSpawnWrap) { + const { preloadAppend, propagateEnv } = require('node-preload') + + nyc.require.forEach(requireModule => { + const mod = resolveFrom.silent(nyc.cwd, requireModule) || requireModule + preloadAppend(mod) + require(mod) + }) + preloadAppend(require.resolve('../lib/wrap.js')) + Object.assign(propagateEnv, env) + } + + if (argv.all) nyc.addAllFiles() + + if (argv.useSpawnWrap) { + const wrapper = require.resolve('./wrap.js') + // Support running nyc as a user without HOME (e.g. linux 'nobody'), + // https://github.com/istanbuljs/nyc/issues/951 + env.SPAWN_WRAP_SHIM_ROOT = process.env.SPAWN_WRAP_SHIM_ROOT || process.env.XDG_CACHE_HOME || require('os').homedir() + const sw = require('spawn-wrap') + + sw([wrapper], env) + } // Both running the test script invocation and the check-coverage run may // set process.exitCode. Keep track so that both children are run, but diff --git a/bin/wrap.js b/bin/wrap.js index 3aa2a9df1..63d820556 100644 --- a/bin/wrap.js +++ b/bin/wrap.js @@ -1,29 +1,2 @@ -var sw = require('spawn-wrap') -var NYC = require('../index.js') - -var config = {} -if (process.env.NYC_CONFIG) config = JSON.parse(process.env.NYC_CONFIG) -config.isChildProcess = true - -config._processInfo = { - pid: process.pid, - ppid: process.ppid, - parent: process.env.NYC_PROCESS_ID || null -} -if (process.env.NYC_PROCESSINFO_EXTERNAL_ID) { - config._processInfo.externalId = process.env.NYC_PROCESSINFO_EXTERNAL_ID - delete process.env.NYC_PROCESSINFO_EXTERNAL_ID -} - -if (process.env.NYC_CONFIG_OVERRIDE) { - const override = JSON.parse(process.env.NYC_CONFIG_OVERRIDE) - config = { - ...config, - ...override - } - process.env.NYC_CONFIG = JSON.stringify(config) -} - -;(new NYC(config)).wrap() - -sw.runMain() +require('../lib/wrap') +require('spawn-wrap').runMain() diff --git a/index.js b/index.js index 8b45915b1..c51bf7508 100755 --- a/index.js +++ b/index.js @@ -129,6 +129,10 @@ class NYC { } _loadAdditionalModules () { + if (!this.config.useSpawnWrap) { + return + } + this.require.forEach(requireModule => { // Attempt to require the module relative to the directory being instrumented. // Then try other locations, e.g. the nyc node_modules folder. @@ -347,6 +351,12 @@ class NYC { wrap (bin) { process.env.NYC_PROCESS_ID = this.processInfo.uuid + // This is a bug with the spawn-wrap method where + // we cannot force propagation of NYC_PROCESS_ID. + if (!this.config.useSpawnWrap) { + const { propagateEnv } = require('node-preload') + propagateEnv.NYC_PROCESS_ID = this.processInfo.uuid + } this._addRequireHooks() this._wrapExit() this._loadAdditionalModules() diff --git a/lib/config-util.js b/lib/config-util.js index 3acf94a6d..28d61e048 100644 --- a/lib/config-util.js +++ b/lib/config-util.js @@ -201,6 +201,12 @@ async function processConfig (cwd) { type: 'boolean', global: false }) + .option('use-spawn-wrap', { + describe: 'use spawn-wrap instead of setting process.env.NODE_OPTIONS', + default: false, + type: 'boolean', + global: false + }) .option('clean', { describe: 'should the .nyc_output folder be cleaned before executing tests', default: true, diff --git a/lib/wrap.js b/lib/wrap.js new file mode 100644 index 000000000..cb3213340 --- /dev/null +++ b/lib/wrap.js @@ -0,0 +1,29 @@ +const NYC = require('../index.js') + +let config = {} +if (process.env.NYC_CONFIG) { + config = JSON.parse(process.env.NYC_CONFIG) +} +config.isChildProcess = true + +config._processInfo = { + pid: process.pid, + ppid: process.ppid, + parent: process.env.NYC_PROCESS_ID || null +} + +if (process.env.NYC_PROCESSINFO_EXTERNAL_ID) { + config._processInfo.externalId = process.env.NYC_PROCESSINFO_EXTERNAL_ID + delete process.env.NYC_PROCESSINFO_EXTERNAL_ID +} + +if (process.env.NYC_CONFIG_OVERRIDE) { + const override = JSON.parse(process.env.NYC_CONFIG_OVERRIDE) + config = { + ...config, + ...override + } + process.env.NYC_CONFIG = JSON.stringify(config) +} + +;(new NYC(config)).wrap() diff --git a/package-lock.json b/package-lock.json index df614afa4..38b3f93de 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3313,6 +3313,11 @@ "integrity": "sha1-jZ2+KJZKSsVxLpExZCEHxx6Q7EA=", "dev": true }, + "node-preload": { + "version": "0.1.2", + "resolved": "https://registry.npmjs.org/node-preload/-/node-preload-0.1.2.tgz", + "integrity": "sha512-OIVpCWTqvPClyXxt9HtKbHDKqQQzqzQIcv6FJgXGcgrQQiYPXNL38vDp1RbRwsF7HVDGWc7yQa5DNu2Zbs9MVw==" + }, "normalize-package-data": { "version": "2.5.0", "resolved": "https://registry.npmjs.org/normalize-package-data/-/normalize-package-data-2.5.0.tgz", diff --git a/package.json b/package.json index 7e5de9d8e..ff2f0a10f 100644 --- a/package.json +++ b/package.json @@ -87,6 +87,7 @@ "make-dir": "^3.0.0", "merge-source-map": "^1.1.0", "p-map": "^3.0.0", + "node-preload": "^0.1.2", "resolve-from": "^5.0.0", "rimraf": "^3.0.0", "signal-exit": "^3.0.2", diff --git a/tap-snapshots/test-nyc-integration.js-TAP.test.js b/tap-snapshots/test-nyc-integration.js-TAP.test.js index 83d458618..b026b05b6 100644 --- a/tap-snapshots/test-nyc-integration.js-TAP.test.js +++ b/tap-snapshots/test-nyc-integration.js-TAP.test.js @@ -113,7 +113,7 @@ All files | 0 | 0 | 0 | 0 | external-instrumenter.js | 0 | 0 | 0 | 0 | 1 gc.js | 0 | 100 | 100 | 0 | 2,3 half-covered-failing.js | 0 | 0 | 100 | 0 | 1,3,5,6,7,8 - selfspawn-fibonacci.js | 0 | 0 | 0 | 0 | ...19,21,24,25,26,27,28 + selfspawn-fibonacci.js | 0 | 0 | 0 | 0 | ...21,23,26,27,28,29,30 skip-full.js | 0 | 100 | 100 | 0 | 1,2 test.js | 0 | 0 | 0 | 0 | cli/fakebin | 0 | 100 | 100 | 0 | @@ -144,8 +144,8 @@ exports[`test/nyc-integration.js TAP --ignore-class-method skips methods that ma ---------------------------------|---------|----------|---------|---------|------------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------------------|---------|----------|---------|---------|------------------------- -All files | 1.45 | 0 | 5 | 1.89 | - cli | 2.08 | 0 | 5.56 | 3.13 | +All files | 1.44 | 0 | 5 | 1.87 | + cli | 2.06 | 0 | 5.56 | 3.08 | args.js | 0 | 100 | 100 | 0 | 1 by-arg2.js | 0 | 0 | 100 | 0 | 1,2,3,4,5,7 classes.js | 66.67 | 100 | 50 | 66.67 | 6 @@ -158,7 +158,7 @@ All files | 1.45 | 0 | 5 | 1.89 | gc.js | 0 | 100 | 100 | 0 | 2,3 half-covered-failing.js | 0 | 0 | 100 | 0 | 1,3,5,6,7,8 half-covered.js | 0 | 0 | 100 | 0 | 1,3,5,6,7,8 - selfspawn-fibonacci.js | 0 | 0 | 0 | 0 | ...19,21,24,25,26,27,28 + selfspawn-fibonacci.js | 0 | 0 | 0 | 0 | ...21,23,26,27,28,29,30 skip-full.js | 0 | 100 | 100 | 0 | 1,2 cli/fakebin | 0 | 100 | 100 | 0 | npm-template.js | 0 | 100 | 100 | 0 | 2,3,4,7,9 @@ -202,8 +202,8 @@ exports[`test/nyc-integration.js TAP --show-process-tree displays a tree of spaw ------------------------|---------|----------|---------|---------|------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ------------------------|---------|----------|---------|---------|------------------- -All files | 90.91 | 70 | 100 | 100 | - selfspawn-fibonacci.js | 90.91 | 70 | 100 | 100 | 4,25,27 +All files | 91.3 | 70 | 100 | 100 | + selfspawn-fibonacci.js | 91.3 | 70 | 100 | 100 | 6,27,29 ------------------------|---------|----------|---------|---------|------------------- nyc │ 100 % Lines @@ -214,19 +214,41 @@ nyc │ ├─┬ node ./selfspawn-fibonacci.js 3 │ │ │ 100 % Lines │ │ ├── node ./selfspawn-fibonacci.js 2 - │ │ │ 31.58 % Lines + │ │ │ 35 % Lines │ │ └── node ./selfspawn-fibonacci.js 1 - │ │ 26.32 % Lines + │ │ 30 % Lines │ └── node ./selfspawn-fibonacci.js 2 - │ 31.58 % Lines + │ 35 % Lines └─┬ node ./selfspawn-fibonacci.js 3 │ 100 % Lines ├── node ./selfspawn-fibonacci.js 2 - │ 31.58 % Lines + │ 35 % Lines └── node ./selfspawn-fibonacci.js 1 - 26.32 % Lines + 30 % Lines +` + +exports[`test/nyc-integration.js TAP --use-spawn-wrap=false is functional > stdout 1`] = ` +3 +------------------------|---------|----------|---------|---------|------------------- +File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s +------------------------|---------|----------|---------|---------|------------------- +All files | 91.3 | 70 | 100 | 100 | + selfspawn-fibonacci.js | 91.3 | 70 | 100 | 100 | 6,27,29 +------------------------|---------|----------|---------|---------|------------------- + +` + +exports[`test/nyc-integration.js TAP --use-spawn-wrap=true is functional > stdout 1`] = ` +3 +------------------------|---------|----------|---------|---------|------------------- +File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s +------------------------|---------|----------|---------|---------|------------------- +All files | 91.3 | 70 | 100 | 100 | + selfspawn-fibonacci.js | 91.3 | 70 | 100 | 100 | 6,27,29 +------------------------|---------|----------|---------|---------|------------------- + ` exports[`test/nyc-integration.js TAP allows .nycrc configuration to be overridden with command line args > stdout 1`] = ` @@ -853,30 +875,6 @@ exports[`test/nyc-integration.js TAP passes configuration via environment variab ] ` -exports[`test/nyc-integration.js TAP produce-source-map enabled > stdout 1`] = ` -Error: Blarrh - at blah (./stack-trace.js:3:1) -----------------|---------|----------|---------|---------|------------------- -File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -----------------|---------|----------|---------|---------|------------------- -All files | 100 | 100 | 100 | 100 | - stack-trace.js | 100 | 100 | 100 | 100 | -----------------|---------|----------|---------|---------|------------------- - -` - -exports[`test/nyc-integration.js TAP produce-source-map not enabled > stdout 1`] = ` -Error: Blarrh - at blah (./stack-trace.js:1:1037) -----------------|---------|----------|---------|---------|------------------- -File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -----------------|---------|----------|---------|---------|------------------- -All files | 100 | 100 | 100 | 100 | - stack-trace.js | 100 | 100 | 100 | 100 | -----------------|---------|----------|---------|---------|------------------- - -` - exports[`test/nyc-integration.js TAP recursive run does not throw > stdout 1`] = ` ` diff --git a/test/add-all-files.js b/test/add-all-files.js index 38750dc7f..636947300 100644 --- a/test/add-all-files.js +++ b/test/add-all-files.js @@ -60,7 +60,11 @@ t.test('transpiles .js files added via addAllFiles', async t => { 'utf-8' ) - const nyc = new NYC(await parseArgv(fixtures, ['--require', transpileHook])) + const nyc = new NYC(await parseArgv(fixtures, [ + '--use-spawn-wrap=true', + '--require', + transpileHook + ])) await nyc.reset() await nyc.addAllFiles() @@ -83,6 +87,7 @@ t.test('does not attempt to transpile files when they are excluded', async t => ) const nyc = new NYC(await parseArgv(fixtures, [ + '--use-spawn-wrap=true', `--require=${transpileHook}`, '--extension=.do-not-transpile', '--include=needs-transpile.do-not-transpile' @@ -103,6 +108,7 @@ t.test('transpiles non-.js files added via addAllFiles', async t => { ) const nyc = new NYC(await parseArgv(fixtures, [ + '--use-spawn-wrap=true', `--require=${transpileHook}`, '--extension=.whatever' ])) diff --git a/test/fixtures/cli/selfspawn-fibonacci.js b/test/fixtures/cli/selfspawn-fibonacci.js index d176415d7..8d983a68a 100644 --- a/test/fixtures/cli/selfspawn-fibonacci.js +++ b/test/fixtures/cli/selfspawn-fibonacci.js @@ -1,6 +1,8 @@ 'use strict'; var cp = require('child_process'); +process.env = {}; + var index = +process.argv[2] || 0 if (index <= 1) { console.log(0) diff --git a/test/nyc-integration.js b/test/nyc-integration.js index ba68d2280..278d35afa 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -186,6 +186,14 @@ t.test('--show-process-tree displays a tree of spawned processes', t => testSucc args: ['--show-process-tree', process.execPath, 'selfspawn-fibonacci.js', '5'] })) +t.test('--use-spawn-wrap=true is functional', t => testSuccess(t, { + args: ['--use-spawn-wrap=true', process.execPath, 'selfspawn-fibonacci.js', '5'] +})) + +t.test('--use-spawn-wrap=false is functional', t => testSuccess(t, { + args: ['--use-spawn-wrap=false', process.execPath, 'selfspawn-fibonacci.js', '5'] +})) + t.test('can run "npm test" which directly invokes a test file', t => testSuccess(t, { args: ['npm', 'test'], cwd: path.resolve(fixturesCLI, 'run-npm-test')