From 22c4e3f286c3de202451111bc08fd09c10fc47b0 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Tue, 12 Dec 2017 16:49:53 +0100 Subject: [PATCH 1/2] Remove `debug` and `inspect` flags from the arguments sent to the child --- CHANGELOG.md | 15 ++++++----- docs/GlobalAPI.md | 25 ++++++++++--------- packages/jest-worker/README.md | 4 +-- .../jest-worker/src/__tests__/worker.test.js | 12 +++++++-- packages/jest-worker/src/worker.js | 3 +++ 5 files changed, 37 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 417eda4204c1..674bc7498e93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,13 +2,16 @@ ### Fixes +* `[jest-worker]` Remove `debug` and `inspect` flags from the arguments sent to + the child ([#5068](https://github.com/facebook/jest/pull/5068)) * `[jest-config]` Use all `--testPathPattern` and `` args in - `testPathPattern`([#5066](https://github.com/facebook/jest/pull/5066)) -* `[jest-cli]` Do not support `--watch` inside non-version-controlled environments - ([#5060](https://github.com/facebook/jest/pull/5060)) + `testPathPattern` ([#5066](https://github.com/facebook/jest/pull/5066)) +* `[jest-cli]` Do not support `--watch` inside non-version-controlled + environments ([#5060](https://github.com/facebook/jest/pull/5060)) * `[jest-config]` Escape Windows path separator in testPathPattern CLI arguments ([#5054](https://github.com/facebook/jest/pull/5054) -* `[jest-jasmine]` Register sourcemaps as node environment to improve performance with jsdom ([#5045](https://github.com/facebook/jest/pull/5045)) +* `[jest-jasmine]` Register sourcemaps as node environment to improve + performance with jsdom ([#5045](https://github.com/facebook/jest/pull/5045)) * `[pretty-format]` Do not call toJSON recursively ([#5044](https://github.com/facebook/jest/pull/5044)) * `[pretty-format]` Fix errors when identity-obj-proxy mocks CSS Modules @@ -66,8 +69,8 @@ ### Features -* `[jest-config]` Add `testEnvironmentOptions` to apply to jsdom options or node context. - ([#5003](https://github.com/facebook/jest/pull/5003)) +* `[jest-config]` Add `testEnvironmentOptions` to apply to jsdom options or node + context. ([#5003](https://github.com/facebook/jest/pull/5003)) * `[jest-jasmine2]` Update Timeout error message to `jest.timeout` and display current timeout value ([#4990](https://github.com/facebook/jest/pull/4990)) * `[jest-runner]` Enable experimental detection of leaked contexts diff --git a/docs/GlobalAPI.md b/docs/GlobalAPI.md index f391456429b1..b4d17d4167a2 100644 --- a/docs/GlobalAPI.md +++ b/docs/GlobalAPI.md @@ -19,8 +19,8 @@ environment. You don't have to require or import anything to use them. Runs a function after all the tests in this file have completed. If the function returns a promise, Jest waits for that promise to resolve before continuing. -Optionally, you can provide a `timeout` (in milliseconds) for specifying how long to wait -before aborting. _Note: The default timeout is 5 seconds._ +Optionally, you can provide a `timeout` (in milliseconds) for specifying how +long to wait before aborting. _Note: The default timeout is 5 seconds._ This is often useful if you want to clean up some global setup state that is shared across tests. @@ -66,8 +66,8 @@ Runs a function after each one of the tests in this file completes. If the function returns a promise, Jest waits for that promise to resolve before continuing. -Optionally, you can provide a `timeout` (in milliseconds) for specifying how long to wait -before aborting. _Note: The default timeout is 5 seconds._ +Optionally, you can provide a `timeout` (in milliseconds) for specifying how +long to wait before aborting. _Note: The default timeout is 5 seconds._ This is often useful if you want to clean up some temporary state that is created by each test. @@ -112,8 +112,8 @@ If you want to run some cleanup just once, after all of the tests run, use Runs a function before any of the tests in this file run. If the function returns a promise, Jest waits for that promise to resolve before running tests. -Optionally, you can provide a `timeout` (in milliseconds) for specifying how long to wait -before aborting. _Note: The default timeout is 5 seconds._ +Optionally, you can provide a `timeout` (in milliseconds) for specifying how +long to wait before aborting. _Note: The default timeout is 5 seconds._ This is often useful if you want to set up some global state that will be used by many tests. @@ -157,8 +157,8 @@ Runs a function before each of the tests in this file runs. If the function returns a promise, Jest waits for that promise to resolve before running the test. -Optionally, you can provide a `timeout` (in milliseconds) for specifying how long to wait -before aborting. _Note: The default timeout is 5 seconds._ +Optionally, you can provide a `timeout` (in milliseconds) for specifying how +long to wait before aborting. _Note: The default timeout is 5 seconds._ This is often useful if you want to reset some global state that will be used by many tests. @@ -327,8 +327,9 @@ test('did not rain', () => { ``` The first argument is the test name; the second argument is a function that -contains the expectations to test. The third argument (optional) is `timeout` (in milliseconds) -for specifying how long to wait before aborting. _Note: The default timeout is 5 seconds._ +contains the expectations to test. The third argument (optional) is `timeout` +(in milliseconds) for specifying how long to wait before aborting. _Note: The +default timeout is 5 seconds._ > Note: If a **promise is returned** from `test`, Jest will wait for the promise > to resolve before letting the test complete. Jest will also wait if you @@ -358,8 +359,8 @@ When you are debugging a large codebase, you will often only want to run a subset of tests. You can use `.only` to specify which tests are the only ones you want to run. -Optionally, you can provide a `timeout` (in milliseconds) for specifying how long to wait -before aborting. _Note: The default timeout is 5 seconds._ +Optionally, you can provide a `timeout` (in milliseconds) for specifying how +long to wait before aborting. _Note: The default timeout is 5 seconds._ For example, let's say you had these tests: diff --git a/packages/jest-worker/README.md b/packages/jest-worker/README.md index 0ea84f232081..ad5c886e6702 100644 --- a/packages/jest-worker/README.md +++ b/packages/jest-worker/README.md @@ -87,8 +87,8 @@ to `3`, pass `Infinity` to allow endless retries. #### `forkOptions: Object` (optional) Allow customizing all options passed to `childProcess.fork`. By default, some -values are set (`cwd` and `env`), but you can override them and customize the -rest. For a list of valid values, check +values are set (`cwd`, `env` and `execArgv`), but you can override them and +customize the rest. For a list of valid values, check [the Node documentation](https://nodejs.org/api/child_process.html#child_process_child_process_fork_modulepath_args_options). #### `computeWorkerKey: (method: string, ...args: Array) => ?string` (optional) diff --git a/packages/jest-worker/src/__tests__/worker.test.js b/packages/jest-worker/src/__tests__/worker.test.js index f54b62432617..a155c94848c1 100644 --- a/packages/jest-worker/src/__tests__/worker.test.js +++ b/packages/jest-worker/src/__tests__/worker.test.js @@ -21,9 +21,11 @@ import { let Worker; let forkInterface; let childProcess; +let properProcess; beforeEach(() => { jest.mock('child_process'); + properProcess = process; childProcess = require('child_process'); childProcess.fork.mockImplementation(() => { @@ -41,14 +43,19 @@ beforeEach(() => { afterEach(() => { jest.resetModules(); + // eslint-disable-next-line no-native-reassign + process = properProcess; }); it('passes fork options down to child_process.fork, adding the defaults', () => { const child = require.resolve('../child'); + + Object.assign(process, {execArgv: ['--inspect', '-p']}); + new Worker({ forkOptions: { cwd: '/tmp', - execArgv: ['--no-warnings'], + execPath: 'hello', }, maxRetries: 3, workerPath: '/tmp/foo/bar/baz.js', @@ -58,7 +65,8 @@ it('passes fork options down to child_process.fork, adding the defaults', () => expect(childProcess.fork.mock.calls[0][1]).toEqual({ cwd: '/tmp', // Overridden default option. env: process.env, // Default option. - execArgv: ['--no-warnings'], // Added option. + execArgv: ['-p'], // Filtered option. + execPath: 'hello', // Added option. silent: true, // Default option. }); }); diff --git a/packages/jest-worker/src/worker.js b/packages/jest-worker/src/worker.js index be8fb8b1c73b..bbb759b2b4f6 100644 --- a/packages/jest-worker/src/worker.js +++ b/packages/jest-worker/src/worker.js @@ -80,6 +80,9 @@ export default class { { cwd: process.cwd(), env: process.env, + // suppress --debug / --inspect flags while preserving others (like --harmony) + // inspired by https://github.com/rvagg/node-worker-farm/blob/f63d988c307a6805e03b1650f8ef0fb7ca6f1546/lib/fork.js + execArgv: process.execArgv.filter(v => !/^--(debug|inspect)/.test(v)), silent: true, }, this._options.forkOptions, From 14381eed8309fc10292cf5d9f1a0926d0727a31e Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Tue, 12 Dec 2017 17:08:51 +0100 Subject: [PATCH 2/2] address PR comments --- packages/jest-worker/src/__tests__/worker.test.js | 9 ++++----- packages/jest-worker/src/worker.js | 1 - 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/jest-worker/src/__tests__/worker.test.js b/packages/jest-worker/src/__tests__/worker.test.js index a155c94848c1..4ab2d08aaee2 100644 --- a/packages/jest-worker/src/__tests__/worker.test.js +++ b/packages/jest-worker/src/__tests__/worker.test.js @@ -21,11 +21,11 @@ import { let Worker; let forkInterface; let childProcess; -let properProcess; +let originalExecArgv; beforeEach(() => { jest.mock('child_process'); - properProcess = process; + originalExecArgv = process.execArgv; childProcess = require('child_process'); childProcess.fork.mockImplementation(() => { @@ -43,14 +43,13 @@ beforeEach(() => { afterEach(() => { jest.resetModules(); - // eslint-disable-next-line no-native-reassign - process = properProcess; + process.execArgv = originalExecArgv; }); it('passes fork options down to child_process.fork, adding the defaults', () => { const child = require.resolve('../child'); - Object.assign(process, {execArgv: ['--inspect', '-p']}); + process.execArgv = ['--inspect', '-p']; new Worker({ forkOptions: { diff --git a/packages/jest-worker/src/worker.js b/packages/jest-worker/src/worker.js index bbb759b2b4f6..ca7be4e3614c 100644 --- a/packages/jest-worker/src/worker.js +++ b/packages/jest-worker/src/worker.js @@ -81,7 +81,6 @@ export default class { cwd: process.cwd(), env: process.env, // suppress --debug / --inspect flags while preserving others (like --harmony) - // inspired by https://github.com/rvagg/node-worker-farm/blob/f63d988c307a6805e03b1650f8ef0fb7ca6f1546/lib/fork.js execArgv: process.execArgv.filter(v => !/^--(debug|inspect)/.test(v)), silent: true, },