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: Add --use-spawn-wrap=true option #1169

Merged
merged 1 commit into from Oct 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
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