Skip to content

Commit

Permalink
feat: Add --use-spawn-wrap=true option (#1169)
Browse files Browse the repository at this point in the history
Completely bypass spawn-wrap unless overridden with this new option.
  • Loading branch information
coreyfarrell committed Oct 7, 2019
1 parent 408c1cb commit df4de4d
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 73 deletions.
33 changes: 25 additions & 8 deletions bin/nyc.js
Expand Up @@ -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:
Expand Down Expand Up @@ -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()
}
Expand All @@ -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
Expand Down
31 changes: 2 additions & 29 deletions 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()
10 changes: 10 additions & 0 deletions index.js
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 6 additions & 0 deletions lib/config-util.js
Expand Up @@ -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,
Expand Down
29 changes: 29 additions & 0 deletions 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()
5 changes: 5 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Expand Up @@ -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",
Expand Down
68 changes: 33 additions & 35 deletions tap-snapshots/test-nyc-integration.js-TAP.test.js
Expand Up @@ -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 |
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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`] = `
Expand Down Expand Up @@ -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`] = `
`
Expand Down
8 changes: 7 additions & 1 deletion test/add-all-files.js
Expand Up @@ -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()

Expand All @@ -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'
Expand All @@ -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'
]))
Expand Down
2 changes: 2 additions & 0 deletions 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)
Expand Down
8 changes: 8 additions & 0 deletions test/nyc-integration.js
Expand Up @@ -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')
Expand Down

0 comments on commit df4de4d

Please sign in to comment.